From 850938bf75e75aa1d36d3ebe80f2ddcbb14d6714 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Wed, 8 May 2024 17:04:58 +0100 Subject: [PATCH 01/16] :truck: Rename AzureADApplication to EntraIDApplication --- .../infrastructure/components/__init__.py | 8 ++--- .../components/dynamic/__init__.py | 6 ++-- ...application.py => entra_id_application.py} | 30 +++++++++---------- .../infrastructure/programs/sre/identity.py | 8 ++--- .../programs/sre/remote_desktop.py | 8 ++--- 5 files changed, 30 insertions(+), 30 deletions(-) rename data_safe_haven/infrastructure/components/dynamic/{azuread_application.py => entra_id_application.py} (90%) diff --git a/data_safe_haven/infrastructure/components/__init__.py b/data_safe_haven/infrastructure/components/__init__.py index 6fcb8d3f9b..d2c7e2df04 100644 --- a/data_safe_haven/infrastructure/components/__init__.py +++ b/data_safe_haven/infrastructure/components/__init__.py @@ -12,12 +12,12 @@ WindowsVMComponentProps, ) from .dynamic import ( - AzureADApplication, - AzureADApplicationProps, BlobContainerAcl, BlobContainerAclProps, CompiledDsc, CompiledDscProps, + EntraIDApplication, + EntraIDApplicationProps, FileShareFile, FileShareFileProps, FileUpload, @@ -35,12 +35,12 @@ __all__ = [ "AutomationDscNode", "AutomationDscNodeProps", - "AzureADApplication", - "AzureADApplicationProps", "BlobContainerAcl", "BlobContainerAclProps", "CompiledDsc", "CompiledDscProps", + "EntraIDApplication", + "EntraIDApplicationProps", "FileShareFile", "FileShareFileProps", "FileUpload", diff --git a/data_safe_haven/infrastructure/components/dynamic/__init__.py b/data_safe_haven/infrastructure/components/dynamic/__init__.py index 4fdfb12dfc..ca1b86c731 100644 --- a/data_safe_haven/infrastructure/components/dynamic/__init__.py +++ b/data_safe_haven/infrastructure/components/dynamic/__init__.py @@ -1,18 +1,18 @@ -from .azuread_application import AzureADApplication, AzureADApplicationProps from .blob_container_acl import BlobContainerAcl, BlobContainerAclProps from .compiled_dsc import CompiledDsc, CompiledDscProps +from .entra_id_application import EntraIDApplication, EntraIDApplicationProps from .file_share_file import FileShareFile, FileShareFileProps from .file_upload import FileUpload, FileUploadProps from .remote_script import RemoteScript, RemoteScriptProps from .ssl_certificate import SSLCertificate, SSLCertificateProps __all__ = [ - "AzureADApplication", - "AzureADApplicationProps", "BlobContainerAcl", "BlobContainerAclProps", "CompiledDsc", "CompiledDscProps", + "EntraIDApplication", + "EntraIDApplicationProps", "FileShareFile", "FileShareFileProps", "FileUpload", diff --git a/data_safe_haven/infrastructure/components/dynamic/azuread_application.py b/data_safe_haven/infrastructure/components/dynamic/entra_id_application.py similarity index 90% rename from data_safe_haven/infrastructure/components/dynamic/azuread_application.py rename to data_safe_haven/infrastructure/components/dynamic/entra_id_application.py index e24c3f25d7..8391bc964a 100644 --- a/data_safe_haven/infrastructure/components/dynamic/azuread_application.py +++ b/data_safe_haven/infrastructure/components/dynamic/entra_id_application.py @@ -1,4 +1,4 @@ -"""Pulumi dynamic component for AzureAD applications.""" +"""Pulumi dynamic component for Entra ID applications.""" from contextlib import suppress from typing import Any @@ -12,8 +12,8 @@ from .dsh_resource_provider import DshResourceProvider -class AzureADApplicationProps: - """Props for the AzureADApplication class""" +class EntraIDApplicationProps: + """Props for the EntraIDApplication class""" def __init__( self, @@ -34,7 +34,7 @@ def __init__( self.web_redirect_url = web_redirect_url -class AzureADApplicationProvider(DshResourceProvider): +class EntraIDApplicationProvider(DshResourceProvider): @staticmethod def refresh(props: dict[str, Any]) -> dict[str, Any]: try: @@ -61,11 +61,11 @@ def refresh(props: dict[str, Any]) -> dict[str, Any]: ) return outs except Exception as exc: - msg = f"Failed to refresh application [green]{props['application_name']}[/] in AzureAD.\n{exc}" + msg = f"Failed to refresh application [green]{props['application_name']}[/] in Entra ID.\n{exc}" raise DataSafeHavenMicrosoftGraphError(msg) from exc def create(self, props: dict[str, Any]) -> CreateResult: - """Create new AzureAD application.""" + """Create new Entra ID application.""" outs = dict(**props) try: graph_api = GraphApi(auth_token=props["auth_token"], disable_logging=True) @@ -112,22 +112,22 @@ def create(self, props: dict[str, Any]) -> CreateResult: else "" ) except Exception as exc: - msg = f"Failed to create application [green]{props['application_name']}[/] in AzureAD.\n{exc}" + msg = f"Failed to create application [green]{props['application_name']}[/] in Entra ID.\n{exc}" raise DataSafeHavenMicrosoftGraphError(msg) from exc return CreateResult( - f"AzureADApplication-{props['application_name']}", + f"EntraIDApplication-{props['application_name']}", outs=outs, ) def delete(self, id_: str, props: dict[str, Any]) -> None: - """Delete an AzureAD application.""" + """Delete an Entra ID application.""" # Use `id` as a no-op to avoid ARG002 while maintaining function signature id(id_) try: graph_api = GraphApi(auth_token=props["auth_token"], disable_logging=True) graph_api.delete_application(props["application_name"]) except Exception as exc: - msg = f"Failed to delete application [green]{props['application_name']}[/] from AzureAD.\n{exc}" + msg = f"Failed to delete application [green]{props['application_name']}[/] from Entra ID.\n{exc}" raise DataSafeHavenMicrosoftGraphError(msg) from exc def diff( @@ -158,24 +158,24 @@ def update( updated = self.create(new_props) return UpdateResult(outs=updated.outs) except Exception as exc: - msg = f"Failed to update application [green]{new_props['application_name']}[/] in AzureAD.\n{exc}" + msg = f"Failed to update application [green]{new_props['application_name']}[/] in Entra ID.\n{exc}" raise DataSafeHavenMicrosoftGraphError(msg) from exc -class AzureADApplication(Resource): +class EntraIDApplication(Resource): application_id: Output[str] application_secret: Output[str] object_id: Output[str] - _resource_type_name = "dsh:common:AzureADApplication" # set resource type + _resource_type_name = "dsh:common:EntraIDApplication" # set resource type def __init__( self, name: str, - props: AzureADApplicationProps, + props: EntraIDApplicationProps, opts: ResourceOptions | None = None, ): super().__init__( - AzureADApplicationProvider(), + EntraIDApplicationProvider(), name, { "application_id": None, diff --git a/data_safe_haven/infrastructure/programs/sre/identity.py b/data_safe_haven/infrastructure/programs/sre/identity.py index 98701690d2..111f9d9fc5 100644 --- a/data_safe_haven/infrastructure/programs/sre/identity.py +++ b/data_safe_haven/infrastructure/programs/sre/identity.py @@ -10,8 +10,8 @@ get_ip_address_from_container_group, ) from data_safe_haven.infrastructure.components import ( - AzureADApplication, - AzureADApplicationProps, + EntraIDApplication, + EntraIDApplicationProps, LocalDnsRecordComponent, LocalDnsRecordProps, ) @@ -93,9 +93,9 @@ def __init__( ) # Define AzureAD application - aad_application = AzureADApplication( + aad_application = EntraIDApplication( f"{self._name}_aad_application", - AzureADApplicationProps( + EntraIDApplicationProps( application_name=props.aad_application_name, application_role_assignments=["User.Read.All", "GroupMember.Read.All"], application_secret_name="Apricot Authentication Secret", diff --git a/data_safe_haven/infrastructure/programs/sre/remote_desktop.py b/data_safe_haven/infrastructure/programs/sre/remote_desktop.py index 59eb637a0c..39f7747267 100644 --- a/data_safe_haven/infrastructure/programs/sre/remote_desktop.py +++ b/data_safe_haven/infrastructure/programs/sre/remote_desktop.py @@ -15,8 +15,8 @@ get_id_from_subnet, ) from data_safe_haven.infrastructure.components import ( - AzureADApplication, - AzureADApplicationProps, + EntraIDApplication, + EntraIDApplicationProps, FileShareFile, FileShareFileProps, PostgresqlDatabaseComponent, @@ -131,9 +131,9 @@ def __init__( ) # Define AzureAD application - aad_application = AzureADApplication( + aad_application = EntraIDApplication( f"{self._name}_aad_application", - AzureADApplicationProps( + EntraIDApplicationProps( application_name=props.aad_application_name, auth_token=props.aad_auth_token, web_redirect_url=props.aad_application_url, From 7d26f745fef67a77461dc4d4963f7928b166b4a2 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Wed, 8 May 2024 17:12:21 +0100 Subject: [PATCH 02/16] :truck: Updated identity and remote desktop components to use Entra ID --- .../programs/declarative_sre.py | 14 +++---- .../infrastructure/programs/sre/identity.py | 28 ++++++------- .../programs/sre/remote_desktop.py | 41 ++++++++++--------- 3 files changed, 43 insertions(+), 40 deletions(-) diff --git a/data_safe_haven/infrastructure/programs/declarative_sre.py b/data_safe_haven/infrastructure/programs/declarative_sre.py index f0f2e9be9e..a2fce52bd4 100644 --- a/data_safe_haven/infrastructure/programs/declarative_sre.py +++ b/data_safe_haven/infrastructure/programs/declarative_sre.py @@ -237,11 +237,11 @@ def __call__(self) -> None: "sre_identity", self.stack_name, SREIdentityProps( - aad_application_name=f"sre-{self.sre_name}-apricot", - aad_auth_token=self.graph_api_token, - aad_tenant_id=self.cfg.shm.aad_tenant_id, dns_resource_group_name=dns.resource_group.name, dns_server_ip=dns.ip_address, + entra_id_application_name=f"sre-{self.sre_name}-apricot", + entra_id_auth_token=self.graph_api_token, + entra_id_tenant_id=self.cfg.shm.aad_tenant_id, location=self.context.location, networking_resource_group_name=networking.resource_group.name, shm_fqdn=self.cfg.shm.fqdn, @@ -274,14 +274,14 @@ def __call__(self) -> None: "sre_remote_desktop", self.stack_name, SRERemoteDesktopProps( - aad_application_fqdn=networking.sre_fqdn, - aad_application_name=f"sre-{self.sre_name}-guacamole", - aad_auth_token=self.graph_api_token, - aad_tenant_id=self.cfg.shm.aad_tenant_id, allow_copy=self.cfg.sre(self.sre_name).remote_desktop.allow_copy, allow_paste=self.cfg.sre(self.sre_name).remote_desktop.allow_paste, database_password=data.password_user_database_admin, dns_server_ip=dns.ip_address, + entra_id_application_fqdn=networking.sre_fqdn, + entra_id_application_name=f"sre-{self.sre_name}-guacamole", + entra_id_auth_token=self.graph_api_token, + entra_id_tenant_id=self.cfg.shm.aad_tenant_id, ldap_group_filter=ldap_group_filter, ldap_group_search_base=ldap_group_search_base, ldap_server_hostname=identity.hostname, diff --git a/data_safe_haven/infrastructure/programs/sre/identity.py b/data_safe_haven/infrastructure/programs/sre/identity.py index 111f9d9fc5..b1ee3dce0e 100644 --- a/data_safe_haven/infrastructure/programs/sre/identity.py +++ b/data_safe_haven/infrastructure/programs/sre/identity.py @@ -22,11 +22,11 @@ class SREIdentityProps: def __init__( self, - aad_application_name: Input[str], - aad_auth_token: Input[str], - aad_tenant_id: Input[str], dns_resource_group_name: Input[str], dns_server_ip: Input[str], + entra_id_application_name: Input[str], + entra_id_auth_token: Input[str], + entra_id_tenant_id: Input[str], location: Input[str], networking_resource_group_name: Input[str], shm_fqdn: Input[str], @@ -36,11 +36,11 @@ def __init__( storage_account_resource_group_name: Input[str], subnet_containers: Input[network.GetSubnetResult], ) -> None: - self.aad_application_name = aad_application_name - self.aad_auth_token = aad_auth_token - self.aad_tenant_id = aad_tenant_id self.dns_resource_group_name = dns_resource_group_name self.dns_server_ip = dns_server_ip + self.entra_id_application_name = entra_id_application_name + self.entra_id_auth_token = entra_id_auth_token + self.entra_id_tenant_id = entra_id_tenant_id self.location = location self.networking_resource_group_name = networking_resource_group_name self.shm_fqdn = shm_fqdn @@ -92,14 +92,14 @@ def __init__( opts=child_opts, ) - # Define AzureAD application - aad_application = EntraIDApplication( - f"{self._name}_aad_application", + # Define Entra ID application + entra_id_application = EntraIDApplication( + f"{self._name}_entra_id_application", EntraIDApplicationProps( - application_name=props.aad_application_name, + application_name=props.entra_id_application_name, application_role_assignments=["User.Read.All", "GroupMember.Read.All"], application_secret_name="Apricot Authentication Secret", - auth_token=props.aad_auth_token, + auth_token=props.entra_id_auth_token, delegated_role_assignments=["User.Read.All"], public_client_redirect_uri="urn:ietf:wg:oauth:2.0:oob", ), @@ -121,11 +121,11 @@ def __init__( ), containerinstance.EnvironmentVariableArgs( name="CLIENT_ID", - value=aad_application.application_id, + value=entra_id_application.application_id, ), containerinstance.EnvironmentVariableArgs( name="CLIENT_SECRET", - secure_value=aad_application.application_secret, + secure_value=entra_id_application.application_secret, ), containerinstance.EnvironmentVariableArgs( name="DEBUG", @@ -137,7 +137,7 @@ def __init__( ), containerinstance.EnvironmentVariableArgs( name="ENTRA_TENANT_ID", - value=props.aad_tenant_id, + value=props.entra_id_tenant_id, ), containerinstance.EnvironmentVariableArgs( name="REDIS_HOST", diff --git a/data_safe_haven/infrastructure/programs/sre/remote_desktop.py b/data_safe_haven/infrastructure/programs/sre/remote_desktop.py index 39f7747267..511bca5f4f 100644 --- a/data_safe_haven/infrastructure/programs/sre/remote_desktop.py +++ b/data_safe_haven/infrastructure/programs/sre/remote_desktop.py @@ -31,14 +31,14 @@ class SRERemoteDesktopProps: def __init__( self, - aad_application_name: Input[str], - aad_application_fqdn: Input[str], - aad_auth_token: Input[str], - aad_tenant_id: Input[str], allow_copy: Input[bool], allow_paste: Input[bool], database_password: Input[str], dns_server_ip: Input[str], + entra_id_application_fqdn: Input[str], + entra_id_application_name: Input[str], + entra_id_auth_token: Input[str], + entra_id_tenant_id: Input[str], ldap_group_filter: Input[str], ldap_group_search_base: Input[str], ldap_server_hostname: Input[str], @@ -53,10 +53,6 @@ def __init__( subnet_guacamole_containers_support: Input[network.GetSubnetResult], database_username: Input[str] | None = "postgresadmin", ) -> None: - self.aad_application_name = aad_application_name - self.aad_application_url = Output.concat("https://", aad_application_fqdn) - self.aad_auth_token = aad_auth_token - self.aad_tenant_id = aad_tenant_id self.database_password = database_password self.database_username = ( database_username if database_username else "postgresadmin" @@ -64,6 +60,12 @@ def __init__( self.disable_copy = not allow_copy self.disable_paste = not allow_paste self.dns_server_ip = dns_server_ip + self.entra_id_application_name = entra_id_application_name + self.entra_id_application_url = Output.concat( + "https://", entra_id_application_fqdn + ) + self.entra_id_auth_token = entra_id_auth_token + self.entra_id_tenant_id = entra_id_tenant_id self.ldap_group_filter = ldap_group_filter self.ldap_group_search_base = ldap_group_search_base self.ldap_server_hostname = ldap_server_hostname @@ -130,13 +132,13 @@ def __init__( tags=child_tags, ) - # Define AzureAD application - aad_application = EntraIDApplication( - f"{self._name}_aad_application", + # Define Entra ID application + entra_id_application = EntraIDApplication( + f"{self._name}_entra_id_application", EntraIDApplicationProps( - application_name=props.aad_application_name, - auth_token=props.aad_auth_token, - web_redirect_url=props.aad_application_url, + application_name=props.entra_id_application_name, + auth_token=props.entra_id_auth_token, + web_redirect_url=props.entra_id_application_url, ), opts=child_opts, ) @@ -228,19 +230,19 @@ def __init__( name="OPENID_AUTHORIZATION_ENDPOINT", value=Output.concat( "https://login.microsoftonline.com/", - props.aad_tenant_id, + props.entra_id_tenant_id, "/oauth2/v2.0/authorize", ), ), containerinstance.EnvironmentVariableArgs( name="OPENID_CLIENT_ID", - value=aad_application.application_id, + value=entra_id_application.application_id, ), containerinstance.EnvironmentVariableArgs( name="OPENID_ISSUER", value=Output.concat( "https://login.microsoftonline.com/", - props.aad_tenant_id, + props.entra_id_tenant_id, "/v2.0", ), ), @@ -248,12 +250,13 @@ def __init__( name="OPENID_JWKS_ENDPOINT", value=Output.concat( "https://login.microsoftonline.com/", - props.aad_tenant_id, + props.entra_id_tenant_id, "/discovery/v2.0/keys", ), ), containerinstance.EnvironmentVariableArgs( - name="OPENID_REDIRECT_URI", value=props.aad_application_url + name="OPENID_REDIRECT_URI", + value=props.entra_id_application_url, ), containerinstance.EnvironmentVariableArgs( name="OPENID_USERNAME_CLAIM_TYPE", From 0b4145c3422bdb6c1c6973d82cededd56dee8236 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Wed, 8 May 2024 17:13:13 +0100 Subject: [PATCH 03/16] :truck: Update SHM config to use Entra ID --- data_safe_haven/commands/admin_add_users.py | 2 +- data_safe_haven/commands/admin_list_users.py | 2 +- data_safe_haven/commands/admin_register_users.py | 2 +- data_safe_haven/commands/admin_remove_users.py | 2 +- .../commands/admin_unregister_users.py | 2 +- data_safe_haven/commands/deploy.py | 4 ++-- data_safe_haven/commands/teardown.py | 2 +- data_safe_haven/config/config.py | 16 ++++++++-------- .../infrastructure/programs/declarative_sre.py | 4 ++-- tests/config/test_config.py | 2 +- tests/conftest.py | 4 ++-- 11 files changed, 21 insertions(+), 21 deletions(-) diff --git a/data_safe_haven/commands/admin_add_users.py b/data_safe_haven/commands/admin_add_users.py index 5a5535709c..ec3de65d49 100644 --- a/data_safe_haven/commands/admin_add_users.py +++ b/data_safe_haven/commands/admin_add_users.py @@ -20,7 +20,7 @@ def admin_add_users(csv_path: pathlib.Path) -> None: try: # Load GraphAPI graph_api = GraphApi( - tenant_id=config.shm.aad_tenant_id, + tenant_id=config.shm.entra_id_tenant_id, default_scopes=[ "Group.Read.All", "User.ReadWrite.All", diff --git a/data_safe_haven/commands/admin_list_users.py b/data_safe_haven/commands/admin_list_users.py index 274b525e2e..9e912a5cf3 100644 --- a/data_safe_haven/commands/admin_list_users.py +++ b/data_safe_haven/commands/admin_list_users.py @@ -18,7 +18,7 @@ def admin_list_users() -> None: try: # Load GraphAPI graph_api = GraphApi( - tenant_id=config.shm.aad_tenant_id, + tenant_id=config.shm.entra_id_tenant_id, default_scopes=["Directory.Read.All", "Group.Read.All"], ) diff --git a/data_safe_haven/commands/admin_register_users.py b/data_safe_haven/commands/admin_register_users.py index a4630b25ff..d281ede3b7 100644 --- a/data_safe_haven/commands/admin_register_users.py +++ b/data_safe_haven/commands/admin_register_users.py @@ -33,7 +33,7 @@ def admin_register_users( # Load GraphAPI graph_api = GraphApi( - tenant_id=config.shm.aad_tenant_id, + tenant_id=config.shm.entra_id_tenant_id, default_scopes=["Group.ReadWrite.All", "GroupMember.ReadWrite.All"], ) diff --git a/data_safe_haven/commands/admin_remove_users.py b/data_safe_haven/commands/admin_remove_users.py index fc9a01a00d..1e1cd111f6 100644 --- a/data_safe_haven/commands/admin_remove_users.py +++ b/data_safe_haven/commands/admin_remove_users.py @@ -20,7 +20,7 @@ def admin_remove_users( try: # Load GraphAPI graph_api = GraphApi( - tenant_id=config.shm.aad_tenant_id, + tenant_id=config.shm.entra_id_tenant_id, default_scopes=["User.ReadWrite.All"], ) diff --git a/data_safe_haven/commands/admin_unregister_users.py b/data_safe_haven/commands/admin_unregister_users.py index d13216891f..d14c475cdf 100644 --- a/data_safe_haven/commands/admin_unregister_users.py +++ b/data_safe_haven/commands/admin_unregister_users.py @@ -34,7 +34,7 @@ def admin_unregister_users( # Load GraphAPI graph_api = GraphApi( - tenant_id=config.shm.aad_tenant_id, + tenant_id=config.shm.entra_id_tenant_id, default_scopes=["Group.ReadWrite.All", "GroupMember.ReadWrite.All"], ) diff --git a/data_safe_haven/commands/deploy.py b/data_safe_haven/commands/deploy.py index d1ec83ecf2..34884e473a 100644 --- a/data_safe_haven/commands/deploy.py +++ b/data_safe_haven/commands/deploy.py @@ -36,7 +36,7 @@ def shm( try: # Add the SHM domain to AzureAD as a custom domain graph_api = GraphApi( - tenant_id=config.shm.aad_tenant_id, + tenant_id=config.shm.entra_id_tenant_id, default_scopes=[ "Application.ReadWrite.All", "Domain.ReadWrite.All", @@ -114,7 +114,7 @@ def sre( # Load GraphAPI as this may require user-interaction that is not possible as # part of a Pulumi declarative command graph_api = GraphApi( - tenant_id=config.shm.aad_tenant_id, + tenant_id=config.shm.entra_id_tenant_id, default_scopes=[ "Application.ReadWrite.All", "AppRoleAssignment.ReadWrite.All", diff --git a/data_safe_haven/commands/teardown.py b/data_safe_haven/commands/teardown.py index d2f87355d6..5eb4486de0 100644 --- a/data_safe_haven/commands/teardown.py +++ b/data_safe_haven/commands/teardown.py @@ -61,7 +61,7 @@ def sre( # Load GraphAPI as this may require user-interaction that is not possible as # part of a Pulumi declarative command graph_api = GraphApi( - tenant_id=config.shm.aad_tenant_id, + tenant_id=config.shm.entra_id_tenant_id, default_scopes=["Application.ReadWrite.All", "Group.ReadWrite.All"], ) diff --git a/data_safe_haven/config/config.py b/data_safe_haven/config/config.py index c0dfa05140..d8bd2e4d78 100644 --- a/data_safe_haven/config/config.py +++ b/data_safe_haven/config/config.py @@ -35,7 +35,7 @@ class ConfigSectionAzure(BaseModel, validate_assignment=True): class ConfigSectionSHM(BaseModel, validate_assignment=True): - aad_tenant_id: Guid + entra_id_tenant_id: Guid admin_email_address: EmailAddress admin_ip_addresses: list[IpAddress] fqdn: Fqdn @@ -44,7 +44,7 @@ class ConfigSectionSHM(BaseModel, validate_assignment=True): def update( self, *, - aad_tenant_id: str | None = None, + entra_id_tenant_id: str | None = None, admin_email_address: str | None = None, admin_ip_addresses: list[str] | None = None, fqdn: str | None = None, @@ -53,18 +53,18 @@ def update( """Update SHM settings Args: - aad_tenant_id: AzureAD tenant containing users + entra_id_tenant_id: Entra ID tenant containing users admin_email_address: Email address shared by all administrators admin_ip_addresses: List of IP addresses belonging to administrators fqdn: Fully-qualified domain name to use for this SHM timezone: Timezone in pytz format (eg. Europe/London) """ logger = LoggingSingleton() - # Set AzureAD tenant ID - if aad_tenant_id: - self.aad_tenant_id = aad_tenant_id + # Set Entra ID tenant ID + if entra_id_tenant_id: + self.entra_id_tenant_id = entra_id_tenant_id logger.info( - f"[bold]AzureAD tenant ID[/] will be [green]{self.aad_tenant_id}[/]." + f"[bold]Entra ID tenant ID[/] will be [green]{self.entra_id_tenant_id}[/]." ) # Set admin email address if admin_email_address: @@ -237,7 +237,7 @@ def template(cls) -> Config: tenant_id="Azure tenant ID", ), shm=ConfigSectionSHM.model_construct( - aad_tenant_id="Azure Active Directory tenant ID", + entra_id_tenant_id="Entra ID tenant ID", admin_email_address="Admin email address", admin_ip_addresses=["Admin IP addresses"], fqdn="TRE domain name", diff --git a/data_safe_haven/infrastructure/programs/declarative_sre.py b/data_safe_haven/infrastructure/programs/declarative_sre.py index a2fce52bd4..458354a293 100644 --- a/data_safe_haven/infrastructure/programs/declarative_sre.py +++ b/data_safe_haven/infrastructure/programs/declarative_sre.py @@ -241,7 +241,7 @@ def __call__(self) -> None: dns_server_ip=dns.ip_address, entra_id_application_name=f"sre-{self.sre_name}-apricot", entra_id_auth_token=self.graph_api_token, - entra_id_tenant_id=self.cfg.shm.aad_tenant_id, + entra_id_tenant_id=self.cfg.shm.entra_id_tenant_id, location=self.context.location, networking_resource_group_name=networking.resource_group.name, shm_fqdn=self.cfg.shm.fqdn, @@ -281,7 +281,7 @@ def __call__(self) -> None: entra_id_application_fqdn=networking.sre_fqdn, entra_id_application_name=f"sre-{self.sre_name}-guacamole", entra_id_auth_token=self.graph_api_token, - entra_id_tenant_id=self.cfg.shm.aad_tenant_id, + entra_id_tenant_id=self.cfg.shm.entra_id_tenant_id, ldap_group_filter=ldap_group_filter, ldap_group_search_base=ldap_group_search_base, ldap_server_hostname=identity.hostname, diff --git a/tests/config/test_config.py b/tests/config/test_config.py index 92ffa07098..b9bc1ed72e 100644 --- a/tests/config/test_config.py +++ b/tests/config/test_config.py @@ -27,7 +27,7 @@ def test_constructor(self): class TestConfigSectionSHM: def test_constructor(self): ConfigSectionSHM( - aad_tenant_id="d5c5c439-1115-4cb6-ab50-b8e547b6c8dd", + entra_id_tenant_id="d5c5c439-1115-4cb6-ab50-b8e547b6c8dd", admin_email_address="admin@example.com", admin_ip_addresses=["0.0.0.0"], # noqa: S104 fqdn="shm.acme.com", diff --git a/tests/conftest.py b/tests/conftest.py index 3fc83910ce..339782c5d3 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -61,7 +61,7 @@ def config_yaml(): subscription_id: d5c5c439-1115-4cb6-ab50-b8e547b6c8dd tenant_id: d5c5c439-1115-4cb6-ab50-b8e547b6c8dd shm: - aad_tenant_id: d5c5c439-1115-4cb6-ab50-b8e547b6c8dd + entra_id_tenant_id: d5c5c439-1115-4cb6-ab50-b8e547b6c8dd admin_email_address: admin@example.com admin_ip_addresses: - 0.0.0.0/32 @@ -110,7 +110,7 @@ def azure_config(): @fixture def shm_config(): return ConfigSectionSHM( - aad_tenant_id="d5c5c439-1115-4cb6-ab50-b8e547b6c8dd", + entra_id_tenant_id="d5c5c439-1115-4cb6-ab50-b8e547b6c8dd", admin_email_address="admin@example.com", admin_ip_addresses=["0.0.0.0"], # noqa: S104 fqdn="shm.acme.com", From b519634103f9aecd874621f61f3d23bf5cf4bd60 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Wed, 8 May 2024 17:36:05 +0100 Subject: [PATCH 04/16] :memo: Updated docstrings and log messages --- data_safe_haven/commands/deploy.py | 4 +- data_safe_haven/external/api/graph_api.py | 54 +++++++++---------- .../provisioning/sre_provisioning_manager.py | 2 +- 3 files changed, 30 insertions(+), 30 deletions(-) diff --git a/data_safe_haven/commands/deploy.py b/data_safe_haven/commands/deploy.py index 34884e473a..24ecae56fa 100644 --- a/data_safe_haven/commands/deploy.py +++ b/data_safe_haven/commands/deploy.py @@ -34,7 +34,7 @@ def shm( pulumi_project = pulumi_config.create_or_select_project(context.shm_name) try: - # Add the SHM domain to AzureAD as a custom domain + # Connect to GraphAPI interactively graph_api = GraphApi( tenant_id=config.shm.entra_id_tenant_id, default_scopes=[ @@ -66,7 +66,7 @@ def shm( else: stack.deploy(force=force) - # Add the SHM domain as a custom domain in AzureAD + # Add the SHM domain to Entra ID as a custom domain graph_api.verify_custom_domain( config.shm.fqdn, stack.output("networking")["fqdn_nameservers"], diff --git a/data_safe_haven/external/api/graph_api.py b/data_safe_haven/external/api/graph_api.py index abf930ed04..d65cc4e6b2 100644 --- a/data_safe_haven/external/api/graph_api.py +++ b/data_safe_haven/external/api/graph_api.py @@ -95,7 +95,7 @@ def __init__( self.token = self.create_token_administrator() def add_custom_domain(self, domain_name: str) -> str: - """Add AzureAD custom domain + """Add Entra ID custom domain Returns: str: Registration TXT record @@ -104,7 +104,7 @@ def add_custom_domain(self, domain_name: str) -> str: DataSafeHavenMicrosoftGraphError if domain could not be added """ try: - # Create the AzureAD custom domain if it does not already exist + # Create the Entra ID custom domain if it does not already exist domains = self.read_domains() domain_exists = any(domain["id"] == domain_name for domain in domains) if not domain_exists: @@ -173,7 +173,7 @@ def create_application( delegated_scopes: Sequence[str] = [], request_json: dict[str, Any] | None = None, ) -> dict[str, Any]: - """Create an AzureAD application if it does not already exist + """Create an Entra ID application if it does not already exist Raises: DataSafeHavenMicrosoftGraphError if the application could not be created @@ -257,7 +257,7 @@ def create_application( msg = "Maximum attempts to validate service principle permissions exceeded" raise DataSafeHavenMicrosoftGraphError(msg) - # Return JSON representation of the AzureAD application + # Return JSON representation of the Entra ID application return json_response except Exception as exc: msg = f"Could not create application '{application_name}'.\n{exc}" @@ -266,7 +266,7 @@ def create_application( def create_application_secret( self, application_name: str, application_secret_name: str ) -> str: - """Add a secret to an existing AzureAD application + """Add a secret to an existing Entra ID application Returns: str: Contents of newly-created secret @@ -312,7 +312,7 @@ def create_application_secret( raise DataSafeHavenMicrosoftGraphError(msg) from exc def create_group(self, group_name: str) -> None: - """Create an AzureAD group if it does not already exist + """Create an Entra ID group if it does not already exist Raises: DataSafeHavenMicrosoftGraphError if the group could not be created @@ -320,11 +320,11 @@ def create_group(self, group_name: str) -> None: try: if self.get_id_from_groupname(group_name): self.logger.info( - f"Found existing AzureAD group '[green]{group_name}[/]'.", + f"Found existing Entra ID group '[green]{group_name}[/]'.", ) return self.logger.debug( - f"Creating AzureAD group '[green]{group_name}[/]'...", + f"Creating Entra ID group '[green]{group_name}[/]'...", ) request_json = { "description": group_name, @@ -339,16 +339,16 @@ def create_group(self, group_name: str) -> None: json=request_json, ).json() self.logger.info( - f"Created AzureAD group '[green]{group_name}[/]'.", + f"Created Entra ID group '[green]{group_name}[/]'.", ) except Exception as exc: - msg = f"Could not create AzureAD group '{group_name}'.\n{exc}" + msg = f"Could not create Entra ID group '{group_name}'.\n{exc}" raise DataSafeHavenMicrosoftGraphError(msg) from exc def ensure_application_service_principal( self, application_name: str ) -> dict[str, Any]: - """Create a service principal for an AzureAD application if it does not already exist + """Create a service principal for an Entra ID application if it does not already exist Raises: DataSafeHavenMicrosoftGraphError if the service principal could not be created @@ -470,7 +470,7 @@ def create_user( email_address: str, phone_number: str, ) -> None: - """Create an AzureAD user if it does not already exist + """Create an Entra ID user if it does not already exist Raises: DataSafeHavenMicrosoftGraphError if the user could not be created @@ -482,12 +482,12 @@ def create_user( user_id = self.get_id_from_username(username) if user_id: self.logger.debug( - f"Updating AzureAD user '[green]{username}[/]'...", + f"Updating Entra ID user '[green]{username}[/]'...", ) final_verb = "Update" else: self.logger.debug( - f"Creating AzureAD user '[green]{username}[/]'...", + f"Creating Entra ID user '[green]{username}[/]'...", ) final_verb = "Create" # If they do not then create them @@ -523,7 +523,7 @@ def create_user( json={"accountEnabled": True}, ) self.logger.info( - f"{final_verb}d AzureAD user '[green]{username}[/]'.", + f"{final_verb}d Entra ID user '[green]{username}[/]'.", ) except DataSafeHavenMicrosoftGraphError as exc: msg = f"Could not {final_verb.lower()} user {username}.\n{exc}" @@ -533,7 +533,7 @@ def delete_application( self, application_name: str, ) -> None: - """Remove an application from AzureAD + """Remove an application from Entra ID Raises: DataSafeHavenMicrosoftGraphError if the application could not be deleted @@ -917,10 +917,10 @@ def read_application_permissions( raise DataSafeHavenMicrosoftGraphError(msg) from exc def read_domains(self) -> Sequence[dict[str, Any]]: - """Get details of AzureAD domains + """Get details of Entra ID domains Returns: - JSON: A JSON list of AzureAD domains + JSON: A JSON list of Entra ID domains Raises: DataSafeHavenMicrosoftGraphError if domains could not be loaded @@ -936,10 +936,10 @@ def read_groups( self, attributes: Sequence[str] | None = None, ) -> Sequence[dict[str, Any]]: - """Get details of AzureAD groups + """Get details of Entra ID groups Returns: - JSON: A JSON list of AzureAD groups + JSON: A JSON list of Entra ID groups Raises: DataSafeHavenMicrosoftGraphError if groups could not be loaded @@ -969,10 +969,10 @@ def read_service_principals(self) -> Sequence[dict[str, Any]]: def read_users( self, attributes: Sequence[str] | None = None ) -> Sequence[dict[str, Any]]: - """Get details of AzureAD users + """Get details of Entra ID users Returns: - JSON: A JSON list of AzureAD users + JSON: A JSON list of Entra ID users Raises: DataSafeHavenMicrosoftGraphError if users could not be loaded @@ -1020,7 +1020,7 @@ def remove_user( self, username: str, ) -> None: - """Remove a user from AzureAD + """Remove a user from Entra ID Raises: DataSafeHavenMicrosoftGraphError if the user could not be removed @@ -1041,7 +1041,7 @@ def remove_user_from_group( username: str, group_name: str, ) -> None: - """Remove a user from an AzureAD group + """Remove a user from an Entra ID group Raises: DataSafeHavenMicrosoftGraphError if the user could not be removed @@ -1076,16 +1076,16 @@ def remove_user_from_group( def verify_custom_domain( self, domain_name: str, expected_nameservers: Sequence[str] ) -> None: - """Verify AzureAD custom domain + """Verify Entra ID custom domain Raises: DataSafeHavenMicrosoftGraphError if domain could not be verified """ try: - # Create the AzureAD custom domain if it does not already exist + # Create the Entra ID custom domain if it does not already exist domains = self.read_domains() if not any(d["id"] == domain_name for d in domains): - msg = f"Domain {domain_name} has not been added to AzureAD." + msg = f"Domain {domain_name} has not been added to Entra ID." raise DataSafeHavenMicrosoftGraphError(msg) # Wait until domain delegation is complete while True: diff --git a/data_safe_haven/provisioning/sre_provisioning_manager.py b/data_safe_haven/provisioning/sre_provisioning_manager.py index cf25ed2a86..b7ea37a9f4 100644 --- a/data_safe_haven/provisioning/sre_provisioning_manager.py +++ b/data_safe_haven/provisioning/sre_provisioning_manager.py @@ -72,7 +72,7 @@ def available_vm_skus(self) -> dict[str, dict[str, Any]]: return self._available_vm_skus def create_security_groups(self) -> None: - """Create groups in AzureAD""" + """Create groups in Entra ID""" for group_name in self.security_group_params.values(): self.graph_api.create_group(group_name) From 4ec140ebad46f5e6be9a6bbc73cb60bb13d4703d Mon Sep 17 00:00:00 2001 From: James Robinson Date: Wed, 8 May 2024 17:40:57 +0100 Subject: [PATCH 05/16] :truck: Rename AzureADUsers to EntraIDUsers --- .../{azure_ad_users.py => entra_id_users.py} | 16 ++++---- .../administration/users/user_handler.py | 40 +++++++++---------- 2 files changed, 28 insertions(+), 28 deletions(-) rename data_safe_haven/administration/users/{azure_ad_users.py => entra_id_users.py} (91%) diff --git a/data_safe_haven/administration/users/azure_ad_users.py b/data_safe_haven/administration/users/entra_id_users.py similarity index 91% rename from data_safe_haven/administration/users/azure_ad_users.py rename to data_safe_haven/administration/users/entra_id_users.py index 24dc129594..b7bf82b4bb 100644 --- a/data_safe_haven/administration/users/azure_ad_users.py +++ b/data_safe_haven/administration/users/entra_id_users.py @@ -11,8 +11,8 @@ from .research_user import ResearchUser -class AzureADUsers: - """Interact with users in an Azure Active Directory""" +class EntraIDUsers: + """Interact with users in an Entra ID directory.""" def __init__( self, @@ -25,7 +25,7 @@ def __init__( self.logger = LoggingSingleton() def add(self, new_users: Sequence[ResearchUser]) -> None: - """Add list of users to AzureAD""" + """Add list of users to Entra ID""" # Get the default domain default_domain = next( domain["id"] @@ -47,7 +47,7 @@ def add(self, new_users: Sequence[ResearchUser]) -> None: request_json, user.email_address, user.phone_number ) self.logger.info( - f"Ensured user '[green]{user.preferred_username}[/]' exists in AzureAD" + f"Ensured user '[green]{user.preferred_username}[/]' exists in Entra ID" ) def list(self) -> Sequence[ResearchUser]: @@ -74,13 +74,13 @@ def list(self) -> Sequence[ResearchUser]: ] def register(self, sre_name: str, usernames: Sequence[str]) -> None: - """Add usernames to SRE security group""" + """Add usernames to SRE group""" group_name = f"Data Safe Haven SRE {sre_name} Users" for username in usernames: self.graph_api.add_user_to_group(username, group_name) def remove(self, users: Sequence[ResearchUser]) -> None: - """Remove list of users from AzureAD""" + """Remove list of users from Entra ID""" for user in filter( lambda existing_user: any(existing_user == user for user in users), self.list(), @@ -92,14 +92,14 @@ def remove(self, users: Sequence[ResearchUser]) -> None: self.logger.error(f"Unable to remove '{user.preferred_username}'.") def set(self, users: Sequence[ResearchUser]) -> None: - """Set AzureAD users to specified list""" + """Set Entra ID users to specified list""" users_to_remove = [user for user in self.list() if user not in users] self.remove(users_to_remove) users_to_add = [user for user in users if user not in self.list()] self.add(users_to_add) def unregister(self, sre_name: str, usernames: Sequence[str]) -> None: - """Remove usernames from SRE security group""" + """Remove usernames from SRE group""" group_name = f"Data Safe Haven SRE {sre_name}" for username in usernames: self.graph_api.remove_user_from_group(username, group_name) diff --git a/data_safe_haven/administration/users/user_handler.py b/data_safe_haven/administration/users/user_handler.py index 82cbf27620..a66856531b 100644 --- a/data_safe_haven/administration/users/user_handler.py +++ b/data_safe_haven/administration/users/user_handler.py @@ -8,7 +8,7 @@ from data_safe_haven.external import GraphApi from data_safe_haven.utility import LoggingSingleton -from .azure_ad_users import AzureADUsers +from .entra_id_users import EntraIDUsers from .guacamole_users import GuacamoleUsers from .research_user import ResearchUser @@ -21,7 +21,7 @@ def __init__( pulumi_config: DSHPulumiConfig, graph_api: GraphApi, ): - self.azure_ad_users = AzureADUsers(graph_api) + self.entra_id_users = EntraIDUsers(graph_api) self.context = context self.config = config self.pulumi_config = pulumi_config @@ -29,7 +29,7 @@ def __init__( self.sre_guacamole_users_: dict[str, GuacamoleUsers] = {} def add(self, users_csv_path: pathlib.Path) -> None: - """Add AzureAD and Guacamole users + """Add Entra ID and Guacamole users Raises: DataSafeHavenUserHandlingError if the users could not be added @@ -66,8 +66,8 @@ def add(self, users_csv_path: pathlib.Path) -> None: for user in users: self.logger.debug(f"Processing new user: {user}") - # Add users to AzureAD - self.azure_ad_users.add(users) + # Add users to Entra ID + self.entra_id_users.add(users) except Exception as exc: msg = f"Could not add users from '{users_csv_path}'.\n{exc}" raise DataSafeHavenUserHandlingError(msg) from exc @@ -85,7 +85,7 @@ def get_usernames(self) -> dict[str, list[str]]: def get_usernames_azure_ad(self) -> list[str]: """Load usernames from Azure AD""" - return [user.username for user in self.azure_ad_users.list()] + return [user.username for user in self.entra_id_users.list()] def get_usernames_guacamole( self, sre_name: str, sre_pulumi_project: DSHPulumiProject @@ -104,7 +104,7 @@ def get_usernames_guacamole( return [] def list(self) -> None: - """List Active Directory, AzureAD and Guacamole users + """List Entra ID and Guacamole users Raises: DataSafeHavenUserHandlingError if the users could not be listed @@ -140,13 +140,13 @@ def register(self, sre_name: str, user_names: Sequence[str]) -> None: """ try: # Add users to the SRE security group - self.azure_ad_users.register(sre_name, user_names) + self.entra_id_users.register(sre_name, user_names) except Exception as exc: msg = f"Could not register {len(user_names)} users with SRE '{sre_name}'.\n{exc}" raise DataSafeHavenUserHandlingError(msg) from exc def remove(self, user_names: Sequence[str]) -> None: - """Remove AzureAD and Guacamole users + """Remove Entra ID and Guacamole users Raises: DataSafeHavenUserHandlingError if the users could not be removed @@ -154,23 +154,23 @@ def remove(self, user_names: Sequence[str]) -> None: try: # Construct user lists self.logger.info(f"Attempting to remove {len(user_names)} user(s).") - azuread_users_to_remove = [ + entra_id_users_to_remove = [ user - for user in self.azure_ad_users.list() + for user in self.entra_id_users.list() if user.username in user_names ] # Commit changes self.logger.info( - f"Found {len(azuread_users_to_remove)} valid user(s) to remove." + f"Found {len(entra_id_users_to_remove)} valid user(s) to remove." ) - self.azure_ad_users.remove(azuread_users_to_remove) + self.entra_id_users.remove(entra_id_users_to_remove) except Exception as exc: msg = f"Could not remove users: {user_names}.\n{exc}" raise DataSafeHavenUserHandlingError(msg) from exc def set(self, users_csv_path: str) -> None: - """Set AzureAD and Guacamole users + """Set Entra ID and Guacamole users Raises: DataSafeHavenUserHandlingError if the users could not be set to the desired list @@ -199,19 +199,19 @@ def set(self, users_csv_path: str) -> None: self.logger.debug(f"Processing user: {user}") # Keep existing users with the same username - azuread_desired_users = [ + entra_id_desired_users = [ user - for user in self.azure_ad_users.list() + for user in self.entra_id_users.list() if user.username in [u.username for u in desired_users] ] # Construct list of new users - azuread_desired_users = [ - user for user in desired_users if user not in azuread_desired_users + entra_id_desired_users = [ + user for user in desired_users if user not in entra_id_desired_users ] # Commit changes - self.azure_ad_users.set(azuread_desired_users) + self.entra_id_users.set(entra_id_desired_users) except Exception as exc: msg = f"Could not set users from '{users_csv_path}'.\n{exc}" raise DataSafeHavenUserHandlingError(msg) from exc @@ -224,7 +224,7 @@ def unregister(self, sre_name: str, user_names: Sequence[str]) -> None: """ try: # Remove users from the SRE security group - self.azure_ad_users.unregister(sre_name, user_names) + self.entra_id_users.unregister(sre_name, user_names) except Exception as exc: msg = f"Could not unregister {len(user_names)} users with SRE '{sre_name}'.\n{exc}" raise DataSafeHavenUserHandlingError(msg) from exc From b3ae3b05ce8887c21c5f540934bba9ddbd062fd6 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 9 May 2024 09:39:49 +0100 Subject: [PATCH 06/16] :rotating_light: Fix linting/test issues --- data_safe_haven/config/config.py | 18 +++++++++--------- tests/config/test_config.py | 2 +- tests/conftest.py | 4 ++-- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/data_safe_haven/config/config.py b/data_safe_haven/config/config.py index d8bd2e4d78..f4853cecd2 100644 --- a/data_safe_haven/config/config.py +++ b/data_safe_haven/config/config.py @@ -35,18 +35,18 @@ class ConfigSectionAzure(BaseModel, validate_assignment=True): class ConfigSectionSHM(BaseModel, validate_assignment=True): - entra_id_tenant_id: Guid admin_email_address: EmailAddress admin_ip_addresses: list[IpAddress] + entra_id_tenant_id: Guid fqdn: Fqdn timezone: TimeZone def update( self, *, - entra_id_tenant_id: str | None = None, admin_email_address: str | None = None, admin_ip_addresses: list[str] | None = None, + entra_id_tenant_id: str | None = None, fqdn: str | None = None, timezone: TimeZone | None = None, ) -> None: @@ -60,12 +60,6 @@ def update( timezone: Timezone in pytz format (eg. Europe/London) """ logger = LoggingSingleton() - # Set Entra ID tenant ID - if entra_id_tenant_id: - self.entra_id_tenant_id = entra_id_tenant_id - logger.info( - f"[bold]Entra ID tenant ID[/] will be [green]{self.entra_id_tenant_id}[/]." - ) # Set admin email address if admin_email_address: self.admin_email_address = admin_email_address @@ -78,6 +72,12 @@ def update( logger.info( f"[bold]IP addresses used by administrators[/] will be [green]{self.admin_ip_addresses}[/]." ) + # Set Entra ID tenant ID + if entra_id_tenant_id: + self.entra_id_tenant_id = entra_id_tenant_id + logger.info( + f"[bold]Entra ID tenant ID[/] will be [green]{self.entra_id_tenant_id}[/]." + ) # Set fully-qualified domain name if fqdn: self.fqdn = fqdn @@ -237,9 +237,9 @@ def template(cls) -> Config: tenant_id="Azure tenant ID", ), shm=ConfigSectionSHM.model_construct( - entra_id_tenant_id="Entra ID tenant ID", admin_email_address="Admin email address", admin_ip_addresses=["Admin IP addresses"], + entra_id_tenant_id="Entra ID tenant ID", fqdn="TRE domain name", timezone="Timezone", ), diff --git a/tests/config/test_config.py b/tests/config/test_config.py index b9bc1ed72e..308c4920e3 100644 --- a/tests/config/test_config.py +++ b/tests/config/test_config.py @@ -27,9 +27,9 @@ def test_constructor(self): class TestConfigSectionSHM: def test_constructor(self): ConfigSectionSHM( - entra_id_tenant_id="d5c5c439-1115-4cb6-ab50-b8e547b6c8dd", admin_email_address="admin@example.com", admin_ip_addresses=["0.0.0.0"], # noqa: S104 + entra_id_tenant_id="d5c5c439-1115-4cb6-ab50-b8e547b6c8dd", fqdn="shm.acme.com", timezone="UTC", ) diff --git a/tests/conftest.py b/tests/conftest.py index 339782c5d3..f0726e7998 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -61,10 +61,10 @@ def config_yaml(): subscription_id: d5c5c439-1115-4cb6-ab50-b8e547b6c8dd tenant_id: d5c5c439-1115-4cb6-ab50-b8e547b6c8dd shm: - entra_id_tenant_id: d5c5c439-1115-4cb6-ab50-b8e547b6c8dd admin_email_address: admin@example.com admin_ip_addresses: - 0.0.0.0/32 + entra_id_tenant_id: d5c5c439-1115-4cb6-ab50-b8e547b6c8dd fqdn: shm.acme.com timezone: UTC sres: @@ -110,9 +110,9 @@ def azure_config(): @fixture def shm_config(): return ConfigSectionSHM( - entra_id_tenant_id="d5c5c439-1115-4cb6-ab50-b8e547b6c8dd", admin_email_address="admin@example.com", admin_ip_addresses=["0.0.0.0"], # noqa: S104 + entra_id_tenant_id="d5c5c439-1115-4cb6-ab50-b8e547b6c8dd", fqdn="shm.acme.com", timezone="UTC", ) From b74d5a24ce833ef59b9beeeec19747ad8a5ed3ee Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 9 May 2024 11:21:21 +0100 Subject: [PATCH 07/16] :truck: Rename entra_id_tenant_id to entra_tenant_id --- data_safe_haven/commands/admin_add_users.py | 2 +- data_safe_haven/commands/admin_list_users.py | 2 +- data_safe_haven/commands/admin_register_users.py | 2 +- data_safe_haven/commands/admin_remove_users.py | 2 +- data_safe_haven/commands/admin_unregister_users.py | 2 +- data_safe_haven/commands/deploy.py | 4 ++-- data_safe_haven/commands/teardown.py | 2 +- data_safe_haven/config/config.py | 14 +++++++------- .../infrastructure/programs/declarative_sre.py | 4 ++-- .../infrastructure/programs/sre/identity.py | 6 +++--- .../infrastructure/programs/sre/remote_desktop.py | 10 +++++----- tests/config/test_config.py | 2 +- tests/conftest.py | 4 ++-- 13 files changed, 28 insertions(+), 28 deletions(-) diff --git a/data_safe_haven/commands/admin_add_users.py b/data_safe_haven/commands/admin_add_users.py index ec3de65d49..cad97bf100 100644 --- a/data_safe_haven/commands/admin_add_users.py +++ b/data_safe_haven/commands/admin_add_users.py @@ -20,7 +20,7 @@ def admin_add_users(csv_path: pathlib.Path) -> None: try: # Load GraphAPI graph_api = GraphApi( - tenant_id=config.shm.entra_id_tenant_id, + tenant_id=config.shm.entra_tenant_id, default_scopes=[ "Group.Read.All", "User.ReadWrite.All", diff --git a/data_safe_haven/commands/admin_list_users.py b/data_safe_haven/commands/admin_list_users.py index 9e912a5cf3..9b0fc7dea4 100644 --- a/data_safe_haven/commands/admin_list_users.py +++ b/data_safe_haven/commands/admin_list_users.py @@ -18,7 +18,7 @@ def admin_list_users() -> None: try: # Load GraphAPI graph_api = GraphApi( - tenant_id=config.shm.entra_id_tenant_id, + tenant_id=config.shm.entra_tenant_id, default_scopes=["Directory.Read.All", "Group.Read.All"], ) diff --git a/data_safe_haven/commands/admin_register_users.py b/data_safe_haven/commands/admin_register_users.py index d281ede3b7..03aa68ecd4 100644 --- a/data_safe_haven/commands/admin_register_users.py +++ b/data_safe_haven/commands/admin_register_users.py @@ -33,7 +33,7 @@ def admin_register_users( # Load GraphAPI graph_api = GraphApi( - tenant_id=config.shm.entra_id_tenant_id, + tenant_id=config.shm.entra_tenant_id, default_scopes=["Group.ReadWrite.All", "GroupMember.ReadWrite.All"], ) diff --git a/data_safe_haven/commands/admin_remove_users.py b/data_safe_haven/commands/admin_remove_users.py index 1e1cd111f6..387655e567 100644 --- a/data_safe_haven/commands/admin_remove_users.py +++ b/data_safe_haven/commands/admin_remove_users.py @@ -20,7 +20,7 @@ def admin_remove_users( try: # Load GraphAPI graph_api = GraphApi( - tenant_id=config.shm.entra_id_tenant_id, + tenant_id=config.shm.entra_tenant_id, default_scopes=["User.ReadWrite.All"], ) diff --git a/data_safe_haven/commands/admin_unregister_users.py b/data_safe_haven/commands/admin_unregister_users.py index d14c475cdf..af5a67c8ee 100644 --- a/data_safe_haven/commands/admin_unregister_users.py +++ b/data_safe_haven/commands/admin_unregister_users.py @@ -34,7 +34,7 @@ def admin_unregister_users( # Load GraphAPI graph_api = GraphApi( - tenant_id=config.shm.entra_id_tenant_id, + tenant_id=config.shm.entra_tenant_id, default_scopes=["Group.ReadWrite.All", "GroupMember.ReadWrite.All"], ) diff --git a/data_safe_haven/commands/deploy.py b/data_safe_haven/commands/deploy.py index 24ecae56fa..ae2c7b0db5 100644 --- a/data_safe_haven/commands/deploy.py +++ b/data_safe_haven/commands/deploy.py @@ -36,7 +36,7 @@ def shm( try: # Connect to GraphAPI interactively graph_api = GraphApi( - tenant_id=config.shm.entra_id_tenant_id, + tenant_id=config.shm.entra_tenant_id, default_scopes=[ "Application.ReadWrite.All", "Domain.ReadWrite.All", @@ -114,7 +114,7 @@ def sre( # Load GraphAPI as this may require user-interaction that is not possible as # part of a Pulumi declarative command graph_api = GraphApi( - tenant_id=config.shm.entra_id_tenant_id, + tenant_id=config.shm.entra_tenant_id, default_scopes=[ "Application.ReadWrite.All", "AppRoleAssignment.ReadWrite.All", diff --git a/data_safe_haven/commands/teardown.py b/data_safe_haven/commands/teardown.py index 5eb4486de0..49ae5b9832 100644 --- a/data_safe_haven/commands/teardown.py +++ b/data_safe_haven/commands/teardown.py @@ -61,7 +61,7 @@ def sre( # Load GraphAPI as this may require user-interaction that is not possible as # part of a Pulumi declarative command graph_api = GraphApi( - tenant_id=config.shm.entra_id_tenant_id, + tenant_id=config.shm.entra_tenant_id, default_scopes=["Application.ReadWrite.All", "Group.ReadWrite.All"], ) diff --git a/data_safe_haven/config/config.py b/data_safe_haven/config/config.py index f4853cecd2..2fd1b3443b 100644 --- a/data_safe_haven/config/config.py +++ b/data_safe_haven/config/config.py @@ -37,7 +37,7 @@ class ConfigSectionAzure(BaseModel, validate_assignment=True): class ConfigSectionSHM(BaseModel, validate_assignment=True): admin_email_address: EmailAddress admin_ip_addresses: list[IpAddress] - entra_id_tenant_id: Guid + entra_tenant_id: Guid fqdn: Fqdn timezone: TimeZone @@ -46,14 +46,14 @@ def update( *, admin_email_address: str | None = None, admin_ip_addresses: list[str] | None = None, - entra_id_tenant_id: str | None = None, + entra_tenant_id: str | None = None, fqdn: str | None = None, timezone: TimeZone | None = None, ) -> None: """Update SHM settings Args: - entra_id_tenant_id: Entra ID tenant containing users + entra_tenant_id: Entra ID tenant containing users admin_email_address: Email address shared by all administrators admin_ip_addresses: List of IP addresses belonging to administrators fqdn: Fully-qualified domain name to use for this SHM @@ -73,10 +73,10 @@ def update( f"[bold]IP addresses used by administrators[/] will be [green]{self.admin_ip_addresses}[/]." ) # Set Entra ID tenant ID - if entra_id_tenant_id: - self.entra_id_tenant_id = entra_id_tenant_id + if entra_tenant_id: + self.entra_tenant_id = entra_tenant_id logger.info( - f"[bold]Entra ID tenant ID[/] will be [green]{self.entra_id_tenant_id}[/]." + f"[bold]Entra ID tenant ID[/] will be [green]{self.entra_tenant_id}[/]." ) # Set fully-qualified domain name if fqdn: @@ -239,7 +239,7 @@ def template(cls) -> Config: shm=ConfigSectionSHM.model_construct( admin_email_address="Admin email address", admin_ip_addresses=["Admin IP addresses"], - entra_id_tenant_id="Entra ID tenant ID", + entra_tenant_id="Entra ID tenant ID", fqdn="TRE domain name", timezone="Timezone", ), diff --git a/data_safe_haven/infrastructure/programs/declarative_sre.py b/data_safe_haven/infrastructure/programs/declarative_sre.py index 458354a293..22f46aa15a 100644 --- a/data_safe_haven/infrastructure/programs/declarative_sre.py +++ b/data_safe_haven/infrastructure/programs/declarative_sre.py @@ -241,7 +241,7 @@ def __call__(self) -> None: dns_server_ip=dns.ip_address, entra_id_application_name=f"sre-{self.sre_name}-apricot", entra_id_auth_token=self.graph_api_token, - entra_id_tenant_id=self.cfg.shm.entra_id_tenant_id, + entra_tenant_id=self.cfg.shm.entra_tenant_id, location=self.context.location, networking_resource_group_name=networking.resource_group.name, shm_fqdn=self.cfg.shm.fqdn, @@ -281,7 +281,7 @@ def __call__(self) -> None: entra_id_application_fqdn=networking.sre_fqdn, entra_id_application_name=f"sre-{self.sre_name}-guacamole", entra_id_auth_token=self.graph_api_token, - entra_id_tenant_id=self.cfg.shm.entra_id_tenant_id, + entra_tenant_id=self.cfg.shm.entra_tenant_id, ldap_group_filter=ldap_group_filter, ldap_group_search_base=ldap_group_search_base, ldap_server_hostname=identity.hostname, diff --git a/data_safe_haven/infrastructure/programs/sre/identity.py b/data_safe_haven/infrastructure/programs/sre/identity.py index b1ee3dce0e..29fcba1167 100644 --- a/data_safe_haven/infrastructure/programs/sre/identity.py +++ b/data_safe_haven/infrastructure/programs/sre/identity.py @@ -26,7 +26,7 @@ def __init__( dns_server_ip: Input[str], entra_id_application_name: Input[str], entra_id_auth_token: Input[str], - entra_id_tenant_id: Input[str], + entra_tenant_id: Input[str], location: Input[str], networking_resource_group_name: Input[str], shm_fqdn: Input[str], @@ -40,7 +40,7 @@ def __init__( self.dns_server_ip = dns_server_ip self.entra_id_application_name = entra_id_application_name self.entra_id_auth_token = entra_id_auth_token - self.entra_id_tenant_id = entra_id_tenant_id + self.entra_tenant_id = entra_tenant_id self.location = location self.networking_resource_group_name = networking_resource_group_name self.shm_fqdn = shm_fqdn @@ -137,7 +137,7 @@ def __init__( ), containerinstance.EnvironmentVariableArgs( name="ENTRA_TENANT_ID", - value=props.entra_id_tenant_id, + value=props.entra_tenant_id, ), containerinstance.EnvironmentVariableArgs( name="REDIS_HOST", diff --git a/data_safe_haven/infrastructure/programs/sre/remote_desktop.py b/data_safe_haven/infrastructure/programs/sre/remote_desktop.py index 511bca5f4f..41c11850d3 100644 --- a/data_safe_haven/infrastructure/programs/sre/remote_desktop.py +++ b/data_safe_haven/infrastructure/programs/sre/remote_desktop.py @@ -38,7 +38,7 @@ def __init__( entra_id_application_fqdn: Input[str], entra_id_application_name: Input[str], entra_id_auth_token: Input[str], - entra_id_tenant_id: Input[str], + entra_tenant_id: Input[str], ldap_group_filter: Input[str], ldap_group_search_base: Input[str], ldap_server_hostname: Input[str], @@ -65,7 +65,7 @@ def __init__( "https://", entra_id_application_fqdn ) self.entra_id_auth_token = entra_id_auth_token - self.entra_id_tenant_id = entra_id_tenant_id + self.entra_tenant_id = entra_tenant_id self.ldap_group_filter = ldap_group_filter self.ldap_group_search_base = ldap_group_search_base self.ldap_server_hostname = ldap_server_hostname @@ -230,7 +230,7 @@ def __init__( name="OPENID_AUTHORIZATION_ENDPOINT", value=Output.concat( "https://login.microsoftonline.com/", - props.entra_id_tenant_id, + props.entra_tenant_id, "/oauth2/v2.0/authorize", ), ), @@ -242,7 +242,7 @@ def __init__( name="OPENID_ISSUER", value=Output.concat( "https://login.microsoftonline.com/", - props.entra_id_tenant_id, + props.entra_tenant_id, "/v2.0", ), ), @@ -250,7 +250,7 @@ def __init__( name="OPENID_JWKS_ENDPOINT", value=Output.concat( "https://login.microsoftonline.com/", - props.entra_id_tenant_id, + props.entra_tenant_id, "/discovery/v2.0/keys", ), ), diff --git a/tests/config/test_config.py b/tests/config/test_config.py index 308c4920e3..3939a88206 100644 --- a/tests/config/test_config.py +++ b/tests/config/test_config.py @@ -29,7 +29,7 @@ def test_constructor(self): ConfigSectionSHM( admin_email_address="admin@example.com", admin_ip_addresses=["0.0.0.0"], # noqa: S104 - entra_id_tenant_id="d5c5c439-1115-4cb6-ab50-b8e547b6c8dd", + entra_tenant_id="d5c5c439-1115-4cb6-ab50-b8e547b6c8dd", fqdn="shm.acme.com", timezone="UTC", ) diff --git a/tests/conftest.py b/tests/conftest.py index f0726e7998..5201009af4 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -64,7 +64,7 @@ def config_yaml(): admin_email_address: admin@example.com admin_ip_addresses: - 0.0.0.0/32 - entra_id_tenant_id: d5c5c439-1115-4cb6-ab50-b8e547b6c8dd + entra_tenant_id: d5c5c439-1115-4cb6-ab50-b8e547b6c8dd fqdn: shm.acme.com timezone: UTC sres: @@ -112,7 +112,7 @@ def shm_config(): return ConfigSectionSHM( admin_email_address="admin@example.com", admin_ip_addresses=["0.0.0.0"], # noqa: S104 - entra_id_tenant_id="d5c5c439-1115-4cb6-ab50-b8e547b6c8dd", + entra_tenant_id="d5c5c439-1115-4cb6-ab50-b8e547b6c8dd", fqdn="shm.acme.com", timezone="UTC", ) From ee0ffd989f1ec5127c18ab54902d032b16e71c3f Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 9 May 2024 11:24:54 +0100 Subject: [PATCH 08/16] :truck: Replace Entra ID directory with Entra ID. --- data_safe_haven/administration/users/entra_id_users.py | 4 ++-- data_safe_haven/external/api/graph_api.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/data_safe_haven/administration/users/entra_id_users.py b/data_safe_haven/administration/users/entra_id_users.py index b7bf82b4bb..870f5844ec 100644 --- a/data_safe_haven/administration/users/entra_id_users.py +++ b/data_safe_haven/administration/users/entra_id_users.py @@ -1,4 +1,4 @@ -"""Interact with users in an Azure Active Directory""" +"""Interact with users in Entra ID.""" from collections.abc import Sequence from typing import Any @@ -12,7 +12,7 @@ class EntraIDUsers: - """Interact with users in an Entra ID directory.""" + """Interact with users in Entra ID.""" def __init__( self, diff --git a/data_safe_haven/external/api/graph_api.py b/data_safe_haven/external/api/graph_api.py index d65cc4e6b2..968118be1a 100644 --- a/data_safe_haven/external/api/graph_api.py +++ b/data_safe_haven/external/api/graph_api.py @@ -411,11 +411,11 @@ def create_token_administrator(self) -> str: msg = f"Could not initiate device login for scopes {self.default_scopes}." raise DataSafeHavenMicrosoftGraphError(msg) self.logger.info( - "Administrator approval is needed in order to interact with Azure Active Directory." + "Administrator approval is needed in order to interact with Entra ID." ) self.logger.info( "Please sign-in with [bold]global administrator[/] credentials for" - f" Azure Active Directory [green]{self.tenant_id}[/]." + f" Entra ID tenant [green]{self.tenant_id}[/]." ) self.logger.info( "Note that the sign-in screen will prompt you to sign-in to" From 68ccc326f5f6b300f353ce2a424dc6d1b0991e01 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 9 May 2024 11:32:20 +0100 Subject: [PATCH 09/16] :coffin: Remove unused DataSafeHavenActiveDirectoryError --- data_safe_haven/exceptions/__init__.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/data_safe_haven/exceptions/__init__.py b/data_safe_haven/exceptions/__init__.py index 7941e762c3..2a519ec604 100644 --- a/data_safe_haven/exceptions/__init__.py +++ b/data_safe_haven/exceptions/__init__.py @@ -48,7 +48,3 @@ class DataSafeHavenMicrosoftGraphError(DataSafeHavenAzureError): class DataSafeHavenPulumiError(DataSafeHavenCloudError): pass - - -class DataSafeHavenActiveDirectoryError(DataSafeHavenCloudError): - pass From 3d1fc4768e3091aa4de2d71e7c31673b0f2bf717 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 9 May 2024 11:47:13 +0100 Subject: [PATCH 10/16] :loud_sound: Better exception handling for EntraIDUsers --- .../administration/users/entra_id_users.py | 190 ++++++++++++------ data_safe_haven/exceptions/__init__.py | 4 + data_safe_haven/external/api/graph_api.py | 2 +- 3 files changed, 131 insertions(+), 65 deletions(-) diff --git a/data_safe_haven/administration/users/entra_id_users.py b/data_safe_haven/administration/users/entra_id_users.py index 870f5844ec..7a99de4d5a 100644 --- a/data_safe_haven/administration/users/entra_id_users.py +++ b/data_safe_haven/administration/users/entra_id_users.py @@ -3,7 +3,11 @@ from collections.abc import Sequence from typing import Any -from data_safe_haven.exceptions import DataSafeHavenMicrosoftGraphError +from data_safe_haven.exceptions import ( + DataSafeHavenEntraIDError, + DataSafeHavenError, + DataSafeHavenInputError, +) from data_safe_haven.external import GraphApi from data_safe_haven.functions import password from data_safe_haven.utility import LoggingSingleton @@ -25,81 +29,139 @@ def __init__( self.logger = LoggingSingleton() def add(self, new_users: Sequence[ResearchUser]) -> None: - """Add list of users to Entra ID""" - # Get the default domain - default_domain = next( - domain["id"] - for domain in self.graph_api.read_domains() - if domain["isDefault"] - ) - for user in new_users: - request_json = { - "accountEnabled": user.account_enabled, - "displayName": user.display_name, - "givenName": user.given_name, - "surname": user.surname, - "mailNickname": user.username, - "passwordProfile": {"password": password(20)}, - "userPrincipalName": f"{user.username}@{default_domain}", - } - if user.email_address and user.phone_number: + """ + Add list of users to Entra ID + + Raises: + DataSafeHavenEntraIDError if any user could not be created + """ + try: + default_domain = next( + domain["id"] + for domain in self.graph_api.read_domains() + if domain["isDefault"] + ) + for user in new_users: + request_json = { + "accountEnabled": user.account_enabled, + "displayName": user.display_name, + "givenName": user.given_name, + "surname": user.surname, + "mailNickname": user.username, + "passwordProfile": {"password": password(20)}, + "userPrincipalName": f"{user.username}@{default_domain}", + } + if not user.email_address: + msg = ( + f"User '[green]{user.username}[/]' is missing an email address." + ) + raise DataSafeHavenInputError(msg) + if not user.phone_number: + msg = f"User '[green]{user.username}[/]' is missing a phone number." + raise DataSafeHavenInputError(msg) self.graph_api.create_user( request_json, user.email_address, user.phone_number ) - self.logger.info( - f"Ensured user '[green]{user.preferred_username}[/]' exists in Entra ID" - ) + self.logger.info( + f"Ensured user '[green]{user.preferred_username}[/]' exists in Entra ID" + ) + except DataSafeHavenError as exc: + msg = f"Unable to add users to Entra ID.\n{exc}" + raise DataSafeHavenEntraIDError(msg) from exc def list(self) -> Sequence[ResearchUser]: - user_list = self.graph_api.read_users() - return [ - ResearchUser( - account_enabled=user_details["accountEnabled"], - email_address=user_details["mail"], - given_name=user_details["givenName"], - phone_number=( - user_details["businessPhones"][0] - if len(user_details["businessPhones"]) - else None - ), - sam_account_name=( - user_details["onPremisesSamAccountName"] - if user_details["onPremisesSamAccountName"] - else user_details["mailNickname"] - ), - surname=user_details["surname"], - user_principal_name=user_details["userPrincipalName"], - ) - for user_details in user_list - ] + """ + List available Entra ID users + + Raises: + DataSafeHavenEntraIDError if users could not be loaded + """ + try: + user_list = self.graph_api.read_users() + return [ + ResearchUser( + account_enabled=user_details["accountEnabled"], + email_address=user_details["mail"], + given_name=user_details["givenName"], + phone_number=( + user_details["businessPhones"][0] + if len(user_details["businessPhones"]) + else None + ), + sam_account_name=( + user_details["onPremisesSamAccountName"] + if user_details["onPremisesSamAccountName"] + else user_details["mailNickname"] + ), + surname=user_details["surname"], + user_principal_name=user_details["userPrincipalName"], + ) + for user_details in user_list + ] + except DataSafeHavenError as exc: + msg = f"Unable list Entra ID users.\n{exc}" + raise DataSafeHavenEntraIDError(msg) from exc def register(self, sre_name: str, usernames: Sequence[str]) -> None: - """Add usernames to SRE group""" - group_name = f"Data Safe Haven SRE {sre_name} Users" - for username in usernames: - self.graph_api.add_user_to_group(username, group_name) + """ + Add usernames to SRE group + + Raises: + DataSafeHavenEntraIDError if any user could not be added to the group. + """ + try: + group_name = f"Data Safe Haven SRE {sre_name} Users" + for username in usernames: + self.graph_api.add_user_to_group(username, group_name) + except DataSafeHavenError as exc: + msg = f"Unable add users to group '{group_name}'.\n{exc}" + raise DataSafeHavenEntraIDError(msg) from exc def remove(self, users: Sequence[ResearchUser]) -> None: - """Remove list of users from Entra ID""" - for user in filter( - lambda existing_user: any(existing_user == user for user in users), - self.list(), - ): - try: + """ + Remove list of users from Entra ID + + Raises: + DataSafeHavenEntraIDError if any user could not be removed. + """ + try: + for user in filter( + lambda existing_user: any(existing_user == user for user in users), + self.list(), + ): self.graph_api.remove_user(user.username) self.logger.info(f"Removed '{user.preferred_username}'.") - except DataSafeHavenMicrosoftGraphError: - self.logger.error(f"Unable to remove '{user.preferred_username}'.") + except DataSafeHavenError as exc: + msg = f"Unable to remove users from Entra ID.\n{exc}" + raise DataSafeHavenEntraIDError(msg) from exc def set(self, users: Sequence[ResearchUser]) -> None: - """Set Entra ID users to specified list""" - users_to_remove = [user for user in self.list() if user not in users] - self.remove(users_to_remove) - users_to_add = [user for user in users if user not in self.list()] - self.add(users_to_add) + """ + Set Entra ID users to specified list + + Raises: + DataSafeHavenEntraIDError if user list could not be set + """ + try: + users_to_remove = [user for user in self.list() if user not in users] + self.remove(users_to_remove) + users_to_add = [user for user in users if user not in self.list()] + self.add(users_to_add) + except DataSafeHavenError as exc: + msg = f"Unable to set desired user list in Entra ID.\n{exc}" + raise DataSafeHavenEntraIDError(msg) from exc def unregister(self, sre_name: str, usernames: Sequence[str]) -> None: - """Remove usernames from SRE group""" - group_name = f"Data Safe Haven SRE {sre_name}" - for username in usernames: - self.graph_api.remove_user_from_group(username, group_name) + """ + Remove usernames from SRE group + + Raises: + DataSafeHavenEntraIDError if any user could not be added to the group. + """ + try: + group_name = f"Data Safe Haven SRE {sre_name}" + for username in usernames: + self.graph_api.remove_user_from_group(username, group_name) + except DataSafeHavenError as exc: + msg = f"Unable to remove users from group {group_name}.\n{exc}" + raise DataSafeHavenEntraIDError(msg) from exc diff --git a/data_safe_haven/exceptions/__init__.py b/data_safe_haven/exceptions/__init__.py index 2a519ec604..3ef0bbcac8 100644 --- a/data_safe_haven/exceptions/__init__.py +++ b/data_safe_haven/exceptions/__init__.py @@ -10,6 +10,10 @@ class DataSafeHavenConfigError(DataSafeHavenError): pass +class DataSafeHavenEntraIDError(DataSafeHavenCloudError): + pass + + class DataSafeHavenInputError(DataSafeHavenError): pass diff --git a/data_safe_haven/external/api/graph_api.py b/data_safe_haven/external/api/graph_api.py index 968118be1a..b55677a99c 100644 --- a/data_safe_haven/external/api/graph_api.py +++ b/data_safe_haven/external/api/graph_api.py @@ -137,7 +137,7 @@ def add_user_to_group( """Add a user to a group Raises: - DataSafeHavenMicrosoftGraphError if the token could not be created + DataSafeHavenMicrosoftGraphError if the user could not be added to the group. """ try: user_id = self.get_id_from_username(username) From 0e8809dee3346ccb2f621c8b8af433569797d528 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 9 May 2024 11:52:38 +0100 Subject: [PATCH 11/16] :loud_sound: Replace Entra ID user with Entra user Co-authored-by: Matt Craddock --- data_safe_haven/external/api/graph_api.py | 42 +++++++++---------- .../dynamic/entra_id_application.py | 2 +- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/data_safe_haven/external/api/graph_api.py b/data_safe_haven/external/api/graph_api.py index b55677a99c..847e6d0da8 100644 --- a/data_safe_haven/external/api/graph_api.py +++ b/data_safe_haven/external/api/graph_api.py @@ -173,7 +173,7 @@ def create_application( delegated_scopes: Sequence[str] = [], request_json: dict[str, Any] | None = None, ) -> dict[str, Any]: - """Create an Entra ID application if it does not already exist + """Create an Entra application if it does not already exist Raises: DataSafeHavenMicrosoftGraphError if the application could not be created @@ -257,7 +257,7 @@ def create_application( msg = "Maximum attempts to validate service principle permissions exceeded" raise DataSafeHavenMicrosoftGraphError(msg) - # Return JSON representation of the Entra ID application + # Return JSON representation of the Entra application return json_response except Exception as exc: msg = f"Could not create application '{application_name}'.\n{exc}" @@ -266,7 +266,7 @@ def create_application( def create_application_secret( self, application_name: str, application_secret_name: str ) -> str: - """Add a secret to an existing Entra ID application + """Add a secret to an existing Entra application Returns: str: Contents of newly-created secret @@ -312,7 +312,7 @@ def create_application_secret( raise DataSafeHavenMicrosoftGraphError(msg) from exc def create_group(self, group_name: str) -> None: - """Create an Entra ID group if it does not already exist + """Create an Entra group if it does not already exist Raises: DataSafeHavenMicrosoftGraphError if the group could not be created @@ -320,11 +320,11 @@ def create_group(self, group_name: str) -> None: try: if self.get_id_from_groupname(group_name): self.logger.info( - f"Found existing Entra ID group '[green]{group_name}[/]'.", + f"Found existing Entra group '[green]{group_name}[/]'.", ) return self.logger.debug( - f"Creating Entra ID group '[green]{group_name}[/]'...", + f"Creating Entra group '[green]{group_name}[/]'...", ) request_json = { "description": group_name, @@ -339,16 +339,16 @@ def create_group(self, group_name: str) -> None: json=request_json, ).json() self.logger.info( - f"Created Entra ID group '[green]{group_name}[/]'.", + f"Created Entra group '[green]{group_name}[/]'.", ) except Exception as exc: - msg = f"Could not create Entra ID group '{group_name}'.\n{exc}" + msg = f"Could not create Entra group '{group_name}'.\n{exc}" raise DataSafeHavenMicrosoftGraphError(msg) from exc def ensure_application_service_principal( self, application_name: str ) -> dict[str, Any]: - """Create a service principal for an Entra ID application if it does not already exist + """Create a service principal for an Entra application if it does not already exist Raises: DataSafeHavenMicrosoftGraphError if the service principal could not be created @@ -470,7 +470,7 @@ def create_user( email_address: str, phone_number: str, ) -> None: - """Create an Entra ID user if it does not already exist + """Create an Entra user if it does not already exist Raises: DataSafeHavenMicrosoftGraphError if the user could not be created @@ -482,12 +482,12 @@ def create_user( user_id = self.get_id_from_username(username) if user_id: self.logger.debug( - f"Updating Entra ID user '[green]{username}[/]'...", + f"Updating Entra user '[green]{username}[/]'...", ) final_verb = "Update" else: self.logger.debug( - f"Creating Entra ID user '[green]{username}[/]'...", + f"Creating Entra user '[green]{username}[/]'...", ) final_verb = "Create" # If they do not then create them @@ -523,7 +523,7 @@ def create_user( json={"accountEnabled": True}, ) self.logger.info( - f"{final_verb}d Entra ID user '[green]{username}[/]'.", + f"{final_verb}d Entra user '[green]{username}[/]'.", ) except DataSafeHavenMicrosoftGraphError as exc: msg = f"Could not {final_verb.lower()} user {username}.\n{exc}" @@ -917,10 +917,10 @@ def read_application_permissions( raise DataSafeHavenMicrosoftGraphError(msg) from exc def read_domains(self) -> Sequence[dict[str, Any]]: - """Get details of Entra ID domains + """Get details of Entra domains Returns: - JSON: A JSON list of Entra ID domains + JSON: A JSON list of Entra domains Raises: DataSafeHavenMicrosoftGraphError if domains could not be loaded @@ -936,7 +936,7 @@ def read_groups( self, attributes: Sequence[str] | None = None, ) -> Sequence[dict[str, Any]]: - """Get details of Entra ID groups + """Get details of Entra groups Returns: JSON: A JSON list of Entra ID groups @@ -969,10 +969,10 @@ def read_service_principals(self) -> Sequence[dict[str, Any]]: def read_users( self, attributes: Sequence[str] | None = None ) -> Sequence[dict[str, Any]]: - """Get details of Entra ID users + """Get details of Entra users Returns: - JSON: A JSON list of Entra ID users + JSON: A JSON list of Entra users Raises: DataSafeHavenMicrosoftGraphError if users could not be loaded @@ -1041,7 +1041,7 @@ def remove_user_from_group( username: str, group_name: str, ) -> None: - """Remove a user from an Entra ID group + """Remove a user from an Entra group Raises: DataSafeHavenMicrosoftGraphError if the user could not be removed @@ -1076,13 +1076,13 @@ def remove_user_from_group( def verify_custom_domain( self, domain_name: str, expected_nameservers: Sequence[str] ) -> None: - """Verify Entra ID custom domain + """Verify Entra custom domain Raises: DataSafeHavenMicrosoftGraphError if domain could not be verified """ try: - # Create the Entra ID custom domain if it does not already exist + # Create the Entra custom domain if it does not already exist domains = self.read_domains() if not any(d["id"] == domain_name for d in domains): msg = f"Domain {domain_name} has not been added to Entra ID." diff --git a/data_safe_haven/infrastructure/components/dynamic/entra_id_application.py b/data_safe_haven/infrastructure/components/dynamic/entra_id_application.py index 8391bc964a..0e8425bb31 100644 --- a/data_safe_haven/infrastructure/components/dynamic/entra_id_application.py +++ b/data_safe_haven/infrastructure/components/dynamic/entra_id_application.py @@ -120,7 +120,7 @@ def create(self, props: dict[str, Any]) -> CreateResult: ) def delete(self, id_: str, props: dict[str, Any]) -> None: - """Delete an Entra ID application.""" + """Delete an Entra application.""" # Use `id` as a no-op to avoid ARG002 while maintaining function signature id(id_) try: From 9a704addba974884f128c3745dc4d391fcad4b0a Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 9 May 2024 11:57:11 +0100 Subject: [PATCH 12/16] :truck: Rename EntraIDApplication to EntraApplication --- .../infrastructure/components/__init__.py | 8 ++--- .../components/dynamic/__init__.py | 6 ++-- ...id_application.py => entra_application.py} | 16 ++++----- .../programs/declarative_sre.py | 10 +++--- .../infrastructure/programs/sre/identity.py | 26 +++++++------- .../programs/sre/remote_desktop.py | 34 +++++++++---------- 6 files changed, 49 insertions(+), 51 deletions(-) rename data_safe_haven/infrastructure/components/dynamic/{entra_id_application.py => entra_application.py} (94%) diff --git a/data_safe_haven/infrastructure/components/__init__.py b/data_safe_haven/infrastructure/components/__init__.py index d2c7e2df04..8342aa18bd 100644 --- a/data_safe_haven/infrastructure/components/__init__.py +++ b/data_safe_haven/infrastructure/components/__init__.py @@ -16,8 +16,8 @@ BlobContainerAclProps, CompiledDsc, CompiledDscProps, - EntraIDApplication, - EntraIDApplicationProps, + EntraApplication, + EntraApplicationProps, FileShareFile, FileShareFileProps, FileUpload, @@ -39,8 +39,8 @@ "BlobContainerAclProps", "CompiledDsc", "CompiledDscProps", - "EntraIDApplication", - "EntraIDApplicationProps", + "EntraApplication", + "EntraApplicationProps", "FileShareFile", "FileShareFileProps", "FileUpload", diff --git a/data_safe_haven/infrastructure/components/dynamic/__init__.py b/data_safe_haven/infrastructure/components/dynamic/__init__.py index ca1b86c731..f28be2aa31 100644 --- a/data_safe_haven/infrastructure/components/dynamic/__init__.py +++ b/data_safe_haven/infrastructure/components/dynamic/__init__.py @@ -1,6 +1,6 @@ from .blob_container_acl import BlobContainerAcl, BlobContainerAclProps from .compiled_dsc import CompiledDsc, CompiledDscProps -from .entra_id_application import EntraIDApplication, EntraIDApplicationProps +from .entra_application import EntraApplication, EntraApplicationProps from .file_share_file import FileShareFile, FileShareFileProps from .file_upload import FileUpload, FileUploadProps from .remote_script import RemoteScript, RemoteScriptProps @@ -11,8 +11,8 @@ "BlobContainerAclProps", "CompiledDsc", "CompiledDscProps", - "EntraIDApplication", - "EntraIDApplicationProps", + "EntraApplication", + "EntraApplicationProps", "FileShareFile", "FileShareFileProps", "FileUpload", diff --git a/data_safe_haven/infrastructure/components/dynamic/entra_id_application.py b/data_safe_haven/infrastructure/components/dynamic/entra_application.py similarity index 94% rename from data_safe_haven/infrastructure/components/dynamic/entra_id_application.py rename to data_safe_haven/infrastructure/components/dynamic/entra_application.py index 0e8425bb31..9e5bd6d155 100644 --- a/data_safe_haven/infrastructure/components/dynamic/entra_id_application.py +++ b/data_safe_haven/infrastructure/components/dynamic/entra_application.py @@ -12,8 +12,8 @@ from .dsh_resource_provider import DshResourceProvider -class EntraIDApplicationProps: - """Props for the EntraIDApplication class""" +class EntraApplicationProps: + """Props for the EntraApplication class""" def __init__( self, @@ -34,7 +34,7 @@ def __init__( self.web_redirect_url = web_redirect_url -class EntraIDApplicationProvider(DshResourceProvider): +class EntraApplicationProvider(DshResourceProvider): @staticmethod def refresh(props: dict[str, Any]) -> dict[str, Any]: try: @@ -115,7 +115,7 @@ def create(self, props: dict[str, Any]) -> CreateResult: msg = f"Failed to create application [green]{props['application_name']}[/] in Entra ID.\n{exc}" raise DataSafeHavenMicrosoftGraphError(msg) from exc return CreateResult( - f"EntraIDApplication-{props['application_name']}", + f"EntraApplication-{props['application_name']}", outs=outs, ) @@ -162,20 +162,20 @@ def update( raise DataSafeHavenMicrosoftGraphError(msg) from exc -class EntraIDApplication(Resource): +class EntraApplication(Resource): application_id: Output[str] application_secret: Output[str] object_id: Output[str] - _resource_type_name = "dsh:common:EntraIDApplication" # set resource type + _resource_type_name = "dsh:common:EntraApplication" # set resource type def __init__( self, name: str, - props: EntraIDApplicationProps, + props: EntraApplicationProps, opts: ResourceOptions | None = None, ): super().__init__( - EntraIDApplicationProvider(), + EntraApplicationProvider(), name, { "application_id": None, diff --git a/data_safe_haven/infrastructure/programs/declarative_sre.py b/data_safe_haven/infrastructure/programs/declarative_sre.py index 22f46aa15a..8385ad5a80 100644 --- a/data_safe_haven/infrastructure/programs/declarative_sre.py +++ b/data_safe_haven/infrastructure/programs/declarative_sre.py @@ -239,8 +239,8 @@ def __call__(self) -> None: SREIdentityProps( dns_resource_group_name=dns.resource_group.name, dns_server_ip=dns.ip_address, - entra_id_application_name=f"sre-{self.sre_name}-apricot", - entra_id_auth_token=self.graph_api_token, + entra_application_name=f"sre-{self.sre_name}-apricot", + entra_auth_token=self.graph_api_token, entra_tenant_id=self.cfg.shm.entra_tenant_id, location=self.context.location, networking_resource_group_name=networking.resource_group.name, @@ -278,9 +278,9 @@ def __call__(self) -> None: allow_paste=self.cfg.sre(self.sre_name).remote_desktop.allow_paste, database_password=data.password_user_database_admin, dns_server_ip=dns.ip_address, - entra_id_application_fqdn=networking.sre_fqdn, - entra_id_application_name=f"sre-{self.sre_name}-guacamole", - entra_id_auth_token=self.graph_api_token, + entra_application_fqdn=networking.sre_fqdn, + entra_application_name=f"sre-{self.sre_name}-guacamole", + entra_auth_token=self.graph_api_token, entra_tenant_id=self.cfg.shm.entra_tenant_id, ldap_group_filter=ldap_group_filter, ldap_group_search_base=ldap_group_search_base, diff --git a/data_safe_haven/infrastructure/programs/sre/identity.py b/data_safe_haven/infrastructure/programs/sre/identity.py index 29fcba1167..467409bc90 100644 --- a/data_safe_haven/infrastructure/programs/sre/identity.py +++ b/data_safe_haven/infrastructure/programs/sre/identity.py @@ -10,8 +10,8 @@ get_ip_address_from_container_group, ) from data_safe_haven.infrastructure.components import ( - EntraIDApplication, - EntraIDApplicationProps, + EntraApplication, + EntraApplicationProps, LocalDnsRecordComponent, LocalDnsRecordProps, ) @@ -24,8 +24,8 @@ def __init__( self, dns_resource_group_name: Input[str], dns_server_ip: Input[str], - entra_id_application_name: Input[str], - entra_id_auth_token: Input[str], + entra_application_name: Input[str], + entra_auth_token: Input[str], entra_tenant_id: Input[str], location: Input[str], networking_resource_group_name: Input[str], @@ -38,8 +38,8 @@ def __init__( ) -> None: self.dns_resource_group_name = dns_resource_group_name self.dns_server_ip = dns_server_ip - self.entra_id_application_name = entra_id_application_name - self.entra_id_auth_token = entra_id_auth_token + self.entra_application_name = entra_application_name + self.entra_auth_token = entra_auth_token self.entra_tenant_id = entra_tenant_id self.location = location self.networking_resource_group_name = networking_resource_group_name @@ -93,13 +93,13 @@ def __init__( ) # Define Entra ID application - entra_id_application = EntraIDApplication( - f"{self._name}_entra_id_application", - EntraIDApplicationProps( - application_name=props.entra_id_application_name, + entra_application = EntraApplication( + f"{self._name}_entra_application", + EntraApplicationProps( + application_name=props.entra_application_name, application_role_assignments=["User.Read.All", "GroupMember.Read.All"], application_secret_name="Apricot Authentication Secret", - auth_token=props.entra_id_auth_token, + auth_token=props.entra_auth_token, delegated_role_assignments=["User.Read.All"], public_client_redirect_uri="urn:ietf:wg:oauth:2.0:oob", ), @@ -121,11 +121,11 @@ def __init__( ), containerinstance.EnvironmentVariableArgs( name="CLIENT_ID", - value=entra_id_application.application_id, + value=entra_application.application_id, ), containerinstance.EnvironmentVariableArgs( name="CLIENT_SECRET", - secure_value=entra_id_application.application_secret, + secure_value=entra_application.application_secret, ), containerinstance.EnvironmentVariableArgs( name="DEBUG", diff --git a/data_safe_haven/infrastructure/programs/sre/remote_desktop.py b/data_safe_haven/infrastructure/programs/sre/remote_desktop.py index 41c11850d3..f17b89907b 100644 --- a/data_safe_haven/infrastructure/programs/sre/remote_desktop.py +++ b/data_safe_haven/infrastructure/programs/sre/remote_desktop.py @@ -15,8 +15,8 @@ get_id_from_subnet, ) from data_safe_haven.infrastructure.components import ( - EntraIDApplication, - EntraIDApplicationProps, + EntraApplication, + EntraApplicationProps, FileShareFile, FileShareFileProps, PostgresqlDatabaseComponent, @@ -35,9 +35,9 @@ def __init__( allow_paste: Input[bool], database_password: Input[str], dns_server_ip: Input[str], - entra_id_application_fqdn: Input[str], - entra_id_application_name: Input[str], - entra_id_auth_token: Input[str], + entra_application_fqdn: Input[str], + entra_application_name: Input[str], + entra_auth_token: Input[str], entra_tenant_id: Input[str], ldap_group_filter: Input[str], ldap_group_search_base: Input[str], @@ -60,11 +60,9 @@ def __init__( self.disable_copy = not allow_copy self.disable_paste = not allow_paste self.dns_server_ip = dns_server_ip - self.entra_id_application_name = entra_id_application_name - self.entra_id_application_url = Output.concat( - "https://", entra_id_application_fqdn - ) - self.entra_id_auth_token = entra_id_auth_token + self.entra_application_name = entra_application_name + self.entra_application_url = Output.concat("https://", entra_application_fqdn) + self.entra_auth_token = entra_auth_token self.entra_tenant_id = entra_tenant_id self.ldap_group_filter = ldap_group_filter self.ldap_group_search_base = ldap_group_search_base @@ -133,12 +131,12 @@ def __init__( ) # Define Entra ID application - entra_id_application = EntraIDApplication( - f"{self._name}_entra_id_application", - EntraIDApplicationProps( - application_name=props.entra_id_application_name, - auth_token=props.entra_id_auth_token, - web_redirect_url=props.entra_id_application_url, + entra_application = EntraApplication( + f"{self._name}_entra_application", + EntraApplicationProps( + application_name=props.entra_application_name, + auth_token=props.entra_auth_token, + web_redirect_url=props.entra_application_url, ), opts=child_opts, ) @@ -236,7 +234,7 @@ def __init__( ), containerinstance.EnvironmentVariableArgs( name="OPENID_CLIENT_ID", - value=entra_id_application.application_id, + value=entra_application.application_id, ), containerinstance.EnvironmentVariableArgs( name="OPENID_ISSUER", @@ -256,7 +254,7 @@ def __init__( ), containerinstance.EnvironmentVariableArgs( name="OPENID_REDIRECT_URI", - value=props.entra_id_application_url, + value=props.entra_application_url, ), containerinstance.EnvironmentVariableArgs( name="OPENID_USERNAME_CLAIM_TYPE", From 39a0642afd13f7c2079ead31304bd59d6f891e35 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 9 May 2024 12:02:50 +0100 Subject: [PATCH 13/16] :truck: Rename EntraIDUsers to EntraUsers --- .../users/{entra_id_users.py => entra_users.py} | 10 +++++----- data_safe_haven/administration/users/user_handler.py | 12 ++++++------ data_safe_haven/commands/admin_register_users.py | 2 +- data_safe_haven/commands/admin_unregister_users.py | 4 +--- 4 files changed, 13 insertions(+), 15 deletions(-) rename data_safe_haven/administration/users/{entra_id_users.py => entra_users.py} (96%) diff --git a/data_safe_haven/administration/users/entra_id_users.py b/data_safe_haven/administration/users/entra_users.py similarity index 96% rename from data_safe_haven/administration/users/entra_id_users.py rename to data_safe_haven/administration/users/entra_users.py index 7a99de4d5a..755f441573 100644 --- a/data_safe_haven/administration/users/entra_id_users.py +++ b/data_safe_haven/administration/users/entra_users.py @@ -15,7 +15,7 @@ from .research_user import ResearchUser -class EntraIDUsers: +class EntraUsers: """Interact with users in Entra ID.""" def __init__( @@ -71,7 +71,7 @@ def add(self, new_users: Sequence[ResearchUser]) -> None: def list(self) -> Sequence[ResearchUser]: """ - List available Entra ID users + List available Entra users Raises: DataSafeHavenEntraIDError if users could not be loaded @@ -104,7 +104,7 @@ def list(self) -> Sequence[ResearchUser]: def register(self, sre_name: str, usernames: Sequence[str]) -> None: """ - Add usernames to SRE group + Add usernames to SRE group in Entra ID Raises: DataSafeHavenEntraIDError if any user could not be added to the group. @@ -137,7 +137,7 @@ def remove(self, users: Sequence[ResearchUser]) -> None: def set(self, users: Sequence[ResearchUser]) -> None: """ - Set Entra ID users to specified list + Set Entra users to specified list Raises: DataSafeHavenEntraIDError if user list could not be set @@ -153,7 +153,7 @@ def set(self, users: Sequence[ResearchUser]) -> None: def unregister(self, sre_name: str, usernames: Sequence[str]) -> None: """ - Remove usernames from SRE group + Remove usernames from SRE group in Entra ID Raises: DataSafeHavenEntraIDError if any user could not be added to the group. diff --git a/data_safe_haven/administration/users/user_handler.py b/data_safe_haven/administration/users/user_handler.py index a66856531b..25bb35dfb2 100644 --- a/data_safe_haven/administration/users/user_handler.py +++ b/data_safe_haven/administration/users/user_handler.py @@ -8,7 +8,7 @@ from data_safe_haven.external import GraphApi from data_safe_haven.utility import LoggingSingleton -from .entra_id_users import EntraIDUsers +from .entra_users import EntraUsers from .guacamole_users import GuacamoleUsers from .research_user import ResearchUser @@ -21,7 +21,7 @@ def __init__( pulumi_config: DSHPulumiConfig, graph_api: GraphApi, ): - self.entra_id_users = EntraIDUsers(graph_api) + self.entra_id_users = EntraUsers(graph_api) self.context = context self.config = config self.pulumi_config = pulumi_config @@ -29,7 +29,7 @@ def __init__( self.sre_guacamole_users_: dict[str, GuacamoleUsers] = {} def add(self, users_csv_path: pathlib.Path) -> None: - """Add Entra ID and Guacamole users + """Add users to Entra ID and Guacamole Raises: DataSafeHavenUserHandlingError if the users could not be added @@ -75,7 +75,7 @@ def add(self, users_csv_path: pathlib.Path) -> None: def get_usernames(self) -> dict[str, list[str]]: """Load usernames from all sources""" usernames = {} - usernames["Azure AD"] = self.get_usernames_azure_ad() + usernames["Entra ID"] = self.get_usernames_entra_id() for sre_name in self.config.sre_names: usernames[f"SRE {sre_name}"] = self.get_usernames_guacamole( sre_name, @@ -83,8 +83,8 @@ def get_usernames(self) -> dict[str, list[str]]: ) return usernames - def get_usernames_azure_ad(self) -> list[str]: - """Load usernames from Azure AD""" + def get_usernames_entra_id(self) -> list[str]: + """Load usernames from Entra ID""" return [user.username for user in self.entra_id_users.list()] def get_usernames_guacamole( diff --git a/data_safe_haven/commands/admin_register_users.py b/data_safe_haven/commands/admin_register_users.py index 03aa68ecd4..23d51d2069 100644 --- a/data_safe_haven/commands/admin_register_users.py +++ b/data_safe_haven/commands/admin_register_users.py @@ -39,7 +39,7 @@ def admin_register_users( # List users users = UserHandler(context, config, pulumi_config, graph_api) - available_usernames = users.get_usernames_azure_ad() + available_usernames = users.get_usernames_entra_id() usernames_to_register = [] for username in usernames: if username in available_usernames: diff --git a/data_safe_haven/commands/admin_unregister_users.py b/data_safe_haven/commands/admin_unregister_users.py index af5a67c8ee..f5aa2aae82 100644 --- a/data_safe_haven/commands/admin_unregister_users.py +++ b/data_safe_haven/commands/admin_unregister_users.py @@ -21,8 +21,6 @@ def admin_unregister_users( shm_name = context.shm_name sre_name = sanitise_sre_name(sre) - # sre_pulumi_project = pulumi_config.create_or_select_project(sre_name) - try: # Check that SRE option has been provided if not sre_name: @@ -40,7 +38,7 @@ def admin_unregister_users( # List users users = UserHandler(context, config, pulumi_config, graph_api) - available_usernames = users.get_usernames_azure_ad() + available_usernames = users.get_usernames_entra_id() usernames_to_unregister = [] for username in usernames: if username in available_usernames: From 3fba250ef48d52b7af1e1e88333ee8cd12700282 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 9 May 2024 12:04:07 +0100 Subject: [PATCH 14/16] :truck: Use entra_users instead of entra_id_users --- .../administration/users/user_handler.py | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/data_safe_haven/administration/users/user_handler.py b/data_safe_haven/administration/users/user_handler.py index 25bb35dfb2..570e142090 100644 --- a/data_safe_haven/administration/users/user_handler.py +++ b/data_safe_haven/administration/users/user_handler.py @@ -21,7 +21,7 @@ def __init__( pulumi_config: DSHPulumiConfig, graph_api: GraphApi, ): - self.entra_id_users = EntraUsers(graph_api) + self.entra_users = EntraUsers(graph_api) self.context = context self.config = config self.pulumi_config = pulumi_config @@ -67,7 +67,7 @@ def add(self, users_csv_path: pathlib.Path) -> None: self.logger.debug(f"Processing new user: {user}") # Add users to Entra ID - self.entra_id_users.add(users) + self.entra_users.add(users) except Exception as exc: msg = f"Could not add users from '{users_csv_path}'.\n{exc}" raise DataSafeHavenUserHandlingError(msg) from exc @@ -85,7 +85,7 @@ def get_usernames(self) -> dict[str, list[str]]: def get_usernames_entra_id(self) -> list[str]: """Load usernames from Entra ID""" - return [user.username for user in self.entra_id_users.list()] + return [user.username for user in self.entra_users.list()] def get_usernames_guacamole( self, sre_name: str, sre_pulumi_project: DSHPulumiProject @@ -140,7 +140,7 @@ def register(self, sre_name: str, user_names: Sequence[str]) -> None: """ try: # Add users to the SRE security group - self.entra_id_users.register(sre_name, user_names) + self.entra_users.register(sre_name, user_names) except Exception as exc: msg = f"Could not register {len(user_names)} users with SRE '{sre_name}'.\n{exc}" raise DataSafeHavenUserHandlingError(msg) from exc @@ -154,17 +154,17 @@ def remove(self, user_names: Sequence[str]) -> None: try: # Construct user lists self.logger.info(f"Attempting to remove {len(user_names)} user(s).") - entra_id_users_to_remove = [ + entra_users_to_remove = [ user - for user in self.entra_id_users.list() + for user in self.entra_users.list() if user.username in user_names ] # Commit changes self.logger.info( - f"Found {len(entra_id_users_to_remove)} valid user(s) to remove." + f"Found {len(entra_users_to_remove)} valid user(s) to remove." ) - self.entra_id_users.remove(entra_id_users_to_remove) + self.entra_users.remove(entra_users_to_remove) except Exception as exc: msg = f"Could not remove users: {user_names}.\n{exc}" raise DataSafeHavenUserHandlingError(msg) from exc @@ -199,19 +199,19 @@ def set(self, users_csv_path: str) -> None: self.logger.debug(f"Processing user: {user}") # Keep existing users with the same username - entra_id_desired_users = [ + entra_desired_users = [ user - for user in self.entra_id_users.list() + for user in self.entra_users.list() if user.username in [u.username for u in desired_users] ] # Construct list of new users - entra_id_desired_users = [ - user for user in desired_users if user not in entra_id_desired_users + entra_desired_users = [ + user for user in desired_users if user not in entra_desired_users ] # Commit changes - self.entra_id_users.set(entra_id_desired_users) + self.entra_users.set(entra_desired_users) except Exception as exc: msg = f"Could not set users from '{users_csv_path}'.\n{exc}" raise DataSafeHavenUserHandlingError(msg) from exc @@ -224,7 +224,7 @@ def unregister(self, sre_name: str, user_names: Sequence[str]) -> None: """ try: # Remove users from the SRE security group - self.entra_id_users.unregister(sre_name, user_names) + self.entra_users.unregister(sre_name, user_names) except Exception as exc: msg = f"Could not unregister {len(user_names)} users with SRE '{sre_name}'.\n{exc}" raise DataSafeHavenUserHandlingError(msg) from exc From 7f15260be4eb93f5db6a73b2f4c308eb93030f3b Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 9 May 2024 12:10:32 +0100 Subject: [PATCH 15/16] :memo: Additional Entra ID typos --- data_safe_haven/config/config.py | 6 +++--- data_safe_haven/external/api/graph_api.py | 2 +- .../infrastructure/components/dynamic/entra_application.py | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/data_safe_haven/config/config.py b/data_safe_haven/config/config.py index 2fd1b3443b..c751651667 100644 --- a/data_safe_haven/config/config.py +++ b/data_safe_haven/config/config.py @@ -72,11 +72,11 @@ def update( logger.info( f"[bold]IP addresses used by administrators[/] will be [green]{self.admin_ip_addresses}[/]." ) - # Set Entra ID tenant ID + # Set Entra tenant ID if entra_tenant_id: self.entra_tenant_id = entra_tenant_id logger.info( - f"[bold]Entra ID tenant ID[/] will be [green]{self.entra_tenant_id}[/]." + f"[bold]Entra tenant ID[/] will be [green]{self.entra_tenant_id}[/]." ) # Set fully-qualified domain name if fqdn: @@ -239,7 +239,7 @@ def template(cls) -> Config: shm=ConfigSectionSHM.model_construct( admin_email_address="Admin email address", admin_ip_addresses=["Admin IP addresses"], - entra_tenant_id="Entra ID tenant ID", + entra_tenant_id="Entra tenant ID", fqdn="TRE domain name", timezone="Timezone", ), diff --git a/data_safe_haven/external/api/graph_api.py b/data_safe_haven/external/api/graph_api.py index 847e6d0da8..bb9441cd6d 100644 --- a/data_safe_haven/external/api/graph_api.py +++ b/data_safe_haven/external/api/graph_api.py @@ -415,7 +415,7 @@ def create_token_administrator(self) -> str: ) self.logger.info( "Please sign-in with [bold]global administrator[/] credentials for" - f" Entra ID tenant [green]{self.tenant_id}[/]." + f" Entra tenant '[green]{self.tenant_id}[/]'." ) self.logger.info( "Note that the sign-in screen will prompt you to sign-in to" diff --git a/data_safe_haven/infrastructure/components/dynamic/entra_application.py b/data_safe_haven/infrastructure/components/dynamic/entra_application.py index 9e5bd6d155..c6c0e3758a 100644 --- a/data_safe_haven/infrastructure/components/dynamic/entra_application.py +++ b/data_safe_haven/infrastructure/components/dynamic/entra_application.py @@ -1,4 +1,4 @@ -"""Pulumi dynamic component for Entra ID applications.""" +"""Pulumi dynamic component for Entra applications.""" from contextlib import suppress from typing import Any @@ -65,7 +65,7 @@ def refresh(props: dict[str, Any]) -> dict[str, Any]: raise DataSafeHavenMicrosoftGraphError(msg) from exc def create(self, props: dict[str, Any]) -> CreateResult: - """Create new Entra ID application.""" + """Create new Entra application.""" outs = dict(**props) try: graph_api = GraphApi(auth_token=props["auth_token"], disable_logging=True) From e08e24b126167303f811319ed5785bca52bb4a6b Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 9 May 2024 12:13:53 +0100 Subject: [PATCH 16/16] :rotating_light: Apply linting fixes --- data_safe_haven/administration/users/user_handler.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/data_safe_haven/administration/users/user_handler.py b/data_safe_haven/administration/users/user_handler.py index 570e142090..57c339c7a0 100644 --- a/data_safe_haven/administration/users/user_handler.py +++ b/data_safe_haven/administration/users/user_handler.py @@ -155,9 +155,7 @@ def remove(self, user_names: Sequence[str]) -> None: # Construct user lists self.logger.info(f"Attempting to remove {len(user_names)} user(s).") entra_users_to_remove = [ - user - for user in self.entra_users.list() - if user.username in user_names + user for user in self.entra_users.list() if user.username in user_names ] # Commit changes