Remove need to provide client secret when creating workspace and remove Directory.Read.All Dependency from automation admin

CHANGELOG.md

@@ -2,13 +2,18 @@
 ## 0.27.0 (Unreleased)
 **BREAKING CHANGES**
 * Fix missing arguments for airlock manager requests - change in API contract  ([#4544](https://github.com/microsoft/AzureTRE/issues/4544))
-* Clarify cost label time period and aggregation scope in UI tooltips ([#4607](https://github.com/microsoft/AzureTRE/pull/4607))
+
+* Base workspace bundle 3.0.0 (major upgrade from 2.8.0) now creates and rotates the workspace Microsoft Entra application secret automatically and removes the manual identity passthrough parameters (`client_secret`, `register_aad_application`, `scope_id`, `sp_id`, `app_role_id_*`).
+  - Existing workspaces that relied on manually managed secrets continue to operate without interruption; upgrade them at your own pace.
+  - The automation admin (`APPLICATION_ADMIN_CLIENT_ID`) no longer needs the `Directory.Read.All` Microsoft Graph permission; keep the documented `Application.ReadWrite.*`, `Group.*`, `User.ReadBasic.All`, and `DelegatedPermissionGrant.ReadWrite.All` permissions in place. ([#4775](https://github.com/microsoft/AzureTRE/pull/4775))
+
 
 ENHANCEMENTS:
 * Upgrade Guacamole to v1.6.0 with Java 17 and other security updates ([#4754](https://github.com/microsoft/AzureTRE/pull/4754))
 * API: Replace HTTP_422_UNPROCESSABLE_ENTITY response with HTTP_422_UNPROCESSABLE_CONTENT as per RFC 9110 ([#4742](https://github.com/microsoft/AzureTRE/issues/4742))
 * Change Group.ReadWrite.All permission to Group.Create for AUTO_WORKSPACE_GROUP_CREATION ([#4772](https://github.com/microsoft/AzureTRE/issues/4772))
 * Make workspace shared storage quota updateable ([#4314](https://github.com/microsoft/AzureTRE/issues/4314))
+* Clarify cost label time period and aggregation scope in UI tooltips ([#4607](https://github.com/microsoft/AzureTRE/pull/4607))
 
 BUG FIXES:
 * Fix circular dependancy in base workspace. ([#4756](https://github.com/microsoft/AzureTRE/pull/4756))

api_app/_version.py

@@ -1 +1 @@
-__version__ = "0.25.4"
+__version__ = "0.26.0"

api_app/api/routes/workspaces.py

@@ -30,7 +30,6 @@
     get_current_workspace_owner_or_researcher_user_or_airlock_manager, \
     get_current_workspace_owner_or_airlock_manager, \
     get_current_workspace_owner_or_researcher_user_or_airlock_manager_or_tre_admin
-from services.authentication import extract_auth_information
 from services.azure_resource_status import get_azure_resource_status
 from azure.cosmos.exceptions import CosmosAccessConditionFailedError
 from .resource_helpers import cascaded_update_resource, delete_validation, enrich_resource_with_available_upgrades, get_identity_role_assignments, save_and_deploy_resource, construct_location_header, send_uninstall_message, \
@@ -99,9 +98,7 @@ async def retrieve_workspace_scope_id_by_workspace_id(workspace=Depends(get_work
 @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)])
 async def create_workspace(workspace_create: WorkspaceInCreate, response: Response, user=Depends(get_current_admin_user), workspace_repo=Depends(get_repository(WorkspaceRepository)), resource_template_repo=Depends(get_repository(ResourceTemplateRepository)), operations_repo=Depends(get_repository(OperationRepository)), resource_history_repo=Depends(get_repository(ResourceHistoryRepository))) -> OperationInResponse:
     try:
-        # TODO: This requires Directory.ReadAll ( Application.Read.All ) to be enabled in the Azure AD application to enable a users workspaces to be listed. This should be made optional.
-        auth_info = extract_auth_information(workspace_create.properties)
-        workspace, resource_template = await workspace_repo.create_workspace_item(workspace_create, auth_info, user.id, user.roles)
+        workspace, resource_template = await workspace_repo.create_workspace_item(workspace_create, user.id, user.roles)
     except (ValidationError, ValueError) as e:
         logger.exception("Failed to create workspace model instance")
         raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=str(e))

api_app/db/repositories/workspaces.py

@@ -89,7 +89,7 @@ async def is_workspace_storage_account_available(self, workspace_id: str) -> boo
         )
         return availability_result.name_available
 
-    async def create_workspace_item(self, workspace_input: WorkspaceInCreate, auth_info: dict, workspace_owner_object_id: str, user_roles: List[str]) -> Tuple[Workspace, ResourceTemplate]:
+    async def create_workspace_item(self, workspace_input: WorkspaceInCreate, workspace_owner_object_id: str, user_roles: List[str]) -> Tuple[Workspace, ResourceTemplate]:
 
         full_workspace_id = str(uuid.uuid4())
 
@@ -104,17 +104,14 @@ async def create_workspace_item(self, workspace_input: WorkspaceInCreate, auth_i
         address_space_param = {"address_space": intial_address_space}
         address_spaces_param = {"address_spaces": [intial_address_space]}
 
-        auto_app_registration_param = {"register_aad_application": self.automatically_create_application_registration(workspace_input.properties)}
         workspace_owner_param = {"workspace_owner_object_id": self.get_workspace_owner(workspace_input.properties, workspace_owner_object_id)}
 
         # we don't want something in the input to overwrite the system parameters,
         # so dict.update can't work. Priorities from right to left.
         resource_spec_parameters = {**workspace_input.properties,
                                     **address_space_param,
                                     **address_spaces_param,
-                                    **auto_app_registration_param,
                                     **workspace_owner_param,
-                                    **auth_info,
                                     **self.get_workspace_spec_params(full_workspace_id)}
 
         workspace = Workspace(
@@ -134,9 +131,6 @@ def get_workspace_owner(self, workspace_properties: dict, workspace_owner_object
         user_defined_workspace_owner_object_id = workspace_properties.get("workspace_owner_object_id")
         return workspace_owner_object_id if user_defined_workspace_owner_object_id is None else user_defined_workspace_owner_object_id
 
-    def automatically_create_application_registration(self, workspace_properties: dict) -> bool:
-        return True if ("auth_type" in workspace_properties and workspace_properties["auth_type"] == "Automatic") else False
-
     async def get_address_space_based_on_size(self, workspace_properties: dict):
         # Default the address space to 'small' if not supplied.
         address_space_size = workspace_properties.get("address_space_size", "small").lower()

api_app/models/schemas/workspace.py

@@ -84,7 +84,6 @@ class Config:
                     "description": "workspace description",
                     "auth_type": "Manual",
                     "client_id": "<WORKSPACE_CLIENT_ID>",
-                    "client_secret": "<WORKSPACE_CLIENT_SECRET>",
                     "address_space_size": "small"
                 }
             }

api_app/models/schemas/workspace_template.py

@@ -18,7 +18,6 @@ def get_sample_workspace_template_object(template_name: str = "tre-workspace-bas
             "display_name": Property(type="string"),
             "description": Property(type="string"),
             "client_id": Property(type="string"),
-            "client_secret": Property(type="string"),
             "address_space_size": Property(
                 type="string",
                 default="small",

api_app/services/aad_authentication.py

@@ -480,25 +480,6 @@ def _get_batch_users_by_role_assignments_body(self, roles_graph_data):
 
         return request_body
 
-    # This method is called when you create a workspace and you already have an AAD App Registration
-    # to link it to. You pass in the client_id and go and get the extra information you need from AAD
-    # If the auth_type is `Automatic`, then these values will be written by Terraform.
-    def _get_app_auth_info(self, client_id: str) -> dict:
-        graph_data = self._get_app_sp_graph_data(client_id)
-        if 'value' not in graph_data or len(graph_data['value']) == 0:
-            logger.debug(graph_data)
-            raise AuthConfigValidationError(f"{strings.ACCESS_UNABLE_TO_GET_INFO_FOR_APP} {client_id}")
-
-        app_info = graph_data['value'][0]
-        authInfo = {'sp_id': app_info['id'], 'scope_id': app_info['servicePrincipalNames'][0]}
-
-        # Convert the roles into ids (We could have more roles defined in the app than we need.)
-        for appRole in app_info['appRoles']:
-            if appRole['value'] in self.WORKSPACE_ROLES_DICT.keys():
-                authInfo[self.WORKSPACE_ROLES_DICT[appRole['value']]] = appRole['id']
-
-        return authInfo
-
     def _ms_graph_query(self, url: str, http_method: str, json=None) -> dict:
         msgraph_token = self._get_msgraph_token()
         auth_headers = self._get_auth_header(msgraph_token)
@@ -550,23 +531,6 @@ def _get_identity_type(self, id: str) -> str:
 
         return object_info["@odata.type"]
 
-    def extract_workspace_auth_information(self, data: dict) -> dict:
-        if ("auth_type" not in data) or (data["auth_type"] != "Automatic" and "client_id" not in data):
-            raise AuthConfigValidationError(strings.ACCESS_PLEASE_SUPPLY_CLIENT_ID)
-
-        auth_info = {}
-        # The user may want us to create the AAD workspace app and therefore they
-        # don't know the client_id yet.
-        if data["auth_type"] != "Automatic":
-            auth_info = self._get_app_auth_info(data["client_id"])
-
-            # Check we've get all our required roles
-            for role in self.WORKSPACE_ROLES_DICT.items():
-                if role[1] not in auth_info:
-                    raise AuthConfigValidationError(f"{strings.ACCESS_APP_IS_MISSING_ROLE} {role[0]}")
-
-        return auth_info
-
     def get_identity_role_assignments(self, user_id: str) -> List[RoleAssignment]:
         identity_type = self._get_identity_type(user_id)
         if identity_type == "#microsoft.graph.user":

api_app/services/access_service.py

@@ -15,10 +15,6 @@ class UserRoleAssignmentError(Exception):
 
 
 class AccessService(OAuth2AuthorizationCodeBearer):
-    @abstractmethod
-    def extract_workspace_auth_information(self, data: dict) -> dict:
-        pass
-
     @abstractmethod
     def get_identity_role_assignments(self, user_id: str) -> dict:
         pass

api_app/services/authentication.py

@@ -4,15 +4,7 @@
 from models.schemas.workspace import AuthProvider
 from resources import strings
 from services.aad_authentication import AzureADAuthorization
-from services.access_service import AccessService, AuthConfigValidationError
-
-
-def extract_auth_information(workspace_creation_properties: dict) -> dict:
-    access_service = get_access_service('AAD')
-    try:
-        return access_service.extract_workspace_auth_information(workspace_creation_properties)
-    except AuthConfigValidationError as e:
-        raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=str(e))
+from services.access_service import AccessService
 
 
 def get_access_service(provider: str = AuthProvider.AAD) -> AccessService:

api_app/tests_ma/test_api/test_routes/test_workspace_users.py

@@ -24,7 +24,7 @@
 OPERATION_ID = '11111111-7265-4b5f-9eae-a1a62928772f'
 
 
-def sample_workspace(workspace_id=WORKSPACE_ID, auth_info: dict = {}) -> Workspace:
+def sample_workspace(workspace_id=WORKSPACE_ID) -> Workspace:
     workspace = Workspace(
         id=workspace_id,
         templateName="tre-workspace-base",
@@ -39,8 +39,7 @@ def sample_workspace(workspace_id=WORKSPACE_ID, auth_info: dict = {}) -> Workspa
         updatedWhen=FAKE_CREATE_TIMESTAMP,
         user=create_admin_user()
     )
-    if auth_info:
-        workspace.properties = {**auth_info}
+
     return workspace
 
 

api_app/tests_ma/test_api/test_routes/test_workspaces.py

@@ -436,8 +436,7 @@ async def test_get_workspace_history_returns_empty_list_when_no_history(self, ac
     @patch("api.routes.resource_helpers.send_resource_request_message", return_value=sample_resource_operation(resource_id=WORKSPACE_ID, operation_id=OPERATION_ID))
     @patch("api.routes.workspaces.WorkspaceRepository.save_item")
     @patch("api.routes.workspaces.WorkspaceRepository.create_workspace_item")
-    @patch("api.routes.workspaces.extract_auth_information")
-    async def test_post_workspaces_creates_workspace(self, _, create_workspace_item, __, ___, resource_template_repo, app, client, workspace_input, basic_resource_template):
+    async def test_post_workspaces_creates_workspace(self, create_workspace_item, __, ___, resource_template_repo, app, client, workspace_input, basic_resource_template):
         resource_template_repo.return_value = basic_resource_template
         create_workspace_item.return_value = [sample_workspace(), basic_resource_template]
         response = await client.post(app.url_path_for(strings.API_CREATE_WORKSPACE), json=workspace_input)
@@ -451,8 +450,7 @@ async def test_post_workspaces_creates_workspace(self, _, create_workspace_item,
     @patch("api.routes.workspaces.WorkspaceRepository.save_item")
     @patch("api.routes.workspaces.WorkspaceRepository.create_workspace_item")
     @patch("api.routes.workspaces.WorkspaceRepository._validate_resource_parameters")
-    @patch("api.routes.workspaces.extract_auth_information")
-    async def test_post_workspaces_calls_db_and_service_bus(self, _, __, create_workspace_item, save_item_mock, send_resource_request_message_mock, resource_template_repo, app, client, workspace_input, basic_resource_template):
+    async def test_post_workspaces_calls_db_and_service_bus(self, __, create_workspace_item, save_item_mock, send_resource_request_message_mock, resource_template_repo, app, client, workspace_input, basic_resource_template):
         resource_template_repo.return_value = basic_resource_template
         create_workspace_item.return_value = [sample_workspace(), basic_resource_template]
         await client.post(app.url_path_for(strings.API_CREATE_WORKSPACE), json=workspace_input)
@@ -466,8 +464,7 @@ async def test_post_workspaces_calls_db_and_service_bus(self, _, __, create_work
     @patch("api.routes.workspaces.WorkspaceRepository.save_item")
     @patch("api.routes.workspaces.WorkspaceRepository.create_workspace_item")
     @patch("api.routes.workspaces.WorkspaceRepository._validate_resource_parameters")
-    @patch("api.routes.workspaces.extract_auth_information")
-    async def test_post_workspaces_returns_202_on_successful_create(self, _, __, create_workspace_item, ____, _____, resource_template_repo, app, client, workspace_input, basic_resource_template):
+    async def test_post_workspaces_returns_202_on_successful_create(self, __, create_workspace_item, ____, _____, resource_template_repo, app, client, workspace_input, basic_resource_template):
         resource_template_repo.return_value = basic_resource_template
         create_workspace_item.return_value = [sample_workspace(), basic_resource_template]
         response = await client.post(app.url_path_for(strings.API_CREATE_WORKSPACE), json=workspace_input)
@@ -482,17 +479,16 @@ async def test_post_workspaces_returns_202_on_successful_create(self, _, __, cre
     @patch("api.routes.workspaces.WorkspaceRepository.save_item")
     @patch("api.routes.workspaces.WorkspaceRepository.create_workspace_item", return_value=[sample_workspace(), sample_resource_template()])
     @patch("api.routes.workspaces.WorkspaceRepository._validate_resource_parameters")
-    @patch("api.routes.workspaces.extract_auth_information")
-    async def test_post_workspaces_returns_503_if_service_bus_call_fails(self, _, __, ___, ____, _____, delete_item_mock, resource_template_repo, app, client, workspace_input, basic_resource_template):
+    async def test_post_workspaces_returns_503_if_service_bus_call_fails(self, __, ___, ____, _____, delete_item_mock, resource_template_repo, app, client, workspace_input, basic_resource_template):
         resource_template_repo.return_value = basic_resource_template
         response = await client.post(app.url_path_for(strings.API_CREATE_WORKSPACE), json=workspace_input)
 
         assert response.status_code == status.HTTP_503_SERVICE_UNAVAILABLE
         delete_item_mock.assert_called_once_with(WORKSPACE_ID)
 
     # [POST] /workspaces/
-    @patch("api.routes.workspaces.WorkspaceRepository.validate_input_against_template", side_effect=ValueError)
-    async def test_post_workspaces_returns_400_if_template_does_not_exist(self, _, app, client, workspace_input):
+    @patch("api.routes.workspaces.WorkspaceRepository.create_workspace_item", side_effect=ValueError)
+    async def test_post_workspaces_returns_400_if_template_does_not_exist(self, mock_create, app, client, workspace_input):
         response = await client.post(app.url_path_for(strings.API_CREATE_WORKSPACE), json=workspace_input)
         assert response.status_code == status.HTTP_400_BAD_REQUEST
 

api_app/tests_ma/test_db/test_repositories/test_workpaces_repository.py

@@ -117,7 +117,7 @@ async def test_create_workspace_item_creates_a_workspace_with_the_right_values(m
     validate_input_mock.return_value = basic_resource_template
     new_cidr_mock.return_value = "1.2.3.4/24"
 
-    workspace, _ = await workspace_repo.create_workspace_item(workspace_to_create, {}, "test_object_id", ["test_role"])
+    workspace, _ = await workspace_repo.create_workspace_item(workspace_to_create, "test_object_id", ["test_role"])
 
     assert workspace.templateName == workspace_to_create.templateName
     assert workspace.resourceType == ResourceType.Workspace
@@ -186,7 +186,7 @@ async def test_create_workspace_item_creates_a_workspace_with_custom_address_spa
     mock_is_workspace_storage_account_available.return_value.return_value = False
     validate_input_mock.return_value = basic_resource_template
 
-    workspace, _ = await workspace_repo.create_workspace_item(workspace_to_create, {}, "test_object_id", ["test_role"])
+    workspace, _ = await workspace_repo.create_workspace_item(workspace_to_create, "test_object_id", ["test_role"])
 
     assert workspace.properties["address_space"] == workspace_to_create.properties["address_space"]
 
@@ -208,7 +208,7 @@ async def test_create_workspace_item_throws_exception_with_bad_custom_address_sp
     validate_input_mock.return_value = basic_resource_template
 
     with pytest.raises(InvalidInput):
-        await workspace_repo.create_workspace_item(workspace_to_create, {}, "test_object_id", ["test_role"])
+        await workspace_repo.create_workspace_item(workspace_to_create, "test_object_id", ["test_role"])
 
 
 @pytest.mark.asyncio
@@ -273,19 +273,7 @@ async def test_create_workspace_item_raises_value_error_if_template_is_invalid(m
     validate_input_mock.side_effect = ValueError
 
     with pytest.raises(ValueError):
-        await workspace_repo.create_workspace_item(workspace_input, {}, "test_object_id", ["test_role"])
-
-
-def test_automatically_create_application_registration_returns_true(workspace_repo):
-    dictToTest = {"auth_type": "Automatic"}
-
-    assert workspace_repo.automatically_create_application_registration(dictToTest) is True
-
-
-def test_automatically_create_application_registration_returns_false(workspace_repo):
-    dictToTest = {"client_id": "12345"}
-
-    assert workspace_repo.automatically_create_application_registration(dictToTest) is False
+        await workspace_repo.create_workspace_item(workspace_input, "test_object_id", ["test_role"])
 
 
 def test_workspace_owner_is_set_if_not_present_in_workspace_properties(workspace_repo):