Adding Airlock Manager Role - Part I (#2349)

* Initial airlock manager role

* Add only airlock manager review requests in api

* Fix tests

* cr changes

* fix tests

* Add to env.sample

* edit api version

* add to changelog

* update api ver

CHANGELOG.md

@@ -5,6 +5,7 @@
 **BREAKING CHANGES & MIGRATIONS**:
 
 * Guacamole workspace service configures firewall requirements with deployment pipeline ([#2371](https://github.com/microsoft/AzureTRE/pull/2371)). **Migration** is manual - update the templateVersion of `tre-shared-service-firewall` in Cosmos to `0.4.0` in order to use this capability.
+* Workspace now has an AirlockManager role that has the permissions to review airlock requests  ([#2349](https://github.com/microsoft/AzureTRE/pull/2349)).
 
 FEATURES:
 

api_app/_version.py

@@ -1 +1 @@
-__version__ = "0.4.3"
+__version__ = "0.4.4"

api_app/api/routes/airlock.py

@@ -15,15 +15,15 @@
 from db.repositories.airlock_requests import AirlockRequestRepository
 from models.schemas.airlock_request import AirlockRequestInCreate, AirlockRequestInResponse, AirlockRequestInList
 from resources import strings
-from services.authentication import get_current_workspace_owner_or_researcher_user, get_current_workspace_owner_user
+from services.authentication import get_current_workspace_owner_or_researcher_user_or_airlock_manager, get_current_workspace_owner_or_researcher_user, get_current_airlock_manager_user
 
 from .airlock_resource_helpers import save_airlock_review, save_and_publish_event_airlock_request, \
     update_status_and_publish_event_airlock_request, RequestAccountDetails
 
 from services.airlock import get_storage_management_client, validate_user_allowed_to_access_storage_account, \
     get_account_and_rg_by_request, get_airlock_request_container_sas_token, validate_request_status
 
-airlock_workspace_router = APIRouter(dependencies=[Depends(get_current_workspace_owner_or_researcher_user)])
+airlock_workspace_router = APIRouter(dependencies=[Depends(get_current_workspace_owner_or_researcher_user_or_airlock_manager)])
 storage_client = get_storage_management_client()
 
 
@@ -45,7 +45,7 @@ async def create_draft_request(airlock_request_input: AirlockRequestInCreate, us
                               status_code=status.HTTP_200_OK,
                               response_model=AirlockRequestInList,
                               name=strings.API_LIST_AIRLOCK_REQUESTS,
-                              dependencies=[Depends(get_current_workspace_owner_user), Depends(get_workspace_by_id_from_path)])
+                              dependencies=[Depends(get_current_workspace_owner_or_researcher_user), Depends(get_workspace_by_id_from_path)])
 async def get_all_airlock_requests_by_workspace(
         airlock_request_repo=Depends(get_repository(AirlockRequestRepository)),
         workspace=Depends(get_deployed_workspace_by_id_from_path)) -> AirlockRequestInList:
@@ -62,20 +62,20 @@ async def retrieve_airlock_request_by_id(airlock_request=Depends(get_airlock_req
     return AirlockRequestInResponse(airlockRequest=airlock_request)
 
 
-@airlock_workspace_router.post("/workspaces/{workspace_id}/requests/{airlock_request_id}/submit", status_code=status.HTTP_200_OK, response_model=AirlockRequestInResponse, name=strings.API_SUBMIT_AIRLOCK_REQUEST, dependencies=[Depends(get_current_workspace_owner_or_researcher_user)])
+@airlock_workspace_router.post("/workspaces/{workspace_id}/requests/{airlock_request_id}/submit", status_code=status.HTTP_200_OK, response_model=AirlockRequestInResponse, name=strings.API_SUBMIT_AIRLOCK_REQUEST, dependencies=[Depends(get_current_workspace_owner_or_researcher_user), Depends(get_workspace_by_id_from_path)])
 async def create_submit_request(airlock_request=Depends(get_airlock_request_by_id_from_path), user=Depends(get_current_workspace_owner_or_researcher_user), airlock_request_repo=Depends(get_repository(AirlockRequestRepository)), workspace=Depends(get_workspace_by_id_from_path)) -> AirlockRequestInResponse:
     updated_resource = await update_status_and_publish_event_airlock_request(airlock_request, airlock_request_repo, user, AirlockRequestStatus.Submitted, workspace)
     return AirlockRequestInResponse(airlockRequest=updated_resource)
 
 
-@airlock_workspace_router.post("/workspaces/{workspace_id}/requests/{airlock_request_id}/cancel", status_code=status.HTTP_200_OK, response_model=AirlockRequestInResponse, name=strings.API_CANCEL_AIRLOCK_REQUEST, dependencies=[Depends(get_current_workspace_owner_or_researcher_user)])
+@airlock_workspace_router.post("/workspaces/{workspace_id}/requests/{airlock_request_id}/cancel", status_code=status.HTTP_200_OK, response_model=AirlockRequestInResponse, name=strings.API_CANCEL_AIRLOCK_REQUEST, dependencies=[Depends(get_current_workspace_owner_or_researcher_user), Depends(get_workspace_by_id_from_path)])
 async def create_cancel_request(airlock_request=Depends(get_airlock_request_by_id_from_path), user=Depends(get_current_workspace_owner_or_researcher_user), airlock_request_repo=Depends(get_repository(AirlockRequestRepository)), workspace=Depends(get_workspace_by_id_from_path)) -> AirlockRequestInResponse:
     updated_resource = await update_status_and_publish_event_airlock_request(airlock_request, airlock_request_repo, user, AirlockRequestStatus.Cancelled, workspace)
     return AirlockRequestInResponse(airlockRequest=updated_resource)
 
 
-@airlock_workspace_router.post("/workspaces/{workspace_id}/requests/{airlock_request_id}/reviews", status_code=status.HTTP_200_OK, response_model=AirlockReviewInResponse, name=strings.API_REVIEW_AIRLOCK_REQUEST, dependencies=[Depends(get_current_workspace_owner_user), Depends(get_workspace_by_id_from_path)])
-async def create_airlock_review(airlock_review_input: AirlockReviewInCreate, airlock_request=Depends(get_airlock_request_by_id_from_path), user=Depends(get_current_workspace_owner_user), airlock_request_repo=Depends(get_repository(AirlockRequestRepository)), airlock_review_repo=Depends(get_repository(AirlockReviewRepository)), workspace=Depends(get_deployed_workspace_by_id_from_path)) -> AirlockReviewInResponse:
+@airlock_workspace_router.post("/workspaces/{workspace_id}/requests/{airlock_request_id}/reviews", status_code=status.HTTP_200_OK, response_model=AirlockReviewInResponse, name=strings.API_REVIEW_AIRLOCK_REQUEST, dependencies=[Depends(get_current_airlock_manager_user), Depends(get_workspace_by_id_from_path)])
+async def create_airlock_review(airlock_review_input: AirlockReviewInCreate, airlock_request=Depends(get_airlock_request_by_id_from_path), user=Depends(get_current_airlock_manager_user), airlock_request_repo=Depends(get_repository(AirlockRequestRepository)), airlock_review_repo=Depends(get_repository(AirlockReviewRepository)), workspace=Depends(get_deployed_workspace_by_id_from_path)) -> AirlockReviewInResponse:
     # Create the review model and save in cosmos
     try:
         airlock_review = airlock_review_repo.create_airlock_review_item(airlock_review_input, workspace.id, airlock_request.id)

api_app/models/domain/workspace.py

@@ -7,6 +7,7 @@ class WorkspaceRole(Enum):
     NoRole = 0
     Researcher = 1
     Owner = 2
+    AirlockManager = 3
 
 
 class Workspace(Resource):

api_app/services/aad_authentication.py

@@ -32,7 +32,7 @@ class AzureADAuthorization(AccessService):
     require_one_of_roles = None
 
     TRE_CORE_ROLES = ['TREAdmin', 'TREUser']
-    WORKSPACE_ROLES_DICT = {'WorkspaceOwner': 'app_role_id_workspace_owner', 'WorkspaceResearcher': 'app_role_id_workspace_researcher'}
+    WORKSPACE_ROLES_DICT = {'WorkspaceOwner': 'app_role_id_workspace_owner', 'WorkspaceResearcher': 'app_role_id_workspace_researcher', 'AirlockManager': 'app_role_id_workspace_airlock_manager'}
 
     def __init__(self, auto_error: bool = True, require_one_of_roles: list = None):
         super(AzureADAuthorization, self).__init__(
@@ -328,4 +328,6 @@ def get_workspace_role(self, user: User, workspace: Workspace, user_role_assignm
             return WorkspaceRole.Owner
         if RoleAssignment(resource_id=workspace_sp_id, role_id=workspace.properties['app_role_id_workspace_researcher']) in user_role_assignments:
             return WorkspaceRole.Researcher
+        if RoleAssignment(resource_id=workspace_sp_id, role_id=workspace.properties['app_role_id_workspace_airlock_manager']) in user_role_assignments:
+            return WorkspaceRole.AirlockManager
         return WorkspaceRole.NoRole

api_app/services/authentication.py

@@ -36,9 +36,15 @@ def get_access_service(provider: str = AuthProvider.AAD) -> AccessService:
 get_current_workspace_researcher_user = AzureADAuthorization(require_one_of_roles=['WorkspaceResearcher'])
 
 
+get_current_airlock_manager_user = AzureADAuthorization(require_one_of_roles=['AirlockManager'])
+
+
 get_current_workspace_owner_or_researcher_user = AzureADAuthorization(require_one_of_roles=['WorkspaceOwner', 'WorkspaceResearcher'])
 
 
+get_current_workspace_owner_or_researcher_user_or_airlock_manager = AzureADAuthorization(require_one_of_roles=['WorkspaceOwner', 'WorkspaceResearcher', 'AirlockManager'])
+
+
 get_current_workspace_owner_or_researcher_user_or_tre_admin = AzureADAuthorization(require_one_of_roles=["TREAdmin", "WorkspaceOwner", "WorkspaceResearcher"])
 
 

api_app/tests_ma/test_api/conftest.py

@@ -62,6 +62,12 @@ def create_workspace_researcher_user() -> User:
     return user
 
 
+def create_workspace_airlock_manager_user() -> User:
+    user = create_test_user()
+    user.roles = ["AirlockManager"]
+    return user
+
+
 def override_get_user():
     user = create_test_user()
     user.roles = []
@@ -97,6 +103,13 @@ def inner():
     return inner
 
 
+@pytest.fixture(scope='module')
+def airlock_manager_user():
+    def inner():
+        return create_workspace_airlock_manager_user()
+    return inner
+
+
 @pytest.fixture(scope='module')
 def no_workspace_role_user():
     def inner():

api_app/tests_ma/test_api/test_routes/test_airlock.py

@@ -9,7 +9,7 @@
 
 from models.domain.workspace import Workspace
 from resources import strings
-from services.authentication import get_current_workspace_owner_or_researcher_user_or_tre_admin, get_current_workspace_owner_or_researcher_user, get_current_workspace_owner_user
+from services.authentication import get_current_workspace_owner_or_researcher_user, get_current_workspace_owner_or_researcher_user_or_airlock_manager, get_current_airlock_manager_user
 pytestmark = pytest.mark.asyncio
 WORKSPACE_ID = "abc000d3-82da-4bfc-b6e9-9a7853ef753e"
 AIRLOCK_REQUEST_ID = "af89dccd-cdf8-4e47-8cfe-995faeac0f09"
@@ -69,16 +69,22 @@ def sample_workspace(workspace_id=WORKSPACE_ID, workspace_properties: dict = {})
 class TestAirlockRoutesThatRequireOwnerOrResearcherRights():
     @pytest.fixture(autouse=True, scope='class')
     def log_in_with_researcher_user(self, app, researcher_user):
-        # The following ws services requires the WS app registration
-        app.dependency_overrides[get_current_workspace_owner_or_researcher_user_or_tre_admin] = researcher_user
         app.dependency_overrides[get_current_workspace_owner_or_researcher_user] = researcher_user
+        app.dependency_overrides[get_current_workspace_owner_or_researcher_user_or_airlock_manager] = researcher_user
         with patch("api.routes.airlock.AirlockRequestRepository.create_airlock_request_item", return_value=sample_airlock_request_object()), \
                 patch("api.routes.workspaces.OperationRepository.resource_has_deployed_operation"), \
                 patch("api.routes.airlock.AirlockRequestRepository.save_item"), \
-                patch("api.dependencies.workspaces.WorkspaceRepository.get_workspace_by_id"):
+                patch("api.dependencies.workspaces.WorkspaceRepository.get_workspace_by_id"), \
+                patch("services.aad_authentication.AzureADAuthorization.get_workspace_role_assignment_details", return_value={"researcher_emails": ["researcher@outlook.com"], "owner_emails": ["owner@outlook.com"]}):
             yield
         app.dependency_overrides = {}
 
+    # [GET] /workspaces/{workspace_id}/requests}
+    @patch("api.routes.airlock.AirlockRequestRepository.get_airlock_requests_by_workspace_id", return_value=[])
+    async def test_get_all_airlock_requests_by_workspace_returns_200(self, _, app, client):
+        response = await client.get(app.url_path_for(strings.API_LIST_AIRLOCK_REQUESTS, workspace_id=WORKSPACE_ID))
+        assert response.status_code == status.HTTP_200_OK
+
     # [POST] /workspaces/{workspace_id}/requests
     @patch("api.routes.airlock.save_and_publish_event_airlock_request")
     async def test_post_airlock_request_creates_airlock_request_returns_201(self, _, app, client, sample_airlock_request_input_data):
@@ -96,16 +102,14 @@ async def test_post_airlock_request_with_non_deployed_workspace_id_returns_404(s
         response = await client.post(app.url_path_for(strings.API_CREATE_AIRLOCK_REQUEST, workspace_id=WORKSPACE_ID), json=sample_airlock_request_input_data)
         assert response.status_code == status.HTTP_404_NOT_FOUND
 
-    @patch("services.aad_authentication.AzureADAuthorization.get_workspace_role_assignment_details", return_value={"researcher_emails": ["researcher@outlook.com"], "owner_emails": ["owner@outlook.com"]})
     @patch("api.routes.airlock.AirlockRequestRepository.save_item", side_effect=UnableToAccessDatabase)
-    async def test_post_airlock_request_with_state_store_endpoint_not_responding_returns_503(self, _, __, app, client, sample_airlock_request_input_data):
+    async def test_post_airlock_request_with_state_store_endpoint_not_responding_returns_503(self, _, app, client, sample_airlock_request_input_data):
         response = await client.post(app.url_path_for(strings.API_CREATE_AIRLOCK_REQUEST, workspace_id=WORKSPACE_ID), json=sample_airlock_request_input_data)
         assert response.status_code == status.HTTP_503_SERVICE_UNAVAILABLE
 
-    @patch("services.aad_authentication.AzureADAuthorization.get_workspace_role_assignment_details", return_value={"researcher_emails": ["researcher@outlook.com"], "owner_emails": ["owner@outlook.com"]})
     @patch("api.routes.airlock.AirlockRequestRepository.delete_item")
     @patch("event_grid.event_sender.send_status_changed_event", side_effect=HttpResponseError)
-    async def test_post_airlock_request_with_event_grid_not_responding_returns_503(self, _, __, ___, app, client, sample_airlock_request_input_data):
+    async def test_post_airlock_request_with_event_grid_not_responding_returns_503(self, _, __, app, client, sample_airlock_request_input_data):
         response = await client.post(app.url_path_for(strings.API_CREATE_AIRLOCK_REQUEST, workspace_id=WORKSPACE_ID), json=sample_airlock_request_input_data)
         assert response.status_code == status.HTTP_503_SERVICE_UNAVAILABLE
 
@@ -227,12 +231,11 @@ async def test_get_airlock_container_link_returned_as_expected(self, get_airlock
         assert response.json()["containerUrl"] == get_airlock_request_container_sas_token_mock.return_value
 
 
-class TestAirlockRoutesThatRequireOwnerRights():
+class TestAirlockRoutesThatRequireAirlockManagerRights():
     @pytest.fixture(autouse=True, scope='class')
-    def log_in_with_researcher_user(self, app, owner_user):
-        # The following ws services requires the WS app registration
-        app.dependency_overrides[get_current_workspace_owner_user] = owner_user
-        app.dependency_overrides[get_current_workspace_owner_or_researcher_user] = owner_user
+    def log_in_with_airlock_manager_user(self, app, airlock_manager_user):
+        app.dependency_overrides[get_current_airlock_manager_user] = airlock_manager_user
+        app.dependency_overrides[get_current_workspace_owner_or_researcher_user_or_airlock_manager] = airlock_manager_user
         with patch("api.routes.airlock.AirlockRequestRepository.create_airlock_request_item", return_value=sample_airlock_request_object()), \
                 patch("api.routes.workspaces.OperationRepository.resource_has_deployed_operation"), \
                 patch("api.routes.airlock.AirlockRequestRepository.save_item"), \
@@ -281,9 +284,3 @@ async def test_post_create_airlock_review_with_event_grid_not_responding_returns
     async def test_post_create_airlock_review_with_illegal_status_change_returns_400(self, _, __, ___, ____, app, client, sample_airlock_review_input_data):
         response = await client.post(app.url_path_for(strings.API_REVIEW_AIRLOCK_REQUEST, workspace_id=WORKSPACE_ID, airlock_request_id=AIRLOCK_REQUEST_ID), json=sample_airlock_review_input_data)
         assert response.status_code == status.HTTP_400_BAD_REQUEST
-
-    # [GET] /workspaces/{workspace_id}/requests}
-    @patch("api.routes.airlock.AirlockRequestRepository.get_airlock_requests_by_workspace_id", return_value=[])
-    async def test_get_all_airlock_requests_by_workspace_returns_200(self, _, app, client):
-        response = await client.get(app.url_path_for(strings.API_LIST_AIRLOCK_REQUESTS, workspace_id=WORKSPACE_ID))
-        assert response.status_code == status.HTTP_200_OK

api_app/tests_ma/test_api/test_routes/test_workspaces.py

@@ -231,9 +231,9 @@ async def test_get_workspaces_returns_empty_list_when_no_resources_exist(self, a
     @patch("api.routes.workspaces.WorkspaceRepository.get_active_workspaces")
     @patch("api.routes.workspaces.get_user_role_assignments")
     async def test_get_workspaces_returns_correct_data_when_resources_exist(self, access_service_mock, get_workspaces_mock, app, client) -> None:
-        auth_info_user_in_workspace_owner_role = {'sp_id': 'ab123', 'app_role_id_workspace_owner': 'ab124', 'app_role_id_workspace_researcher': 'ab125'}
-        auth_info_user_in_workspace_researcher_role = {'sp_id': 'ab123', 'app_role_id_workspace_owner': 'ab127', 'app_role_id_workspace_researcher': 'ab126'}
-        auth_info_user_not_in_workspace_role = {'sp_id': 'ab127', 'app_role_id_workspace_owner': 'ab128', 'app_role_id_workspace_researcher': 'ab129'}
+        auth_info_user_in_workspace_owner_role = {'sp_id': 'ab123', 'app_role_id_workspace_owner': 'ab124', 'app_role_id_workspace_researcher': 'ab125', 'app_role_id_workspace_airlock_manager': 'ab130'}
+        auth_info_user_in_workspace_researcher_role = {'sp_id': 'ab123', 'app_role_id_workspace_owner': 'ab127', 'app_role_id_workspace_researcher': 'ab126', 'app_role_id_workspace_airlock_manager': 'ab130'}
+        auth_info_user_not_in_workspace_role = {'sp_id': 'ab127', 'app_role_id_workspace_owner': 'ab128', 'app_role_id_workspace_researcher': 'ab129', 'app_role_id_workspace_airlock_manager': 'ab130'}
 
         valid_ws_1 = sample_workspace(workspace_id=str(uuid.uuid4()), auth_info=auth_info_user_in_workspace_owner_role)
         valid_ws_2 = sample_workspace(workspace_id=str(uuid.uuid4()), auth_info=auth_info_user_in_workspace_researcher_role)