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

.github/actions/devcontainer_run_command/action.yml

@@ -30,12 +30,6 @@ inputs:
   TEST_ACCOUNT_CLIENT_SECRET:
     description: "The Test Automation Account Client Secret used to interact with the API."
     required: false
-  TEST_WORKSPACE_APP_ID:
-    description: "The Test Workspace application Id used to interact with the API."
-    required: false
-  TEST_WORKSPACE_APP_SECRET:
-    description: "The Test Workspace application secret used to interact with the API."
-    required: false
   TRE_ID:
     description: "The TRE Id."
     required: false
@@ -273,8 +267,6 @@ runs:
           -e TRE_ID="${{ inputs.TRE_ID }}" \
           -e TF_VAR_tre_id="${{ inputs.TRE_ID }}" \
           -e TRE_URL="${{ env.TRE_URL }}" \
-          -e TEST_WORKSPACE_APP_ID="${{ inputs.TEST_WORKSPACE_APP_ID }}" \
-          -e TEST_WORKSPACE_APP_SECRET="${{ inputs.TEST_WORKSPACE_APP_SECRET }}" \
           -e TEST_APP_ID="${{ inputs.TEST_APP_ID }}" \
           -e TEST_ACCOUNT_CLIENT_ID="${{ inputs.TEST_ACCOUNT_CLIENT_ID }}" \
           -e TEST_ACCOUNT_CLIENT_SECRET="${{ inputs.TEST_ACCOUNT_CLIENT_SECRET }}" \

.github/scripts/build.js

@@ -114,6 +114,15 @@ async function getCommandFromComment({ core, context, github }) {
           break;
         }
 
