diff --git a/.github/actions/devcontainer_run_command/action.yml b/.github/actions/devcontainer_run_command/action.yml index d4d9926e9d..301e9c0f9a 100644 --- a/.github/actions/devcontainer_run_command/action.yml +++ b/.github/actions/devcontainer_run_command/action.yml @@ -70,6 +70,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 @@ -205,6 +211,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' }}" \ diff --git a/.github/workflows/deploy_tre.yml b/.github/workflows/deploy_tre.yml index bddea6979f..317ba5487e 100644 --- a/.github/workflows/deploy_tre.yml +++ b/.github/workflows/deploy_tre.yml @@ -46,6 +46,10 @@ 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 }} MS_TEAMS_WEBHOOK_URI: ${{ secrets.MS_TEAMS_WEBHOOK_URI }} MGMT_STORAGE_ACCOUNT_NAME: ${{ secrets.MGMT_STORAGE_ACCOUNT_NAME }} diff --git a/.github/workflows/deploy_tre_branch.yml b/.github/workflows/deploy_tre_branch.yml index a6b65f9196..28bf46b6f6 100644 --- a/.github/workflows/deploy_tre_branch.yml +++ b/.github/workflows/deploy_tre_branch.yml @@ -77,6 +77,10 @@ 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) }} MS_TEAMS_WEBHOOK_URI: ${{ secrets.MS_TEAMS_WEBHOOK_URI }} MGMT_STORAGE_ACCOUNT_NAME: ${{ format('tre{0}mgmt', needs.prepare-not-main.outputs.refid) }} diff --git a/.github/workflows/deploy_tre_reusable.yml b/.github/workflows/deploy_tre_reusable.yml index 4b43636ab6..9a15f22640 100644 --- a/.github/workflows/deploy_tre_reusable.yml +++ b/.github/workflows/deploy_tre_reusable.yml @@ -135,6 +135,15 @@ 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 if [ "${{ secrets.MGMT_RESOURCE_GROUP_NAME }}" == '' ]; then echo "Missing secret: MGMT_RESOURCE_GROUP_NAME" && exit 1 fi @@ -352,6 +361,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 }}" KV_PURGE_PROTECTION_ENABLED: ${{ vars.KV_PURGE_PROTECTION_ENABLED || true }} CORE_APP_SERVICE_PLAN_SKU: ${{ vars.CORE_APP_SERVICE_PLAN_SKU }} diff --git a/.github/workflows/pr_comment_bot.yml b/.github/workflows/pr_comment_bot.yml index cfc14dc739..9604329a7f 100644 --- a/.github/workflows/pr_comment_bot.yml +++ b/.github/workflows/pr_comment_bot.yml @@ -174,6 +174,10 @@ 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) }} MS_TEAMS_WEBHOOK_URI: ${{ secrets.MS_TEAMS_WEBHOOK_URI }} MGMT_STORAGE_ACCOUNT_NAME: ${{ format('tre{0}mgmt', needs.pr_comment.outputs.prRefId) }} diff --git a/CHANGELOG.md b/CHANGELOG.md index b9fc942a5e..ef52a0b44d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ FEATURES: ENHANCEMENTS: +* Remove AAD params from templates and configure in API. ([#3407](https://github.com/microsoft/AzureTRE/pull/3407)) BUG FIXES: @@ -542,7 +543,6 @@ ENHANCEMENTS: * Update Porter (1.0.11), Docker (23.0.3), Terraform (1.4.5) ([#3430](https://github.com/microsoft/AzureTRE/issues/3430)) * Build, publish and register Databricks bundles in workflow ([#3447](https://github.com/microsoft/AzureTRE/issues/3447)) - BUG FIXES: * Fix ENABLE_SWAGGER configuration being ignored in CI ([#3355](https://github.com/microsoft/AzureTRE/pull/3355)) * Set yq output format when reading a json file ([#3441](https://github.com/microsoft/AzureTRE/pull/3441)) diff --git a/api_app/_version.py b/api_app/_version.py index 963aa2b5eb..fce337455e 100644 --- a/api_app/_version.py +++ b/api_app/_version.py @@ -1 +1 @@ -__version__ = "0.18.9" +__version__ = "0.18.10" diff --git a/api_app/core/config.py b/api_app/core/config.py index 5e338830d7..644cffd97a 100644 --- a/api_app/core/config.py +++ b/api_app/core/config.py @@ -66,10 +66,10 @@ API_CLIENT_SECRET: str = config("API_CLIENT_SECRET", default="") 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) - AIRLOCK_SAS_TOKEN_EXPIRY_PERIOD_IN_HOURS: int = config("AIRLOCK_SAS_TOKEN_EXPIRY_PERIOD_IN_HOURS", default=1) ENABLE_AIRLOCK_EMAIL_CHECK: bool = config("ENABLE_AIRLOCK_EMAIL_CHECK", cast=bool, default=False) - API_ROOT_SCOPE: str = f"api://{API_CLIENT_ID}/user_impersonation" diff --git a/api_app/db/repositories/workspaces.py b/api_app/db/repositories/workspaces.py index d66a875f7e..4d4a958230 100644 --- a/api_app/db/repositories/workspaces.py +++ b/api_app/db/repositories/workspaces.py @@ -91,7 +91,6 @@ 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, @@ -99,7 +98,6 @@ async def create_workspace_item(self, workspace_input: WorkspaceInCreate, auth_i 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)} @@ -121,9 +119,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() diff --git a/api_app/models/schemas/workspace.py b/api_app/models/schemas/workspace.py index 424c82b46a..6d692c377a 100644 --- a/api_app/models/schemas/workspace.py +++ b/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": "", "client_secret": "", "address_space_size": "small" diff --git a/api_app/schemas/azuread.json b/api_app/schemas/azuread.json deleted file mode 100644 index 8c4fa52189..0000000000 --- a/api_app/schemas/azuread.json +++ /dev/null @@ -1,12 +0,0 @@ -{ - "$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": { - - } -} diff --git a/api_app/schemas/azuread_auto.json b/api_app/schemas/azuread_auto.json new file mode 100644 index 0000000000..c84fef16a5 --- /dev/null +++ b/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/" + ] + } + } + } + } + } +} diff --git a/api_app/schemas/azuread_manual.json b/api_app/schemas/azuread_manual.json new file mode 100644 index 0000000000..7f670d249e --- /dev/null +++ b/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 + } + } +} diff --git a/api_app/services/aad_authentication.py b/api_app/services/aad_authentication.py index 81dd486a8f..532887838b 100644 --- a/api_app/services/aad_authentication.py +++ b/api_app/services/aad_authentication.py @@ -9,7 +9,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 @@ -42,9 +42,9 @@ class AzureADAuthorization(AccessService): def __init__(self, auto_error: bool = True, require_one_of_roles: Optional[list] = None): super(AzureADAuthorization, self).__init__( - authorizationUrl=f"{self.aad_instance}/{config.AAD_TENANT_ID}/oauth2/v2.0/authorize", - tokenUrl=f"{self.aad_instance}/{config.AAD_TENANT_ID}/oauth2/v2.0/token", - refreshUrl=f"{self.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 ) @@ -76,7 +76,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: logger.debug("Failed to decode using TRE API app registration (Invalid Signatrue)") raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail=strings.INVALID_SIGNATURE) @@ -171,7 +171,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"{self.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: @@ -196,12 +196,9 @@ def _get_token_key(self, key_id: str) -> str: # If there is no need to list all workspaces for a specific user, then Directory.ReadAll permissions are not required. @staticmethod def _get_msgraph_token() -> str: - scopes = [f"{MICROSOFT_GRAPH_URL}/.default"] - app = ConfidentialClientApplication(client_id=config.API_CLIENT_ID, client_credential=config.API_CLIENT_SECRET, authority=f"{config.AAD_AUTHORITY_URL}/{config.AAD_TENANT_ID}") - try: - result = app.acquire_token_silent(scopes=scopes, account=None) - except Exception: - result = None + scopes = ["https://graph.microsoft.com/.default"] + 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: logger.debug('No suitable token exists in cache, getting a new one from AAD') result = app.acquire_token_for_client(scopes=scopes) @@ -323,7 +320,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: @@ -392,13 +389,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 diff --git a/api_app/services/schema_service.py b/api_app/services/schema_service.py index 65e98012aa..d077aaa7f4 100644 --- a/api_app/services/schema_service.py +++ b/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 @@ -82,7 +83,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) diff --git a/api_app/tests_ma/test_db/test_repositories/test_workpaces_repository.py b/api_app/tests_ma/test_db/test_repositories/test_workpaces_repository.py index a53ff0bc94..77c487f408 100644 --- a/api_app/tests_ma/test_db/test_repositories/test_workpaces_repository.py +++ b/api_app/tests_ma/test_db/test_repositories/test_workpaces_repository.py @@ -272,18 +272,6 @@ async def test_create_workspace_item_raises_value_error_if_template_is_invalid(m 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" diff --git a/api_app/tests_ma/test_services/test_aad_access_service.py b/api_app/tests_ma/test_services/test_aad_access_service.py index eeb7754106..1275f2e58b 100644 --- a/api_app/tests_ma/test_services/test_aad_access_service.py +++ b/api_app/tests_ma/test_services/test_aad_access_service.py @@ -28,10 +28,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( @@ -68,8 +69,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": [ { @@ -93,7 +95,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 diff --git a/api_app/tests_ma/test_services/test_schema_service.py b/api_app/tests_ma/test_services/test_schema_service.py index 76cae2ea1a..52a1a563ec 100644 --- a/api_app/tests_ma/test_services/test_schema_service.py +++ b/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) diff --git a/core/terraform/api-webapp.tf b/core/terraform/api-webapp.tf index f2d4beab1a..da777c1e2e 100644 --- a/core/terraform/api-webapp.tf +++ b/core/terraform/api-webapp.tf @@ -49,6 +49,8 @@ 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 diff --git a/core/terraform/variables.tf b/core/terraform/variables.tf index 478325be7a..9abe948ade 100644 --- a/core/terraform/variables.tf +++ b/core/terraform/variables.tf @@ -100,6 +100,18 @@ variable "api_client_secret" { sensitive = true } +variable "auto_workspace_app_registration" { + type = bool + default = false + description = "If true, the API will automatically create an app registration for each workspace." +} + +variable "auto_workspace_group_creation" { + type = bool + default = false + description = "If true, the API will automatically create a group for each workspace." +} + variable "application_admin_client_id" { type = string description = "The client id (app id) of the registration in Azure AD for creating AAD Applications." diff --git a/core/version.txt b/core/version.txt index 1f4c4d43b2..17c1a6260b 100644 --- a/core/version.txt +++ b/core/version.txt @@ -1 +1 @@ -__version__ = "0.10.1" +__version__ = "0.10.2" diff --git a/docs/tre-admins/setup-instructions/cicd-pre-deployment-steps.md b/docs/tre-admins/setup-instructions/cicd-pre-deployment-steps.md index af61f035f7..6f86284c0a 100644 --- a/docs/tre-admins/setup-instructions/cicd-pre-deployment-steps.md +++ b/docs/tre-admins/setup-instructions/cicd-pre-deployment-steps.md @@ -93,8 +93,10 @@ In a previous [Setup Auth configuration](./setup-auth-entities.md) step authenti | Secret Name | Description | | -------- | ----------- | | `AAD_TENANT_ID` | Tenant id against which auth is performed. | - | `APPLICATION_ADMIN_CLIENT_ID`| This client will administer Microsoft Entra ID Applications for TRE | - | `APPLICATION_ADMIN_CLIENT_SECRET`| This client will administer Microsoft Entra ID Applications for TRE | + | `APPLICATION_ADMIN_CLIENT_ID`| This client will administer AAD Applications for TRE | + | `APPLICATION_ADMIN_CLIENT_SECRET`| This client will administer AAD Applications for TRE | + | `AUTO_WORKSPACE_APP_REGISTRATION`| App registration for workspaces will be auto created. | + | `AUTO_WORKSPACE_GROUP_CREATION`| Workspace groups will be auto created. | | `TEST_ACCOUNT_CLIENT_ID`| This will be created by default, but can be disabled by editing `/devops/scripts/create_aad_assets.sh`. This is the user that will run the tests for you | | `TEST_ACCOUNT_CLIENT_SECRET` | This will be created by default, but can be disabled by editing `/devops/scripts/create_aad_assets.sh`. This is the user that will run the tests for you | | `API_CLIENT_ID` | API application (client) ID. | diff --git a/e2e_tests/conftest.py b/e2e_tests/conftest.py index 7195a14588..04e41036e7 100644 --- a/e2e_tests/conftest.py +++ b/e2e_tests/conftest.py @@ -38,7 +38,6 @@ def verify(pytestconfig): async def create_or_get_test_workspace( - auth_type: str, verify: bool, template_name: str = resource_strings.BASE_WORKSPACE, pre_created_workspace_id: str = "", @@ -53,16 +52,15 @@ async def create_or_get_test_workspace( payload = { "templateName": template_name, "properties": { - "display_name": f"E2E {description} workspace ({auth_type} AAD)", + "display_name": f"E2E {description} workspace)", "description": f"{template_name} test workspace for E2E tests", - "auth_type": auth_type, "address_space_size": "small" } } if config.TEST_WORKSPACE_APP_PLAN != "": payload["properties"]["app_service_plan_sku"] = config.TEST_WORKSPACE_APP_PLAN - if auth_type == "Manual": + if not config.AUTO_WORKSPACE_APP_REGISTRATION: payload["properties"]["client_id"] = client_id payload["properties"]["client_secret"] = client_secret @@ -118,7 +116,7 @@ async def setup_test_workspace(verify) -> Tuple[str, str, str]: pre_created_workspace_id = config.TEST_WORKSPACE_ID # Set up - uses a pre created app reg as has appropriate roles assigned workspace_path, workspace_id = await create_or_get_test_workspace( - auth_type="Manual", verify=verify, pre_created_workspace_id=pre_created_workspace_id, client_id=config.TEST_WORKSPACE_APP_ID, client_secret=config.TEST_WORKSPACE_APP_SECRET) + verify=verify, pre_created_workspace_id=pre_created_workspace_id, client_id=config.TEST_WORKSPACE_APP_ID, client_secret=config.TEST_WORKSPACE_APP_SECRET) yield workspace_path, workspace_id @@ -150,7 +148,7 @@ async def setup_test_workspace_and_guacamole_service(setup_test_workspace, verif async def setup_test_aad_workspace(verify) -> Tuple[str, str, str]: pre_created_workspace_id = config.TEST_AAD_WORKSPACE_ID # Set up - workspace_path, workspace_id = await create_or_get_test_workspace(auth_type="Automatic", verify=verify, pre_created_workspace_id=pre_created_workspace_id) + workspace_path, workspace_id = await create_or_get_test_workspace(verify=verify, pre_created_workspace_id=pre_created_workspace_id) yield workspace_path, workspace_id @@ -179,7 +177,7 @@ async def disable_and_delete_tre_resource(resource_path, verify): async def setup_test_airlock_import_review_workspace_and_guacamole_service(verify) -> Tuple[str, str, str, str, str]: pre_created_workspace_id = config.TEST_AIRLOCK_IMPORT_REVIEW_WORKSPACE_ID # Set up - workspace_path, workspace_id = await create_or_get_test_workspace(auth_type="Automatic", verify=verify, template_name=resource_strings.AIRLOCK_IMPORT_REVIEW_WORKSPACE, pre_created_workspace_id=pre_created_workspace_id) + workspace_path, workspace_id = await create_or_get_test_workspace(verify=verify, template_name=resource_strings.AIRLOCK_IMPORT_REVIEW_WORKSPACE, pre_created_workspace_id=pre_created_workspace_id) admin_token = await get_admin_token(verify=verify) workspace_owner_token, _ = await get_workspace_auth_details(admin_token=admin_token, workspace_id=workspace_id, verify=verify) diff --git a/e2e_tests/test_performance.py b/e2e_tests/test_performance.py index 6c6d836d9d..f8643d4962 100644 --- a/e2e_tests/test_performance.py +++ b/e2e_tests/test_performance.py @@ -27,7 +27,6 @@ async def test_parallel_resource_creations(verify) -> None: "display_name": f'Perf Test Workspace {i}', "description": "workspace for perf test", "address_space_size": "small", - "auth_type": "Manual", "client_id": f"{config.TEST_WORKSPACE_APP_ID}" } } @@ -67,7 +66,6 @@ async def test_bulk_updates_to_ensure_each_resource_updated_in_series(verify) -> "display_name": "E2E test guacamole service", "description": "", "address_space_size": "small", - "auth_type": "Manual", "client_id": f"{config.TEST_WORKSPACE_APP_ID}", "client_secret": f"{config.TEST_WORKSPACE_APP_SECRET}" } diff --git a/templates/workspaces/airlock-import-review/porter.yaml b/templates/workspaces/airlock-import-review/porter.yaml index 5943388107..9ab7fe1685 100644 --- a/templates/workspaces/airlock-import-review/porter.yaml +++ b/templates/workspaces/airlock-import-review/porter.yaml @@ -1,7 +1,7 @@ --- schemaVersion: 1.0.0 name: tre-workspace-airlock-import-review -version: 0.12.16 +version: 0.13.0 description: "A workspace to do Airlock Data Import Reviews for Azure TRE" dockerfile: Dockerfile.tmpl registry: azuretre diff --git a/templates/workspaces/airlock-import-review/template_schema.json b/templates/workspaces/airlock-import-review/template_schema.json index e05a0d87e7..afb1ea9083 100644 --- a/templates/workspaces/airlock-import-review/template_schema.json +++ b/templates/workspaces/airlock-import-review/template_schema.json @@ -5,7 +5,6 @@ "title": "Airlock Import Review Workspace", "description": "This workspace template is intended to conduct Airlock Data Import reviews from.", "required": [ - "auth_type", "address_space_size" ], "authorizedRoles": [], @@ -38,17 +37,6 @@ "title": "Address spaces", "description": "Network address space to be used by the workspace.", "updateable": true - }, - "auth_type": { - "type": "string", - "title": "Workspace Authentication Type", - "description": "", - "default": "Automatic", - "enum": [ - "Automatic", - "Manual" - ], - "updateable": true } }, "allOf": [ @@ -77,82 +65,6 @@ "address_space" ] } - }, - { - "if": { - "properties": { - "auth_type": { - "const": "Manual" - } - }, - "required": [ - "auth_type" - ] - }, - "then": { - "properties": { - "client_id": { - "type": "string", - "title": "Application (Client) ID", - "description": "The AAD Application Registration ID for the workspace.", - "updateable": true - }, - "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, - "updateable": true - } - }, - "required": [ - "client_id" - ] - }, - "else": { - "properties": { - "create_aad_groups": { - "type": "boolean", - "title": "Create AAD Groups for each workspace role", - "description": "Create AAD Groups for the workspace roles. If this is set to true, the workspace will create new AAD Groups.", - "default": false, - "updateable": true - }, - "aad_redirect_uris": { - "type": "array", - "title": "AAD Redirect URIs", - "description": "Redirect URIs for the AAD app in Automatic Auth mode", - "updateable": true, - "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/" - ] - } - } - } - } - } - } } ], "actions": [], @@ -172,7 +84,6 @@ "app_service_plan_sku", "address_space_size", "address_spaces", - "auth_type", "create_aad_groups", "client_id", "client_secret", diff --git a/templates/workspaces/base/porter.yaml b/templates/workspaces/base/porter.yaml index f7a1802477..2bbe140c7b 100644 --- a/templates/workspaces/base/porter.yaml +++ b/templates/workspaces/base/porter.yaml @@ -1,7 +1,7 @@ --- schemaVersion: 1.0.0 name: tre-workspace-base -version: 1.5.3 +version: 1.6.0 description: "A base Azure TRE workspace" dockerfile: Dockerfile.tmpl registry: azuretre diff --git a/templates/workspaces/base/template_schema.json b/templates/workspaces/base/template_schema.json index 3d6cdf0e16..239c62a5f4 100644 --- a/templates/workspaces/base/template_schema.json +++ b/templates/workspaces/base/template_schema.json @@ -5,7 +5,6 @@ "title": "Base Workspace", "description": "This workspace template is the foundation for TRE workspaces.", "required": [ - "auth_type", "address_space_size" ], "authorizedRoles": [], @@ -50,17 +49,6 @@ "title": "Address spaces", "description": "Network address space to be used by the workspace.", "updateable": true - }, - "auth_type": { - "type": "string", - "title": "Workspace Authentication Type", - "description": "", - "default": "Automatic", - "enum": [ - "Automatic", - "Manual" - ], - "updateable": true } }, "allOf": [ @@ -191,82 +179,6 @@ "address_space" ] } - }, - { - "if": { - "properties": { - "auth_type": { - "const": "Manual" - } - }, - "required": [ - "auth_type" - ] - }, - "then": { - "properties": { - "client_id": { - "type": "string", - "title": "Application (Client) ID", - "description": "The AAD Application Registration ID for the workspace.", - "updateable": true - }, - "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, - "updateable": true - } - }, - "required": [ - "client_id" - ] - }, - "else": { - "properties": { - "create_aad_groups": { - "type": "boolean", - "title": "Create AAD Groups for each workspace role", - "description": "Create AAD Groups for the workspace roles. If this is set to true, the workspace will create new AAD Groups.", - "default": false, - "updateable": true - }, - "aad_redirect_uris": { - "type": "array", - "title": "AAD Redirect URIs", - "description": "Redirect URIs for the AAD app in Automatic Auth mode", - "updateable": true, - "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/" - ] - } - } - } - } - } - } } ], "actions": [], @@ -287,7 +199,6 @@ "app_service_plan_sku", "address_space_size", "address_space", - "auth_type", "create_aad_groups", "client_id", "client_secret", diff --git a/templates/workspaces/unrestricted/porter.yaml b/templates/workspaces/unrestricted/porter.yaml index fd3a07583c..bccd58e790 100644 --- a/templates/workspaces/unrestricted/porter.yaml +++ b/templates/workspaces/unrestricted/porter.yaml @@ -1,7 +1,7 @@ --- schemaVersion: 1.0.0 name: tre-workspace-unrestricted -version: 0.11.4 +version: 0.12.0 description: "A base Azure TRE workspace" dockerfile: Dockerfile.tmpl registry: azuretre diff --git a/templates/workspaces/unrestricted/template_schema.json b/templates/workspaces/unrestricted/template_schema.json index 3fbaa16a3a..f44ed09c51 100644 --- a/templates/workspaces/unrestricted/template_schema.json +++ b/templates/workspaces/unrestricted/template_schema.json @@ -5,7 +5,6 @@ "title": "Unrestricted Workspace", "description": "Workspace with unrestricted access to the Internet", "required": [ - "auth_type", "address_space_size" ], "authorizedRoles": [], @@ -50,17 +49,6 @@ "title": "Address spaces", "description": "Network address space to be used by the workspace.", "updateable": true - }, - "auth_type": { - "type": "string", - "title": "Workspace Authentication Type", - "description": "", - "default": "Automatic", - "enum": [ - "Automatic", - "Manual" - ], - "updateable": true } }, "allOf": [ @@ -189,82 +177,6 @@ "address_space" ] } - }, - { - "if": { - "properties": { - "auth_type": { - "const": "Manual" - } - }, - "required": [ - "auth_type" - ] - }, - "then": { - "properties": { - "client_id": { - "type": "string", - "title": "Application (Client) ID", - "description": "The AAD Application Registration ID for the workspace.", - "updateable": true - }, - "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, - "updateable": true - } - }, - "required": [ - "client_id" - ] - }, - "else": { - "properties": { - "create_aad_groups": { - "type": "boolean", - "title": "Create AAD Groups for each workspace role", - "description": "Create AAD Groups for the workspace roles. If this is set to true, the workspace will create new AAD Groups.", - "default": false, - "updateable": true - }, - "aad_redirect_uris": { - "type": "array", - "title": "AAD Redirect URIs", - "description": "Redirect URIs for the AAD app in Automatic Auth mode", - "updateable": true, - "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/" - ] - } - } - } - } - } - } } ], "actions": [], @@ -284,7 +196,6 @@ "app_service_plan_sku", "address_space_size", "address_spaces", - "auth_type", "create_aad_groups", "client_id", "client_secret",