diff --git a/data_safe_haven/commands/users.py b/data_safe_haven/commands/users.py index fe413fa781..8c8b232ceb 100644 --- a/data_safe_haven/commands/users.py +++ b/data_safe_haven/commands/users.py @@ -5,6 +5,7 @@ import typer +from data_safe_haven import console from data_safe_haven.administration.users import UserHandler from data_safe_haven.config import ContextManager, DSHPulumiConfig, SHMConfig, SREConfig from data_safe_haven.exceptions import DataSafeHavenError @@ -120,9 +121,9 @@ def register( # Load SHMConfig try: shm_config = SHMConfig.from_remote(context) - except DataSafeHavenError: + except DataSafeHavenError as exc: logger.error("Have you deployed the SHM?") - raise + raise typer.Exit(1) from exc # Load Pulumi config pulumi_config = DSHPulumiConfig.from_remote(context) @@ -132,7 +133,7 @@ def register( if sre_config.name not in pulumi_config.project_names: msg = f"Could not load Pulumi settings for '{sre_config.name}'. Have you deployed the SRE?" logger.error(msg) - raise DataSafeHavenError(msg) + raise typer.Exit(1) # Load GraphAPI graph_api = GraphApi.from_scopes( @@ -146,16 +147,29 @@ def register( # List users users = UserHandler(context, graph_api) - available_usernames = users.get_usernames_entra_id() + available_users = users.entra_users.list() + user_dict = { + user.preferred_username.split("@")[0]: user.preferred_username.split("@")[1] + for user in available_users + } usernames_to_register = [] for username in usernames: - if username in available_usernames: - usernames_to_register.append(username) + if user_domain := user_dict.get(username): + if shm_config.shm.fqdn not in user_domain: + console.print( + f"User [green]'{username}[/green]'s principal domain name is [blue]'{user_domain}'[/blue].\n" + f"SRE [yellow]'{sre}'[/yellow] belongs to SHM domain [blue]'{shm_config.shm.fqdn}'[/blue]." + ) + logger.error( + "The user's principal domain name must match the domain of the SRE to be registered." + ) + else: + usernames_to_register.append(username) else: logger.error( f"Username '{username}' does not belong to this Data Safe Haven deployment." - " Please use 'dsh users add' to create it." ) + console.print("Please use 'dsh users add' to create this user.") users.register(sre_config.name, usernames_to_register) except DataSafeHavenError as exc: logger.critical(f"Could not register Data Safe Haven users with SRE '{sre}'.") @@ -259,8 +273,8 @@ def unregister( else: logger.error( f"Username '{username}' does not belong to this Data Safe Haven deployment." - " Please use 'dsh users add' to create it." ) + console.print("Please use 'dsh users add' to create it.") for group_name in ( f"{sre_config.name} Users", f"{sre_config.name} Privileged Users", diff --git a/docs/source/deployment/security_checklist.md b/docs/source/deployment/security_checklist.md index b2f8308181..b96cdc38da 100644 --- a/docs/source/deployment/security_checklist.md +++ b/docs/source/deployment/security_checklist.md @@ -8,6 +8,7 @@ Organisations are responsible for making their own decisions about the suitabili ``` In this check list we aim to evaluate our deployment against the {ref}`security configuration ` that we apply at the Alan Turing Institute. +A copy of this template in Markdown format is {download}`available for download `. The security checklist currently focuses on checks that can evaluate these security requirements for {ref}`policy_tier_2` (or greater) SREs (with some steps noted as specific to a tier): ## How to use this checklist diff --git a/docs/source/deployment/security_checklist/security_checklist_template.md b/docs/source/deployment/security_checklist/security_checklist_template.md index 5c1a64a119..6233e56f5c 100644 --- a/docs/source/deployment/security_checklist/security_checklist_template.md +++ b/docs/source/deployment/security_checklist/security_checklist_template.md @@ -1,139 +1,136 @@ # Security checklist -Running on SHM/SREs deployed using commit +Running on SHM/SREs deployed using commit xxxxxx ## Summary -- :white_check_mark: tests passed -- :partly_sunny: tests partially passed (see below for more details) -- :fast_forward: tests skipped (see below for more details) -- :x: tests failed (see below for more details) +- :white_check_mark: x tests passed +- :partly_sunny: x tests partially passed (see below for more details) +- :fast_forward: x tests skipped (see below for more details) +- :x: x tests failed (see below for more details) ## Details -Some security checks were skipped because: - -- … -- … +- Any additional details as referred to in the summary ### Multifactor Authentication and Password strength -- :white_check_mark: Check: Users can reset their own password -- Verify that: User can reset their own password - +- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Check: Users can reset their own password + - Verify that: User can reset their own password -- :white_check_mark: Check: non-registered users cannot connect to any SRE workspace - - Verify that: User can authenticate but cannot see any workspaces +- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Check: non-registered users cannot connect to any SRE workspace + - Verify that: User can authenticate but cannot see any workspaces -- :white_check_mark: Check: registered users can see SRE workspaces - - Verify that: User can authenticate and can see workspaces +- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Check: registered users can see SRE workspaces + - Verify that: User can authenticate and can see workspaces -- :white_check_mark: Check: Authenticated user can access workspaces - - Verify that: You can connect to any workspace +- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Check: Authenticated user can access workspaces + - Verify that: You can connect to any workspace ### Isolated Network -- :white_check_mark: Fail to connect to the internet from a workspace - - Verify that: Browsing to the service fails +- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Fail to connect to the internet from a workspace + - Verify that: Browsing to the service fails - - Verify that: You cannot access the service using curl + - Verify that: You cannot access the service using curl - - Verify: You cannot get the IP address for the service using nslookup + - Verify: You cannot get the IP address for the service using nslookup ### User devices -#### Tier 2 +#### Tier 2: - Connect to the environment using an allowed IP address and credentials - - :white_check_mark: Verify that: Connection succeeds + - :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Verify that: Connection succeeds - Connect to the environment from an IP address that is not allowed but with correct credentials - - :white_check_mark: Verify that: Connection fails + - :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Verify that: Connection fails -#### Tier 3 +#### Tier 3: - All managed devices should be provided by a known IT team at an approved organisation. - - :fast_forward: Verify that: the IT team of the approved organisation take responsibility for managing the device. - - :fast_forward: Verify that: the user does not have administrator permissions on the device. - - :fast_forward: Verify that: allowed IP addresses are exclusive to managed devices. + - :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Verify that: the IT team of the approved organisation take responsibility for managing the device. + - :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Verify that: the user does not have administrator permissions on the device. + - :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Verify that: allowed IP addresses are exclusive to managed devices. - Connect to the environment using an allowed IP address and credentials - - :fast_forward: Verify that: Connection succeeds + - :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Verify that: Connection succeeds - Connect to the environment from an IP address that is not allowed but with correct credentials - - :fast_forward: Verify that: Connection fails + - :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Verify that: Connection fails -#### Tiers 2 and above +#### Tiers 2 and above: -- :white_check_mark: Network rules permit access only from allow-listed IP addresses - - In the Azure portal navigate to the Guacamole application gateway NSG for this SRE shm--sre--nsg-application-gateway - - Verify that: the NSG has network rules allowing Inbound access from allowed IP addresses only +- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Network rules permit access only from allow-listed IP addresses + - In the Azure portal navigate to the Guacamole application gateway NSG for this SRE shm--sre--nsg-application-gateway + - Verify that: the NSG has network rules allowing Inbound access from allowed IP addresses only -- :white_check_mark: all other NSGs have an inbound Deny All rule and no higher priority rule allowing inbound connections from outside the Virtual Network +- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: all other NSGs have an inbound Deny All rule and no higher priority rule allowing inbound connections from outside the Virtual Network ### Physical security #### Tier 3 only - Attempt to connect to the Tier 3 SRE web client from home using a managed device and the correct VPN connection and credentials. - - :fast_forward: Verify that: connection fails. + - :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Verify that: connection fails. - Attempt to connect from research office using a managed device and the correct VPN connection and credentials. - - :fast_forward: Verify that: connection succeeds - - :fast_forward: Verify that: the network IP ranges corresponding to the research spaces correspond to those allowed by storage account firewall - - :fast_forward: Verify that: physical measures such as screen adaptions or desk partitions are present if risk of visual eavesdropping is high + - :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Verify that: connection succeeds + - :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Verify that: the network IP ranges corresponding to the research spaces correspond to those allowed by storage account firewall + - :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Verify that: physical measures such as screen adaptions or desk partitions are present if risk of visual eavesdropping is high ### Remote connections -- :white_check_mark: Unable to connect as a user to the remote desktop server via SSH - - Verify that: SSH login by fully-qualified domain name fails +- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Unable to connect as a user to the remote desktop server via SSH + - Verify that: SSH login by fully-qualified domain name fails - - Verify that: SSH login by public IP address fails + - Verify that: SSH login by public IP address fails -- :white_check_mark: Verify that: the remote desktop web client application gateway (shm--sre--ag-entrypoint) and the firewall are the only SRE resources with public IP addresses. + +- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Verify that: the remote desktop web client application gateway (shm--sre--ag-entrypoint) and the firewall are the only SRE resources with public IP addresses. ### Copy-and-paste - Unable to paste text from a local device into a workspace - - :white_check_mark: Verify that: paste fails + - :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Verify that: paste fails - Unable to copy text from a workspace to a local device - - :white_check_mark: Verify that: paste fails + - :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Verify that: paste fails ### Data ingress - Check that the **System Manager** can send an upload token to the **Dataset Provider Representative** - - :white_check_mark: Verify that: the upload token is successfully created. - - :white_check_mark: Verify that: you are able to send this token using a secure mechanism. + - :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Verify that: the upload token is successfully created. + - :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Verify that: you are able to send this token using a secure mechanism. - Ensure that data ingress works only for connections from the accepted IP address range - - :white_check_mark: Verify that: writing succeeds by uploading a file - - :white_check_mark: Verify that: attempting to open or download any of the files results in the following error: "Failed to start transfer: Insufficient credentials" under the Activities pane at the bottom of the MS Azure Storage Explorer window. - - :white_check_mark: Verify that: the access token fails when using a device with a non-allowed IP address + - :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Verify that: writing succeeds by uploading a file + - :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Verify that: attempting to open or download any of the files results in the following error: "Failed to start transfer: Insufficient credentials" under the Activities pane at the bottom of the MS Azure Storage Explorer window. + - :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Verify that: the access token fails when using a device with a non-allowed IP address - Check that the upload fails if the token has expired - - :white_check_mark: Verify that: you can connect and write with the token during the duration - - :white_check_mark: Verify that: you cannot connect and write with the token after the duration has expired - - :white_check_mark: Verify that:the data ingress process works by uploading different kinds of files, e.g. data, images, scripts (if appropriate) + - :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Verify that: you can connect and write with the token during the duration + - :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Verify that: you cannot connect and write with the token after the duration has expired + - :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Verify that:the data ingress process works by uploading different kinds of files, e.g. data, images, scripts (if appropriate) ### Data egress - Confirm that a non-privileged user is able to read the different storage volumes and write to output - - :white_check_mark: Verify that: the `/mnt/output` volume exists and can be written to - - :white_check_mark: Verify that: the permissions of other storage volumes match that described in the user guide + - :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Verify that: the `/mnt/output` volume exists and can be written to + - :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Verify that: the permissions of other storage volumes match that described in the user guide - Confirm that System Manager can see and download files from output - - :white_check_mark: Verify that: you can see the files written to the `/mnt/output` storage volume. - - :white_check_mark: Verify that: a written file can be taken out of the environment via download + - :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Verify that: you can see the files written to the `/mnt/output` storage volume. + - :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Verify that: a written file can be taken out of the environment via download ### Software package repositories -#### Tier 2 +#### Tier 2: -- :white_check_mark: Can install any packages - - Verify that: pytz can be installed +- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Can install any packages + - Verify that: pytz can be installed - - Verify that: awscli can be installed + - Verify that: awscli can be installed -#### Tier 3 +#### Tier 3: -- :white_check_mark: Can install only allow-listed packages - - Verify: pytz can be installed +- :white_check_mark:/:partly_sunny:/:fast_forward:/:x: Can install only allow-listed packages + - Verify: pytz can be installed - - Verify: awscli cannot be installed + - Verify: awscli cannot be installed diff --git a/tests/commands/conftest.py b/tests/commands/conftest.py index d675398bfc..de60eb29d0 100644 --- a/tests/commands/conftest.py +++ b/tests/commands/conftest.py @@ -1,6 +1,8 @@ from pytest import fixture from typer.testing import CliRunner +from data_safe_haven.administration.users.entra_users import EntraUsers +from data_safe_haven.administration.users.research_user import ResearchUser from data_safe_haven.config import ( Context, ContextManager, @@ -260,3 +262,14 @@ def tmp_contexts_none(tmp_path, context_yaml): with open(config_file_path, "w") as f: f.write(context_yaml) return tmp_path + + +@fixture +def mock_entra_user_list(mocker): + test_user = ResearchUser( + given_name="Harry", + surname="Lime", + sam_account_name="harry.lime", + user_principal_name="harry.lime@acme.testing", + ) + mocker.patch.object(EntraUsers, "list", return_value=[test_user]) diff --git a/tests/commands/test_users.py b/tests/commands/test_users.py index c1b183c922..5c11e29cc9 100644 --- a/tests/commands/test_users.py +++ b/tests/commands/test_users.py @@ -52,6 +52,26 @@ def test_invalid_shm( assert result.exit_code == 1 assert "Have you deployed the SHM?" in result.stdout + def test_mismatched_domain( + self, + mock_graphapi_get_credential, # noqa: ARG002 + mock_pulumi_config_no_key_from_remote, # noqa: ARG002 + mock_shm_config_from_remote, # noqa: ARG002 + mock_sre_config_from_remote, # noqa: ARG002 + mock_entra_user_list, # noqa: ARG002 + runner, + tmp_contexts, # noqa: ARG002 + ): + result = runner.invoke( + users_command_group, ["register", "-u", "harry.lime", "sandbox"] + ) + + assert result.exit_code == 0 + assert ( + "principal domain name must match the domain of the SRE to be registered" + in result.stdout + ) + def test_invalid_sre( self, mock_pulumi_config_from_remote, # noqa: ARG002