+      case "/test-manual-app":
+        {
+          const runTests = await handleTestCommand({ core, github }, parts, "manual app tests", runId, { number: prNumber, authorUsername: prAuthorUsername, repoOwner, repoName, headSha: prHeadSha, refId: prRefId, details: pr }, { username: commentUsername, link: commentLink });
+          if (runTests) {
+            command = "run-tests-manual-app";
+          }
+          break;
+        }
+
       case "/test-force-approve":
         {
           command = "test-force-approve";
@@ -250,6 +259,7 @@ You can use the following commands:
     /test-extended - build, deploy and run smoke & extended tests on a PR
     /test-extended-aad - build, deploy and run smoke & extended AAD tests on a PR
     /test-shared-services - test the deployment of shared services on a PR build
+    /test-manual-app - run the manual workspace application test suite on a PR build
     /test-force-approve - force approval of the PR tests (i.e. skip the deployment checks)
     /test-destroy-env - delete the validation environment for a PR (e.g. to enable testing a deployment from a clean start after previous tests)
     /help - show this help`;

.github/scripts/build.test.js

@@ -412,6 +412,32 @@ describe('getCommandFromComment', () => {
         });
       });
 
+      describe(`for '/test-manual-app'`, () => {
+        test(`should set command to 'run-tests-manual-app'`, async () => {
+          const context = createCommentContext({
+            username: 'admin',
+            body: '/test-manual-app',
+          });
+          await getCommandFromComment({ core, context, github });
+          expect(outputFor(mockCoreSetOutput, 'command')).toBe('run-tests-manual-app');
+        });
+
+        test(`should add comment with run link`, async () => {
+          const context = createCommentContext({
+            username: 'admin',
+            body: '/test-manual-app',
+            pullRequestNumber: PR_NUMBER.UPSTREAM_NON_DOCS_CHANGES,
+          });
+          await getCommandFromComment({ core, context, github });
+          expect(mockGithubRestIssuesCreateComment).toHaveComment({
+            owner: 'someOwner',
+            repo: 'someRepo',
+            issue_number: PR_NUMBER.UPSTREAM_NON_DOCS_CHANGES,
+            bodyMatcher: /Running manual app tests: https:\/\/github.com\/someOwner\/someRepo\/actions\/runs\/11112222 \(with refid `291ae84f`\)/,
+          });
+        });
+      });
+
       describe(`for '/test-shared-services'`, () => {
         test(`should set command to 'run-tests-shared-services'`, async () => {
           const context = createCommentContext({

.github/workflows/deploy_tre.yml

@@ -50,8 +50,6 @@ jobs:
       MGMT_STORAGE_ACCOUNT_NAME: ${{ secrets.MGMT_STORAGE_ACCOUNT_NAME }}
       SWAGGER_UI_CLIENT_ID: ${{ secrets.SWAGGER_UI_CLIENT_ID }}
       TEST_APP_ID: ${{ secrets.TEST_APP_ID }}
-      TEST_WORKSPACE_APP_ID: ${{ secrets.TEST_WORKSPACE_APP_ID }}
-      TEST_WORKSPACE_APP_SECRET: "${{ secrets.TEST_WORKSPACE_APP_SECRET }}"
       TEST_ACCOUNT_CLIENT_ID: "${{ secrets.TEST_ACCOUNT_CLIENT_ID }}"
       TEST_ACCOUNT_CLIENT_SECRET: "${{ secrets.TEST_ACCOUNT_CLIENT_SECRET }}"
       TRE_ID: ${{ secrets.TRE_ID }}

.github/workflows/deploy_tre_branch.yml

@@ -81,8 +81,6 @@ jobs:
       MGMT_STORAGE_ACCOUNT_NAME: ${{ format('tre{0}mgmt', needs.prepare-not-main.outputs.refid) }}
       SWAGGER_UI_CLIENT_ID: ${{ secrets.SWAGGER_UI_CLIENT_ID }}
       TEST_APP_ID: ${{ secrets.TEST_APP_ID }}
-      TEST_WORKSPACE_APP_ID: ${{ secrets.TEST_WORKSPACE_APP_ID }}
-      TEST_WORKSPACE_APP_SECRET: ${{ secrets.TEST_WORKSPACE_APP_SECRET }}
       TEST_ACCOUNT_CLIENT_ID: "${{ secrets.TEST_ACCOUNT_CLIENT_ID }}"
       TEST_ACCOUNT_CLIENT_SECRET: "${{ secrets.TEST_ACCOUNT_CLIENT_SECRET }}"
       TRE_ID: ${{ format('tre{0}', needs.prepare-not-main.outputs.refid) }}

.github/workflows/deploy_tre_reusable.yml

@@ -76,12 +76,6 @@ on: # yamllint disable-line rule:truthy
       TEST_APP_ID:
         description: ""
         required: true
-      TEST_WORKSPACE_APP_ID:
-        description: ""
-        required: true
-      TEST_WORKSPACE_APP_SECRET:
-        description: ""
-        required: true
       TEST_ACCOUNT_CLIENT_ID:
         description: ""
         required: true
@@ -163,12 +157,6 @@ jobs:
           if [ "${{ secrets.TEST_APP_ID }}" == '' ]; then
             echo "Missing secret: TEST_APP_ID" && exit 1
           fi
-          if [ "${{ secrets.TEST_WORKSPACE_APP_ID }}" == '' ]; then
-            echo "Missing secret: TEST_WORKSPACE_APP_ID" && exit 1
-          fi
-          if [ "${{ secrets.TEST_WORKSPACE_APP_SECRET }}" == '' ]; then
-            echo "Missing secret: TEST_WORKSPACE_APP_SECRET" && exit 1
-          fi
           if [ "${{ secrets.TEST_ACCOUNT_CLIENT_ID }}" == '' ]; then
             echo "Missing secret: TEST_ACCOUNT_CLIENT_ID" && exit 1
           fi
@@ -301,7 +289,7 @@ jobs:
           [
             build-and-push-api,
             build-and-push-resource-processor,
-            build-and-push-airlock-processor
+            build-and-push-airlock-processor,
           ]
 
     steps:
@@ -815,8 +803,6 @@ jobs:
           API_CLIENT_ID: "${{ secrets.API_CLIENT_ID }}"
           AAD_TENANT_ID: "${{ secrets.AAD_TENANT_ID }}"
           TEST_APP_ID: "${{ secrets.TEST_APP_ID }}"
-          TEST_WORKSPACE_APP_ID: "${{ secrets.TEST_WORKSPACE_APP_ID }}"
-          TEST_WORKSPACE_APP_SECRET: "${{ secrets.TEST_WORKSPACE_APP_SECRET }}"
           TEST_ACCOUNT_CLIENT_ID: "${{ secrets.TEST_ACCOUNT_CLIENT_ID }}"
           TEST_ACCOUNT_CLIENT_SECRET: "${{ secrets.TEST_ACCOUNT_CLIENT_SECRET }}"
           TRE_ID: ${{ secrets.TRE_ID }}
@@ -859,8 +845,6 @@ jobs:
           API_CLIENT_ID: "${{ secrets.API_CLIENT_ID }}"
           AAD_TENANT_ID: "${{ secrets.AAD_TENANT_ID }}"
           TEST_APP_ID: "${{ secrets.TEST_APP_ID }}"
-          TEST_WORKSPACE_APP_ID: "${{ secrets.TEST_WORKSPACE_APP_ID }}"
-          TEST_WORKSPACE_APP_SECRET: "${{ secrets.TEST_WORKSPACE_APP_SECRET }}"
           TEST_ACCOUNT_CLIENT_ID: "${{ secrets.TEST_ACCOUNT_CLIENT_ID }}"
           TEST_ACCOUNT_CLIENT_SECRET: "${{ secrets.TEST_ACCOUNT_CLIENT_SECRET }}"
           TRE_ID: ${{ secrets.TRE_ID }}

.github/workflows/pr_comment_bot.yml

@@ -147,7 +147,8 @@ jobs:
       needs.pr_comment.outputs.command == 'run-tests' ||
       needs.pr_comment.outputs.command == 'run-tests-extended' ||
       needs.pr_comment.outputs.command == 'run-tests-extended-aad' ||
-      needs.pr_comment.outputs.command == 'run-tests-shared-services'
+      needs.pr_comment.outputs.command == 'run-tests-shared-services' ||
+      needs.pr_comment.outputs.command == 'run-tests-manual-app'
     name: Deploy PR
     uses: ./.github/workflows/deploy_tre_reusable.yml
     permissions:
@@ -162,6 +163,7 @@ jobs:
         ${{ (needs.pr_comment.outputs.command == 'run-tests-extended' && 'extended') ||
         (needs.pr_comment.outputs.command == 'run-tests-extended-aad' && 'extended_aad') ||
         (needs.pr_comment.outputs.command == 'run-tests-shared-services' && 'shared_services') ||
+        (needs.pr_comment.outputs.command == 'run-tests-manual-app' && 'manual_app') ||
         (needs.pr_comment.outputs.command == 'run-tests' && '') }}
       environmentName: CICD
       E2E_TESTS_NUMBER_PROCESSES: 1
@@ -178,8 +180,6 @@ jobs:
       MGMT_STORAGE_ACCOUNT_NAME: ${{ format('tre{0}mgmt', needs.pr_comment.outputs.prRefId) }}
       SWAGGER_UI_CLIENT_ID: ${{ secrets.SWAGGER_UI_CLIENT_ID }}
       TEST_APP_ID: ${{ secrets.TEST_APP_ID }}
-      TEST_WORKSPACE_APP_ID: ${{ secrets.TEST_WORKSPACE_APP_ID }}
-      TEST_WORKSPACE_APP_SECRET: "${{ secrets.TEST_WORKSPACE_APP_SECRET }}"
       TEST_ACCOUNT_CLIENT_ID: "${{ secrets.TEST_ACCOUNT_CLIENT_ID }}"
       TEST_ACCOUNT_CLIENT_SECRET: "${{ secrets.TEST_ACCOUNT_CLIENT_SECRET }}"
       TRE_ID: ${{ format('tre{0}', needs.pr_comment.outputs.prRefId) }}

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.1"

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",