diff --git a/CHANGELOG.md b/CHANGELOG.md index 519192722f..05649d4db0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,11 +3,13 @@ **BREAKING CHANGES & MIGRATIONS**: * Update Core Terraform Provider versions ([[#3919](https://github.com/microsoft/AzureTRE/issues/3919)]) - +* Introduction of config value `enable_airlock_email_check`, which defaults to `false`, this is a change in behaviour. If you require email addresses for users before an airlock request is created, set to `true`. ([#3904](https://github.com/microsoft/AzureTRE/issues/3904)) + FEATURES: ENHANCEMENTS: * Add KeyVault Purge Protection Variable ([#3922](https://github.com/microsoft/AzureTRE/issues/3922)) +* Make check for email addresses prior to an airlock request being created optional. ([#3904](https://github.com/microsoft/AzureTRE/issues/3904)) BUG FIXES: * Update Guacamole Linux VM Images to Ubuntu 22.04 LTS. Part of ([#3523](https://github.com/microsoft/AzureTRE/issues/3523)) diff --git a/api_app/_version.py b/api_app/_version.py index 782e3ece69..963aa2b5eb 100644 --- a/api_app/_version.py +++ b/api_app/_version.py @@ -1 +1 @@ -__version__ = "0.18.8" +__version__ = "0.18.9" diff --git a/api_app/core/config.py b/api_app/core/config.py index 95394d3229..5e338830d7 100644 --- a/api_app/core/config.py +++ b/api_app/core/config.py @@ -70,5 +70,6 @@ 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/resources/strings.py b/api_app/resources/strings.py index 9c2d7ff4b4..aaebcbd1ee 100644 --- a/api_app/resources/strings.py +++ b/api_app/resources/strings.py @@ -215,8 +215,8 @@ AIRLOCK_REQUEST_INVALID_STATUS = "Airlock request status is unknown." AIRLOCK_UNAUTHORIZED_TO_SA = "User is unauthorized to access airlock request files in its current status." AIRLOCK_NOT_ENABLED_IN_WORKSPACE = "Airlock is not enabled in this workspace." -AIRLOCK_NO_RESEARCHER_EMAIL = "There are no Workspace Researchers with an email address." -AIRLOCK_NO_AIRLOCK_MANAGER_EMAIL = "There are no Airlock Managers with an email address." +AIRLOCK_NO_EMAIL = "There are no Workspace Researchers or Workspace Owners in the workspace with an email address." +AIRLOCK_NO_AIRLOCK_MANAGER_EMAIL = "There are no Airlock Managers in the workspace with an email address." # Airlock Actions AIRLOCK_ACTION_REVIEW = "review" diff --git a/api_app/services/airlock.py b/api_app/services/airlock.py index fc33645afb..6e79d49b64 100644 --- a/api_app/services/airlock.py +++ b/api_app/services/airlock.py @@ -272,10 +272,10 @@ async def _handle_existing_review_resource(existing_resource: AirlockReviewUserR async def save_and_publish_event_airlock_request(airlock_request: AirlockRequest, airlock_request_repo: AirlockRequestRepository, user: User, workspace: Workspace): - # First check we have some email addresses so we can notify people. access_service = get_access_service() role_assignment_details = access_service.get_workspace_role_assignment_details(workspace) - check_email_exists(role_assignment_details) + if config.ENABLE_AIRLOCK_EMAIL_CHECK: + check_email_exists(role_assignment_details) try: logger.debug(f"Saving airlock request item: {airlock_request.id}") @@ -345,11 +345,11 @@ def get_timestamp() -> float: def check_email_exists(role_assignment_details: defaultdict(list)): - if "WorkspaceResearcher" not in role_assignment_details or not role_assignment_details["WorkspaceResearcher"]: - logger.error('Creating an airlock request but the researcher does not have an email address.') - raise HTTPException(status_code=status.HTTP_417_EXPECTATION_FAILED, detail=strings.AIRLOCK_NO_RESEARCHER_EMAIL) - if "AirlockManager" not in role_assignment_details or not role_assignment_details["AirlockManager"]: - logger.error('Creating an airlock request but the airlock manager does not have an email address.') + if not role_assignment_details.get("WorkspaceResearcher") and not role_assignment_details.get("WorkspaceOwner"): + logger.error(strings.AIRLOCK_NO_EMAIL) + raise HTTPException(status_code=status.HTTP_417_EXPECTATION_FAILED, detail=strings.AIRLOCK_NO_EMAIL) + if not role_assignment_details.get("AirlockManager"): + logger.error(strings.AIRLOCK_NO_AIRLOCK_MANAGER_EMAIL) raise HTTPException(status_code=status.HTTP_417_EXPECTATION_FAILED, detail=strings.AIRLOCK_NO_AIRLOCK_MANAGER_EMAIL) diff --git a/api_app/tests_ma/test_services/test_airlock.py b/api_app/tests_ma/test_services/test_airlock.py index c7fccec4d9..d8a26a1df8 100644 --- a/api_app/tests_ma/test_services/test_airlock.py +++ b/api_app/tests_ma/test_services/test_airlock.py @@ -317,6 +317,17 @@ async def test_check_email_exists_raises_417_if_email_not_present(role_assignmen assert ex.value.status_code == status.HTTP_417_EXPECTATION_FAILED +@pytest.mark.asyncio +@pytest.mark.parametrize('role_assignment_details_mock_return', [ + {"AirlockManager": ["manager@outlook.com"], "WorkspaceResearcher": ["researcher@outlook.com"], }, + {"AirlockManager": ["manager@outlook.com"], "WorkspaceOwner": ["researcher@outlook.com"], }, + {"AirlockManager": ["manager@outlook.com"], "WorkspaceResearcher": ["researcher@outlook.com"], "WorkspaceOwner": ["owner@outlook.com"]}]) +async def test_check_email_exists_passes_if_researcher_or_owner_and_airlock_manager_email_present(role_assignment_details_mock_return): + role_assignment_details = role_assignment_details_mock_return + result = check_email_exists(role_assignment_details) + assert result is None + + @pytest.mark.asyncio @pytest.mark.parametrize('email_mock_return', [{}, {"AirlockManager": ["owner@outlook.com"]}, @@ -324,6 +335,7 @@ async def test_check_email_exists_raises_417_if_email_not_present(role_assignmen {"WorkspaceResearcher": ["researcher@outlook.com"], "owner_emails": []}, {"WorkspaceResearcher": ["researcher@outlook.com"]}]) @patch("services.aad_authentication.AzureADAuthorization.get_workspace_role_assignment_details") +@patch('core.config.ENABLE_AIRLOCK_EMAIL_CHECK', "True") async def test_save_and_publish_event_airlock_request_raises_417_if_email_not_present(get_workspace_role_assignment_details_patched, email_mock_return): get_workspace_role_assignment_details_patched.return_value = email_mock_return @@ -338,6 +350,26 @@ async def test_save_and_publish_event_airlock_request_raises_417_if_email_not_pr assert ex.value.status_code == status.HTTP_417_EXPECTATION_FAILED +@pytest.mark.asyncio +@pytest.mark.parametrize('email_mock_return', [{}, + {"WorkspaceResearcher": [], "AirlockManager": []}]) +@patch("services.aad_authentication.AzureADAuthorization.get_workspace_role_assignment_details") +@patch("event_grid.event_sender.publish_event", return_value=AsyncMock()) +async def test_save_and_publish_event_airlock_notification_if_email_not_present(publish_event_mock, get_workspace_role_assignment_details_patched, email_mock_return, airlock_request_repo_mock): + + get_workspace_role_assignment_details_patched.return_value = email_mock_return + airlock_request_mock = sample_airlock_request() + airlock_request_repo_mock.save_item = AsyncMock() + + await save_and_publish_event_airlock_request( + airlock_request=airlock_request_mock, + airlock_request_repo=airlock_request_repo_mock, + user=create_test_user(), + workspace=sample_workspace()) + + assert publish_event_mock.call_count == 2 + + @pytest.mark.asyncio @patch("event_grid.helpers.EventGridPublisherClient", return_value=AsyncMock()) @patch("services.aad_authentication.AzureADAuthorization.get_workspace_role_assignment_details", return_value={"WorkspaceResearcher": ["researcher@outlook.com"], "WorkspaceOwner": ["owner@outlook.com"], "AirlockManager": ["manager@outlook.com"]}) diff --git a/config.sample.yaml b/config.sample.yaml index ad2aa6078c..21e757fe89 100644 --- a/config.sample.yaml +++ b/config.sample.yaml @@ -33,6 +33,10 @@ tre: enable_swagger: true enable_airlock_malware_scanning: true + # Set to true if want to ensure users have an email address before airlock request is created + # Used if rely on email notifications for governance purposes + # enable_airlock_email_check: true + # TODO: move to RP default with https://github.com/microsoft/AzureTRE/issues/2948 workspace_app_service_plan_sku: P1v2 # The TRE Web UI is deployed by default. diff --git a/config_schema.json b/config_schema.json index a8ae1711d6..d16ff8f775 100644 --- a/config_schema.json +++ b/config_schema.json @@ -69,6 +69,10 @@ "description": "Allow airlock malware scanning.", "type": "boolean" }, + "enable_airlock_email_check": { + "description": "Require email check for airlock.", + "type": "boolean" + }, "core_address_space": { "description": "TRE core address spaces.", "type": "string" diff --git a/core/terraform/api-webapp.tf b/core/terraform/api-webapp.tf index a38d6ed26f..f2d4beab1a 100644 --- a/core/terraform/api-webapp.tf +++ b/core/terraform/api-webapp.tf @@ -58,6 +58,7 @@ resource "azurerm_linux_web_app" "api" { RESOURCE_MANAGER_ENDPOINT = module.terraform_azurerm_environment_configuration.resource_manager_endpoint MICROSOFT_GRAPH_URL = module.terraform_azurerm_environment_configuration.microsoft_graph_endpoint STORAGE_ENDPOINT_SUFFIX = module.terraform_azurerm_environment_configuration.storage_suffix + ENABLE_AIRLOCK_EMAIL_CHECK = var.enable_airlock_email_check LOGGING_LEVEL = var.logging_level OTEL_RESOURCE_ATTRIBUTES = "service.name=api,service.version=${local.version}" OTEL_EXPERIMENTAL_RESOURCE_DETECTORS = "azure_app_service" diff --git a/core/terraform/variables.tf b/core/terraform/variables.tf index 93d8c3a765..478325be7a 100644 --- a/core/terraform/variables.tf +++ b/core/terraform/variables.tf @@ -174,6 +174,12 @@ variable "enable_airlock_malware_scanning" { description = "If False, Airlock requests will skip the malware scanning stage" } +variable "enable_airlock_email_check" { + type = bool + default = false + description = "If True, prior to airlock requests creation will check users have email addresses" +} + variable "firewall_sku" { description = "Azure Firewall SKU" type = string diff --git a/core/version.txt b/core/version.txt index a9b029e7c7..1f4c4d43b2 100644 --- a/core/version.txt +++ b/core/version.txt @@ -1 +1 @@ -__version__ = "0.10.0" \ No newline at end of file +__version__ = "0.10.1"