From b2b3d126a0f18087f4d95b4ae56f91334339f712 Mon Sep 17 00:00:00 2001 From: marrobi Date: Mon, 3 Apr 2023 18:18:24 +0000 Subject: [PATCH 1/3] Remove AAD params from temapltes so configured in API. --- api_app/_version.py | 2 +- api_app/core/config.py | 3 + api_app/db/repositories/workspaces.py | 5 -- api_app/models/schemas/workspace.py | 1 - api_app/schemas/azuread.json | 12 --- api_app/schemas/azuread_auto.json | 43 ++++++++++ api_app/schemas/azuread_manual.json | 24 ++++++ api_app/services/aad_authentication.py | 23 ++--- api_app/services/schema_service.py | 8 +- .../test_workpaces_repository.py | 12 --- .../test_services/test_aad_access_service.py | 10 ++- .../test_services/test_schema_service.py | 2 +- core/terraform/api-webapp.tf | 6 +- core/terraform/variables.tf | 12 +++ core/version.txt | 2 +- e2e_tests/conftest.py | 12 ++- e2e_tests/test_performance.py | 2 - .../airlock-import-review/porter.yaml | 2 +- .../template_schema.json | 85 ------------------- templates/workspaces/base/porter.yaml | 2 +- .../workspaces/base/template_schema.json | 85 ------------------- templates/workspaces/unrestricted/porter.yaml | 2 +- .../unrestricted/template_schema.json | 85 ------------------- 23 files changed, 123 insertions(+), 317 deletions(-) delete mode 100644 api_app/schemas/azuread.json create mode 100644 api_app/schemas/azuread_auto.json create mode 100644 api_app/schemas/azuread_manual.json diff --git a/api_app/_version.py b/api_app/_version.py index 9e78220f94..f075dd36ab 100644 --- a/api_app/_version.py +++ b/api_app/_version.py @@ -1 +1 @@ -__version__ = "0.14.0" +__version__ = "0.14.1" diff --git a/api_app/core/config.py b/api_app/core/config.py index 0199bd47d8..1273f47fca 100644 --- a/api_app/core/config.py +++ b/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) diff --git a/api_app/db/repositories/workspaces.py b/api_app/db/repositories/workspaces.py index 60a5340c25..a56ffc4ed4 100644 --- a/api_app/db/repositories/workspaces.py +++ b/api_app/db/repositories/workspaces.py @@ -76,7 +76,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, @@ -84,7 +83,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)} @@ -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() 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 5cafab1228..d1fca64b62 100644 --- a/api_app/services/aad_authentication.py +++ b/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 diff --git a/api_app/services/schema_service.py b/api_app/services/schema_service.py index f01e20385d..fbfdbd50e0 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 @@ -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) 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 ce4960c54e..852ea03a59 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 @@ -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" 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 b5cdd04182..2cacc963c5 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 @@ -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 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 b8ba17ccac..574fd79b5b 100644 --- a/core/terraform/api-webapp.tf +++ b/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 { diff --git a/core/terraform/variables.tf b/core/terraform/variables.tf index f0ed9c495a..e3524018a4 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 4e27eedb8d..777f190df0 100644 --- a/core/version.txt +++ b/core/version.txt @@ -1 +1 @@ -__version__ = "0.7.7" +__version__ = "0.8.0" diff --git a/e2e_tests/conftest.py b/e2e_tests/conftest.py index 63d7214855..cd194d6cc9 100644 --- a/e2e_tests/conftest.py +++ b/e2e_tests/conftest.py @@ -33,7 +33,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 = "", @@ -48,16 +47,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 @@ -113,7 +111,7 @@ async def setup_test_workspace(verify) -> Tuple[str, str, str]: pre_created_workspace_id = config.TEST_WORKSPACE_ID # Set up 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 @@ -145,7 +143,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 @@ -174,7 +172,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 9f284fc235..0b1f33e9df 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 21717a8efa..cbad160e30 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.9.0 +version: 0.10.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 be6c1ccf01..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,78 +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." - }, - "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 - } - }, - "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 - }, - "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/" - ] - } - } - } - } - } - } } ], "actions": [], @@ -168,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 c686c2ecc9..52d2cb81be 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.1.0 +version: 1.2.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 2ae9c9d504..b3e58656cf 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": [ @@ -189,78 +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." - }, - "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 - } - }, - "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 - }, - "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/" - ] - } - } - } - } - } - } } ], "actions": [], @@ -281,7 +197,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 72ef618a9c..5430a3ab2b 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.8.1 +version: 0.9.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 40e051d568..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,78 +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." - }, - "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 - } - }, - "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 - }, - "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/" - ] - } - } - } - } - } - } } ], "actions": [], @@ -280,7 +196,6 @@ "app_service_plan_sku", "address_space_size", "address_spaces", - "auth_type", "create_aad_groups", "client_id", "client_secret", From 0c389921e216970210c8618caf5ac4c83f206ded Mon Sep 17 00:00:00 2001 From: marrobi Date: Mon, 3 Apr 2023 18:39:26 +0000 Subject: [PATCH 2/3] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 12f5d01bd6..84afe6686c 100644 --- a/CHANGELOG.md +++ b/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)) From 53ed8afa613521dfec466de1c7171f57caba6450 Mon Sep 17 00:00:00 2001 From: marrobi Date: Mon, 3 Apr 2023 19:02:49 +0000 Subject: [PATCH 3/3] Update workflows --- .github/actions/devcontainer_run_command/action.yml | 8 ++++++++ .github/workflows/deploy_tre.yml | 2 ++ .github/workflows/deploy_tre_branch.yml | 2 ++ .github/workflows/deploy_tre_reusable.yml | 8 ++++++++ .github/workflows/pr_comment_bot.yml | 2 ++ .../setup-instructions/cicd-pre-deployment-steps.md | 2 ++ 6 files changed, 24 insertions(+) diff --git a/.github/actions/devcontainer_run_command/action.yml b/.github/actions/devcontainer_run_command/action.yml index 0b8cc6e693..8622926376 100644 --- a/.github/actions/devcontainer_run_command/action.yml +++ b/.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' }}" \ diff --git a/.github/workflows/deploy_tre.yml b/.github/workflows/deploy_tre.yml index 179d7dc8e3..0204176815 100644 --- a/.github/workflows/deploy_tre.yml +++ b/.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 }} diff --git a/.github/workflows/deploy_tre_branch.yml b/.github/workflows/deploy_tre_branch.yml index 5205298b49..82552485ed 100644 --- a/.github/workflows/deploy_tre_branch.yml +++ b/.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) }} diff --git a/.github/workflows/deploy_tre_reusable.yml b/.github/workflows/deploy_tre_reusable.yml index df3e7d9e7d..a822f7381b 100644 --- a/.github/workflows/deploy_tre_reusable.yml +++ b/.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 }} diff --git a/.github/workflows/pr_comment_bot.yml b/.github/workflows/pr_comment_bot.yml index 9ec2ed01fe..5440db28a0 100644 --- a/.github/workflows/pr_comment_bot.yml +++ b/.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) }} 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 f58369b651..305d695003 100644 --- a/docs/tre-admins/setup-instructions/cicd-pre-deployment-steps.md +++ b/docs/tre-admins/setup-instructions/cicd-pre-deployment-steps.md @@ -84,6 +84,8 @@ In a previous [Setup Auth configuration](./setup-auth-entities.md) step authenti | `AAD_TENANT_ID` | Tenant id against which auth is performed. | | `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. |