Get Workspace by ID (#2651)

* TRE Admin allowed to GET workspace by id

* get by workspace id for TRE admin OR workspace user. infer AAD scope_id in URI

* TRE Admin allowed to GET workspace by id

* moved + added back templates tests

* api v

* api v

api_app/_version.py

@@ -1 +1 @@
-__version__ = "0.4.42"
+__version__ = "0.4.43"

api_app/api/routes/workspaces.py

@@ -76,15 +76,9 @@ def _safe_get_workspace_role(user, workspace, user_role_assignments):
         return WorkspacesInList(workspaces=user_workspaces)
 
 
-@workspaces_core_router.get("/workspaces/{workspace_id}", response_model=WorkspaceInResponse, name=strings.API_GET_WORKSPACE_BY_ID)
-async def retrieve_workspace_by_workspace_id(user=Depends(get_current_tre_user_or_tre_admin), workspace=Depends(get_workspace_by_id_from_path)) -> WorkspaceInResponse:
-    access_service = get_access_service()
-    user_role_assignments = get_identity_role_assignments(user)
-    if access_service.get_workspace_role(user, workspace, user_role_assignments) != WorkspaceRole.NoRole:
-        return WorkspaceInResponse(workspace=workspace)
-    else:
-        logging.debug("User doesn't have roles in workspace.")
-        raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail=strings.ACCESS_USER_IS_NOT_OWNER_OR_RESEARCHER)
+@workspaces_shared_router.get("/workspaces/{workspace_id}", response_model=WorkspaceInResponse, name=strings.API_GET_WORKSPACE_BY_ID)
+async def retrieve_workspace_by_workspace_id(workspace=Depends(get_workspace_by_id_from_path)) -> WorkspaceInResponse:
+    return WorkspaceInResponse(workspace=workspace)
 
 
 @workspaces_core_router.post("/workspaces", status_code=status.HTTP_202_ACCEPTED, response_model=OperationInResponse, name=strings.API_CREATE_WORKSPACE, dependencies=[Depends(get_current_admin_user)])

api_app/services/aad_authentication.py

@@ -57,6 +57,8 @@ async def __call__(self, request: Request) -> User:
             try:
                 app_reg_id = self._fetch_ws_app_reg_id_from_ws_id(request)
                 decoded_token = self._decode_token(token, app_reg_id)
+            except HTTPException as h:
+                raise h
             except Exception as e:
                 logging.debug(e)
                 logging.debug("Failed to decode using workspace_id, trying with TRE API app registration")

api_app/tests_ma/test_api/test_routes/test_workspaces.py

@@ -18,8 +18,8 @@
 from models.domain.user_resource import UserResource
 from models.domain.workspace import Workspace, WorkspaceRole
 from models.domain.workspace_service import WorkspaceService
-from models.schemas.resource_template import ResourceTemplateInformation
 from resources import strings
+from models.schemas.resource_template import ResourceTemplateInformation
 from services.authentication import get_current_admin_user, \
     get_current_tre_user_or_tre_admin, get_current_workspace_owner_user, \
     get_current_workspace_owner_or_researcher_user, \
@@ -218,11 +218,7 @@ class TestWorkspaceRoutesThatDontRequireAdminRights:
     @pytest.fixture(autouse=True, scope='class')
     def log_in_with_non_admin_user(self, app, non_admin_user):
         with patch('services.aad_authentication.AzureADAuthorization._get_user_from_token', return_value=non_admin_user()):
-            app.dependency_overrides[get_current_tre_user_or_tre_admin] = non_admin_user
-            app.dependency_overrides[get_current_workspace_owner_or_researcher_user_or_airlock_manager] = non_admin_user
-            app.dependency_overrides[get_current_workspace_owner_or_researcher_user_or_airlock_manager_or_tre_admin] = non_admin_user
             yield
-            app.dependency_overrides = {}
 
     # [GET] /workspaces
     @patch("api.routes.workspaces.WorkspaceRepository.get_active_workspaces")
