Remove AAD params from templates and configure in API.

.github/actions/devcontainer_run_command/action.yml

@@ -66,6 +66,12 @@ inputs:
   APPLICATION_ADMIN_CLIENT_SECRET:
     description: "The Client secret of an identity that can manage the AAD Applications."
     required: false
+  AUTO_WORKSPACE_APP_REGISTRATION:
+    description: "Determines whether the API will automatically register the workspace application."
+    required: false
+  AUTO_WORKSPACE_GROUP_CREATION:
+    description: "Determines whether the API will automatically create AAD groups for the workspace."
+    required: false
   ACR_NAME:
     description: "The Container Registry that holds our Research images."
     required: false
@@ -164,6 +170,8 @@ runs:
           -e TF_VAR_api_client_secret="${{ inputs.API_CLIENT_SECRET }}" \
           -e TF_VAR_application_admin_client_id="${{ inputs.APPLICATION_ADMIN_CLIENT_ID }}" \
           -e TF_VAR_application_admin_client_secret="${{ inputs.APPLICATION_ADMIN_CLIENT_SECRET }}" \
+          -e TF_VAR_auto_workspace_app_registration="${{ inputs.AUTO_WORKSPACE_APP_REGISTRATION }}" \
+          -e TF_VAR_auto_workspace_group_creation="${{ inputs.AUTO_WORKSPACE_GROUP_CREATION }}" \
           -e TF_VAR_arm_subscription_id="${{ fromJSON(inputs.AZURE_CREDENTIALS).subscriptionId }}" \
           -e TF_VAR_enable_swagger="${{ (inputs.ENABLE_SWAGGER != ''
             && inputs.ENABLE_SWAGGER) || 'false' }}" \

.github/workflows/deploy_tre.yml

@@ -42,6 +42,8 @@ jobs:
       API_CLIENT_SECRET: ${{ secrets.API_CLIENT_SECRET }}
       APPLICATION_ADMIN_CLIENT_ID: ${{ secrets.APPLICATION_ADMIN_CLIENT_ID }}
       APPLICATION_ADMIN_CLIENT_SECRET: ${{ secrets.APPLICATION_ADMIN_CLIENT_SECRET }}
+      AUTO_WORKSPACE_APP_REGISTRATION: ${{ secrets.AUTO_WORKSPACE_APP_REGISTRATION }}
+      AUTO_WORKSPACE_GROUP_CREATION: ${{ secrets.AUTO_WORKSPACE_GROUP_CREATION }}
       CORE_ADDRESS_SPACE: ${{ secrets.CORE_ADDRESS_SPACE }}
       LOCATION: ${{ secrets.LOCATION }}
       MGMT_RESOURCE_GROUP_NAME: ${{ secrets.MGMT_RESOURCE_GROUP_NAME }}

.github/workflows/deploy_tre_branch.yml

@@ -73,6 +73,8 @@ jobs:
       API_CLIENT_SECRET: ${{ secrets.API_CLIENT_SECRET }}
       APPLICATION_ADMIN_CLIENT_ID: ${{ secrets.APPLICATION_ADMIN_CLIENT_ID }}
       APPLICATION_ADMIN_CLIENT_SECRET: ${{ secrets.APPLICATION_ADMIN_CLIENT_SECRET }}
+      AUTO_WORKSPACE_APP_REGISTRATION: ${{ secrets.AUTO_WORKSPACE_APP_REGISTRATION }}
+      AUTO_WORKSPACE_GROUP_CREATION: ${{ secrets.AUTO_WORKSPACE_GROUP_CREATION }}
       CORE_ADDRESS_SPACE: ${{ secrets.CORE_ADDRESS_SPACE }}
       LOCATION: ${{ secrets.LOCATION }}
       MGMT_RESOURCE_GROUP_NAME: ${{ format('rg-tre{0}-mgmt', needs.prepare-not-main.outputs.refid) }}

.github/workflows/deploy_tre_reusable.yml

@@ -163,6 +163,12 @@ jobs:
           if [ "${{ secrets.APPLICATION_ADMIN_CLIENT_SECRET }}" == '' ]; then
             echo "Missing secret: APPLICATION_ADMIN_CLIENT_SECRET" && exit 1
           fi
+          if [ "${{ secrets.AUTO_WORKSPACE_APP_REGISTRATION }}" == '' ]; then
+            echo "Missing secret: AUTO_WORKSPACE_APP_REGISTRATION" && exit 1
+          fi
+          if [ "${{ secrets.AUTO_WORKSPACE_GROUP_CREATION }}" == '' ]; then
+            echo "Missing secret: AUTO_WORKSPACE_GROUP_CREATION" && exit 1
+          fi
           if [ "${{ secrets.CORE_ADDRESS_SPACE }}" == '' ]; then
             echo "Missing secret: CORE_ADDRESS_SPACE" && exit 1
           fi
@@ -375,6 +381,8 @@ jobs:
           API_CLIENT_SECRET: "${{ secrets.API_CLIENT_SECRET }}"
           APPLICATION_ADMIN_CLIENT_ID: "${{ secrets.APPLICATION_ADMIN_CLIENT_ID }}"
           APPLICATION_ADMIN_CLIENT_SECRET: "${{ secrets.APPLICATION_ADMIN_CLIENT_SECRET }}"
+          AUTO_WORKSPACE_APP_REGISTRATION: ${{ secrets.AUTO_WORKSPACE_APP_REGISTRATION }}
+          AUTO_WORKSPACE_GROUP_CREATION: ${{ secrets.AUTO_WORKSPACE_GROUP_CREATION }}
           STATEFUL_RESOURCES_LOCKED: "${{ github.ref == 'refs/heads/main' && inputs.prRef == '' && true || false }}"
           CORE_APP_SERVICE_PLAN_SKU: ${{ secrets.CORE_APP_SERVICE_PLAN_SKU }}
           RESOURCE_PROCESSOR_NUMBER_PROCESSES_PER_INSTANCE: ${{ secrets.RESOURCE_PROCESSOR_NUMBER_PROCESSES_PER_INSTANCE }}

.github/workflows/pr_comment_bot.yml

@@ -160,6 +160,8 @@ jobs:
       API_CLIENT_SECRET: ${{ secrets.API_CLIENT_SECRET }}
       APPLICATION_ADMIN_CLIENT_ID: ${{ secrets.APPLICATION_ADMIN_CLIENT_ID }}
       APPLICATION_ADMIN_CLIENT_SECRET: ${{ secrets.APPLICATION_ADMIN_CLIENT_SECRET }}
+      AUTO_WORKSPACE_APP_REGISTRATION: ${{ secrets.AUTO_WORKSPACE_APP_REGISTRATION }}
+      AUTO_WORKSPACE_GROUP_CREATION: ${{ secrets.AUTO_WORKSPACE_GROUP_CREATION }}
       CORE_ADDRESS_SPACE: ${{ secrets.CORE_ADDRESS_SPACE }}
       LOCATION: ${{ secrets.LOCATION }}
       MGMT_RESOURCE_GROUP_NAME: ${{ format('rg-tre{0}-mgmt', needs.pr_comment.outputs.prRefId) }}

CHANGELOG.md