@@ -259,15 +255,13 @@ async def test_get_workspaces_returns_correct_data_when_resources_exist(self, ac
     # [GET] /workspaces/{workspace_id}
     @patch("api.dependencies.workspaces.WorkspaceRepository.get_workspace_by_id")
     @patch("api.routes.workspaces.get_identity_role_assignments")
-    async def test_get_workspace_by_id_get_returns_workspace_if_found(self, access_service_mock, get_workspace_mock, app, client):
-        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'}
-        workspace = sample_workspace(auth_info=auth_info_user_in_workspace_owner_role)
+    async def test_get_workspace_by_id_get_as_tre_user_returns_403(self, access_service_mock, get_workspace_mock, app, client):
+        auth_info_user_in_workspace_owner_role = {'sp_id': 'ab123', 'client_id': 'cl123', 'app_role_id_workspace_owner': 'ab124', 'app_role_id_workspace_researcher': 'ab125', 'app_role_id_workspace_airlock_manager': 'ab130'}
         get_workspace_mock.return_value = sample_workspace(auth_info=auth_info_user_in_workspace_owner_role)
         access_service_mock.return_value = [RoleAssignment('ab123', 'ab124')]
 
         response = await client.get(app.url_path_for(strings.API_GET_WORKSPACE_BY_ID, workspace_id=WORKSPACE_ID))
-        actual_resource = response.json()["workspace"]
-        assert actual_resource["id"] == workspace.id
+        assert response.status_code == status.HTTP_403_FORBIDDEN
 
     # [GET] /workspaces/{workspace_id}
     @patch("api.dependencies.workspaces.WorkspaceRepository.get_workspace_by_id", side_effect=EntityDoesNotExist)
@@ -277,31 +271,12 @@ async def test_get_workspace_by_id_get_returns_404_if_resource_is_not_found(self
         response = await client.get(app.url_path_for(strings.API_GET_WORKSPACE_BY_ID, workspace_id=WORKSPACE_ID))
         assert response.status_code == status.HTTP_404_NOT_FOUND
 
-    # [GET] /workspaces/{workspace_id}
-    @patch("api.dependencies.workspaces.WorkspaceRepository.get_workspace_by_id")
-    @patch("api.routes.workspaces.get_identity_role_assignments")
-    async def test_get_workspace_by_id_get_returns_422_if_workspace_id_is_not_a_uuid(self, access_service_mock, _, app, client):
-        access_service_mock.return_value = [RoleAssignment('ab123', 'ab124')]
-        response = await client.get(app.url_path_for(strings.API_GET_WORKSPACE_BY_ID, workspace_id="not_valid"))
-        assert response.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY
-
-    @patch("api.dependencies.workspaces.WorkspaceRepository.get_workspace_by_id")
-    @patch("api.routes.workspaces.ResourceTemplateRepository.get_templates_information", return_value=[ResourceTemplateInformation(name="test")])
-    async def test_get_workspace_service_templates_returns_templates(self, _, __, app, client):
-        response = await client.get(app.url_path_for(strings.API_GET_WORKSPACE_SERVICE_TEMPLATES_IN_WORKSPACE, workspace_id=WORKSPACE_ID))
-        assert response.status_code == status.HTTP_200_OK
-
-    @patch("api.dependencies.workspaces.WorkspaceRepository.get_workspace_by_id")
-    @patch("api.routes.workspaces.ResourceTemplateRepository.get_templates_information", return_value=[ResourceTemplateInformation(name="test")])
-    async def test_get_user_resource_templates_returns_templates(self, _, __, app, client):
-        response = await client.get(app.url_path_for(strings.API_GET_USER_RESOURCE_TEMPLATES_IN_WORKSPACE, workspace_id=WORKSPACE_ID, service_template_name="guacamole"))
-        assert response.status_code == status.HTTP_200_OK
-
 
 class TestWorkspaceRoutesThatRequireAdminRights:
     @pytest.fixture(autouse=True, scope='class')
     def _prepare(self, app, admin_user):
         with patch('services.aad_authentication.AzureADAuthorization._get_user_from_token', return_value=admin_user()):
+            app.dependency_overrides[get_current_workspace_owner_or_researcher_user_or_airlock_manager_or_tre_admin] = admin_user
             app.dependency_overrides[get_current_tre_user_or_tre_admin] = admin_user
             app.dependency_overrides[get_current_workspace_owner_or_researcher_user_or_airlock_manager] = admin_user
             app.dependency_overrides[get_current_admin_user] = admin_user
@@ -329,6 +304,27 @@ async def test_get_workspaces_returns_correct_data_when_resources_exist(self, ge
         assert workspaces_from_response[1]["id"] == valid_ws_2.id
         assert workspaces_from_response[2]["id"] == valid_ws_3.id
 
+    # [GET] /workspaces/{workspace_id}
+    @patch("api.dependencies.workspaces.WorkspaceRepository.get_workspace_by_id")
+    @patch("api.routes.workspaces.get_identity_role_assignments")
+    async def test_get_workspace_by_id_as_tre_admin(self, access_service_mock, get_workspace_mock, app, client):
+        auth_info_user_in_workspace_owner_role = {'sp_id': 'ab123', 'client_id': 'cl123', 'app_role_id_workspace_owner': 'ab124', 'app_role_id_workspace_researcher': 'ab125', 'app_role_id_workspace_airlock_manager': 'ab130'}
+        workspace = sample_workspace(auth_info=auth_info_user_in_workspace_owner_role)
+        get_workspace_mock.return_value = sample_workspace(auth_info=auth_info_user_in_workspace_owner_role)
+        access_service_mock.return_value = [RoleAssignment('ab123', 'ab124')]
+
+        response = await client.get(app.url_path_for(strings.API_GET_WORKSPACE_BY_ID, workspace_id=WORKSPACE_ID))
+        actual_resource = response.json()["workspace"]
+        assert actual_resource["id"] == workspace.id
+
+    # [GET] /workspaces/{workspace_id}
+    @patch("api.dependencies.workspaces.WorkspaceRepository.get_workspace_by_id")
+    @patch("api.routes.workspaces.get_identity_role_assignments")
+    async def test_get_workspace_by_id_get_returns_422_if_workspace_id_is_not_a_uuid(self, access_service_mock, _, app, client):
+        access_service_mock.return_value = [RoleAssignment('ab123', 'ab124')]
+        response = await client.get(app.url_path_for(strings.API_GET_WORKSPACE_BY_ID, workspace_id="not_valid"))
+        assert response.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY
+
     # [POST] /workspaces/
     @ patch("api.routes.workspaces.ResourceTemplateRepository.get_template_by_name_and_version")
     @ patch("api.routes.resource_helpers.send_resource_request_message", return_value=sample_resource_operation(resource_id=WORKSPACE_ID, operation_id=OPERATION_ID))
@@ -735,9 +731,28 @@ 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_airlock_manager] = 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_or_tre_admin] = researcher_user
         yield
         app.dependency_overrides = {}
 
+    # [GET] /workspaces/{workspace_id}
+    @patch("api.dependencies.workspaces.WorkspaceRepository.get_workspace_by_id", return_value=sample_workspace())
+    async def test_get_workspace_by_id_get_as_workspace_researcher(self, _, app, client):
+        response = await client.get(app.url_path_for(strings.API_GET_WORKSPACE_BY_ID, workspace_id=WORKSPACE_ID))
+        assert response.status_code == status.HTTP_200_OK
+
+    @patch("api.dependencies.workspaces.WorkspaceRepository.get_workspace_by_id")
+    @patch("api.routes.workspaces.ResourceTemplateRepository.get_templates_information", return_value=[ResourceTemplateInformation(name="test")])
+    async def test_get_workspace_service_templates_returns_templates(self, _, __, app, client):
+        response = await client.get(app.url_path_for(strings.API_GET_WORKSPACE_SERVICE_TEMPLATES_IN_WORKSPACE, workspace_id=WORKSPACE_ID))
+        assert response.status_code == status.HTTP_200_OK
+
+    @patch("api.dependencies.workspaces.WorkspaceRepository.get_workspace_by_id")
+    @patch("api.routes.workspaces.ResourceTemplateRepository.get_templates_information", return_value=[ResourceTemplateInformation(name="test")])
+    async def test_get_user_resource_templates_returns_templates(self, _, __, app, client):
+        response = await client.get(app.url_path_for(strings.API_GET_USER_RESOURCE_TEMPLATES_IN_WORKSPACE, workspace_id=WORKSPACE_ID, service_template_name="guacamole"))
+        assert response.status_code == status.HTTP_200_OK
+
     # [GET] /workspaces/{workspace_id}/workspace-services
     @patch("api.dependencies.workspaces.WorkspaceRepository.get_workspace_by_id", return_value=sample_workspace())
     @patch("api.routes.workspaces.WorkspaceServiceRepository.get_active_workspace_services_for_workspace",

devops/scripts/build_deploy_ui.sh

@@ -13,7 +13,8 @@ jq --arg rootClientId "${SWAGGER_UI_CLIENT_ID}" \
   --arg rootTenantId "${AAD_TENANT_ID}" \
   --arg treApplicationId "api://${API_CLIENT_ID}" \
   --arg treUrl "https://${FQDN}/api" \
-  '.rootClientId = $rootClientId | .rootTenantId = $rootTenantId | .treApplicationId = $treApplicationId | .treUrl = $treUrl' ./src/config.source.json > ./src/config.json
+  --arg treId "${TRE_ID}" \
+  '.rootClientId = $rootClientId | .rootTenantId = $rootTenantId | .treApplicationId = $treApplicationId | .treUrl = $treUrl | .treId = $treId' ./src/config.source.json > ./src/config.json
 
 # build and deploy the app
 yarn install

ui/app/src/components/workspaces/WorkspaceProvider.tsx

@@ -36,7 +36,11 @@ export const WorkspaceProvider: React.FunctionComponent = () => {
     const getWorkspace = async () => {
       try {
         // get the workspace
-        const ws = (await apiCall(`${ApiEndpoint.Workspaces}/${workspaceId}`, HttpMethod.Get)).workspace;
+        // for this call, we infer the scope_id for the aad app to authenticate against
+        // {TRE-ID}-ws-{last-4-chars-of-workspace-id}
+        // following this, we set the workspace in the context and all other calls use the context values
+        let inferredScopeId = `api://${config.treId}-ws-${workspaceId?.substring(workspaceId.length-4)}`;
+        const ws = (await apiCall(`${ApiEndpoint.Workspaces}/${workspaceId}`, HttpMethod.Get, inferredScopeId)).workspace;
         workspaceCtx.current.setWorkspace(ws);
         const ws_application_id_uri = ws.properties.scope_id;
 

ui/app/src/config.source.json

@@ -4,5 +4,6 @@
     "treApplicationId": "api://",
     "treUrl": "https://my-tre.northeurope.cloudapp.azure.com/api",
     "pollingDelayMilliseconds": 10000,
+    "treId": "my-tre",
     "debug": false
 }