@@ -8,6 +8,7 @@ FEATURES:
 
 ENHANCEMENTS:
 * Added 'availableUpgrades' field to Resources in GET/GET all Resources endpoints. The field indicates whether there are template versions that a resource can be upgraded to [#3234](https://github.com/microsoft/AzureTRE/pull/3234)
+* Remove AAD params from templates and configure in API. ([#3407](https://github.com/microsoft/AzureTRE/pull/3407))
 
 BUG FIXES:
 * Fix ENABLE_SWAGGER configuration being ignored in CI ([#3355](https://github.com/microsoft/AzureTRE/pull/3355))

api_app/_version.py

@@ -1 +1 @@
-__version__ = "0.14.0"
+__version__ = "0.14.1"

api_app/core/config.py

@@ -52,6 +52,9 @@
 SWAGGER_UI_CLIENT_ID: str = config("SWAGGER_UI_CLIENT_ID", default="")
 AAD_TENANT_ID: str = config("AAD_TENANT_ID", default="")
 
+AUTO_WORKSPACE_APP_REGISTRATION: bool = config("AUTO_WORKSPACE_APP_REGISTRATION", cast=bool, default=False)
+AUTO_WORKSPACE_GROUP_REGISTRATION: bool = config("AUTO_WORKSPACE_GROUP_REGISTRATION", cast=bool, default=False)
+
 AAD_INSTANCE: str = config("AAD_INSTANCE", default="https://login.microsoftonline.com")
 API_AUDIENCE: str = config("API_AUDIENCE", default=API_CLIENT_ID)
 

api_app/db/repositories/workspaces.py

@@ -76,15 +76,13 @@ 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)}
@@ -106,9 +104,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

@@ -82,7 +82,6 @@ class Config:
                 "properties": {
                     "display_name": "the workspace display name",
                     "description": "workspace description",
-                    "auth_type": "Manual",
                     "client_id": "<WORKSPACE_CLIENT_ID>",
                     "client_secret": "<WORKSPACE_CLIENT_SECRET>",
                     "address_space_size": "small"

api_app/schemas/azuread_auto.json

@@ -0,0 +1,43 @@
+{
+    "$schema": "http://json-schema.org/draft-07/schema",
+    "$id": "https://github.com/microsoft/AzureTRE/schema/azuread.json",
+    "type": "object",
+    "title": "Azure AD Authorisation Schema",
+    "default": {},
+    "required": [
+    ],
+    "properties": {
+      "aad_redirect_uris": {
+        "type": "array",
+        "title": "AAD Redirect URIs",
+        "description": "Redirect URIs for the AAD app in Automatic Auth mode",
+        "items": {
+          "title": "items",
+          "type": "object",
+          "required": [
+            "name",
+            "value"
+          ],
+          "properties": {
+            "name": {
+              "title": "name",
+              "type": "string",
+              "description": "Redirect URI Name",
+              "examples": [
+                "My Redirect URI"
+              ],
+              "pattern": "^.*$"
+            },
+            "value": {
+              "title": "value",
+              "type": "string",
+              "description": "Redirect URI Value",
+              "examples": [
+                "https://a-domain-name.com/oauth/"
+              ]
+            }
+          }
+        }
+      }
+    }
+}

api_app/schemas/azuread_manual.json

@@ -0,0 +1,24 @@
+{
+    "$schema": "http://json-schema.org/draft-07/schema",
+    "$id": "https://github.com/microsoft/AzureTRE/schema/azuread.json",
+    "type": "object",
+    "title": "Azure AD Authorisation Schema",
+    "default": {},
+    "required": [
+      "client_id",
+      "client_secret"
+    ],
+    "properties": {
+      "client_id": {
+        "type": "string",
+        "title": "Application (Client) ID",
+        "description": "The AAD Application Registration ID for the workspace."
+      },
+      "client_secret": {
+        "type": "string",
+        "title": "Application (Client) Secret",
+        "description": "The AAD Application Registration secret for the workspace. This value will be stored in the Workspace Key Vault.",
+        "sensitive": true
+      }
+    }
+}

api_app/services/aad_authentication.py

@@ -11,7 +11,7 @@
 from msal import ConfidentialClientApplication
 
 from services.access_service import AccessService, AuthConfigValidationError
-from core import config
+from core.config import AAD_INSTANCE, AAD_TENANT_ID, API_AUDIENCE, API_CLIENT_ID, API_CLIENT_SECRET, AUTO_WORKSPACE_APP_REGISTRATION, AUTO_WORKSPACE_GROUP_REGISTRATION
 from db.errors import EntityDoesNotExist
 from models.domain.authentication import User, RoleAssignment
 from models.domain.workspace import Workspace, WorkspaceRole
@@ -36,9 +36,9 @@ class AzureADAuthorization(AccessService):
 
     def __init__(self, auto_error: bool = True, require_one_of_roles: Optional[list] = None):
         super(AzureADAuthorization, self).__init__(
-            authorizationUrl=f"{config.AAD_INSTANCE}/{config.AAD_TENANT_ID}/oauth2/v2.0/authorize",
-            tokenUrl=f"{config.AAD_INSTANCE}/{config.AAD_TENANT_ID}/oauth2/v2.0/token",
-            refreshUrl=f"{config.AAD_INSTANCE}/{config.AAD_TENANT_ID}/oauth2/v2.0/token",
+            authorizationUrl=f"{AAD_INSTANCE}/{AAD_TENANT_ID}/oauth2/v2.0/authorize",
+            tokenUrl=f"{AAD_INSTANCE}/{AAD_TENANT_ID}/oauth2/v2.0/token",
+            refreshUrl=f"{AAD_INSTANCE}/{AAD_TENANT_ID}/oauth2/v2.0/token",
             scheme_name="oauth2",
             auto_error=auto_error
         )
@@ -70,7 +70,7 @@ async def __call__(self, request: Request) -> User:
         # Try TRE API app registration if appropriate
         if decoded_token is None and any(role in self.require_one_of_roles for role in self.TRE_CORE_ROLES):
             try:
-                decoded_token = self._decode_token(token, config.API_AUDIENCE)
+                decoded_token = self._decode_token(token, API_AUDIENCE)
             except jwt.exceptions.InvalidSignatureError:
                 logging.debug("Failed to decode using TRE API app registration (Invalid Signatrue)")
                 raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail=strings.INVALID_SIGNATURE)
@@ -166,7 +166,7 @@ def _get_token_key(self, key_id: str) -> str:
         Rather tha use PyJWKClient.get_signing_key_from_jwt every time, we'll get all the keys from AAD and cache them.
         """
         if key_id not in AzureADAuthorization._jwt_keys:
-            response = requests.get(f"{config.AAD_INSTANCE}/{config.AAD_TENANT_ID}/v2.0/.well-known/openid-configuration")
+            response = requests.get(f"{AAD_INSTANCE}/{AAD_TENANT_ID}/v2.0/.well-known/openid-configuration")
             aad_metadata = response.json() if response.ok else None
             jwks_uri = aad_metadata['jwks_uri'] if aad_metadata and 'jwks_uri' in aad_metadata else None
             if jwks_uri:
@@ -188,7 +188,7 @@ def _get_token_key(self, key_id: str) -> str:
     @staticmethod
     def _get_msgraph_token() -> str:
         scopes = ["https://graph.microsoft.com/.default"]
-        app = ConfidentialClientApplication(client_id=config.API_CLIENT_ID, client_credential=config.API_CLIENT_SECRET, authority=f"{config.AAD_INSTANCE}/{config.AAD_TENANT_ID}")
+        app = ConfidentialClientApplication(client_id=API_CLIENT_ID, client_credential=API_CLIENT_SECRET, authority=f"{AAD_INSTANCE}/{AAD_TENANT_ID}")
         result = app.acquire_token_silent(scopes=scopes, account=None)
         if not result:
             logging.debug('No suitable token exists in cache, getting a new one from AAD')
@@ -303,7 +303,7 @@ def _get_batch_users_by_role_assignments_body(self, roles_graph_data):
 
     # 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.
+    # If the AUTO_WORKSPACE_APP_REGISTRATION is `True`, 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:
@@ -372,13 +372,16 @@ 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):
+        if not AUTO_WORKSPACE_APP_REGISTRATION and "client_id" not in data:
             raise AuthConfigValidationError(strings.ACCESS_PLEASE_SUPPLY_CLIENT_ID)
 
         auth_info = {}
+        auth_info["register_aad_application"] = AUTO_WORKSPACE_APP_REGISTRATION
+        auth_info["create_aad_groups"] = AUTO_WORKSPACE_GROUP_REGISTRATION
+
         # 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":
+        if not AUTO_WORKSPACE_APP_REGISTRATION:
             auth_info = self._get_app_auth_info(data["client_id"])
 
             # Check we've get all our required roles

api_app/services/schema_service.py

@@ -1,3 +1,4 @@
+from core.config import AUTO_WORKSPACE_APP_REGISTRATION
 import json
 from pathlib import Path
 from typing import List, Dict, Tuple
@@ -74,7 +75,12 @@ def enrich_workspace_template(template, is_update: bool = False) -> dict:
         [Dict]: [Enriched template with all required and system properties added]
     """
     workspace_default_properties = read_schema('workspace.json')
-    azure_ad_properties = read_schema('azuread.json')
+
+    if AUTO_WORKSPACE_APP_REGISTRATION:
+        azure_ad_properties = read_schema('azuread_auto.json')
+    else:
+        azure_ad_properties = read_schema('azuread_manual.json')
+
     return enrich_template(template, [workspace_default_properties, azure_ad_properties], is_update=is_update)
 
 

api_app/tests_ma/test_db/test_repositories/test_workpaces_repository.py

@@ -259,18 +259,6 @@ async def test_create_workspace_item_raises_value_error_if_template_is_invalid(v
         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
-
-
 def test_workspace_owner_is_set_if_not_present_in_workspace_properties(workspace_repo):
     dictToTest = {}
     expected_object_id = "Expected"

api_app/tests_ma/test_services/test_aad_access_service.py

@@ -26,10 +26,11 @@ def __init__(self, principal_id, members):
         self.members = members
 
 
-def test_extract_workspace__raises_error_if_client_id_not_available():
+@patch("core.config.AUTO_WORKSPACE_APP_REGISTRATION", return_value=False)
+def test_extract_workspace__raises_error_if_client_id_not_available(config_mock):
     access_service = AzureADAuthorization()
     with pytest.raises(AuthConfigValidationError):
-        access_service.extract_workspace_auth_information(data={"auth_type": "Manual"})
+        access_service.extract_workspace_auth_information(data={})
 
 
 @patch(
@@ -66,8 +67,9 @@ def test_extract_workspace__raises_error_if_graph_data_is_invalid(
         access_service.extract_workspace_auth_information(data={"client_id": "1234"})
 
 
+@patch("core.config.AUTO_WORKSPACE_APP_REGISTRATION", return_value=False)
 @patch("services.aad_authentication.AzureADAuthorization._get_app_sp_graph_data")
-def test_extract_workspace__returns_sp_id_and_roles(get_app_sp_graph_data_mock):
+def test_extract_workspace__returns_sp_id_and_roles(get_app_sp_graph_data_mock, config_mock):
     get_app_sp_graph_data_mock.return_value = {
         "value": [
             {
@@ -91,7 +93,7 @@ def test_extract_workspace__returns_sp_id_and_roles(get_app_sp_graph_data_mock):
 
     access_service = AzureADAuthorization()
     actual_auth_info = access_service.extract_workspace_auth_information(
-        data={"auth_type": "Manual", "client_id": "1234"}
+        data={"client_id": "1234"}
     )
 
     assert actual_auth_info == expected_auth_info

api_app/tests_ma/test_services/test_schema_service.py

@@ -15,7 +15,7 @@ def test_enrich_workspace_template_enriches_with_workspace_defaults_and_aad(enri
 
     services.schema_service.enrich_workspace_template(workspace_template)
 
-    read_schema_mock.assert_has_calls([call('workspace.json'), call('azuread.json')])
+    read_schema_mock.assert_has_calls([call('workspace.json'), call('azuread_manual.json')])
     enrich_template_mock.assert_called_once_with(workspace_template, [default_props, aad_props], is_update=False)
 
 

core/terraform/api-webapp.tf

@@ -49,10 +49,12 @@ resource "azurerm_linux_web_app" "api" {
     "AAD_TENANT_ID"                                  = "@Microsoft.KeyVault(SecretUri=${azurerm_key_vault_secret.auth_tenant_id.id})"
     "API_CLIENT_ID"                                  = "@Microsoft.KeyVault(SecretUri=${azurerm_key_vault_secret.api_client_id.id})"
     "API_CLIENT_SECRET"                              = "@Microsoft.KeyVault(SecretUri=${azurerm_key_vault_secret.api_client_secret.id})"
+    "AUTO_WORKSPACE_APP_REGISTRATION"                = var.auto_workspace_app_registration
+    "AUTO_WORKSPACE_GROUP_CREATION"                  = var.auto_workspace_group_creation
     "RESOURCE_GROUP_NAME"                            = azurerm_resource_group.core.name
     "SUBSCRIPTION_ID"                                = data.azurerm_subscription.current.subscription_id
-    CORE_ADDRESS_SPACE                               = var.core_address_space
-    TRE_ADDRESS_SPACE                                = var.tre_address_space
+    "CORE_ADDRESS_SPACE"                             = var.core_address_space
+    "TRE_ADDRESS_SPACE"                              = var.tre_address_space
   }
 
   identity {