From de17e608cddbba563219ab211ceffdc4a13a37d8 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Tue, 13 Feb 2024 10:39:30 +0000 Subject: [PATCH 01/32] :sparkles: Initial skeleton of SRE identity component --- .../infrastructure/stacks/declarative_sre.py | 13 ++++++ .../infrastructure/stacks/sre/identity.py | 41 +++++++++++++++++++ 2 files changed, 54 insertions(+) create mode 100644 data_safe_haven/infrastructure/stacks/sre/identity.py diff --git a/data_safe_haven/infrastructure/stacks/declarative_sre.py b/data_safe_haven/infrastructure/stacks/declarative_sre.py index bca7f651b4..0cc370d15f 100644 --- a/data_safe_haven/infrastructure/stacks/declarative_sre.py +++ b/data_safe_haven/infrastructure/stacks/declarative_sre.py @@ -21,6 +21,10 @@ SREDnsServerComponent, SREDnsServerProps, ) +from .sre.identity import ( + SREIdentityComponent, + SREIdentityProps, +) from .sre.monitoring import ( SREMonitoringComponent, SREMonitoringProps, @@ -132,6 +136,15 @@ def run(self) -> None: tags=self.cfg.tags.model_dump(), ) + # Deploy identity server + SREIdentityComponent( + "sre_identity", + self.stack_name, + SREIdentityProps( + location=self.cfg.azure.location, + ), + ) + # Deploy automated monitoring SREMonitoringComponent( "sre_monitoring", diff --git a/data_safe_haven/infrastructure/stacks/sre/identity.py b/data_safe_haven/infrastructure/stacks/sre/identity.py new file mode 100644 index 0000000000..c37dbe1e72 --- /dev/null +++ b/data_safe_haven/infrastructure/stacks/sre/identity.py @@ -0,0 +1,41 @@ +"""Pulumi component for SRE identity""" + +from collections.abc import Mapping + +from pulumi import ComponentResource, Input, ResourceOptions +from pulumi_azure_native import resources + + +class SREIdentityProps: + """Properties for SREIdentityComponent""" + + def __init__( + self, + location: Input[str], + ) -> None: + self.location = location + + +class SREIdentityComponent(ComponentResource): + """Deploy SRE backup with Pulumi""" + + def __init__( + self, + name: str, + stack_name: str, + props: SREIdentityProps, + opts: ResourceOptions | None = None, + tags: Input[Mapping[str, Input[str]]] | None = None, + ) -> None: + super().__init__("dsh:sre:IdentityComponent", name, {}, opts) + child_opts = ResourceOptions.merge(opts, ResourceOptions(parent=self)) + child_tags = tags if tags else {} + + # Deploy resource group + resource_group = resources.ResourceGroup( + f"{self._name}_resource_group", + location=props.location, + resource_group_name=f"{stack_name}-rg-identity", + opts=child_opts, + tags=child_tags, + ) From d4b4b6fbbc3df0ddc7ea1d3b956de2f8420fc783 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Tue, 13 Feb 2024 14:45:22 +0000 Subject: [PATCH 02/32] :sparkles: Add identity subnet and container group --- .../infrastructure/common/ip_ranges.py | 2 + .../infrastructure/stacks/declarative_sre.py | 1 + .../infrastructure/stacks/sre/identity.py | 88 ++++++++++++++++++- .../infrastructure/stacks/sre/networking.py | 72 +++++++++++++++ 4 files changed, 161 insertions(+), 2 deletions(-) diff --git a/data_safe_haven/infrastructure/common/ip_ranges.py b/data_safe_haven/infrastructure/common/ip_ranges.py index e636531295..cb605e4dfe 100644 --- a/data_safe_haven/infrastructure/common/ip_ranges.py +++ b/data_safe_haven/infrastructure/common/ip_ranges.py @@ -16,6 +16,8 @@ def __init__(self, index: int) -> None: self.data_private = self.vnet.next_subnet(8) self.guacamole_containers = self.vnet.next_subnet(8) self.guacamole_containers_support = self.vnet.next_subnet(8) + self.identity_containers = self.vnet.next_subnet(8) + self.identity_containers_support = self.vnet.next_subnet(8) self.user_services_containers = self.vnet.next_subnet(8) self.user_services_containers_support = self.vnet.next_subnet(8) self.user_services_databases = self.vnet.next_subnet(8) diff --git a/data_safe_haven/infrastructure/stacks/declarative_sre.py b/data_safe_haven/infrastructure/stacks/declarative_sre.py index 0cc370d15f..69aa02e6ec 100644 --- a/data_safe_haven/infrastructure/stacks/declarative_sre.py +++ b/data_safe_haven/infrastructure/stacks/declarative_sre.py @@ -142,6 +142,7 @@ def run(self) -> None: self.stack_name, SREIdentityProps( location=self.cfg.azure.location, + subnet_containers=networking.subnet_identity_containers ), ) diff --git a/data_safe_haven/infrastructure/stacks/sre/identity.py b/data_safe_haven/infrastructure/stacks/sre/identity.py index c37dbe1e72..c255387383 100644 --- a/data_safe_haven/infrastructure/stacks/sre/identity.py +++ b/data_safe_haven/infrastructure/stacks/sre/identity.py @@ -2,8 +2,8 @@ from collections.abc import Mapping -from pulumi import ComponentResource, Input, ResourceOptions -from pulumi_azure_native import resources +from pulumi import ComponentResource, Input, Output, ResourceOptions +from pulumi_azure_native import containerinstance, network, resources class SREIdentityProps: @@ -12,8 +12,10 @@ class SREIdentityProps: def __init__( self, location: Input[str], + subnet_containers: Input[network.GetSubnetResult] ) -> None: self.location = location + self.subnet_containers_id = Output.from_input(subnet_containers).apply(lambda s: str(s.id)) class SREIdentityComponent(ComponentResource): @@ -39,3 +41,85 @@ def __init__( opts=child_opts, tags=child_tags, ) + + # Define the LDAP server container group with Apricot + container_group = containerinstance.ContainerGroup( + f"{self._name}_container_group", + container_group_name=f"{stack_name}-container-group-identity", + containers=[ + containerinstance.ContainerArgs( + image="ghcr.io/alan-turing-institute/apricot:0.0.4", + name="apricot", + environment_variables=[ + containerinstance.EnvironmentVariableArgs( + name="BACKEND", + value="MicrosoftEntra", + ), + containerinstance.EnvironmentVariableArgs( + name="CLIENT_ID", + value="MicrosoftEntra", + ), + containerinstance.EnvironmentVariableArgs( + name="CLIENT_SECRET", + value="MicrosoftEntra", + ), + containerinstance.EnvironmentVariableArgs( + name="DOMAIN", + value="MicrosoftEntra", + ), + containerinstance.EnvironmentVariableArgs( + name="ENTRA_TENANT_ID", + value="MicrosoftEntra", + ), + ], + # All Azure Container Instances need to expose port 80 on at least + # one container even if there's nothing behind it. + ports=[ + containerinstance.ContainerPortArgs( + port=80, + protocol=containerinstance.ContainerGroupNetworkProtocol.TCP, + ), + containerinstance.ContainerPortArgs( + port=1389, + protocol=containerinstance.ContainerGroupNetworkProtocol.TCP, + ), + ], + resources=containerinstance.ResourceRequirementsArgs( + requests=containerinstance.ResourceRequestsArgs( + cpu=1, + memory_in_gb=1, + ), + ), + volume_mounts=[ + # containerinstance.VolumeMountArgs( + # mount_path="/opt/adguardhome/custom", + # name="adguard-opt-adguardhome-custom", + # read_only=True, + # ), + ], + ), + ], + ip_address=containerinstance.IpAddressArgs( + ports=[ + containerinstance.PortArgs( + port=80, + protocol=containerinstance.ContainerGroupNetworkProtocol.TCP, + ) + ], + type=containerinstance.ContainerGroupIpAddressType.PRIVATE, + ), + os_type=containerinstance.OperatingSystemTypes.LINUX, + resource_group_name=resource_group.name, + restart_policy=containerinstance.ContainerGroupRestartPolicy.ALWAYS, + sku=containerinstance.ContainerGroupSku.STANDARD, + subnet_ids=[containerinstance.ContainerGroupSubnetIdArgs(id=props.subnet_containers_id)], + volumes=[], + opts=ResourceOptions.merge( + child_opts, + ResourceOptions( + delete_before_replace=True, + replace_on_changes=["containers"], + ), + ), + tags=child_tags, + ) diff --git a/data_safe_haven/infrastructure/stacks/sre/networking.py b/data_safe_haven/infrastructure/stacks/sre/networking.py index 499e59fa26..8ce1027782 100644 --- a/data_safe_haven/infrastructure/stacks/sre/networking.py +++ b/data_safe_haven/infrastructure/stacks/sre/networking.py @@ -53,6 +53,12 @@ def __init__( self.subnet_guacamole_containers_support_iprange = subnet_ranges.apply( lambda s: s.guacamole_containers_support ) + self.subnet_identity_containers_iprange = subnet_ranges.apply( + lambda s: s.identity_containers + ) + self.subnet_identity_containers_support_iprange = subnet_ranges.apply( + lambda s: s.identity_containers_support + ) self.subnet_user_services_containers_iprange = subnet_ranges.apply( lambda s: s.user_services_containers ) @@ -147,6 +153,12 @@ def __init__( subnet_guacamole_containers_support_prefix = ( props.subnet_guacamole_containers_support_iprange.apply(lambda r: str(r)) ) + subnet_identity_containers_prefix = ( + props.subnet_identity_containers_iprange.apply(lambda r: str(r)) + ) + subnet_identity_containers_support_prefix = ( + props.subnet_identity_containers_support_iprange.apply(lambda r: str(r)) + ) subnet_user_services_containers_prefix = ( props.subnet_user_services_containers_iprange.apply(lambda r: str(r)) ) @@ -609,6 +621,28 @@ def __init__( opts=child_opts, tags=child_tags, ) + nsg_identity_containers = network.NetworkSecurityGroup( + f"{self._name}_nsg_identity_containers", + network_security_group_name=f"{stack_name}-nsg-identity-containers", + resource_group_name=resource_group.name, + security_rules=[ + # Inbound + # Outbound + ], + opts=child_opts, + tags=child_tags, + ) + nsg_identity_containers_support = network.NetworkSecurityGroup( + f"{self._name}_nsg_identity_containers_support", + network_security_group_name=f"{stack_name}-nsg-identity-containers-support", + resource_group_name=resource_group.name, + security_rules=[ + # Inbound + # Outbound + ], + opts=child_opts, + tags=child_tags, + ) nsg_user_services_containers = network.NetworkSecurityGroup( f"{self._name}_nsg_user_services_containers", network_security_group_name=f"{stack_name}-nsg-user-services-containers", @@ -1142,6 +1176,8 @@ def __init__( subnet_data_private_name = "DataPrivateSubnet" subnet_guacamole_containers_name = "GuacamoleContainersSubnet" subnet_guacamole_containers_support_name = "GuacamoleContainersSupportSubnet" + subnet_identity_containers_name = "IdentityContainersSubnet" + subnet_identity_containers_support_name = "IdentityContainersSupportSubnet" subnet_user_services_containers_name = "UserServicesContainersSubnet" subnet_user_services_containers_support_name = ( "UserServicesContainersSupportSubnet" @@ -1225,6 +1261,32 @@ def __init__( private_endpoint_network_policies=network.VirtualNetworkPrivateEndpointNetworkPolicies.ENABLED, route_table=network.RouteTableArgs(id=route_table.id), ), + # Identity containers + network.SubnetArgs( + address_prefix=subnet_identity_containers_prefix, + delegations=[ + network.DelegationArgs( + name="SubnetDelegationContainerGroups", + service_name="Microsoft.ContainerInstance/containerGroups", + type="Microsoft.Network/virtualNetworks/subnets/delegations", + ), + ], + name=subnet_identity_containers_name, + network_security_group=network.NetworkSecurityGroupArgs( + id=nsg_identity_containers.id + ), + route_table=network.RouteTableArgs(id=route_table.id), + ), + # Identity containers support + network.SubnetArgs( + address_prefix=subnet_identity_containers_support_prefix, + name=subnet_identity_containers_support_name, + network_security_group=network.NetworkSecurityGroupArgs( + id=nsg_identity_containers_support.id + ), + private_endpoint_network_policies=network.VirtualNetworkPrivateEndpointNetworkPolicies.ENABLED, + route_table=network.RouteTableArgs(id=route_table.id), + ), # User services containers network.SubnetArgs( address_prefix=subnet_user_services_containers_prefix, @@ -1490,6 +1552,16 @@ def __init__( resource_group_name=resource_group.name, virtual_network_name=sre_virtual_network.name, ) + self.subnet_identity_containers = network.get_subnet_output( + subnet_name=subnet_identity_containers_name, + resource_group_name=resource_group.name, + virtual_network_name=sre_virtual_network.name, + ) + self.subnet_identity_containers_support = network.get_subnet_output( + subnet_name=subnet_identity_containers_support_name, + resource_group_name=resource_group.name, + virtual_network_name=sre_virtual_network.name, + ) self.subnet_data_private = network.get_subnet_output( subnet_name=subnet_data_private_name, resource_group_name=resource_group.name, From 6d3a6d15f3c292e2cadf3f6f9c967315a8c80410 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Wed, 14 Feb 2024 15:44:18 +0000 Subject: [PATCH 03/32] :sparkles: Add an AzureADApplication to the identity component --- data_safe_haven/external/api/graph_api.py | 2 +- .../infrastructure/stacks/declarative_sre.py | 8 +++- .../infrastructure/stacks/sre/identity.py | 46 ++++++++++++++++--- 3 files changed, 46 insertions(+), 10 deletions(-) diff --git a/data_safe_haven/external/api/graph_api.py b/data_safe_haven/external/api/graph_api.py index adf3705c3b..808ed0c2da 100644 --- a/data_safe_haven/external/api/graph_api.py +++ b/data_safe_haven/external/api/graph_api.py @@ -682,7 +682,7 @@ def grant_application_role_permissions( "resourceId": microsoft_graph_sp["id"], "appRoleId": app_role_id, } - response = self.http_post( + self.http_post( f"{self.base_endpoint}/servicePrincipals/{microsoft_graph_sp['id']}/appRoleAssignments", json=request_json, ) diff --git a/data_safe_haven/infrastructure/stacks/declarative_sre.py b/data_safe_haven/infrastructure/stacks/declarative_sre.py index 69aa02e6ec..f951cf9c49 100644 --- a/data_safe_haven/infrastructure/stacks/declarative_sre.py +++ b/data_safe_haven/infrastructure/stacks/declarative_sre.py @@ -141,8 +141,12 @@ def run(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, location=self.cfg.azure.location, - subnet_containers=networking.subnet_identity_containers + shm_fqdn=self.cfg.shm.fqdn, + subnet_containers=networking.subnet_identity_containers, ), ) @@ -213,7 +217,7 @@ def run(self) -> None: "sre_remote_desktop", self.stack_name, SRERemoteDesktopProps( - aad_application_name=f"sre-{self.sre_name}-azuread-guacamole", + aad_application_name=f"sre-{self.sre_name}-guacamole", aad_application_fqdn=networking.sre_fqdn, aad_auth_token=self.graph_api_token, aad_tenant_id=self.cfg.shm.aad_tenant_id, diff --git a/data_safe_haven/infrastructure/stacks/sre/identity.py b/data_safe_haven/infrastructure/stacks/sre/identity.py index c255387383..d0c41cfeeb 100644 --- a/data_safe_haven/infrastructure/stacks/sre/identity.py +++ b/data_safe_haven/infrastructure/stacks/sre/identity.py @@ -5,17 +5,32 @@ from pulumi import ComponentResource, Input, Output, ResourceOptions from pulumi_azure_native import containerinstance, network, resources +from data_safe_haven.infrastructure.components import ( + AzureADApplication, + AzureADApplicationProps, +) + class SREIdentityProps: """Properties for SREIdentityComponent""" def __init__( self, + aad_application_name: Input[str], + aad_auth_token: Input[str], + aad_tenant_id: Input[str], location: Input[str], - subnet_containers: Input[network.GetSubnetResult] + shm_fqdn: 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.location = location - self.subnet_containers_id = Output.from_input(subnet_containers).apply(lambda s: str(s.id)) + self.shm_fqdn = shm_fqdn + self.subnet_containers_id = Output.from_input(subnet_containers).apply( + lambda s: str(s.id) + ) class SREIdentityComponent(ComponentResource): @@ -42,6 +57,19 @@ def __init__( tags=child_tags, ) + # Define AzureAD application + aad_application = AzureADApplication( + f"{self._name}_aad_application", + AzureADApplicationProps( + application_name=props.aad_application_name, + application_role_assignments=["User.Read.All", "GroupMember.Read.All"], + application_secret_name="Apricot Authentication Secret", + auth_token=props.aad_auth_token, + public_client_redirect_uri="urn:ietf:wg:oauth:2.0:oob", + ), + opts=child_opts, + ) + # Define the LDAP server container group with Apricot container_group = containerinstance.ContainerGroup( f"{self._name}_container_group", @@ -57,19 +85,19 @@ def __init__( ), containerinstance.EnvironmentVariableArgs( name="CLIENT_ID", - value="MicrosoftEntra", + value=aad_application.application_id, ), containerinstance.EnvironmentVariableArgs( name="CLIENT_SECRET", - value="MicrosoftEntra", + secure_value=aad_application.application_secret, ), containerinstance.EnvironmentVariableArgs( name="DOMAIN", - value="MicrosoftEntra", + value=props.shm_fqdn, ), containerinstance.EnvironmentVariableArgs( name="ENTRA_TENANT_ID", - value="MicrosoftEntra", + value=props.aad_tenant_id, ), ], # All Azure Container Instances need to expose port 80 on at least @@ -112,7 +140,11 @@ def __init__( resource_group_name=resource_group.name, restart_policy=containerinstance.ContainerGroupRestartPolicy.ALWAYS, sku=containerinstance.ContainerGroupSku.STANDARD, - subnet_ids=[containerinstance.ContainerGroupSubnetIdArgs(id=props.subnet_containers_id)], + subnet_ids=[ + containerinstance.ContainerGroupSubnetIdArgs( + id=props.subnet_containers_id + ) + ], volumes=[], opts=ResourceOptions.merge( child_opts, From f69cb841c6707ba2a15259d896bb1d9c8a5abd47 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 15 Feb 2024 15:35:20 +0000 Subject: [PATCH 04/32] :sparkles: Add a firewall rule for SRE identity servers --- .../infrastructure/stacks/shm/firewall.py | 32 +++++++++++++++++-- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/data_safe_haven/infrastructure/stacks/shm/firewall.py b/data_safe_haven/infrastructure/stacks/shm/firewall.py index 2e003341e7..92f520e5e7 100644 --- a/data_safe_haven/infrastructure/stacks/shm/firewall.py +++ b/data_safe_haven/infrastructure/stacks/shm/firewall.py @@ -69,6 +69,10 @@ def __init__( "time3.google.com", "time4.google.com", ] + sre_identity_server_subnets = [ + str(SREIpRanges(idx).identity_containers) + for idx in range(1, SREIpRanges.max_index) + ] sre_package_repositories_subnets = [ str(SREIpRanges(idx).user_services_software_repositories) for idx in range(1, SREIpRanges.max_index) @@ -375,8 +379,30 @@ def __init__( ), network.AzureFirewallApplicationRuleCollectionArgs( action=network.AzureFirewallRCActionArgs(type="Allow"), - name=f"{stack_name}-sre-package-repositories", + name=f"{stack_name}-sre-identity-servers", priority=1100, + rules=[ + network.AzureFirewallApplicationRuleArgs( + description="Allow external OAuth login requests", + name="AllowExternalOAuthLogin", + protocols=[ + network.AzureFirewallApplicationRuleProtocolArgs( + port=443, + protocol_type="Https", + ) + ], + source_addresses=sre_identity_server_subnets, + target_fqdns=[ + "graph.microsoft.com", + "login.microsoftonline.com", + ], + ), + ], + ), + network.AzureFirewallApplicationRuleCollectionArgs( + action=network.AzureFirewallRCActionArgs(type="Allow"), + name=f"{stack_name}-sre-package-repositories", + priority=1110, rules=[ network.AzureFirewallApplicationRuleArgs( description="Allow external CRAN package requests", @@ -407,7 +433,7 @@ def __init__( network.AzureFirewallApplicationRuleCollectionArgs( action=network.AzureFirewallRCActionArgs(type="Allow"), name=f"{stack_name}-sre-remote-desktop-gateways", - priority=1110, + priority=1120, rules=[ network.AzureFirewallApplicationRuleArgs( description="Allow external OAuth login requests", @@ -426,7 +452,7 @@ def __init__( network.AzureFirewallApplicationRuleCollectionArgs( action=network.AzureFirewallRCActionArgs(type="Allow"), name=f"{stack_name}-sre-workspaces", - priority=1120, + priority=1130, rules=[ network.AzureFirewallApplicationRuleArgs( description="Allow external Linux ClamAV update requests", From a2a790fa323040928efb570138d8634df3a88a08 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Wed, 21 Feb 2024 15:43:38 +0000 Subject: [PATCH 05/32] :wrench: Add network rules to allow workspaces to connect to SRE identity server --- .../infrastructure/common/enums.py | 1 + .../infrastructure/stacks/sre/networking.py | 24 +++++++++++++++++++ .../stacks/sre/remote_desktop.py | 8 +++---- .../workspace.cloud_init.mustache.yaml | 4 ++-- 4 files changed, 31 insertions(+), 6 deletions(-) diff --git a/data_safe_haven/infrastructure/common/enums.py b/data_safe_haven/infrastructure/common/enums.py index 78570cb8fa..be20353f12 100644 --- a/data_safe_haven/infrastructure/common/enums.py +++ b/data_safe_haven/infrastructure/common/enums.py @@ -24,6 +24,7 @@ class NetworkingPriorities(int, Enum): INTERNAL_SRE_DATA_PRIVATE = 1700 INTERNAL_SRE_GUACAMOLE_CONTAINERS = 1800 INTERNAL_SRE_GUACAMOLE_CONTAINERS_SUPPORT = 1900 + INTERNAL_SRE_IDENTITY_CONTAINERS = 1950 INTERNAL_SRE_USER_SERVICES_CONTAINERS = 2000 INTERNAL_SRE_USER_SERVICES_CONTAINERS_SUPPORT = 2100 INTERNAL_SRE_USER_SERVICES_DATABASES = 2200 diff --git a/data_safe_haven/infrastructure/stacks/sre/networking.py b/data_safe_haven/infrastructure/stacks/sre/networking.py index 8ce1027782..155fbaf4fe 100644 --- a/data_safe_haven/infrastructure/stacks/sre/networking.py +++ b/data_safe_haven/infrastructure/stacks/sre/networking.py @@ -627,6 +627,18 @@ def __init__( resource_group_name=resource_group.name, security_rules=[ # Inbound + network.SecurityRuleArgs( + access=network.SecurityRuleAccess.ALLOW, + description="Allow LDAP client requests over TCP.", + destination_address_prefix=subnet_identity_containers_prefix, + destination_port_ranges=["389", "1389"], + direction=network.SecurityRuleDirection.INBOUND, + name="AllowLDAPClientTCPInbound", + priority=NetworkingPriorities.INTERNAL_SRE_WORKSPACES, + protocol=network.SecurityRuleProtocol.TCP, + source_address_prefix=subnet_workspaces_prefix, + source_port_range="*", + ), # Outbound ], opts=child_opts, @@ -1045,6 +1057,18 @@ def __init__( source_address_prefix=subnet_workspaces_prefix, source_port_range="*", ), + network.SecurityRuleArgs( + access=network.SecurityRuleAccess.ALLOW, + description="Allow LDAP client requests over TCP.", + destination_address_prefix=subnet_identity_containers_prefix, + destination_port_ranges=["389", "1389"], + direction=network.SecurityRuleDirection.OUTBOUND, + name="AllowSREIdentityServersOutbound", + priority=NetworkingPriorities.INTERNAL_SRE_IDENTITY_CONTAINERS, + protocol=network.SecurityRuleProtocol.TCP, + source_address_prefix=subnet_workspaces_prefix, + source_port_range="*", + ), network.SecurityRuleArgs( access=network.SecurityRuleAccess.ALLOW, description="Allow LDAP client requests over UDP.", diff --git a/data_safe_haven/infrastructure/stacks/sre/remote_desktop.py b/data_safe_haven/infrastructure/stacks/sre/remote_desktop.py index 27263ab814..01e40c67d0 100644 --- a/data_safe_haven/infrastructure/stacks/sre/remote_desktop.py +++ b/data_safe_haven/infrastructure/stacks/sre/remote_desktop.py @@ -227,7 +227,7 @@ def __init__( ), containerinstance.EnvironmentVariableArgs( name="OPENID_AUTHORIZATION_ENDPOINT", - value=f"https://login.microsoftonline.com/{props.aad_tenant_id}/oauth2/v2.0/authorize", + value=Output.concat("https://login.microsoftonline.com/", props.aad_tenant_id, "/oauth2/v2.0/authorize"), ), containerinstance.EnvironmentVariableArgs( name="OPENID_CLIENT_ID", @@ -235,11 +235,11 @@ def __init__( ), containerinstance.EnvironmentVariableArgs( name="OPENID_ISSUER", - value=f"https://login.microsoftonline.com/{props.aad_tenant_id}/v2.0", + value=Output.concat("https://login.microsoftonline.com/", props.aad_tenant_id, "/v2.0"), ), containerinstance.EnvironmentVariableArgs( name="OPENID_JWKS_ENDPOINT", - value=f"https://login.microsoftonline.com/{props.aad_tenant_id}/discovery/v2.0/keys", + value=Output.concat("https://login.microsoftonline.com/", props.aad_tenant_id, "/discovery/v2.0/keys"), ), containerinstance.EnvironmentVariableArgs( name="OPENID_REDIRECT_URI", value=props.aad_application_url @@ -295,7 +295,7 @@ def __init__( ), ), containerinstance.ContainerArgs( - image="ghcr.io/alan-turing-institute/guacamole-user-sync:v0.2.0", + image="ghcr.io/alan-turing-institute/guacamole-user-sync:v0.3.0", name="guacamole-user-sync"[:63], environment_variables=[ containerinstance.EnvironmentVariableArgs( diff --git a/data_safe_haven/resources/workspace/workspace.cloud_init.mustache.yaml b/data_safe_haven/resources/workspace/workspace.cloud_init.mustache.yaml index c4216adb76..647e26ebe6 100644 --- a/data_safe_haven/resources/workspace/workspace.cloud_init.mustache.yaml +++ b/data_safe_haven/resources/workspace/workspace.cloud_init.mustache.yaml @@ -28,10 +28,10 @@ write_files: base {{ldap_group_search_base}} # All users that are members of the correct security group - filter passwd (&(objectClass=user)(memberOf=CN={{ldap_user_security_group_name}},OU=Data Safe Haven Security Groups,{{ldap_root_dn}})) + filter passwd (&(objectClass=user)(memberOf=CN={{ldap_user_group_name}},OU=Data Safe Haven Security Groups,{{ldap_root_dn}})) # One group for each security group and for each user - filter group (|(objectclass=group)(&(objectClass=user)(memberOf=CN={{ldap_user_security_group_name}},OU=Data Safe Haven Security Groups,{{ldap_root_dn}}))) + filter group (|(objectclass=group)(&(objectClass=user)(memberOf=CN={{ldap_user_group_name}},OU=Data Safe Haven Security Groups,{{ldap_root_dn}}))) # Attribute mappings map passwd uid sAMAccountName From 0d4421eb1fb93e55ff523fa0e4b26e483dba5f48 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Mon, 26 Feb 2024 12:53:29 +0000 Subject: [PATCH 06/32] :recycle: Simplify 'security_group_name' to 'group_name' --- .../infrastructure/stacks/declarative_sre.py | 24 +++++++++---------- .../infrastructure/stacks/sre/gitea_server.py | 6 ++--- .../stacks/sre/hedgedoc_server.py | 8 +++---- .../stacks/sre/remote_desktop.py | 6 ++--- .../stacks/sre/user_services.py | 8 +++---- .../infrastructure/stacks/sre/workspaces.py | 13 ++++++---- 6 files changed, 33 insertions(+), 32 deletions(-) diff --git a/data_safe_haven/infrastructure/stacks/declarative_sre.py b/data_safe_haven/infrastructure/stacks/declarative_sre.py index f951cf9c49..2d664c2752 100644 --- a/data_safe_haven/infrastructure/stacks/declarative_sre.py +++ b/data_safe_haven/infrastructure/stacks/declarative_sre.py @@ -75,13 +75,10 @@ def run(self) -> None: ldap_server_ip = self.pulumi_opts.require( "shm-domain_controllers-ldap_server_ip" ) - ldap_admin_security_group_name = ( - f"Data Safe Haven SRE {self.sre_name} Administrators" - ) - ldap_privileged_user_security_group_name = ( - f"Data Safe Haven SRE {self.sre_name} Privileged Users" - ) - ldap_user_security_group_name = f"Data Safe Haven SRE {self.sre_name} Users" + ldap_base_group_name = f"Data Safe Haven SRE {self.sre_name}" + ldap_admin_group_name = f"{ldap_base_group_name} Administrators" + ldap_privileged_user_group_name = f"{ldap_base_group_name} Privileged Users" + ldap_user_group_name = f"{ldap_base_group_name} Users" # Deploy SRE DNS server dns = SREDnsServerComponent( @@ -230,7 +227,7 @@ def run(self) -> None: ldap_search_password=ldap_search_password, ldap_server_ip=ldap_server_ip, ldap_user_search_base=ldap_user_search_base, - ldap_user_security_group_name=ldap_user_security_group_name, + ldap_user_group_name=ldap_user_group_name, location=self.cfg.azure.location, subnet_guacamole_containers=networking.subnet_guacamole_containers, subnet_guacamole_containers_support=networking.subnet_guacamole_containers_support, @@ -250,13 +247,14 @@ def run(self) -> None: domain_sid=self.pulumi_opts.require( "shm-domain_controllers-domain_sid" ), + ldap_base_group_name=ldap_base_group_name, ldap_bind_dn=ldap_bind_dn, ldap_group_search_base=ldap_group_search_base, ldap_root_dn=ldap_root_dn, ldap_search_password=ldap_search_password, ldap_server_ip=ldap_server_ip, ldap_user_search_base=ldap_user_search_base, - ldap_user_security_group_name=ldap_user_security_group_name, + ldap_user_group_name=ldap_user_group_name, linux_update_server_ip=self.pulumi_opts.require( "shm-update_servers-ip_address_linux" ), @@ -298,7 +296,7 @@ def run(self) -> None: ldap_root_dn=ldap_root_dn, ldap_search_password=ldap_search_password, ldap_server_ip=ldap_server_ip, - ldap_user_security_group_name=ldap_user_security_group_name, + ldap_user_group_name=ldap_user_group_name, ldap_user_search_base=ldap_user_search_base, location=self.cfg.azure.location, networking_resource_group_name=networking.resource_group.name, @@ -334,9 +332,9 @@ def run(self) -> None: pulumi.export( "ldap", { - "admin_security_group_name": ldap_admin_security_group_name, - "privileged_user_security_group_name": ldap_privileged_user_security_group_name, - "user_security_group_name": ldap_user_security_group_name, + "admin_security_group_name": ldap_admin_group_name, + "privileged_user_security_group_name": ldap_privileged_user_group_name, + "user_security_group_name": ldap_user_group_name, }, ) pulumi.export("remote_desktop", remote_desktop.exports) diff --git a/data_safe_haven/infrastructure/stacks/sre/gitea_server.py b/data_safe_haven/infrastructure/stacks/sre/gitea_server.py index 4107ae3d26..35c35b50d8 100644 --- a/data_safe_haven/infrastructure/stacks/sre/gitea_server.py +++ b/data_safe_haven/infrastructure/stacks/sre/gitea_server.py @@ -33,7 +33,7 @@ def __init__( ldap_search_password: Input[str], ldap_server_ip: Input[str], ldap_user_search_base: Input[str], - ldap_user_security_group_name: Input[str], + ldap_user_group_name: Input[str], location: Input[str], networking_resource_group_name: Input[str], sre_fqdn: Input[str], @@ -57,7 +57,7 @@ def __init__( self.ldap_search_password = ldap_search_password self.ldap_server_ip = ldap_server_ip self.ldap_user_search_base = ldap_user_search_base - self.ldap_user_security_group_name = ldap_user_security_group_name + self.ldap_user_group_name = ldap_user_group_name self.location = location self.networking_resource_group_name = networking_resource_group_name self.sre_fqdn = sre_fqdn @@ -133,7 +133,7 @@ def __init__( ldap_bind_dn=props.ldap_bind_dn, ldap_root_dn=props.ldap_root_dn, ldap_search_password=props.ldap_search_password, - ldap_user_security_group_name=props.ldap_user_security_group_name, + ldap_user_group_name=props.ldap_user_group_name, ldap_server_ip=props.ldap_server_ip, ldap_user_search_base=props.ldap_user_search_base, ).apply( diff --git a/data_safe_haven/infrastructure/stacks/sre/hedgedoc_server.py b/data_safe_haven/infrastructure/stacks/sre/hedgedoc_server.py index 89e40be7fa..d87d478648 100644 --- a/data_safe_haven/infrastructure/stacks/sre/hedgedoc_server.py +++ b/data_safe_haven/infrastructure/stacks/sre/hedgedoc_server.py @@ -35,7 +35,7 @@ def __init__( ldap_search_password: Input[str], ldap_server_ip: Input[str], ldap_user_search_base: Input[str], - ldap_user_security_group_name: Input[str], + ldap_user_group_name: Input[str], location: Input[str], networking_resource_group_name: Input[str], sre_fqdn: Input[str], @@ -60,8 +60,8 @@ def __init__( self.ldap_search_password = ldap_search_password self.ldap_server_ip = ldap_server_ip self.ldap_user_search_base = ldap_user_search_base - self.ldap_user_security_group_cn = Output.all( - group_name=ldap_user_security_group_name, root_dn=ldap_root_dn + self.ldap_user_group_cn = Output.all( + group_name=ldap_user_group_name, root_dn=ldap_root_dn ).apply( lambda kwargs: ",".join( ( @@ -242,7 +242,7 @@ def __init__( "(&", "(objectClass=user)", "(memberOf=CN=", - props.ldap_user_security_group_cn, + props.ldap_user_group_cn, ")", "(sAMAccountName={{username}}))", ), diff --git a/data_safe_haven/infrastructure/stacks/sre/remote_desktop.py b/data_safe_haven/infrastructure/stacks/sre/remote_desktop.py index 01e40c67d0..040912e1d5 100644 --- a/data_safe_haven/infrastructure/stacks/sre/remote_desktop.py +++ b/data_safe_haven/infrastructure/stacks/sre/remote_desktop.py @@ -45,7 +45,7 @@ def __init__( ldap_search_password: Input[str], ldap_server_ip: Input[str], ldap_user_search_base: Input[str], - ldap_user_security_group_name: Input[str], + ldap_user_group_name: Input[str], location: Input[str], storage_account_key: Input[str], storage_account_name: Input[str], @@ -70,7 +70,7 @@ def __init__( self.ldap_search_password = ldap_search_password self.ldap_server_ip = ldap_server_ip self.ldap_user_search_base = ldap_user_search_base - self.ldap_user_security_group_name = ldap_user_security_group_name + self.ldap_user_group_name = ldap_user_group_name self.location = location self.storage_account_key = storage_account_key self.storage_account_name = storage_account_name @@ -326,7 +326,7 @@ def __init__( name="LDAP_USER_FILTER", value=Output.concat( "(&(objectClass=user)(memberOf=CN=", - props.ldap_user_security_group_name, + props.ldap_user_group_name, ",", props.ldap_group_search_base, "))", diff --git a/data_safe_haven/infrastructure/stacks/sre/user_services.py b/data_safe_haven/infrastructure/stacks/sre/user_services.py index 96beeb769d..331d07b7e7 100644 --- a/data_safe_haven/infrastructure/stacks/sre/user_services.py +++ b/data_safe_haven/infrastructure/stacks/sre/user_services.py @@ -32,7 +32,7 @@ def __init__( ldap_search_password: Input[str], ldap_server_ip: Input[str], ldap_user_search_base: Input[str], - ldap_user_security_group_name: Input[str], + ldap_user_group_name: Input[str], location: Input[str], networking_resource_group_name: Input[str], nexus_admin_password: Input[str], @@ -59,7 +59,7 @@ def __init__( self.ldap_search_password = ldap_search_password self.ldap_server_ip = ldap_server_ip self.ldap_user_search_base = ldap_user_search_base - self.ldap_user_security_group_name = ldap_user_security_group_name + self.ldap_user_group_name = ldap_user_group_name self.location = location self.networking_resource_group_name = networking_resource_group_name self.nexus_admin_password = Output.secret(nexus_admin_password) @@ -121,7 +121,7 @@ def __init__( ldap_root_dn=props.ldap_root_dn, ldap_search_password=props.ldap_search_password, ldap_server_ip=props.ldap_server_ip, - ldap_user_security_group_name=props.ldap_user_security_group_name, + ldap_user_group_name=props.ldap_user_group_name, ldap_user_search_base=props.ldap_user_search_base, location=props.location, networking_resource_group_name=props.networking_resource_group_name, @@ -151,7 +151,7 @@ def __init__( ldap_root_dn=props.ldap_root_dn, ldap_search_password=props.ldap_search_password, ldap_server_ip=props.ldap_server_ip, - ldap_user_security_group_name=props.ldap_user_security_group_name, + ldap_user_group_name=props.ldap_user_group_name, ldap_user_search_base=props.ldap_user_search_base, location=props.location, networking_resource_group_name=props.networking_resource_group_name, diff --git a/data_safe_haven/infrastructure/stacks/sre/workspaces.py b/data_safe_haven/infrastructure/stacks/sre/workspaces.py index fdf3d46c4d..4599e0d87c 100644 --- a/data_safe_haven/infrastructure/stacks/sre/workspaces.py +++ b/data_safe_haven/infrastructure/stacks/sre/workspaces.py @@ -31,13 +31,14 @@ def __init__( self, admin_password: Input[str], domain_sid: Input[str], + ldap_base_group_name: Input[str], ldap_bind_dn: Input[str], ldap_group_search_base: Input[str], ldap_root_dn: Input[str], ldap_search_password: Input[str], ldap_server_ip: Input[str], ldap_user_search_base: Input[str], - ldap_user_security_group_name: Input[str], + ldap_user_group_name: Input[str], linux_update_server_ip: Input[str], location: Input[str], log_analytics_workspace_id: Input[str], @@ -55,13 +56,14 @@ def __init__( self.admin_password = Output.secret(admin_password) self.admin_username = "dshadmin" self.domain_sid = domain_sid + self.ldap_base_group_name = ldap_base_group_name self.ldap_bind_dn = ldap_bind_dn self.ldap_group_search_base = ldap_group_search_base self.ldap_root_dn = ldap_root_dn self.ldap_search_password = ldap_search_password self.ldap_server_ip = ldap_server_ip self.ldap_user_search_base = ldap_user_search_base - self.ldap_user_security_group_name = ldap_user_security_group_name + self.ldap_user_group_name = ldap_user_group_name self.linux_update_server_ip = linux_update_server_ip self.location = location self.log_analytics_workspace_id = log_analytics_workspace_id @@ -126,11 +128,12 @@ def __init__( # Load cloud-init file b64cloudinit = Output.all( domain_sid=props.domain_sid, + ldap_base_group_name=props.ldap_base_group_name, ldap_bind_dn=props.ldap_bind_dn, ldap_group_search_base=props.ldap_group_search_base, ldap_root_dn=props.ldap_root_dn, ldap_search_password=props.ldap_search_password, - ldap_user_security_group_name=props.ldap_user_security_group_name, + ldap_user_group_name=props.ldap_user_group_name, ldap_server_ip=props.ldap_server_ip, ldap_user_search_base=props.ldap_user_search_base, linux_update_server_ip=props.linux_update_server_ip, @@ -221,7 +224,7 @@ def read_cloudinit( ldap_group_search_base: str, ldap_root_dn: str, ldap_search_password: str, - ldap_user_security_group_name: str, + ldap_user_group_name: str, ldap_server_ip: str, ldap_user_search_base: str, linux_update_server_ip: str, @@ -239,7 +242,7 @@ def read_cloudinit( "ldap_group_search_base": ldap_group_search_base, "ldap_root_dn": ldap_root_dn, "ldap_search_password": ldap_search_password, - "ldap_user_security_group_name": ldap_user_security_group_name, + "ldap_user_group_name": ldap_user_group_name, "ldap_server_ip": ldap_server_ip, "ldap_user_search_base": ldap_user_search_base, "linux_update_server_ip": linux_update_server_ip, From 552509594eef9308a13f08b59a837b6109960659 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Wed, 27 Mar 2024 10:12:34 +0000 Subject: [PATCH 07/32] :wrench: Add network rules to allow Guacamole desktop to connect to SRE identity server --- .../infrastructure/stacks/sre/networking.py | 28 +++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/data_safe_haven/infrastructure/stacks/sre/networking.py b/data_safe_haven/infrastructure/stacks/sre/networking.py index 155fbaf4fe..e01b3656d4 100644 --- a/data_safe_haven/infrastructure/stacks/sre/networking.py +++ b/data_safe_haven/infrastructure/stacks/sre/networking.py @@ -522,6 +522,18 @@ def __init__( source_address_prefix=subnet_guacamole_containers_prefix, source_port_range="*", ), + network.SecurityRuleArgs( + access=network.SecurityRuleAccess.ALLOW, + description="Allow LDAP client requests over TCP.", + destination_address_prefix=subnet_identity_containers_prefix, + destination_port_ranges=["389", "1389"], + direction=network.SecurityRuleDirection.OUTBOUND, + name="AllowSREIdentityServersOutbound", + priority=NetworkingPriorities.INTERNAL_SRE_IDENTITY_CONTAINERS, + protocol=network.SecurityRuleProtocol.TCP, + source_address_prefix=subnet_guacamole_containers_prefix, + source_port_range="*", + ), network.SecurityRuleArgs( access=network.SecurityRuleAccess.ALLOW, description="Allow outbound connections to SRE workspaces.", @@ -629,11 +641,23 @@ def __init__( # Inbound network.SecurityRuleArgs( access=network.SecurityRuleAccess.ALLOW, - description="Allow LDAP client requests over TCP.", + description="Allow LDAP client requests from Guacamole over TCP.", + destination_address_prefix=subnet_identity_containers_prefix, + destination_port_ranges=["389", "1389"], + direction=network.SecurityRuleDirection.INBOUND, + name="AllowGuacamoleLDAPClientTCPInbound", + priority=NetworkingPriorities.INTERNAL_SRE_GUACAMOLE_CONTAINERS, + protocol=network.SecurityRuleProtocol.TCP, + source_address_prefix=subnet_guacamole_containers_prefix, + source_port_range="*", + ), + network.SecurityRuleArgs( + access=network.SecurityRuleAccess.ALLOW, + description="Allow LDAP client requests from workspaces over TCP.", destination_address_prefix=subnet_identity_containers_prefix, destination_port_ranges=["389", "1389"], direction=network.SecurityRuleDirection.INBOUND, - name="AllowLDAPClientTCPInbound", + name="AllowWorkspaceLDAPClientTCPInbound", priority=NetworkingPriorities.INTERNAL_SRE_WORKSPACES, protocol=network.SecurityRuleProtocol.TCP, source_address_prefix=subnet_workspaces_prefix, From 4c189b2989d8f35d96f42000e5f5f33e8d84e329 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Mon, 8 Apr 2024 10:19:44 +0100 Subject: [PATCH 08/32] :truck: Move ordering of SRE component construction --- .../infrastructure/stacks/declarative_sre.py | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/data_safe_haven/infrastructure/stacks/declarative_sre.py b/data_safe_haven/infrastructure/stacks/declarative_sre.py index 2d664c2752..a62df69a77 100644 --- a/data_safe_haven/infrastructure/stacks/declarative_sre.py +++ b/data_safe_haven/infrastructure/stacks/declarative_sre.py @@ -133,20 +133,6 @@ def run(self) -> None: tags=self.cfg.tags.model_dump(), ) - # Deploy identity server - SREIdentityComponent( - "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, - location=self.cfg.azure.location, - shm_fqdn=self.cfg.shm.fqdn, - subnet_containers=networking.subnet_identity_containers, - ), - ) - # Deploy automated monitoring SREMonitoringComponent( "sre_monitoring", @@ -194,6 +180,20 @@ def run(self) -> None: tags=self.cfg.tags.model_dump(), ) + # Deploy identity server + SREIdentityComponent( + "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, + location=self.cfg.azure.location, + shm_fqdn=self.cfg.shm.fqdn, + subnet_containers=networking.subnet_identity_containers, + ), + ) + # Deploy frontend application gateway SREApplicationGatewayComponent( "sre_application_gateway", From d2170360ecbde389f4f04364cc6b1a2daee562f4 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Mon, 8 Apr 2024 10:35:40 +0100 Subject: [PATCH 09/32] :arrow_up: Update to apricot 0.0.5 which allows user/group ID caching --- .../infrastructure/stacks/declarative_sre.py | 5 +- .../infrastructure/stacks/sre/identity.py | 87 ++++++++++++++++--- 2 files changed, 81 insertions(+), 11 deletions(-) diff --git a/data_safe_haven/infrastructure/stacks/declarative_sre.py b/data_safe_haven/infrastructure/stacks/declarative_sre.py index a62df69a77..cdb1ae29fc 100644 --- a/data_safe_haven/infrastructure/stacks/declarative_sre.py +++ b/data_safe_haven/infrastructure/stacks/declarative_sre.py @@ -181,7 +181,7 @@ def run(self) -> None: ) # Deploy identity server - SREIdentityComponent( + identity = SREIdentityComponent( "sre_identity", self.stack_name, SREIdentityProps( @@ -190,6 +190,9 @@ def run(self) -> None: aad_tenant_id=self.cfg.shm.aad_tenant_id, location=self.cfg.azure.location, shm_fqdn=self.cfg.shm.fqdn, + storage_account_key=data.storage_account_data_configuration_key, + storage_account_name=data.storage_account_data_configuration_name, + storage_account_resource_group_name=data.resource_group_name, subnet_containers=networking.subnet_identity_containers, ), ) diff --git a/data_safe_haven/infrastructure/stacks/sre/identity.py b/data_safe_haven/infrastructure/stacks/sre/identity.py index d0c41cfeeb..4af3924e3b 100644 --- a/data_safe_haven/infrastructure/stacks/sre/identity.py +++ b/data_safe_haven/infrastructure/stacks/sre/identity.py @@ -3,8 +3,11 @@ from collections.abc import Mapping from pulumi import ComponentResource, Input, Output, ResourceOptions -from pulumi_azure_native import containerinstance, network, resources +from pulumi_azure_native import containerinstance, network, resources, storage +from data_safe_haven.infrastructure.common import ( + get_ip_address_from_container_group, +) from data_safe_haven.infrastructure.components import ( AzureADApplication, AzureADApplicationProps, @@ -21,6 +24,9 @@ def __init__( aad_tenant_id: Input[str], location: Input[str], shm_fqdn: Input[str], + storage_account_key: Input[str], + storage_account_name: Input[str], + storage_account_resource_group_name: Input[str], subnet_containers: Input[network.GetSubnetResult], ) -> None: self.aad_application_name = aad_application_name @@ -28,6 +34,9 @@ def __init__( self.aad_tenant_id = aad_tenant_id self.location = location self.shm_fqdn = shm_fqdn + self.storage_account_key = storage_account_key + self.storage_account_name = storage_account_name + self.storage_account_resource_group_name = storage_account_resource_group_name self.subnet_containers_id = Output.from_input(subnet_containers).apply( lambda s: str(s.id) ) @@ -48,6 +57,9 @@ def __init__( child_opts = ResourceOptions.merge(opts, ResourceOptions(parent=self)) child_tags = tags if tags else {} + # The port that the server will be hosted on + self.server_port = 1389 + # Deploy resource group resource_group = resources.ResourceGroup( f"{self._name}_resource_group", @@ -57,6 +69,18 @@ def __init__( tags=child_tags, ) + # Define configuration file shares + file_share_redis = storage.FileShare( + f"{self._name}_file_share_redis", + access_tier="TransactionOptimized", + account_name=props.storage_account_name, + resource_group_name=props.storage_account_resource_group_name, + share_name="identity-redis", + share_quota=5120, + signed_identifiers=[], + opts=child_opts, + ) + # Define AzureAD application aad_application = AzureADApplication( f"{self._name}_aad_application", @@ -65,6 +89,7 @@ def __init__( application_role_assignments=["User.Read.All", "GroupMember.Read.All"], application_secret_name="Apricot Authentication Secret", auth_token=props.aad_auth_token, + delegated_role_assignments=["User.Read.All"], public_client_redirect_uri="urn:ietf:wg:oauth:2.0:oob", ), opts=child_opts, @@ -76,7 +101,7 @@ def __init__( container_group_name=f"{stack_name}-container-group-identity", containers=[ containerinstance.ContainerArgs( - image="ghcr.io/alan-turing-institute/apricot:0.0.4", + image="ghcr.io/alan-turing-institute/apricot:0.0.5", name="apricot", environment_variables=[ containerinstance.EnvironmentVariableArgs( @@ -91,6 +116,10 @@ def __init__( name="CLIENT_SECRET", secure_value=aad_application.application_secret, ), + containerinstance.EnvironmentVariableArgs( + name="DEBUG", + value="true", + ), containerinstance.EnvironmentVariableArgs( name="DOMAIN", value=props.shm_fqdn, @@ -99,6 +128,10 @@ def __init__( name="ENTRA_TENANT_ID", value=props.aad_tenant_id, ), + containerinstance.EnvironmentVariableArgs( + name="REDIS_HOST", + value="localhost", + ), ], # All Azure Container Instances need to expose port 80 on at least # one container even if there's nothing behind it. @@ -108,7 +141,25 @@ def __init__( protocol=containerinstance.ContainerGroupNetworkProtocol.TCP, ), containerinstance.ContainerPortArgs( - port=1389, + port=self.server_port, + protocol=containerinstance.ContainerGroupNetworkProtocol.TCP, + ), + ], + resources=containerinstance.ResourceRequirementsArgs( + requests=containerinstance.ResourceRequestsArgs( + cpu=1, + memory_in_gb=1, + ), + ), + volume_mounts=[], + ), + containerinstance.ContainerArgs( + image="redis:7.2", + name="redis", + environment_variables=[], + ports=[ + containerinstance.ContainerPortArgs( + port=6379, protocol=containerinstance.ContainerGroupNetworkProtocol.TCP, ), ], @@ -119,11 +170,11 @@ def __init__( ), ), volume_mounts=[ - # containerinstance.VolumeMountArgs( - # mount_path="/opt/adguardhome/custom", - # name="adguard-opt-adguardhome-custom", - # read_only=True, - # ), + containerinstance.VolumeMountArgs( + mount_path="/data", + name="identity-redis-data", + read_only=False, + ), ], ), ], @@ -132,7 +183,11 @@ def __init__( containerinstance.PortArgs( port=80, protocol=containerinstance.ContainerGroupNetworkProtocol.TCP, - ) + ), + containerinstance.PortArgs( + port=self.server_port, + protocol=containerinstance.ContainerGroupNetworkProtocol.TCP, + ), ], type=containerinstance.ContainerGroupIpAddressType.PRIVATE, ), @@ -145,7 +200,16 @@ def __init__( id=props.subnet_containers_id ) ], - volumes=[], + volumes=[ + containerinstance.VolumeArgs( + azure_file=containerinstance.AzureFileVolumeArgs( + share_name=file_share_redis.name, + storage_account_key=props.storage_account_key, + storage_account_name=props.storage_account_name, + ), + name="identity-redis-data", + ), + ], opts=ResourceOptions.merge( child_opts, ResourceOptions( @@ -155,3 +219,6 @@ def __init__( ), tags=child_tags, ) + + # Register outputs + self.ip_address = get_ip_address_from_container_group(container_group) From 29ca5797e2d1d58dc12c685d840cb6d468214128 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Mon, 8 Apr 2024 10:40:41 +0100 Subject: [PATCH 10/32] :sparkles: Set LDAP lookup variables and filters appropriate for Apricot rather than ActiveDirectory --- .../infrastructure/stacks/declarative_sre.py | 33 +++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/data_safe_haven/infrastructure/stacks/declarative_sre.py b/data_safe_haven/infrastructure/stacks/declarative_sre.py index cdb1ae29fc..0d5a138486 100644 --- a/data_safe_haven/infrastructure/stacks/declarative_sre.py +++ b/data_safe_haven/infrastructure/stacks/declarative_sre.py @@ -69,8 +69,8 @@ def run(self) -> None: ldap_bind_dn = ( f"CN=dshldapsearcher,OU=Data Safe Haven Service Accounts,{ldap_root_dn}" ) - ldap_group_search_base = f"OU=Data Safe Haven Security Groups,{ldap_root_dn}" - ldap_user_search_base = f"OU=Data Safe Haven Research Users,{ldap_root_dn}" + ldap_group_search_base = f"OU=groups,{ldap_root_dn}" + ldap_user_search_base = f"OU=users,{ldap_root_dn}" ldap_search_password = self.pulumi_opts.require("password-domain-ldap-searcher") ldap_server_ip = self.pulumi_opts.require( "shm-domain_controllers-ldap_server_ip" @@ -79,6 +79,35 @@ def run(self) -> None: ldap_admin_group_name = f"{ldap_base_group_name} Administrators" ldap_privileged_user_group_name = f"{ldap_base_group_name} Privileged Users" ldap_user_group_name = f"{ldap_base_group_name} Users" + # Users are a posixAccount belonging to any of these groups + ldap_user_filter = "".join( + [ + "(&", + "(objectClass=posixAccount)", + "(|", + f"(memberOf=CN={ldap_admin_group_name},{ldap_group_search_base})" + f"(memberOf=CN={ldap_privileged_user_group_name},{ldap_group_search_base})" + f"(memberOf=CN={ldap_user_group_name},{ldap_group_search_base})" + ")", + ")", + ] + ) + # Groups are a posixGroup which is either a known, named group or belonging to any of these groups + ldap_group_filter = "".join( + [ + "(&", + "(objectClass=posixGroup)", + "(|", + f"(CN={ldap_admin_group_name})", + f"(CN={ldap_privileged_user_group_name})", + f"(CN={ldap_user_group_name})", + f"(memberOf=CN=Primary user groups for {ldap_admin_group_name},{ldap_group_search_base})", + f"(memberOf=CN=Primary user groups for {ldap_privileged_user_group_name},{ldap_group_search_base})", + f"(memberOf=CN=Primary user groups for {ldap_user_group_name},{ldap_group_search_base})", + ")", + ")", + ] + ) # Deploy SRE DNS server dns = SREDnsServerComponent( From 247edf5d1e805d558c3d91e974d07f9180e57c65 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Mon, 8 Apr 2024 10:41:08 +0100 Subject: [PATCH 11/32] :arrow_up: Update to guacamole-user-sync 0.4.0 which allows anonymous bind and specification of user/group lookup attributes and LDAP ports. --- .../infrastructure/stacks/declarative_sre.py | 13 ++-- .../stacks/sre/remote_desktop.py | 60 ++++++++++++------- 2 files changed, 46 insertions(+), 27 deletions(-) diff --git a/data_safe_haven/infrastructure/stacks/declarative_sre.py b/data_safe_haven/infrastructure/stacks/declarative_sre.py index 0d5a138486..878efe8850 100644 --- a/data_safe_haven/infrastructure/stacks/declarative_sre.py +++ b/data_safe_haven/infrastructure/stacks/declarative_sre.py @@ -246,8 +246,8 @@ def run(self) -> None: "sre_remote_desktop", self.stack_name, SRERemoteDesktopProps( - aad_application_name=f"sre-{self.sre_name}-guacamole", 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, @@ -255,17 +255,20 @@ def run(self) -> None: database_password=data.password_user_database_admin, dns_server_ip=dns.ip_address, ldap_bind_dn=ldap_bind_dn, + ldap_group_filter=ldap_group_filter, ldap_group_search_base=ldap_group_search_base, ldap_search_password=ldap_search_password, - ldap_server_ip=ldap_server_ip, - ldap_user_search_base=ldap_user_search_base, + ldap_server_ip=identity.ip_address, + ldap_server_port=identity.server_port, + ldap_user_filter=ldap_user_filter, ldap_user_group_name=ldap_user_group_name, + ldap_user_search_base=ldap_user_search_base, location=self.cfg.azure.location, - subnet_guacamole_containers=networking.subnet_guacamole_containers, - subnet_guacamole_containers_support=networking.subnet_guacamole_containers_support, storage_account_key=data.storage_account_data_configuration_key, storage_account_name=data.storage_account_data_configuration_name, storage_account_resource_group_name=data.resource_group_name, + subnet_guacamole_containers_support=networking.subnet_guacamole_containers_support, + subnet_guacamole_containers=networking.subnet_guacamole_containers, ), tags=self.cfg.tags.model_dump(), ) diff --git a/data_safe_haven/infrastructure/stacks/sre/remote_desktop.py b/data_safe_haven/infrastructure/stacks/sre/remote_desktop.py index 040912e1d5..3fa459e83e 100644 --- a/data_safe_haven/infrastructure/stacks/sre/remote_desktop.py +++ b/data_safe_haven/infrastructure/stacks/sre/remote_desktop.py @@ -41,11 +41,14 @@ def __init__( database_password: Input[str], dns_server_ip: Input[str], ldap_bind_dn: Input[str], + ldap_group_filter: Input[str], ldap_group_search_base: Input[str], ldap_search_password: Input[str], ldap_server_ip: Input[str], - ldap_user_search_base: Input[str], + ldap_server_port: Input[int], + ldap_user_filter: Input[str], ldap_user_group_name: Input[str], + ldap_user_search_base: Input[str], location: Input[str], storage_account_key: Input[str], storage_account_name: Input[str], @@ -66,11 +69,14 @@ def __init__( self.disable_paste = not allow_paste self.dns_server_ip = dns_server_ip self.ldap_bind_dn = ldap_bind_dn + self.ldap_group_filter = ldap_group_filter self.ldap_group_search_base = ldap_group_search_base self.ldap_search_password = ldap_search_password self.ldap_server_ip = ldap_server_ip - self.ldap_user_search_base = ldap_user_search_base + self.ldap_server_port = ldap_server_port + self.ldap_user_filter = ldap_user_filter self.ldap_user_group_name = ldap_user_group_name + self.ldap_user_search_base = ldap_user_search_base self.location = location self.storage_account_key = storage_account_key self.storage_account_name = storage_account_name @@ -227,7 +233,11 @@ def __init__( ), containerinstance.EnvironmentVariableArgs( name="OPENID_AUTHORIZATION_ENDPOINT", - value=Output.concat("https://login.microsoftonline.com/", props.aad_tenant_id, "/oauth2/v2.0/authorize"), + value=Output.concat( + "https://login.microsoftonline.com/", + props.aad_tenant_id, + "/oauth2/v2.0/authorize", + ), ), containerinstance.EnvironmentVariableArgs( name="OPENID_CLIENT_ID", @@ -235,11 +245,19 @@ def __init__( ), containerinstance.EnvironmentVariableArgs( name="OPENID_ISSUER", - value=Output.concat("https://login.microsoftonline.com/", props.aad_tenant_id, "/v2.0"), + value=Output.concat( + "https://login.microsoftonline.com/", + props.aad_tenant_id, + "/v2.0", + ), ), containerinstance.EnvironmentVariableArgs( name="OPENID_JWKS_ENDPOINT", - value=Output.concat("https://login.microsoftonline.com/", props.aad_tenant_id, "/discovery/v2.0/keys"), + value=Output.concat( + "https://login.microsoftonline.com/", + props.aad_tenant_id, + "/discovery/v2.0/keys", + ), ), containerinstance.EnvironmentVariableArgs( name="OPENID_REDIRECT_URI", value=props.aad_application_url @@ -295,42 +313,40 @@ def __init__( ), ), containerinstance.ContainerArgs( - image="ghcr.io/alan-turing-institute/guacamole-user-sync:v0.3.0", + image="ghcr.io/alan-turing-institute/guacamole-user-sync:v0.4.0", name="guacamole-user-sync"[:63], environment_variables=[ - containerinstance.EnvironmentVariableArgs( - name="LDAP_BIND_DN", - value=props.ldap_bind_dn, - ), - containerinstance.EnvironmentVariableArgs( - name="LDAP_BIND_PASSWORD", - secure_value=props.ldap_search_password, - ), containerinstance.EnvironmentVariableArgs( name="LDAP_GROUP_BASE_DN", value=props.ldap_group_search_base, ), + containerinstance.EnvironmentVariableArgs( + name="LDAP_GROUP_NAME_ATTR", + value="cn", + ), containerinstance.EnvironmentVariableArgs( name="LDAP_GROUP_FILTER", - value="(objectClass=group)", + value=props.ldap_group_filter, ), containerinstance.EnvironmentVariableArgs( name="LDAP_HOST", value=props.ldap_server_ip, ), + containerinstance.EnvironmentVariableArgs( + name="LDAP_PORT", + value=Output.from_input(props.ldap_server_port).apply(str), + ), + containerinstance.EnvironmentVariableArgs( + name="LDAP_USER_NAME_ATTR", + value="oauth_username", # this is the name that users connect with + ), containerinstance.EnvironmentVariableArgs( name="LDAP_USER_BASE_DN", value=props.ldap_user_search_base, ), containerinstance.EnvironmentVariableArgs( name="LDAP_USER_FILTER", - value=Output.concat( - "(&(objectClass=user)(memberOf=CN=", - props.ldap_user_group_name, - ",", - props.ldap_group_search_base, - "))", - ), + value=props.ldap_user_filter, ), containerinstance.EnvironmentVariableArgs( name="POSTGRESQL_DB_NAME", From 06fd05d0ab804d58980095edfc129b912f47df8b Mon Sep 17 00:00:00 2001 From: James Robinson Date: Mon, 8 Apr 2024 10:41:48 +0100 Subject: [PATCH 12/32] :arrow_up: Update workspace image to use SRE identity server instead of SHM --- .../infrastructure/stacks/declarative_sre.py | 7 ++-- .../infrastructure/stacks/sre/workspaces.py | 17 +++++++--- .../workspace.cloud_init.mustache.yaml | 32 +++++++++++++++++-- 3 files changed, 46 insertions(+), 10 deletions(-) diff --git a/data_safe_haven/infrastructure/stacks/declarative_sre.py b/data_safe_haven/infrastructure/stacks/declarative_sre.py index 878efe8850..4a2692a43d 100644 --- a/data_safe_haven/infrastructure/stacks/declarative_sre.py +++ b/data_safe_haven/infrastructure/stacks/declarative_sre.py @@ -282,14 +282,15 @@ def run(self) -> None: domain_sid=self.pulumi_opts.require( "shm-domain_controllers-domain_sid" ), - ldap_base_group_name=ldap_base_group_name, ldap_bind_dn=ldap_bind_dn, + ldap_group_filter=ldap_group_filter, ldap_group_search_base=ldap_group_search_base, ldap_root_dn=ldap_root_dn, ldap_search_password=ldap_search_password, - ldap_server_ip=ldap_server_ip, - ldap_user_search_base=ldap_user_search_base, + ldap_server_ip=identity.ip_address, + ldap_user_filter=ldap_user_filter, ldap_user_group_name=ldap_user_group_name, + ldap_user_search_base=ldap_user_search_base, linux_update_server_ip=self.pulumi_opts.require( "shm-update_servers-ip_address_linux" ), diff --git a/data_safe_haven/infrastructure/stacks/sre/workspaces.py b/data_safe_haven/infrastructure/stacks/sre/workspaces.py index 4599e0d87c..6cc9bbc238 100644 --- a/data_safe_haven/infrastructure/stacks/sre/workspaces.py +++ b/data_safe_haven/infrastructure/stacks/sre/workspaces.py @@ -31,14 +31,15 @@ def __init__( self, admin_password: Input[str], domain_sid: Input[str], - ldap_base_group_name: Input[str], ldap_bind_dn: Input[str], + ldap_group_filter: Input[str], ldap_group_search_base: Input[str], ldap_root_dn: Input[str], ldap_search_password: Input[str], ldap_server_ip: Input[str], - ldap_user_search_base: Input[str], + ldap_user_filter: Input[str], ldap_user_group_name: Input[str], + ldap_user_search_base: Input[str], linux_update_server_ip: Input[str], location: Input[str], log_analytics_workspace_id: Input[str], @@ -56,14 +57,15 @@ def __init__( self.admin_password = Output.secret(admin_password) self.admin_username = "dshadmin" self.domain_sid = domain_sid - self.ldap_base_group_name = ldap_base_group_name self.ldap_bind_dn = ldap_bind_dn + self.ldap_group_filter = ldap_group_filter self.ldap_group_search_base = ldap_group_search_base self.ldap_root_dn = ldap_root_dn self.ldap_search_password = ldap_search_password self.ldap_server_ip = ldap_server_ip - self.ldap_user_search_base = ldap_user_search_base + self.ldap_user_filter = ldap_user_filter self.ldap_user_group_name = ldap_user_group_name + self.ldap_user_search_base = ldap_user_search_base self.linux_update_server_ip = linux_update_server_ip self.location = location self.log_analytics_workspace_id = log_analytics_workspace_id @@ -128,11 +130,12 @@ def __init__( # Load cloud-init file b64cloudinit = Output.all( domain_sid=props.domain_sid, - ldap_base_group_name=props.ldap_base_group_name, ldap_bind_dn=props.ldap_bind_dn, + ldap_group_filter=props.ldap_group_filter, ldap_group_search_base=props.ldap_group_search_base, ldap_root_dn=props.ldap_root_dn, ldap_search_password=props.ldap_search_password, + ldap_user_filter=props.ldap_user_filter, ldap_user_group_name=props.ldap_user_group_name, ldap_server_ip=props.ldap_server_ip, ldap_user_search_base=props.ldap_user_search_base, @@ -221,11 +224,13 @@ def read_cloudinit( self, domain_sid: str, ldap_bind_dn: str, + ldap_group_filter: str, ldap_group_search_base: str, ldap_root_dn: str, ldap_search_password: str, ldap_user_group_name: str, ldap_server_ip: str, + ldap_user_filter: str, ldap_user_search_base: str, linux_update_server_ip: str, sre_fqdn: str, @@ -239,11 +244,13 @@ def read_cloudinit( mustache_values = { "domain_sid": domain_sid, "ldap_bind_dn": ldap_bind_dn, + "ldap_group_filter": ldap_group_filter, "ldap_group_search_base": ldap_group_search_base, "ldap_root_dn": ldap_root_dn, "ldap_search_password": ldap_search_password, "ldap_user_group_name": ldap_user_group_name, "ldap_server_ip": ldap_server_ip, + "ldap_user_filter": ldap_user_filter, "ldap_user_search_base": ldap_user_search_base, "linux_update_server_ip": linux_update_server_ip, "sre_fqdn": sre_fqdn, diff --git a/data_safe_haven/resources/workspace/workspace.cloud_init.mustache.yaml b/data_safe_haven/resources/workspace/workspace.cloud_init.mustache.yaml index 647e26ebe6..f162f839df 100644 --- a/data_safe_haven/resources/workspace/workspace.cloud_init.mustache.yaml +++ b/data_safe_haven/resources/workspace/workspace.cloud_init.mustache.yaml @@ -7,7 +7,7 @@ write_files: Acquire::http::Proxy "http://{{linux_update_server_ip}}:8000"; Acquire::https::Proxy "http://{{linux_update_server_ip}}:8000"; - - path: "/etc/nslcd.conf" + - path: "/etc/nslcd.conf.old" permissions: "0400" content: | # nslcd configuration file. @@ -19,7 +19,7 @@ write_files: log syslog debug # General connection options - uri ldap://{{ldap_server_ip}}:389 + uri ldap://10.0.0.132:389 binddn {{ldap_bind_dn}} bindpw {{ldap_search_password}} @@ -41,6 +41,34 @@ write_files: map group cn sAMAccountName map group gidNumber objectSid:{{domain_sid}} + - path: "/etc/nslcd.conf" + permissions: "0400" + content: | + # nslcd configuration file. + # http://manpages.ubuntu.com/manpages/bionic/man5/nslcd.conf.5.html + + # Runtime options + uid nslcd + gid nslcd + log syslog debug + + # Do not allow uids lower than 2000 to login + nss_min_uid 2000 + + # General connection options + uri ldap://{{ldap_server_ip}}:1389 + + # Search/mapping options + base {{ldap_user_search_base}} + base {{ldap_group_search_base}} + + # All users that are members of the correct group + filter passwd {{{ldap_user_filter}}} + + # One group for each security group belonging to this SRE and for each primary user group for users that belong to a group in this SRE + filter group {{{ldap_group_filter}}} + + - path: "/etc/pip.conf" permissions: "0444" content: | From c80a4cf9bf565f106581fa84abdb9c42634a7bd1 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Mon, 8 Apr 2024 10:48:06 +0100 Subject: [PATCH 13/32] :coffin: Drop Domain SID extraction as this is not needed with Apricot --- data_safe_haven/commands/deploy.py | 5 --- .../infrastructure/stacks/declarative_sre.py | 3 -- .../stacks/shm/domain_controllers.py | 25 +------------- .../infrastructure/stacks/sre/workspaces.py | 5 --- .../resources/active_directory/get_ad_sid.ps1 | 2 -- .../workspace.cloud_init.mustache.yaml | 34 ------------------- 6 files changed, 1 insertion(+), 73 deletions(-) delete mode 100644 data_safe_haven/resources/active_directory/get_ad_sid.ps1 diff --git a/data_safe_haven/commands/deploy.py b/data_safe_haven/commands/deploy.py index 355effc4da..da64fe4079 100644 --- a/data_safe_haven/commands/deploy.py +++ b/data_safe_haven/commands/deploy.py @@ -135,11 +135,6 @@ def sre( ) stack.add_option("azure-native:tenantId", config.azure.tenant_id, replace=False) # Load SHM stack outputs - stack.add_option( - "shm-domain_controllers-domain_sid", - shm_stack.output("domain_controllers")["domain_sid"], - replace=True, - ) stack.add_option( "shm-domain_controllers-ldap_root_dn", shm_stack.output("domain_controllers")["ldap_root_dn"], diff --git a/data_safe_haven/infrastructure/stacks/declarative_sre.py b/data_safe_haven/infrastructure/stacks/declarative_sre.py index 4a2692a43d..8a044a9854 100644 --- a/data_safe_haven/infrastructure/stacks/declarative_sre.py +++ b/data_safe_haven/infrastructure/stacks/declarative_sre.py @@ -279,9 +279,6 @@ def run(self) -> None: self.stack_name, SREWorkspacesProps( admin_password=data.password_workspace_admin, - domain_sid=self.pulumi_opts.require( - "shm-domain_controllers-domain_sid" - ), ldap_bind_dn=ldap_bind_dn, ldap_group_filter=ldap_group_filter, ldap_group_search_base=ldap_group_search_base, diff --git a/data_safe_haven/infrastructure/stacks/shm/domain_controllers.py b/data_safe_haven/infrastructure/stacks/shm/domain_controllers.py index 9fe20512cd..ea43e06ab7 100644 --- a/data_safe_haven/infrastructure/stacks/shm/domain_controllers.py +++ b/data_safe_haven/infrastructure/stacks/shm/domain_controllers.py @@ -9,8 +9,6 @@ from data_safe_haven.infrastructure.components import ( AutomationDscNode, AutomationDscNodeProps, - RemoteScript, - RemoteScriptProps, VMComponent, WindowsVMComponentProps, WrappedAutomationAccount, @@ -122,7 +120,7 @@ def __init__( / "desired_state_configuration" / f"{dsc_configuration_name}.ps1" ) - primary_domain_controller_dsc_node = AutomationDscNode( + AutomationDscNode( f"{self._name}_primary_domain_controller_dsc_node", AutomationDscNodeProps( automation_account=props.automation_account, @@ -154,33 +152,12 @@ def __init__( ), tags=child_tags, ) - # Extract the domain SID - domain_sid_script = FileReader( - resources_path / "active_directory" / "get_ad_sid.ps1" - ) - domain_sid = RemoteScript( - f"{self._name}_get_ad_sid", - RemoteScriptProps( - force_refresh=True, - script_contents=domain_sid_script.file_contents(), - script_hash=domain_sid_script.sha256(), - script_parameters={}, - subscription_name=props.subscription_name, - vm_name=primary_domain_controller.vm_name, - vm_resource_group_name=resource_group.name, - ), - opts=ResourceOptions.merge( - child_opts, - ResourceOptions(parent=primary_domain_controller_dsc_node), - ), - ) # Register outputs self.resource_group_name = resource_group.name # Register exports self.exports = { - "domain_sid": domain_sid.script_output, "ldap_root_dn": props.domain_root_dn, "ldap_search_username": props.username_domain_searcher, "ldap_server_ip": primary_domain_controller.ip_address_private, diff --git a/data_safe_haven/infrastructure/stacks/sre/workspaces.py b/data_safe_haven/infrastructure/stacks/sre/workspaces.py index 6cc9bbc238..204118468a 100644 --- a/data_safe_haven/infrastructure/stacks/sre/workspaces.py +++ b/data_safe_haven/infrastructure/stacks/sre/workspaces.py @@ -30,7 +30,6 @@ class SREWorkspacesProps: def __init__( self, admin_password: Input[str], - domain_sid: Input[str], ldap_bind_dn: Input[str], ldap_group_filter: Input[str], ldap_group_search_base: Input[str], @@ -56,7 +55,6 @@ def __init__( ) -> None: self.admin_password = Output.secret(admin_password) self.admin_username = "dshadmin" - self.domain_sid = domain_sid self.ldap_bind_dn = ldap_bind_dn self.ldap_group_filter = ldap_group_filter self.ldap_group_search_base = ldap_group_search_base @@ -129,7 +127,6 @@ def __init__( # Load cloud-init file b64cloudinit = Output.all( - domain_sid=props.domain_sid, ldap_bind_dn=props.ldap_bind_dn, ldap_group_filter=props.ldap_group_filter, ldap_group_search_base=props.ldap_group_search_base, @@ -222,7 +219,6 @@ def __init__( def read_cloudinit( self, - domain_sid: str, ldap_bind_dn: str, ldap_group_filter: str, ldap_group_search_base: str, @@ -242,7 +238,6 @@ def read_cloudinit( encoding="utf-8", ) as f_cloudinit: mustache_values = { - "domain_sid": domain_sid, "ldap_bind_dn": ldap_bind_dn, "ldap_group_filter": ldap_group_filter, "ldap_group_search_base": ldap_group_search_base, diff --git a/data_safe_haven/resources/active_directory/get_ad_sid.ps1 b/data_safe_haven/resources/active_directory/get_ad_sid.ps1 deleted file mode 100644 index 35846d65d5..0000000000 --- a/data_safe_haven/resources/active_directory/get_ad_sid.ps1 +++ /dev/null @@ -1,2 +0,0 @@ -$DomainControllerSID = $(Get-ADComputer -Filter * | ForEach-Object { $_.SID.Value } | Select-Object -First 1) -Write-Output "$($DomainControllerSID.Substring(0, $DomainControllerSID.Length - 5))" diff --git a/data_safe_haven/resources/workspace/workspace.cloud_init.mustache.yaml b/data_safe_haven/resources/workspace/workspace.cloud_init.mustache.yaml index f162f839df..76632b8b42 100644 --- a/data_safe_haven/resources/workspace/workspace.cloud_init.mustache.yaml +++ b/data_safe_haven/resources/workspace/workspace.cloud_init.mustache.yaml @@ -7,40 +7,6 @@ write_files: Acquire::http::Proxy "http://{{linux_update_server_ip}}:8000"; Acquire::https::Proxy "http://{{linux_update_server_ip}}:8000"; - - path: "/etc/nslcd.conf.old" - permissions: "0400" - content: | - # nslcd configuration file. - # http://manpages.ubuntu.com/manpages/bionic/man5/nslcd.conf.5.html - - # Runtime options - uid nslcd - gid nslcd - log syslog debug - - # General connection options - uri ldap://10.0.0.132:389 - binddn {{ldap_bind_dn}} - bindpw {{ldap_search_password}} - - # Search/mapping options - base {{ldap_user_search_base}} - base {{ldap_group_search_base}} - - # All users that are members of the correct security group - filter passwd (&(objectClass=user)(memberOf=CN={{ldap_user_group_name}},OU=Data Safe Haven Security Groups,{{ldap_root_dn}})) - - # One group for each security group and for each user - filter group (|(objectclass=group)(&(objectClass=user)(memberOf=CN={{ldap_user_group_name}},OU=Data Safe Haven Security Groups,{{ldap_root_dn}}))) - - # Attribute mappings - map passwd uid sAMAccountName - map passwd gidNumber objectSid:{{domain_sid}} - map passwd uidNumber objectSid:{{domain_sid}} - map passwd homeDirectory "${unixHomeDirectory:-/home/$sAMAccountName}" - map group cn sAMAccountName - map group gidNumber objectSid:{{domain_sid}} - - path: "/etc/nslcd.conf" permissions: "0400" content: | From 052a397d60f4ea06cbb9835672a6318d03ec2d7e Mon Sep 17 00:00:00 2001 From: James Robinson Date: Mon, 8 Apr 2024 11:02:54 +0100 Subject: [PATCH 14/32] :recycle: Update Gitea server to use Apricot for identity --- .../infrastructure/stacks/declarative_sre.py | 7 +++---- .../infrastructure/stacks/sre/gitea_server.py | 19 +++++++------------ .../stacks/sre/user_services.py | 16 +++++++++------- .../gitea/gitea/configure.mustache.sh | 6 ++---- 4 files changed, 21 insertions(+), 27 deletions(-) diff --git a/data_safe_haven/infrastructure/stacks/declarative_sre.py b/data_safe_haven/infrastructure/stacks/declarative_sre.py index 8a044a9854..4fb4891bfd 100644 --- a/data_safe_haven/infrastructure/stacks/declarative_sre.py +++ b/data_safe_haven/infrastructure/stacks/declarative_sre.py @@ -72,9 +72,6 @@ def run(self) -> None: ldap_group_search_base = f"OU=groups,{ldap_root_dn}" ldap_user_search_base = f"OU=users,{ldap_root_dn}" ldap_search_password = self.pulumi_opts.require("password-domain-ldap-searcher") - ldap_server_ip = self.pulumi_opts.require( - "shm-domain_controllers-ldap_server_ip" - ) ldap_base_group_name = f"Data Safe Haven SRE {self.sre_name}" ldap_admin_group_name = f"{ldap_base_group_name} Administrators" ldap_privileged_user_group_name = f"{ldap_base_group_name} Privileged Users" @@ -328,7 +325,9 @@ def run(self) -> None: ldap_bind_dn=ldap_bind_dn, ldap_root_dn=ldap_root_dn, ldap_search_password=ldap_search_password, - ldap_server_ip=ldap_server_ip, + ldap_server_ip=identity.ip_address, + ldap_server_port=identity.server_port, + ldap_user_filter=ldap_user_filter, ldap_user_group_name=ldap_user_group_name, ldap_user_search_base=ldap_user_search_base, location=self.cfg.azure.location, diff --git a/data_safe_haven/infrastructure/stacks/sre/gitea_server.py b/data_safe_haven/infrastructure/stacks/sre/gitea_server.py index 35c35b50d8..c6ae7d260c 100644 --- a/data_safe_haven/infrastructure/stacks/sre/gitea_server.py +++ b/data_safe_haven/infrastructure/stacks/sre/gitea_server.py @@ -28,12 +28,10 @@ def __init__( database_subnet_id: Input[str], dns_resource_group_name: Input[str], dns_server_ip: Input[str], - ldap_bind_dn: Input[str], - ldap_root_dn: Input[str], - ldap_search_password: Input[str], ldap_server_ip: Input[str], + ldap_server_port: Input[int], + ldap_user_filter: Input[str], ldap_user_search_base: Input[str], - ldap_user_group_name: Input[str], location: Input[str], networking_resource_group_name: Input[str], sre_fqdn: Input[str], @@ -52,12 +50,10 @@ def __init__( ) self.dns_resource_group_name = dns_resource_group_name self.dns_server_ip = dns_server_ip - self.ldap_bind_dn = ldap_bind_dn - self.ldap_root_dn = ldap_root_dn - self.ldap_search_password = ldap_search_password self.ldap_server_ip = ldap_server_ip + self.ldap_server_port = ldap_server_port + self.ldap_user_filter = ldap_user_filter self.ldap_user_search_base = ldap_user_search_base - self.ldap_user_group_name = ldap_user_group_name self.location = location self.networking_resource_group_name = networking_resource_group_name self.sre_fqdn = sre_fqdn @@ -130,11 +126,10 @@ def __init__( gitea_configure_sh = Output.all( admin_email="dshadmin@example.com", admin_username="dshadmin", - ldap_bind_dn=props.ldap_bind_dn, - ldap_root_dn=props.ldap_root_dn, - ldap_search_password=props.ldap_search_password, - ldap_user_group_name=props.ldap_user_group_name, + ldap_username_attribute="uid", + ldap_user_filter=props.ldap_user_filter, ldap_server_ip=props.ldap_server_ip, + ldap_server_port=props.ldap_server_port, ldap_user_search_base=props.ldap_user_search_base, ).apply( lambda mustache_values: gitea_configure_sh_reader.file_contents( diff --git a/data_safe_haven/infrastructure/stacks/sre/user_services.py b/data_safe_haven/infrastructure/stacks/sre/user_services.py index 331d07b7e7..7d7d48de4f 100644 --- a/data_safe_haven/infrastructure/stacks/sre/user_services.py +++ b/data_safe_haven/infrastructure/stacks/sre/user_services.py @@ -31,8 +31,10 @@ def __init__( ldap_root_dn: Input[str], ldap_search_password: Input[str], ldap_server_ip: Input[str], - ldap_user_search_base: Input[str], + ldap_server_port: Input[int], + ldap_user_filter: Input[str], ldap_user_group_name: Input[str], + ldap_user_search_base: Input[str], location: Input[str], networking_resource_group_name: Input[str], nexus_admin_password: Input[str], @@ -42,8 +44,8 @@ def __init__( storage_account_key: Input[str], storage_account_name: Input[str], storage_account_resource_group_name: Input[str], - subnet_containers: Input[network.GetSubnetResult], subnet_containers_support: Input[network.GetSubnetResult], + subnet_containers: Input[network.GetSubnetResult], subnet_databases: Input[network.GetSubnetResult], subnet_software_repositories: Input[network.GetSubnetResult], ) -> None: @@ -58,8 +60,10 @@ def __init__( self.ldap_root_dn = ldap_root_dn self.ldap_search_password = ldap_search_password self.ldap_server_ip = ldap_server_ip - self.ldap_user_search_base = ldap_user_search_base + self.ldap_server_port = ldap_server_port + self.ldap_user_filter = ldap_user_filter self.ldap_user_group_name = ldap_user_group_name + self.ldap_user_search_base = ldap_user_search_base self.location = location self.networking_resource_group_name = networking_resource_group_name self.nexus_admin_password = Output.secret(nexus_admin_password) @@ -117,11 +121,9 @@ def __init__( database_password=props.gitea_database_password, dns_resource_group_name=props.dns_resource_group_name, dns_server_ip=props.dns_server_ip, - ldap_bind_dn=props.ldap_bind_dn, - ldap_root_dn=props.ldap_root_dn, - ldap_search_password=props.ldap_search_password, ldap_server_ip=props.ldap_server_ip, - ldap_user_group_name=props.ldap_user_group_name, + ldap_server_port=props.ldap_server_port, + ldap_user_filter=props.ldap_user_filter, ldap_user_search_base=props.ldap_user_search_base, location=props.location, networking_resource_group_name=props.networking_resource_group_name, diff --git a/data_safe_haven/resources/gitea/gitea/configure.mustache.sh b/data_safe_haven/resources/gitea/gitea/configure.mustache.sh index f88cd59475..7f4f2a404f 100644 --- a/data_safe_haven/resources/gitea/gitea/configure.mustache.sh +++ b/data_safe_haven/resources/gitea/gitea/configure.mustache.sh @@ -12,13 +12,11 @@ until su-exec "$USER" /usr/local/bin/gitea admin auth list | grep "DataSafeHaven echo "$(date -Iseconds) Attempting to register LDAP authentication..." | tee -a /var/log/configuration su-exec "$USER" /usr/local/bin/gitea admin auth add-ldap \ --name DataSafeHavenLDAP \ - --bind-dn "{{ldap_bind_dn}}" \ - --bind-password "{{ldap_search_password}}" \ --security-protocol "unencrypted" \ --host "{{ldap_server_ip}}" \ - --port "389" \ + --port "{{ldap_server_port}}" \ --user-search-base "{{ldap_user_search_base}}" \ - --user-filter "(&(objectClass=user)(memberOf=CN={{ldap_user_security_group_name}},OU=Data Safe Haven Security Groups,{{ldap_root_dn}})(sAMAccountName=%[1]s))" \ + --user-filter "(&{{{ldap_user_filter}}}({{ldap_username_attribute}}=%[1]s))" \ --email-attribute "mail" sleep 1 done From bea4172accae36bd36008c947a7b5ba3ee0f6cde Mon Sep 17 00:00:00 2001 From: James Robinson Date: Mon, 8 Apr 2024 12:46:19 +0100 Subject: [PATCH 15/32] :alien: Update connection details for Gitea database for new Azure PostgreSQL format --- data_safe_haven/infrastructure/stacks/sre/gitea_server.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/data_safe_haven/infrastructure/stacks/sre/gitea_server.py b/data_safe_haven/infrastructure/stacks/sre/gitea_server.py index c6ae7d260c..4f05723062 100644 --- a/data_safe_haven/infrastructure/stacks/sre/gitea_server.py +++ b/data_safe_haven/infrastructure/stacks/sre/gitea_server.py @@ -235,11 +235,7 @@ def __init__( ), containerinstance.EnvironmentVariableArgs( name="GITEA__database__USER", - value=Output.concat( - props.database_username, - "@", - db_server_gitea.db_server.name, - ), + value=props.database_username, ), containerinstance.EnvironmentVariableArgs( name="GITEA__database__PASSWD", From 6a1e9e9a366be8cf8ea347be6de2598a39a8ee23 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Mon, 8 Apr 2024 13:32:10 +0100 Subject: [PATCH 16/32] :wrench: Allow network connections between user services containers and Apricot server --- .../infrastructure/stacks/sre/networking.py | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/data_safe_haven/infrastructure/stacks/sre/networking.py b/data_safe_haven/infrastructure/stacks/sre/networking.py index e01b3656d4..7cdcf06088 100644 --- a/data_safe_haven/infrastructure/stacks/sre/networking.py +++ b/data_safe_haven/infrastructure/stacks/sre/networking.py @@ -651,6 +651,18 @@ def __init__( source_address_prefix=subnet_guacamole_containers_prefix, source_port_range="*", ), + network.SecurityRuleArgs( + access=network.SecurityRuleAccess.ALLOW, + description="Allow LDAP client requests from user services over TCP.", + destination_address_prefix=subnet_identity_containers_prefix, + destination_port_ranges=["389", "1389"], + direction=network.SecurityRuleDirection.INBOUND, + name="AllowUserServicesLDAPClientTCPInbound", + priority=NetworkingPriorities.INTERNAL_SRE_USER_SERVICES_CONTAINERS, + protocol=network.SecurityRuleProtocol.TCP, + source_address_prefix=subnet_user_services_containers_prefix, + source_port_range="*", + ), network.SecurityRuleArgs( access=network.SecurityRuleAccess.ALLOW, description="Allow LDAP client requests from workspaces over TCP.", @@ -758,6 +770,18 @@ def __init__( source_address_prefix=subnet_user_services_containers_prefix, source_port_range="*", ), + network.SecurityRuleArgs( + access=network.SecurityRuleAccess.ALLOW, + description="Allow LDAP client requests over TCP.", + destination_address_prefix=subnet_identity_containers_prefix, + destination_port_ranges=["389", "1389"], + direction=network.SecurityRuleDirection.OUTBOUND, + name="AllowSREIdentityServersOutbound", + priority=NetworkingPriorities.INTERNAL_SRE_IDENTITY_CONTAINERS, + protocol=network.SecurityRuleProtocol.TCP, + source_address_prefix=subnet_user_services_containers_prefix, + source_port_range="*", + ), network.SecurityRuleArgs( access=network.SecurityRuleAccess.ALLOW, description="Allow outbound connections to container support services.", From 416f748fd677b9b4db04688d12123ba94a90fa5b Mon Sep 17 00:00:00 2001 From: James Robinson Date: Mon, 8 Apr 2024 14:49:36 +0100 Subject: [PATCH 17/32] :recycle: Update HedgeDoc server to use Apricot for identity --- .../infrastructure/stacks/declarative_sre.py | 2 + .../infrastructure/stacks/sre/gitea_server.py | 4 +- .../stacks/sre/hedgedoc_server.py | 55 ++++++------------- .../stacks/sre/user_services.py | 12 ++-- 4 files changed, 30 insertions(+), 43 deletions(-) diff --git a/data_safe_haven/infrastructure/stacks/declarative_sre.py b/data_safe_haven/infrastructure/stacks/declarative_sre.py index 4fb4891bfd..d8e87ae4c5 100644 --- a/data_safe_haven/infrastructure/stacks/declarative_sre.py +++ b/data_safe_haven/infrastructure/stacks/declarative_sre.py @@ -75,6 +75,7 @@ def run(self) -> None: ldap_base_group_name = f"Data Safe Haven SRE {self.sre_name}" ldap_admin_group_name = f"{ldap_base_group_name} Administrators" ldap_privileged_user_group_name = f"{ldap_base_group_name} Privileged Users" + ldap_username_attribute = "uid" ldap_user_group_name = f"{ldap_base_group_name} Users" # Users are a posixAccount belonging to any of these groups ldap_user_filter = "".join( @@ -328,6 +329,7 @@ def run(self) -> None: ldap_server_ip=identity.ip_address, ldap_server_port=identity.server_port, ldap_user_filter=ldap_user_filter, + ldap_username_attribute=ldap_username_attribute, ldap_user_group_name=ldap_user_group_name, ldap_user_search_base=ldap_user_search_base, location=self.cfg.azure.location, diff --git a/data_safe_haven/infrastructure/stacks/sre/gitea_server.py b/data_safe_haven/infrastructure/stacks/sre/gitea_server.py index 4f05723062..983aaa3560 100644 --- a/data_safe_haven/infrastructure/stacks/sre/gitea_server.py +++ b/data_safe_haven/infrastructure/stacks/sre/gitea_server.py @@ -30,6 +30,7 @@ def __init__( dns_server_ip: Input[str], ldap_server_ip: Input[str], ldap_server_port: Input[int], + ldap_username_attribute: Input[str], ldap_user_filter: Input[str], ldap_user_search_base: Input[str], location: Input[str], @@ -52,6 +53,7 @@ def __init__( self.dns_server_ip = dns_server_ip self.ldap_server_ip = ldap_server_ip self.ldap_server_port = ldap_server_port + self.ldap_username_attribute = ldap_username_attribute self.ldap_user_filter = ldap_user_filter self.ldap_user_search_base = ldap_user_search_base self.location = location @@ -126,7 +128,7 @@ def __init__( gitea_configure_sh = Output.all( admin_email="dshadmin@example.com", admin_username="dshadmin", - ldap_username_attribute="uid", + ldap_username_attribute=props.ldap_username_attribute, ldap_user_filter=props.ldap_user_filter, ldap_server_ip=props.ldap_server_ip, ldap_server_port=props.ldap_server_port, diff --git a/data_safe_haven/infrastructure/stacks/sre/hedgedoc_server.py b/data_safe_haven/infrastructure/stacks/sre/hedgedoc_server.py index d87d478648..42dcb0869a 100644 --- a/data_safe_haven/infrastructure/stacks/sre/hedgedoc_server.py +++ b/data_safe_haven/infrastructure/stacks/sre/hedgedoc_server.py @@ -30,12 +30,11 @@ def __init__( dns_resource_group_name: Input[str], dns_server_ip: Input[str], domain_netbios_name: Input[str], - ldap_bind_dn: Input[str], - ldap_root_dn: Input[str], - ldap_search_password: Input[str], ldap_server_ip: Input[str], + ldap_server_port: Input[int], + ldap_user_filter: Input[str], ldap_user_search_base: Input[str], - ldap_user_group_name: Input[str], + ldap_username_attribute: Input[str], location: Input[str], networking_resource_group_name: Input[str], sre_fqdn: Input[str], @@ -55,22 +54,11 @@ def __init__( self.dns_resource_group_name = dns_resource_group_name self.dns_server_ip = dns_server_ip self.domain_netbios_name = domain_netbios_name - self.ldap_bind_dn = ldap_bind_dn - self.ldap_root_dn = ldap_root_dn - self.ldap_search_password = ldap_search_password self.ldap_server_ip = ldap_server_ip + self.ldap_server_port = Output.from_input(ldap_server_port).apply(str) + self.ldap_user_filter = ldap_user_filter self.ldap_user_search_base = ldap_user_search_base - self.ldap_user_group_cn = Output.all( - group_name=ldap_user_group_name, root_dn=ldap_root_dn - ).apply( - lambda kwargs: ",".join( - ( - kwargs["group_name"], - "OU=Data Safe Haven Security Groups", - kwargs["root_dn"], - ) - ) - ) + self.ldap_username_attribute = ldap_username_attribute self.location = location self.networking_resource_group_name = networking_resource_group_name self.sre_fqdn = sre_fqdn @@ -206,11 +194,7 @@ def __init__( ), containerinstance.EnvironmentVariableArgs( name="CMD_DB_USERNAME", - value=Output.concat( - props.database_username, - "@", - db_server_hedgedoc.db_server.name, - ), + value=props.database_username, ), containerinstance.EnvironmentVariableArgs( name="CMD_DOMAIN", @@ -220,14 +204,6 @@ def __init__( name="CMD_EMAIL", value="false", ), - containerinstance.EnvironmentVariableArgs( - name="CMD_LDAP_BINDCREDENTIALS", - secure_value=props.ldap_search_password, - ), - containerinstance.EnvironmentVariableArgs( - name="CMD_LDAP_BINDDN", - value=props.ldap_bind_dn, - ), containerinstance.EnvironmentVariableArgs( name="CMD_LDAP_PROVIDERNAME", value=props.domain_netbios_name, @@ -240,20 +216,25 @@ def __init__( name="CMD_LDAP_SEARCHFILTER", value=Output.concat( "(&", - "(objectClass=user)", - "(memberOf=CN=", - props.ldap_user_group_cn, + props.ldap_user_filter, + "(", + props.ldap_username_attribute, + "={{username}})", ")", - "(sAMAccountName={{username}}))", ), ), containerinstance.EnvironmentVariableArgs( name="CMD_LDAP_URL", - value=f"ldap://{props.ldap_server_ip}", + value=Output.concat( + "ldap://", + props.ldap_server_ip, + ":", + props.ldap_server_port, + ), ), containerinstance.EnvironmentVariableArgs( name="CMD_LDAP_USERIDFIELD", - value="sAMAccountName", + value=props.ldap_username_attribute, ), containerinstance.EnvironmentVariableArgs( name="CMD_LOGLEVEL", diff --git a/data_safe_haven/infrastructure/stacks/sre/user_services.py b/data_safe_haven/infrastructure/stacks/sre/user_services.py index 7d7d48de4f..5b2c1e37dd 100644 --- a/data_safe_haven/infrastructure/stacks/sre/user_services.py +++ b/data_safe_haven/infrastructure/stacks/sre/user_services.py @@ -32,6 +32,7 @@ def __init__( ldap_search_password: Input[str], ldap_server_ip: Input[str], ldap_server_port: Input[int], + ldap_username_attribute: Input[str], ldap_user_filter: Input[str], ldap_user_group_name: Input[str], ldap_user_search_base: Input[str], @@ -61,6 +62,7 @@ def __init__( self.ldap_search_password = ldap_search_password self.ldap_server_ip = ldap_server_ip self.ldap_server_port = ldap_server_port + self.ldap_username_attribute = ldap_username_attribute self.ldap_user_filter = ldap_user_filter self.ldap_user_group_name = ldap_user_group_name self.ldap_user_search_base = ldap_user_search_base @@ -123,6 +125,7 @@ def __init__( dns_server_ip=props.dns_server_ip, ldap_server_ip=props.ldap_server_ip, ldap_server_port=props.ldap_server_port, + ldap_username_attribute=props.ldap_username_attribute, ldap_user_filter=props.ldap_user_filter, ldap_user_search_base=props.ldap_user_search_base, location=props.location, @@ -144,16 +147,15 @@ def __init__( stack_name, SREHedgeDocServerProps( containers_subnet_id=props.subnet_containers_id, - database_subnet_id=props.subnet_containers_support_id, database_password=props.hedgedoc_database_password, + database_subnet_id=props.subnet_containers_support_id, dns_resource_group_name=props.dns_resource_group_name, dns_server_ip=props.dns_server_ip, domain_netbios_name=props.domain_netbios_name, - ldap_bind_dn=props.ldap_bind_dn, - ldap_root_dn=props.ldap_root_dn, - ldap_search_password=props.ldap_search_password, ldap_server_ip=props.ldap_server_ip, - ldap_user_group_name=props.ldap_user_group_name, + ldap_server_port=props.ldap_server_port, + ldap_username_attribute=props.ldap_username_attribute, + ldap_user_filter=props.ldap_user_filter, ldap_user_search_base=props.ldap_user_search_base, location=props.location, networking_resource_group_name=props.networking_resource_group_name, From aea776cdbfbad2e58df83c9d8e4c494769853203 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Mon, 8 Apr 2024 15:21:26 +0100 Subject: [PATCH 18/32] :wrench: Remove unused references to LDAP bind user/password in the SRE --- data_safe_haven/commands/deploy.py | 2 -- .../infrastructure/stacks/declarative_sre.py | 24 ++++----------- .../stacks/sre/remote_desktop.py | 6 ---- .../stacks/sre/user_services.py | 8 ----- .../infrastructure/stacks/sre/workspaces.py | 29 +++++-------------- .../workspace.cloud_init.mustache.yaml | 2 +- 6 files changed, 13 insertions(+), 58 deletions(-) diff --git a/data_safe_haven/commands/deploy.py b/data_safe_haven/commands/deploy.py index da64fe4079..dc4d06f209 100644 --- a/data_safe_haven/commands/deploy.py +++ b/data_safe_haven/commands/deploy.py @@ -210,8 +210,6 @@ def sre( shm_stack.output("update_servers")["ip_address_linux"], replace=True, ) - # Add necessary secrets - stack.copy_secret("password-domain-ldap-searcher", shm_stack) # Deploy Azure infrastructure with Pulumi if force is None: diff --git a/data_safe_haven/infrastructure/stacks/declarative_sre.py b/data_safe_haven/infrastructure/stacks/declarative_sre.py index d8e87ae4c5..ad7f0388cc 100644 --- a/data_safe_haven/infrastructure/stacks/declarative_sre.py +++ b/data_safe_haven/infrastructure/stacks/declarative_sre.py @@ -66,17 +66,13 @@ def run(self) -> None: # Construct LDAP paths ldap_root_dn = self.pulumi_opts.require("shm-domain_controllers-ldap_root_dn") - ldap_bind_dn = ( - f"CN=dshldapsearcher,OU=Data Safe Haven Service Accounts,{ldap_root_dn}" - ) ldap_group_search_base = f"OU=groups,{ldap_root_dn}" ldap_user_search_base = f"OU=users,{ldap_root_dn}" - ldap_search_password = self.pulumi_opts.require("password-domain-ldap-searcher") - ldap_base_group_name = f"Data Safe Haven SRE {self.sre_name}" - ldap_admin_group_name = f"{ldap_base_group_name} Administrators" - ldap_privileged_user_group_name = f"{ldap_base_group_name} Privileged Users" + ldap_group_name_prefix = f"Data Safe Haven SRE {self.sre_name}" + ldap_admin_group_name = f"{ldap_group_name_prefix} Administrators" + ldap_privileged_user_group_name = f"{ldap_group_name_prefix} Privileged Users" ldap_username_attribute = "uid" - ldap_user_group_name = f"{ldap_base_group_name} Users" + ldap_user_group_name = f"{ldap_group_name_prefix} Users" # Users are a posixAccount belonging to any of these groups ldap_user_filter = "".join( [ @@ -252,14 +248,11 @@ def run(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, - ldap_bind_dn=ldap_bind_dn, ldap_group_filter=ldap_group_filter, ldap_group_search_base=ldap_group_search_base, - ldap_search_password=ldap_search_password, ldap_server_ip=identity.ip_address, ldap_server_port=identity.server_port, ldap_user_filter=ldap_user_filter, - ldap_user_group_name=ldap_user_group_name, ldap_user_search_base=ldap_user_search_base, location=self.cfg.azure.location, storage_account_key=data.storage_account_data_configuration_key, @@ -277,14 +270,11 @@ def run(self) -> None: self.stack_name, SREWorkspacesProps( admin_password=data.password_workspace_admin, - ldap_bind_dn=ldap_bind_dn, ldap_group_filter=ldap_group_filter, ldap_group_search_base=ldap_group_search_base, - ldap_root_dn=ldap_root_dn, - ldap_search_password=ldap_search_password, ldap_server_ip=identity.ip_address, + ldap_server_port=identity.server_port, ldap_user_filter=ldap_user_filter, - ldap_user_group_name=ldap_user_group_name, ldap_user_search_base=ldap_user_search_base, linux_update_server_ip=self.pulumi_opts.require( "shm-update_servers-ip_address_linux" @@ -323,14 +313,10 @@ def run(self) -> None: ), gitea_database_password=data.password_gitea_database_admin, hedgedoc_database_password=data.password_hedgedoc_database_admin, - ldap_bind_dn=ldap_bind_dn, - ldap_root_dn=ldap_root_dn, - ldap_search_password=ldap_search_password, ldap_server_ip=identity.ip_address, ldap_server_port=identity.server_port, ldap_user_filter=ldap_user_filter, ldap_username_attribute=ldap_username_attribute, - ldap_user_group_name=ldap_user_group_name, ldap_user_search_base=ldap_user_search_base, location=self.cfg.azure.location, networking_resource_group_name=networking.resource_group.name, diff --git a/data_safe_haven/infrastructure/stacks/sre/remote_desktop.py b/data_safe_haven/infrastructure/stacks/sre/remote_desktop.py index 3fa459e83e..b95641afef 100644 --- a/data_safe_haven/infrastructure/stacks/sre/remote_desktop.py +++ b/data_safe_haven/infrastructure/stacks/sre/remote_desktop.py @@ -40,14 +40,11 @@ def __init__( allow_paste: Input[bool], database_password: Input[str], dns_server_ip: Input[str], - ldap_bind_dn: Input[str], ldap_group_filter: Input[str], ldap_group_search_base: Input[str], - ldap_search_password: Input[str], ldap_server_ip: Input[str], ldap_server_port: Input[int], ldap_user_filter: Input[str], - ldap_user_group_name: Input[str], ldap_user_search_base: Input[str], location: Input[str], storage_account_key: Input[str], @@ -68,14 +65,11 @@ def __init__( self.disable_copy = not allow_copy self.disable_paste = not allow_paste self.dns_server_ip = dns_server_ip - self.ldap_bind_dn = ldap_bind_dn self.ldap_group_filter = ldap_group_filter self.ldap_group_search_base = ldap_group_search_base - self.ldap_search_password = ldap_search_password self.ldap_server_ip = ldap_server_ip self.ldap_server_port = ldap_server_port self.ldap_user_filter = ldap_user_filter - self.ldap_user_group_name = ldap_user_group_name self.ldap_user_search_base = ldap_user_search_base self.location = location self.storage_account_key = storage_account_key diff --git a/data_safe_haven/infrastructure/stacks/sre/user_services.py b/data_safe_haven/infrastructure/stacks/sre/user_services.py index 5b2c1e37dd..0dd0364967 100644 --- a/data_safe_haven/infrastructure/stacks/sre/user_services.py +++ b/data_safe_haven/infrastructure/stacks/sre/user_services.py @@ -27,14 +27,10 @@ def __init__( domain_netbios_name: Input[str], gitea_database_password: Input[str], hedgedoc_database_password: Input[str], - ldap_bind_dn: Input[str], - ldap_root_dn: Input[str], - ldap_search_password: Input[str], ldap_server_ip: Input[str], ldap_server_port: Input[int], ldap_username_attribute: Input[str], ldap_user_filter: Input[str], - ldap_user_group_name: Input[str], ldap_user_search_base: Input[str], location: Input[str], networking_resource_group_name: Input[str], @@ -57,14 +53,10 @@ def __init__( self.domain_netbios_name = domain_netbios_name self.gitea_database_password = gitea_database_password self.hedgedoc_database_password = hedgedoc_database_password - self.ldap_bind_dn = ldap_bind_dn - self.ldap_root_dn = ldap_root_dn - self.ldap_search_password = ldap_search_password self.ldap_server_ip = ldap_server_ip self.ldap_server_port = ldap_server_port self.ldap_username_attribute = ldap_username_attribute self.ldap_user_filter = ldap_user_filter - self.ldap_user_group_name = ldap_user_group_name self.ldap_user_search_base = ldap_user_search_base self.location = location self.networking_resource_group_name = networking_resource_group_name diff --git a/data_safe_haven/infrastructure/stacks/sre/workspaces.py b/data_safe_haven/infrastructure/stacks/sre/workspaces.py index 204118468a..51e64ba988 100644 --- a/data_safe_haven/infrastructure/stacks/sre/workspaces.py +++ b/data_safe_haven/infrastructure/stacks/sre/workspaces.py @@ -30,14 +30,11 @@ class SREWorkspacesProps: def __init__( self, admin_password: Input[str], - ldap_bind_dn: Input[str], ldap_group_filter: Input[str], ldap_group_search_base: Input[str], - ldap_root_dn: Input[str], - ldap_search_password: Input[str], ldap_server_ip: Input[str], + ldap_server_port: Input[int], ldap_user_filter: Input[str], - ldap_user_group_name: Input[str], ldap_user_search_base: Input[str], linux_update_server_ip: Input[str], location: Input[str], @@ -55,14 +52,11 @@ def __init__( ) -> None: self.admin_password = Output.secret(admin_password) self.admin_username = "dshadmin" - self.ldap_bind_dn = ldap_bind_dn self.ldap_group_filter = ldap_group_filter self.ldap_group_search_base = ldap_group_search_base - self.ldap_root_dn = ldap_root_dn - self.ldap_search_password = ldap_search_password self.ldap_server_ip = ldap_server_ip + self.ldap_server_port = Output.from_input(ldap_server_port).apply(str) self.ldap_user_filter = ldap_user_filter - self.ldap_user_group_name = ldap_user_group_name self.ldap_user_search_base = ldap_user_search_base self.linux_update_server_ip = linux_update_server_ip self.location = location @@ -127,14 +121,11 @@ def __init__( # Load cloud-init file b64cloudinit = Output.all( - ldap_bind_dn=props.ldap_bind_dn, ldap_group_filter=props.ldap_group_filter, ldap_group_search_base=props.ldap_group_search_base, - ldap_root_dn=props.ldap_root_dn, - ldap_search_password=props.ldap_search_password, - ldap_user_filter=props.ldap_user_filter, - ldap_user_group_name=props.ldap_user_group_name, ldap_server_ip=props.ldap_server_ip, + ldap_server_port=props.ldap_server_port, + ldap_user_filter=props.ldap_user_filter, ldap_user_search_base=props.ldap_user_search_base, linux_update_server_ip=props.linux_update_server_ip, sre_fqdn=props.sre_fqdn, @@ -219,32 +210,26 @@ def __init__( def read_cloudinit( self, - ldap_bind_dn: str, ldap_group_filter: str, ldap_group_search_base: str, - ldap_root_dn: str, - ldap_search_password: str, - ldap_user_group_name: str, ldap_server_ip: str, + ldap_server_port: str, ldap_user_filter: str, ldap_user_search_base: str, linux_update_server_ip: str, sre_fqdn: str, - storage_account_data_private_user_name: str, storage_account_data_private_sensitive_name: str, + storage_account_data_private_user_name: str, ) -> str: with open( resources_path / "workspace" / "workspace.cloud_init.mustache.yaml", encoding="utf-8", ) as f_cloudinit: mustache_values = { - "ldap_bind_dn": ldap_bind_dn, "ldap_group_filter": ldap_group_filter, "ldap_group_search_base": ldap_group_search_base, - "ldap_root_dn": ldap_root_dn, - "ldap_search_password": ldap_search_password, - "ldap_user_group_name": ldap_user_group_name, "ldap_server_ip": ldap_server_ip, + "ldap_server_port": ldap_server_port, "ldap_user_filter": ldap_user_filter, "ldap_user_search_base": ldap_user_search_base, "linux_update_server_ip": linux_update_server_ip, diff --git a/data_safe_haven/resources/workspace/workspace.cloud_init.mustache.yaml b/data_safe_haven/resources/workspace/workspace.cloud_init.mustache.yaml index 76632b8b42..d67913e63a 100644 --- a/data_safe_haven/resources/workspace/workspace.cloud_init.mustache.yaml +++ b/data_safe_haven/resources/workspace/workspace.cloud_init.mustache.yaml @@ -22,7 +22,7 @@ write_files: nss_min_uid 2000 # General connection options - uri ldap://{{ldap_server_ip}}:1389 + uri ldap://{{ldap_server_ip}}:{{ldap_server_port}} # Search/mapping options base {{ldap_user_search_base}} From 85eede28511f780044dc82ccc0f88efe57960708 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Mon, 8 Apr 2024 15:24:05 +0100 Subject: [PATCH 19/32] :coffin: Drop references to SHM identity subnet --- data_safe_haven/commands/deploy.py | 5 -- .../infrastructure/stacks/declarative_sre.py | 3 - .../infrastructure/stacks/sre/networking.py | 59 +------------------ 3 files changed, 3 insertions(+), 64 deletions(-) diff --git a/data_safe_haven/commands/deploy.py b/data_safe_haven/commands/deploy.py index dc4d06f209..3067f5d0d0 100644 --- a/data_safe_haven/commands/deploy.py +++ b/data_safe_haven/commands/deploy.py @@ -185,11 +185,6 @@ def sre( shm_stack.output("networking")["resource_group_name"], replace=True, ) - stack.add_option( - "shm-networking-subnet_identity_servers_prefix", - shm_stack.output("networking")["subnet_identity_servers_prefix"], - replace=True, - ) stack.add_option( "shm-networking-subnet_subnet_monitoring_prefix", shm_stack.output("networking")["subnet_monitoring_prefix"], diff --git a/data_safe_haven/infrastructure/stacks/declarative_sre.py b/data_safe_haven/infrastructure/stacks/declarative_sre.py index ad7f0388cc..ba7e22b266 100644 --- a/data_safe_haven/infrastructure/stacks/declarative_sre.py +++ b/data_safe_haven/infrastructure/stacks/declarative_sre.py @@ -134,9 +134,6 @@ def run(self) -> None: shm_networking_resource_group_name=self.pulumi_opts.require( "shm-networking-resource_group_name" ), - shm_subnet_identity_servers_prefix=self.pulumi_opts.require( - "shm-networking-subnet_identity_servers_prefix", - ), shm_subnet_monitoring_prefix=self.pulumi_opts.require( "shm-networking-subnet_subnet_monitoring_prefix", ), diff --git a/data_safe_haven/infrastructure/stacks/sre/networking.py b/data_safe_haven/infrastructure/stacks/sre/networking.py index 7cdcf06088..7f788278a2 100644 --- a/data_safe_haven/infrastructure/stacks/sre/networking.py +++ b/data_safe_haven/infrastructure/stacks/sre/networking.py @@ -27,7 +27,6 @@ def __init__( location: Input[str], shm_fqdn: Input[str], shm_networking_resource_group_name: Input[str], - shm_subnet_identity_servers_prefix: Input[str], shm_subnet_monitoring_prefix: Input[str], shm_subnet_update_servers_prefix: Input[str], shm_virtual_network_name: Input[str], @@ -86,7 +85,6 @@ def __init__( self.user_public_ip_ranges = user_public_ip_ranges self.shm_fqdn = shm_fqdn self.shm_networking_resource_group_name = shm_networking_resource_group_name - self.shm_subnet_identity_servers_prefix = shm_subnet_identity_servers_prefix self.shm_subnet_monitoring_prefix = shm_subnet_monitoring_prefix self.shm_subnet_update_servers_prefix = shm_subnet_update_servers_prefix self.shm_virtual_network_name = shm_virtual_network_name @@ -486,18 +484,6 @@ def __init__( source_address_prefix=subnet_guacamole_containers_prefix, source_port_range="*", ), - network.SecurityRuleArgs( - access=network.SecurityRuleAccess.ALLOW, - description="Allow LDAP client requests over TCP.", - destination_address_prefix=props.shm_subnet_identity_servers_prefix, - destination_port_ranges=["389", "636"], - direction=network.SecurityRuleDirection.OUTBOUND, - name="AllowLDAPClientTCPOutbound", - priority=NetworkingPriorities.INTERNAL_SHM_LDAP_TCP, - protocol=network.SecurityRuleProtocol.TCP, - source_address_prefix=subnet_guacamole_containers_prefix, - source_port_range="*", - ), network.SecurityRuleArgs( access=network.SecurityRuleAccess.ALLOW, description="Allow outbound connections to configuration data endpoints.", @@ -528,7 +514,7 @@ def __init__( destination_address_prefix=subnet_identity_containers_prefix, destination_port_ranges=["389", "1389"], direction=network.SecurityRuleDirection.OUTBOUND, - name="AllowSREIdentityServersOutbound", + name="AllowIdentityServersOutbound", priority=NetworkingPriorities.INTERNAL_SRE_IDENTITY_CONTAINERS, protocol=network.SecurityRuleProtocol.TCP, source_address_prefix=subnet_guacamole_containers_prefix, @@ -746,18 +732,6 @@ def __init__( source_address_prefix=subnet_user_services_containers_prefix, source_port_range="*", ), - network.SecurityRuleArgs( - access=network.SecurityRuleAccess.ALLOW, - description="Allow LDAP client requests over TCP.", - destination_address_prefix=props.shm_subnet_identity_servers_prefix, - destination_port_ranges=["389", "636"], - direction=network.SecurityRuleDirection.OUTBOUND, - name="AllowLDAPClientTCPOutbound", - priority=NetworkingPriorities.INTERNAL_SHM_LDAP_TCP, - protocol=network.SecurityRuleProtocol.TCP, - source_address_prefix=subnet_user_services_containers_prefix, - source_port_range="*", - ), network.SecurityRuleArgs( access=network.SecurityRuleAccess.ALLOW, description="Allow outbound connections to configuration data endpoints.", @@ -776,7 +750,7 @@ def __init__( destination_address_prefix=subnet_identity_containers_prefix, destination_port_ranges=["389", "1389"], direction=network.SecurityRuleDirection.OUTBOUND, - name="AllowSREIdentityServersOutbound", + name="AllowIdentityServersOutbound", priority=NetworkingPriorities.INTERNAL_SRE_IDENTITY_CONTAINERS, protocol=network.SecurityRuleProtocol.TCP, source_address_prefix=subnet_user_services_containers_prefix, @@ -1090,45 +1064,18 @@ def __init__( source_address_prefix="*", source_port_range="*", ), - network.SecurityRuleArgs( - access=network.SecurityRuleAccess.ALLOW, - description=( - "Allow LDAP client requests over TCP. " - "See https://devopstales.github.io/linux/pfsense-ad-join/ for details." - ), - destination_address_prefix=props.shm_subnet_identity_servers_prefix, - destination_port_ranges=["389", "636"], - direction=network.SecurityRuleDirection.OUTBOUND, - name="AllowLDAPClientTCPOutbound", - priority=NetworkingPriorities.INTERNAL_SHM_LDAP_TCP, - protocol=network.SecurityRuleProtocol.TCP, - source_address_prefix=subnet_workspaces_prefix, - source_port_range="*", - ), network.SecurityRuleArgs( access=network.SecurityRuleAccess.ALLOW, description="Allow LDAP client requests over TCP.", destination_address_prefix=subnet_identity_containers_prefix, destination_port_ranges=["389", "1389"], direction=network.SecurityRuleDirection.OUTBOUND, - name="AllowSREIdentityServersOutbound", + name="AllowIdentityServersOutbound", priority=NetworkingPriorities.INTERNAL_SRE_IDENTITY_CONTAINERS, protocol=network.SecurityRuleProtocol.TCP, source_address_prefix=subnet_workspaces_prefix, source_port_range="*", ), - network.SecurityRuleArgs( - access=network.SecurityRuleAccess.ALLOW, - description="Allow LDAP client requests over UDP.", - destination_address_prefix=props.shm_subnet_identity_servers_prefix, - destination_port_ranges=["389", "636"], - direction=network.SecurityRuleDirection.OUTBOUND, - name="AllowLDAPClientUDPOutbound", - priority=NetworkingPriorities.INTERNAL_SHM_LDAP_UDP, - protocol=network.SecurityRuleProtocol.UDP, - source_address_prefix=subnet_workspaces_prefix, - source_port_range="*", - ), network.SecurityRuleArgs( access=network.SecurityRuleAccess.ALLOW, description="Allow outbound connections to SHM monitoring tools.", From 1f0548e5f60911fa765fe3b601d39e70996c9807 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Mon, 8 Apr 2024 21:21:41 +0100 Subject: [PATCH 20/32] :coffin: Create AzureAD groups in AzureAD rather than on the SHM DC --- data_safe_haven/commands/deploy.py | 1 + data_safe_haven/external/api/graph_api.py | 2 +- .../provisioning/sre_provisioning_manager.py | 21 +++++----------- .../resources/active_directory/add_group.ps1 | 24 ------------------- 4 files changed, 8 insertions(+), 40 deletions(-) delete mode 100644 data_safe_haven/resources/active_directory/add_group.ps1 diff --git a/data_safe_haven/commands/deploy.py b/data_safe_haven/commands/deploy.py index 3067f5d0d0..13a3a44ded 100644 --- a/data_safe_haven/commands/deploy.py +++ b/data_safe_haven/commands/deploy.py @@ -220,6 +220,7 @@ def sre( # Provision SRE with anything that could not be done in Pulumi manager = SREProvisioningManager( + graph_api_token=graph_api.token, shm_stack=shm_stack, sre_name=sre_name, sre_stack=stack, diff --git a/data_safe_haven/external/api/graph_api.py b/data_safe_haven/external/api/graph_api.py index 808ed0c2da..90018e9856 100644 --- a/data_safe_haven/external/api/graph_api.py +++ b/data_safe_haven/external/api/graph_api.py @@ -328,7 +328,7 @@ def create_group(self, group_name: str) -> None: "displayName": group_name, "groupTypes": [], "mailEnabled": False, - "mailNickname": group_name, + "mailNickname": "".join(filter(str.isalnum, group_name)), "securityEnabled": True, } self.http_post( diff --git a/data_safe_haven/provisioning/sre_provisioning_manager.py b/data_safe_haven/provisioning/sre_provisioning_manager.py index 56d1f0b2d0..db85f3bffa 100644 --- a/data_safe_haven/provisioning/sre_provisioning_manager.py +++ b/data_safe_haven/provisioning/sre_provisioning_manager.py @@ -7,6 +7,7 @@ AzureApi, AzureContainerInstance, AzurePostgreSQLDatabase, + GraphApi, ) from data_safe_haven.infrastructure import SHMStackManager, SREStackManager from data_safe_haven.resources import resources_path @@ -18,6 +19,7 @@ class SREProvisioningManager: def __init__( self, + graph_api_token: str, shm_stack: SHMStackManager, sre_name: str, sre_stack: SREStackManager, @@ -26,6 +28,7 @@ def __init__( ): self._available_vm_skus: dict[str, dict[str, Any]] | None = None self.azure_location = shm_stack.cfg.azure.location + self.graph_api = GraphApi(auth_token=graph_api_token) self.logger = LoggingSingleton() self.sre_name = sre_name self.subscription_name = subscription_name @@ -88,24 +91,12 @@ def available_vm_skus(self) -> dict[str, dict[str, Any]]: return self._available_vm_skus def create_security_groups(self) -> None: - azure_api = AzureApi(self.subscription_name) - script = FileReader(resources_path / "active_directory" / "add_group.ps1") + """Create groups in AzureAD""" for group_name in self.security_group_params["security_group_names"].values(): - script_parameters = { - "GroupName": group_name, - "OuPath": f"OU=Data Safe Haven Security Groups,{self.security_group_params['dn_base']}", - } - output = azure_api.run_remote_script( - self.security_group_params["resource_group_name"], - script.file_contents(), - script_parameters, - self.security_group_params["vm_name"], - ) - for line in output.split("\n"): - self.logger.parse(line) + self.graph_api.create_group(group_name) def restart_remote_desktop_containers(self) -> None: - # Restart the Guacamole container group + """Restart the Guacamole container group""" guacamole_provisioner = AzureContainerInstance( self.remote_desktop_params["container_group_name"], self.remote_desktop_params["resource_group_name"], diff --git a/data_safe_haven/resources/active_directory/add_group.ps1 b/data_safe_haven/resources/active_directory/add_group.ps1 deleted file mode 100644 index 08c7a87bed..0000000000 --- a/data_safe_haven/resources/active_directory/add_group.ps1 +++ /dev/null @@ -1,24 +0,0 @@ -param ( - [Parameter(Mandatory = $false, HelpMessage = "Name of group to create")] - [ValidateNotNullOrEmpty()] - [string]$GroupName, - [Parameter(Mandatory = $false, HelpMessage = "OU path to create group under")] - [ValidateNotNullOrEmpty()] - [string]$OuPath -) - -if (Get-ADGroup -Filter "Name -eq '$GroupName'" -SearchBase $OuPath -ErrorAction SilentlyContinue) { - Write-Output "INFO: Group [green]'$GroupName'[/] already exists." -} else { - try { - New-ADGroup -Description "$GroupName" ` - -GroupCategory "Security" ` - -GroupScope "Global" ` - -Name "$GroupName" ` - -Path $OuPath ` - -ErrorAction Stop - Write-Output "INFO: Created group [green]'$GroupName'[/]." - } catch { - Write-Output "ERROR: Failed to create group [green]'$GroupName'[/]." - } -} From 6b4a8b3fed267ddda8c76407984ca3151e3fd8e1 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Mon, 8 Apr 2024 22:40:31 +0100 Subject: [PATCH 21/32] :recycle: Switch to AzureAD as default place to add/remove/alter user lists --- .../administration/users/azure_ad_users.py | 4 -- .../administration/users/user_handler.py | 38 ++++++++++--------- data_safe_haven/commands/admin_add_users.py | 9 +++-- data_safe_haven/commands/admin_list_users.py | 3 +- .../commands/admin_register_users.py | 7 ++-- .../commands/admin_remove_users.py | 5 +-- .../commands/admin_unregister_users.py | 14 ++++--- data_safe_haven/external/api/graph_api.py | 8 ++-- .../provisioning/sre_provisioning_manager.py | 3 +- 9 files changed, 48 insertions(+), 43 deletions(-) diff --git a/data_safe_haven/administration/users/azure_ad_users.py b/data_safe_haven/administration/users/azure_ad_users.py index 06f648987c..24dc129594 100644 --- a/data_safe_haven/administration/users/azure_ad_users.py +++ b/data_safe_haven/administration/users/azure_ad_users.py @@ -71,10 +71,6 @@ def list(self) -> Sequence[ResearchUser]: user_principal_name=user_details["userPrincipalName"], ) for user_details in user_list - if ( - user_details["onPremisesSamAccountName"] - or user_details["isGlobalAdmin"] - ) ] def register(self, sre_name: str, usernames: Sequence[str]) -> None: diff --git a/data_safe_haven/administration/users/user_handler.py b/data_safe_haven/administration/users/user_handler.py index fb2233cf4b..305dcdb82d 100644 --- a/data_safe_haven/administration/users/user_handler.py +++ b/data_safe_haven/administration/users/user_handler.py @@ -34,7 +34,9 @@ def add(self, users_csv_path: pathlib.Path) -> None: try: # Construct user list with open(users_csv_path, encoding="utf-8") as f_csv: - reader = csv.DictReader(f_csv) + dialect = csv.Sniffer().sniff(f_csv.read(), delimiters=";,") + f_csv.seek(0) + reader = csv.DictReader(f_csv, dialect=dialect) for required_field in [ "GivenName", "Surname", @@ -49,6 +51,7 @@ def add(self, users_csv_path: pathlib.Path) -> None: raise ValueError(msg) users = [ ResearchUser( + account_enabled=True, country=user["CountryCode"], email_address=user["Email"], given_name=user["GivenName"], @@ -60,8 +63,8 @@ def add(self, users_csv_path: pathlib.Path) -> None: for user in users: self.logger.debug(f"Processing new user: {user}") - # Commit changes - self.active_directory_users.add(users) + # Add users to AzureAD + self.azure_ad_users.add(users) except Exception as exc: msg = f"Could not add users from '{users_csv_path}'.\n{exc}" raise DataSafeHavenUserHandlingError(msg) from exc @@ -70,7 +73,6 @@ def get_usernames(self) -> dict[str, list[str]]: """Load usernames from all sources""" usernames = {} usernames["Azure AD"] = self.get_usernames_azure_ad() - usernames["Domain controller"] = self.get_usernames_domain_controller() for sre_name in self.config.sre_names: usernames[f"SRE {sre_name}"] = self.get_usernames_guacamole(sre_name) return usernames @@ -134,7 +136,7 @@ def register(self, sre_name: str, user_names: Sequence[str]) -> None: """ try: # Add users to the SRE security group - self.active_directory_users.register(sre_name, user_names) + self.azure_ad_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 @@ -147,14 +149,18 @@ def remove(self, user_names: Sequence[str]) -> None: """ try: # Construct user lists - active_directory_users_to_remove = [ + self.logger.info(f"Attempting to remove {len(user_names)} user(s).") + azuread_users_to_remove = [ user - for user in self.active_directory_users.list() + for user in self.azure_ad_users.list() if user.username in user_names ] # Commit changes - self.active_directory_users.remove(active_directory_users_to_remove) + self.logger.info( + f"Found {len(azuread_users_to_remove)} valid user(s) to remove." + ) + self.azure_ad_users.remove(azuread_users_to_remove) except Exception as exc: msg = f"Could not remove users: {user_names}.\n{exc}" raise DataSafeHavenUserHandlingError(msg) from exc @@ -189,21 +195,19 @@ def set(self, users_csv_path: str) -> None: self.logger.debug(f"Processing user: {user}") # Keep existing users with the same username - active_directory_desired_users = [ + azuread_desired_users = [ user - for user in self.active_directory_users.list() + for user in self.azure_ad_users.list() if user.username in [u.username for u in desired_users] ] # Construct list of new users - active_directory_desired_users = [ - user - for user in desired_users - if user not in active_directory_desired_users + azuread_desired_users = [ + user for user in desired_users if user not in azuread_desired_users ] # Commit changes - self.active_directory_users.set(active_directory_desired_users) + self.azure_ad_users.set(azuread_desired_users) except Exception as exc: msg = f"Could not set users from '{users_csv_path}'.\n{exc}" raise DataSafeHavenUserHandlingError(msg) from exc @@ -216,7 +220,7 @@ def unregister(self, sre_name: str, user_names: Sequence[str]) -> None: """ try: # Remove users from the SRE security group - self.active_directory_users.unregister(sre_name, user_names) + self.azure_ad_users.unregister(sre_name, user_names) except Exception as exc: - msg = f"Could not register {len(user_names)} users with SRE '{sre_name}'.\n{exc}" + msg = f"Could not unregister {len(user_names)} users with SRE '{sre_name}'.\n{exc}" raise DataSafeHavenUserHandlingError(msg) from exc diff --git a/data_safe_haven/commands/admin_add_users.py b/data_safe_haven/commands/admin_add_users.py index a041d7dc78..67829e0767 100644 --- a/data_safe_haven/commands/admin_add_users.py +++ b/data_safe_haven/commands/admin_add_users.py @@ -16,11 +16,14 @@ def admin_add_users(csv_path: pathlib.Path) -> None: shm_name = context.shm_name try: - # Load GraphAPI as this may require user-interaction that is not - # possible as part of a Pulumi declarative command + # Load GraphAPI graph_api = GraphApi( tenant_id=config.shm.aad_tenant_id, - default_scopes=["Group.Read.All"], + default_scopes=[ + "Group.Read.All", + "User.ReadWrite.All", + "UserAuthenticationMethod.ReadWrite.All", + ], ) # Add users to SHM diff --git a/data_safe_haven/commands/admin_list_users.py b/data_safe_haven/commands/admin_list_users.py index 5d396af47d..eadcca9496 100644 --- a/data_safe_haven/commands/admin_list_users.py +++ b/data_safe_haven/commands/admin_list_users.py @@ -14,8 +14,7 @@ def admin_list_users() -> None: shm_name = context.shm_name try: - # Load GraphAPI as this may require user-interaction that is not - # possible as part of a Pulumi declarative command + # Load GraphAPI graph_api = GraphApi( tenant_id=config.shm.aad_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 7ceb6e169c..90d70f4cd2 100644 --- a/data_safe_haven/commands/admin_register_users.py +++ b/data_safe_haven/commands/admin_register_users.py @@ -28,16 +28,15 @@ def admin_register_users( f"Preparing to register {len(usernames)} user(s) with SRE '{sre_name}'" ) - # Load GraphAPI as this may require user-interaction that is not - # possible as part of a Pulumi declarative command + # Load GraphAPI graph_api = GraphApi( tenant_id=config.shm.aad_tenant_id, - default_scopes=["Group.Read.All"], + default_scopes=["Group.ReadWrite.All", "GroupMember.ReadWrite.All"], ) # List users users = UserHandler(config, graph_api) - available_usernames = users.get_usernames_domain_controller() + available_usernames = users.get_usernames_azure_ad() usernames_to_register = [] for username in usernames: if username in available_usernames: diff --git a/data_safe_haven/commands/admin_remove_users.py b/data_safe_haven/commands/admin_remove_users.py index 36c5ea4763..bd42eae013 100644 --- a/data_safe_haven/commands/admin_remove_users.py +++ b/data_safe_haven/commands/admin_remove_users.py @@ -16,11 +16,10 @@ def admin_remove_users( shm_name = context.shm_name try: - # Load GraphAPI as this may require user-interaction that is not - # possible as part of a Pulumi declarative command + # Load GraphAPI graph_api = GraphApi( tenant_id=config.shm.aad_tenant_id, - default_scopes=["Group.Read.All"], + default_scopes=["User.ReadWrite.All"], ) # Remove users from SHM diff --git a/data_safe_haven/commands/admin_unregister_users.py b/data_safe_haven/commands/admin_unregister_users.py index 64f7fec3dd..3e15f72974 100644 --- a/data_safe_haven/commands/admin_unregister_users.py +++ b/data_safe_haven/commands/admin_unregister_users.py @@ -27,16 +27,15 @@ def admin_unregister_users( f"Preparing to unregister {len(usernames)} users with SRE '{sre_name}'" ) - # Load GraphAPI as this may require user-interaction that is not - # possible as part of a Pulumi declarative command + # Load GraphAPI graph_api = GraphApi( tenant_id=config.shm.aad_tenant_id, - default_scopes=["Group.Read.All"], + default_scopes=["Group.ReadWrite.All", "GroupMember.ReadWrite.All"], ) # List users users = UserHandler(config, graph_api) - available_usernames = users.get_usernames_domain_controller() + available_usernames = users.get_usernames_azure_ad() usernames_to_unregister = [] for username in usernames: if username in available_usernames: @@ -46,7 +45,12 @@ def admin_unregister_users( f"Username '{username}' does not belong to this Data Safe Haven deployment." " Please use 'dsh users add' to create it." ) - users.unregister(sre_name, usernames_to_unregister) + for group_name in ( + f"{sre_name} Users", + f"{sre_name} Privileged Users", + f"{sre_name} Administrators", + ): + users.unregister(group_name, usernames_to_unregister) except DataSafeHavenError as exc: msg = f"Could not unregister users from Data Safe Haven '{shm_name}' with SRE '{sre_name}'.\n{exc}" raise DataSafeHavenError(msg) from exc diff --git a/data_safe_haven/external/api/graph_api.py b/data_safe_haven/external/api/graph_api.py index 90018e9856..394e04867d 100644 --- a/data_safe_haven/external/api/graph_api.py +++ b/data_safe_haven/external/api/graph_api.py @@ -57,6 +57,7 @@ class GraphApi: "Group.Read.All": "5b567255-7703-4780-807c-7be8301ae99b", "Group.ReadWrite.All": "62a82d76-70ea-41e2-9197-370581804d09", "GroupMember.Read.All": "98830695-27a2-44f7-8c18-0c3ebc9698f6", + "GroupMember.ReadWrite.All": "dbaae8cf-10b5-4b86-a4a1-f871c94c6695", "User.Read.All": "df021288-bdef-4463-88db-98f22de89214", "User.ReadWrite.All": "741f803b-c850-494e-b5df-cde7c675a1ca", "UserAuthenticationMethod.ReadWrite.All": "50483e42-d915-4231-9639-7fdb7fd190e5", @@ -325,10 +326,11 @@ def create_group(self, group_name: str) -> None: f"Creating AzureAD group '[green]{group_name}[/]'...", ) request_json = { + "description": group_name, "displayName": group_name, "groupTypes": [], "mailEnabled": False, - "mailNickname": "".join(filter(str.isalnum, group_name)), + "mailNickname": ("".join(filter(str.isalnum, group_name))).lower(), "securityEnabled": True, } self.http_post( @@ -502,7 +504,7 @@ def create_user( ) except DataSafeHavenMicrosoftGraphError as exc: if "already registered" not in str(exc): - msg = f"Invalid email address '{email_address}'.\n{exc}" + msg = f"Failed to add authentication email address '{email_address}'.\n{exc}" raise DataSafeHavenMicrosoftGraphError(msg) from exc # Set the authentication phone number try: @@ -512,7 +514,7 @@ def create_user( ) except DataSafeHavenMicrosoftGraphError as exc: if "already registered" not in str(exc): - msg = f"Invalid phone number '{phone_number}'.\n{exc}" + msg = f"Failed to add authentication phone number '{phone_number}'.\n{exc}" raise DataSafeHavenMicrosoftGraphError(msg) from exc # Ensure user is enabled self.http_patch( diff --git a/data_safe_haven/provisioning/sre_provisioning_manager.py b/data_safe_haven/provisioning/sre_provisioning_manager.py index db85f3bffa..e7d82806c8 100644 --- a/data_safe_haven/provisioning/sre_provisioning_manager.py +++ b/data_safe_haven/provisioning/sre_provisioning_manager.py @@ -10,8 +10,7 @@ GraphApi, ) from data_safe_haven.infrastructure import SHMStackManager, SREStackManager -from data_safe_haven.resources import resources_path -from data_safe_haven.utility import FileReader, LoggingSingleton +from data_safe_haven.utility import LoggingSingleton class SREProvisioningManager: From 0fe059bed8e77a180dd8fad39b789cceb8952ead Mon Sep 17 00:00:00 2001 From: James Robinson Date: Mon, 8 Apr 2024 22:43:20 +0100 Subject: [PATCH 22/32] :coffin: Drop code for interacting with AzureAD on the DC --- .../users/active_directory_users.py | 165 ------------------ .../administration/users/user_handler.py | 6 - .../resources/active_directory/add_users.ps1 | 74 -------- .../active_directory/add_users_to_group.ps1 | 28 --- .../resources/active_directory/list_users.ps1 | 17 -- .../active_directory/remove_users.ps1 | 35 ---- .../remove_users_from_group.ps1 | 22 --- 7 files changed, 347 deletions(-) delete mode 100644 data_safe_haven/administration/users/active_directory_users.py delete mode 100644 data_safe_haven/resources/active_directory/add_users.ps1 delete mode 100644 data_safe_haven/resources/active_directory/add_users_to_group.ps1 delete mode 100644 data_safe_haven/resources/active_directory/list_users.ps1 delete mode 100644 data_safe_haven/resources/active_directory/remove_users.ps1 delete mode 100644 data_safe_haven/resources/active_directory/remove_users_from_group.ps1 diff --git a/data_safe_haven/administration/users/active_directory_users.py b/data_safe_haven/administration/users/active_directory_users.py deleted file mode 100644 index 759175939b..0000000000 --- a/data_safe_haven/administration/users/active_directory_users.py +++ /dev/null @@ -1,165 +0,0 @@ -"""Interact with users in an Azure Active Directory""" - -import pathlib -from collections.abc import Sequence -from typing import Any - -from data_safe_haven.config import Config -from data_safe_haven.exceptions import DataSafeHavenActiveDirectoryError -from data_safe_haven.external import AzureApi -from data_safe_haven.functions import b64encode -from data_safe_haven.infrastructure import SHMStackManager -from data_safe_haven.utility import FileReader, LoggingSingleton - -from .research_user import ResearchUser - - -class ActiveDirectoryUsers: - """Interact with users in an Azure Active Directory""" - - def __init__( - self, - config: Config, - *args: Any, - **kwargs: Any, - ) -> None: - super().__init__(*args, **kwargs) - shm_stack = SHMStackManager(config) - self.azure_api = AzureApi(config.context.subscription_name) - self.logger = LoggingSingleton() - self.resource_group_name = shm_stack.output("domain_controllers")[ - "resource_group_name" - ] - self.resources_path = ( - pathlib.Path(__file__).parent.parent.parent / "resources" - ).resolve() - self.vm_name = shm_stack.output("domain_controllers")["vm_name"] - - def add(self, new_users: Sequence[ResearchUser]) -> None: - """Add list of users to local Active Directory""" - add_users_script = FileReader( - self.resources_path / "active_directory" / "add_users.ps1" - ) - csv_contents = ["SamAccountName;GivenName;Surname;Mobile;Email;Country"] - for user in new_users: - if ( - user.username - and user.given_name - and user.surname - and user.phone_number - and user.email_address - and user.country - ): - csv_contents += [ - ";".join( - [ - user.username, - user.given_name, - user.surname, - user.phone_number, - user.email_address, - user.country, - ] - ) - ] - else: - msg = ( - f"User {user.username} does not have all the required fields to " - "add to the Active Directory." - ) - raise DataSafeHavenActiveDirectoryError(msg) - user_details_b64 = b64encode("\n".join(csv_contents)) - output = self.azure_api.run_remote_script( - self.resource_group_name, - add_users_script.file_contents(), - {"UserDetailsB64": user_details_b64}, - self.vm_name, - ) - for line in output.split("\n"): - self.logger.parse(line) - - def list(self, sre_name: str | None = None) -> Sequence[ResearchUser]: - """List users in a local Active Directory""" - list_users_script = FileReader( - self.resources_path / "active_directory" / "list_users.ps1" - ) - script_params = {"SREName": sre_name} if sre_name else {} - output = self.azure_api.run_remote_script( - self.resource_group_name, - list_users_script.file_contents(), - script_params, - self.vm_name, - ) - users = [] - expected_columns = 6 - for line in output.split("\n"): - tokens = line.split(";") - if (n_tokens := len(tokens)) == expected_columns: - users.append( - ResearchUser( - email_address=tokens[4], - given_name=tokens[1], - phone_number=tokens[3], - sam_account_name=tokens[0], - surname=tokens[2], - user_principal_name=tokens[5], - ) - ) - else: - msg = ( - "Unexpected number of fields returned for Active Directory user.\n" - f"Expected {expected_columns}, got {n_tokens}:\n" - f"{tokens}" - ) - raise DataSafeHavenActiveDirectoryError(msg) - return users - - def register(self, sre_name: str, usernames: Sequence[str]) -> None: - """Add usernames to SRE security group""" - register_users_script = FileReader( - self.resources_path / "active_directory" / "add_users_to_group.ps1" - ) - output = self.azure_api.run_remote_script( - self.resource_group_name, - register_users_script.file_contents(), - {"SREName": sre_name, "UsernamesB64": b64encode("\n".join(usernames))}, - self.vm_name, - ) - for line in output.split("\n"): - self.logger.parse(line) - - def remove(self, users: Sequence[ResearchUser]) -> None: - """Remove list of users from local Active Directory""" - remove_users_script = FileReader( - self.resources_path / "active_directory" / "remove_users.ps1" - ) - usernames_b64 = b64encode("\n".join(user.username for user in users)) - output = self.azure_api.run_remote_script( - self.resource_group_name, - remove_users_script.file_contents(), - {"UsernamesB64": usernames_b64}, - self.vm_name, - ) - for line in output.split("\n"): - self.logger.parse(line) - - def set(self, users: Sequence[ResearchUser]) -> None: - """Set local Active Directory 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""" - register_users_script = FileReader( - self.resources_path / "active_directory" / "remove_users_from_group.ps1" - ) - output = self.azure_api.run_remote_script( - self.resource_group_name, - register_users_script.file_contents(), - {"SREName": sre_name, "UsernamesB64": b64encode("\n".join(usernames))}, - self.vm_name, - ) - for line in output.split("\n"): - self.logger.parse(line) diff --git a/data_safe_haven/administration/users/user_handler.py b/data_safe_haven/administration/users/user_handler.py index 305dcdb82d..6af404c9f4 100644 --- a/data_safe_haven/administration/users/user_handler.py +++ b/data_safe_haven/administration/users/user_handler.py @@ -7,7 +7,6 @@ from data_safe_haven.external import GraphApi from data_safe_haven.utility import LoggingSingleton -from .active_directory_users import ActiveDirectoryUsers from .azure_ad_users import AzureADUsers from .guacamole_users import GuacamoleUsers from .research_user import ResearchUser @@ -19,7 +18,6 @@ def __init__( config: Config, graph_api: GraphApi, ): - self.active_directory_users = ActiveDirectoryUsers(config) self.azure_ad_users = AzureADUsers(graph_api) self.config = config self.logger = LoggingSingleton() @@ -81,10 +79,6 @@ def get_usernames_azure_ad(self) -> list[str]: """Load usernames from Azure AD""" return [user.username for user in self.azure_ad_users.list()] - def get_usernames_domain_controller(self) -> list[str]: - """Load usernames from all domain controller""" - return [user.username for user in self.active_directory_users.list()] - def get_usernames_guacamole(self, sre_name: str) -> list[str]: """Lazy-load usernames from Guacamole""" try: diff --git a/data_safe_haven/resources/active_directory/add_users.ps1 b/data_safe_haven/resources/active_directory/add_users.ps1 deleted file mode 100644 index 98542c1506..0000000000 --- a/data_safe_haven/resources/active_directory/add_users.ps1 +++ /dev/null @@ -1,74 +0,0 @@ -param ( - [Parameter(Mandatory = $false, HelpMessage = "User details as base64-encoded string")] - [ValidateNotNullOrEmpty()] - [string]$UserDetailsB64 -) - -function GeneratePassword ([int] $PasswordLength) { - Add-Type -AssemblyName "System.Web" - $PassComplexityCheck = $false - while (-not $PassComplexityCheck) { - $GeneratedPassword = [System.Web.Security.Membership]::GeneratePassword($PasswordLength, [int]($PasswordLength / 3)) - if (($GeneratedPassword -cmatch "[A-Z]") -and - ($GeneratedPassword -cmatch "[a-z]") -and - ($GeneratedPassword -match "[\d]") -and - ($GeneratedPassword -match "[\W]")) { - $PassComplexityCheck = $True - } - } - return $GeneratedPassword -} - -# Write user details to a local file -$UserFilePath = "C:\DataSafeHaven\ActiveDirectory\users.csv" -$UserDetails = [Text.Encoding]::Utf8.GetString([Convert]::FromBase64String($UserDetailsB64)) -$UserDetails | Out-File $UserFilePath - -# Get common properties -$Domain = (Get-ADForest -Current LocalComputer).Domains -$UserOuPath = (Get-ADObject -Filter * | Where-Object { $_.Name -eq "Data Safe Haven Research Users" }).DistinguishedName - -# Create users if they do not exist -Import-Csv -Path $UserFilePath -Delimiter ";" | ForEach-Object { - $DisplayName = "$($_.GivenName) $($_.Surname)" - $UserPrincipalName = "$($_.SamAccountName)@${Domain}" - # Attempt to create user if they do not exist - try { - New-ADUser -AccountPassword $(ConvertTo-SecureString $(GeneratePassword(12)) -AsPlainText -Force) ` - -Country $_.Country ` - -Department "Data Safe Haven" ` - -Description "Research User" ` - -DisplayName "$DisplayName" ` - -Email $_.Email ` - -Enabled $True ` - -GivenName $_.GivenName ` - -Mobile $_.Mobile ` - -Name "$DisplayName" ` - -PasswordNeverExpires $True ` - -Path "$UserOuPath" ` - -SurName $_.Surname ` - -UserPrincipalName $UserPrincipalName ` - -SamAccountName $_.SamAccountName ` - -ErrorAction Stop - Write-Output "INFO: Created a user with name '[green]$UserPrincipalName[/]'." - } catch [Microsoft.ActiveDirectory.Management.ADIdentityAlreadyExistsException] { - Write-Output "WARNING: User with name '[green]$UserPrincipalName[/]' already exists." - } catch { - Write-Output "ERROR: Failed to create user with name '[green]$UserPrincipalName[/]'!" - Write-Output "ERROR: Cause of error: $($_.Exception)" - } -} -Remove-Item $UserFilePath - -# Force sync with AzureAD. It will still take around 5 minutes for changes to propagate -Write-Output "INFO: Synchronising local Active Directory with Azure" -try { - Import-Module -Name "C:\Program Files\Microsoft Azure AD Sync\Bin\ADSync" -ErrorAction Stop - Start-ADSyncSyncCycle -PolicyType Delta | Out-Null - Write-Output "INFO: Finished synchronising local Active Directory with Azure" -} catch [System.IO.FileNotFoundException] { - Write-Output "WARNING: Skipping as Azure AD Sync is not installed" -} catch { - Write-Output "ERROR: Unable to run Azure Active Directory synchronisation!" - Write-Output "ERROR: Cause of error: $($_.Exception)" -} diff --git a/data_safe_haven/resources/active_directory/add_users_to_group.ps1 b/data_safe_haven/resources/active_directory/add_users_to_group.ps1 deleted file mode 100644 index 4198ea683f..0000000000 --- a/data_safe_haven/resources/active_directory/add_users_to_group.ps1 +++ /dev/null @@ -1,28 +0,0 @@ -param ( - [Parameter(Mandatory = $false, HelpMessage = "SRE name")] - [ValidateNotNullOrEmpty()] - [string]$SREName, - [Parameter(Mandatory = $false, HelpMessage = "Usernames as base64-encoded string")] - [ValidateNotNullOrEmpty()] - [string]$UsernamesB64 -) - -# Find SRE security group -$SREGroup = Get-ADGroup -Filter "Name -eq 'Data Safe Haven SRE $SREName Users'" | Where-Object { $_.DistinguishedName -like '*Data Safe Haven Security Groups*' } | Select-Object -First 1 -if (-not $SREGroup) { - Write-Output "ERROR: No user group found on the domain controller for SRE '[green]$SREName[/]'." -} - -# Load usernames -$Usernames = ([Text.Encoding]::Utf8.GetString([Convert]::FromBase64String($UsernamesB64))).Split() -if (-not $Usernames) { - Write-Output "ERROR: No usernames provided to add to SRE '[green]$SREName[/]'." -} - -# Add each user to the SRE group -if ($SREGroup -and $Usernames) { - foreach ($Username in $Usernames) { - Write-Output "INFO: Adding user '[green]$Username[/]' to group '[green]$($SREGroup.Name)[/]'." - Add-ADGroupMember -Identity "$($SREGroup.Name)" -Members $Username - } -} diff --git a/data_safe_haven/resources/active_directory/list_users.ps1 b/data_safe_haven/resources/active_directory/list_users.ps1 deleted file mode 100644 index da8af0c4f3..0000000000 --- a/data_safe_haven/resources/active_directory/list_users.ps1 +++ /dev/null @@ -1,17 +0,0 @@ -param ( - [Parameter(Mandatory = $false, HelpMessage = "SRE name")] - [ValidateNotNullOrEmpty()] - [string]$SREName = $null -) - -# Find SRE security group if an SRE name is specified -if (![string]::IsNullOrEmpty($SREName)) { - $SREGroup = Get-ADGroup -Filter "Name -eq 'Data Safe Haven SRE $SREName Users'" | Where-Object { $_.DistinguishedName -like '*Data Safe Haven Security Groups*' } | Select-Object -First 1 -} - -# Return all matching users -$UserOuPath = (Get-ADObject -Filter * | Where-Object { $_.Name -eq "Data Safe Haven Research Users" }).DistinguishedName -foreach ($User in $(Get-ADUser -Filter * -SearchBase $UserOuPath -Properties TelephoneNumber,Mail,MemberOf)) { - if ((![string]::IsNullOrEmpty($SREName)) -and -not ($User.MemberOf.Contains($SREGroup.DistinguishedName))) { continue } - Write-Output "$($User.SamAccountName);$($User.GivenName);$($User.Surname);$($User.TelephoneNumber);$($User.Mail);$($User.UserPrincipalName)" -} diff --git a/data_safe_haven/resources/active_directory/remove_users.ps1 b/data_safe_haven/resources/active_directory/remove_users.ps1 deleted file mode 100644 index a3524bf419..0000000000 --- a/data_safe_haven/resources/active_directory/remove_users.ps1 +++ /dev/null @@ -1,35 +0,0 @@ -param ( - [Parameter(Mandatory = $false, HelpMessage = "Usernames as base64-encoded string")] - [ValidateNotNullOrEmpty()] - [string]$UsernamesB64 -) - -# Construct list of users to remove -try { - $Usernames = [Text.Encoding]::Utf8.GetString([Convert]::FromBase64String($UsernamesB64)).Split() - - # Find users in the OU path and remove them - $UserOuPath = (Get-ADObject -Filter * | Where-Object { $_.Name -eq "Data Safe Haven Research Users" }).DistinguishedName - $Usernames | ForEach-Object { - $user = Get-ADUser -Filter "SamAccountName -eq '$_'" -SearchBase $UserOuPath - if ($user) { - Write-Output "INFO: Removing user $($user.SamAccountName)" - $user | Remove-ADUser -Confirm:$false - } - } -} catch { - Write-Output "ERROR: Cause of error: $($_.Exception)" -} - -# Force sync with AzureAD. It will still take around 5 minutes for changes to propagate -Write-Output "INFO: Synchronising local Active Directory with Azure" -try { - Import-Module -Name "C:\Program Files\Microsoft Azure AD Sync\Bin\ADSync" -ErrorAction Stop - Start-ADSyncSyncCycle -PolicyType Delta | Out-Null - Write-Output "INFO: Finished synchronising local Active Directory with Azure" -} catch [System.IO.FileNotFoundException] { - Write-Output "WARNING: Skipping as Azure AD Sync is not installed" -} catch { - Write-Output "ERROR: Unable to run Azure Active Directory synchronisation!" - Write-Output "ERROR: Cause of error: $($_.Exception)" -} diff --git a/data_safe_haven/resources/active_directory/remove_users_from_group.ps1 b/data_safe_haven/resources/active_directory/remove_users_from_group.ps1 deleted file mode 100644 index 435107e91e..0000000000 --- a/data_safe_haven/resources/active_directory/remove_users_from_group.ps1 +++ /dev/null @@ -1,22 +0,0 @@ -param ( - [Parameter(Mandatory = $false, HelpMessage = "SRE name")] - [ValidateNotNullOrEmpty()] - [string]$SREName, - [Parameter(Mandatory = $false, HelpMessage = "Usernames as base64-encoded string")] - [ValidateNotNullOrEmpty()] - [string]$UsernamesB64 -) - -# Find SRE security group -$SREGroup = Get-ADGroup -Filter "Name -eq 'Data Safe Haven SRE $SREName Users'" | Where-Object { $_.DistinguishedName -like '*Data Safe Haven Security Groups*' } | Select-Object -First 1 - -# Load usernames -$Usernames = ([Text.Encoding]::Utf8.GetString([Convert]::FromBase64String($UsernamesB64))).Split() - -# Add each user to the SRE group -if ($SREGroup -and $Usernames) { - foreach ($Username in $Usernames) { - Write-Output "INFO: Removing user '[green]$Username[/]' from group '[green]$($SREGroup.Name)[/]'." - Remove-ADGroupMember -Identity "$($SREGroup.Name)" -Members $Username -Confirm:$false - } -} From 44770d063616639053cbe5844ef7323f635c3bc3 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Mon, 8 Apr 2024 22:50:33 +0100 Subject: [PATCH 23/32] :coffin: Drop password-domain-ldap-searcher from SHM --- data_safe_haven/commands/deploy.py | 2 -- .../infrastructure/stacks/declarative_shm.py | 1 - .../infrastructure/stacks/shm/data.py | 18 ------------------ 3 files changed, 21 deletions(-) diff --git a/data_safe_haven/commands/deploy.py b/data_safe_haven/commands/deploy.py index 13a3a44ded..3dedad8d21 100644 --- a/data_safe_haven/commands/deploy.py +++ b/data_safe_haven/commands/deploy.py @@ -7,7 +7,6 @@ from data_safe_haven.config import Config, ContextSettings from data_safe_haven.exceptions import DataSafeHavenError from data_safe_haven.external import GraphApi -from data_safe_haven.functions import password from data_safe_haven.infrastructure import SHMStackManager, SREStackManager from data_safe_haven.provisioning import SHMProvisioningManager, SREProvisioningManager from data_safe_haven.utility import LoggingSingleton @@ -53,7 +52,6 @@ def shm( ) stack.add_option("azure-native:tenantId", config.azure.tenant_id, replace=False) # Add necessary secrets - stack.add_secret("password-domain-ldap-searcher", password(20), replace=False) stack.add_secret( "verification-azuread-custom-domain", verification_record, replace=False ) diff --git a/data_safe_haven/infrastructure/stacks/declarative_shm.py b/data_safe_haven/infrastructure/stacks/declarative_shm.py index 496f924e44..d0c0ff5828 100644 --- a/data_safe_haven/infrastructure/stacks/declarative_shm.py +++ b/data_safe_haven/infrastructure/stacks/declarative_shm.py @@ -81,7 +81,6 @@ def run(self) -> None: admin_group_id=self.cfg.azure.admin_group_id, admin_ip_addresses=self.cfg.shm.admin_ip_addresses, location=self.cfg.azure.location, - pulumi_opts=self.pulumi_opts, tenant_id=self.cfg.azure.tenant_id, ), tags=self.cfg.tags.model_dump(), diff --git a/data_safe_haven/infrastructure/stacks/shm/data.py b/data_safe_haven/infrastructure/stacks/shm/data.py index cf0b1d1276..e465af948e 100644 --- a/data_safe_haven/infrastructure/stacks/shm/data.py +++ b/data_safe_haven/infrastructure/stacks/shm/data.py @@ -18,15 +18,11 @@ def __init__( admin_group_id: Input[str], admin_ip_addresses: Input[Sequence[str]], location: Input[str], - pulumi_opts: Config, tenant_id: Input[str], ) -> None: self.admin_group_id = admin_group_id self.admin_ip_addresses = admin_ip_addresses self.location = location - self.password_domain_searcher = self.get_secret( - pulumi_opts, "password-domain-ldap-searcher" - ) self.tenant_id = tenant_id def get_secret(self, pulumi_opts: Config, secret_name: str) -> Output[str]: @@ -191,19 +187,6 @@ def __init__( tags=child_tags, ) - # Add other Pulumi secrets to key vault - keyvault.Secret( - f"{self._name}_kvs_password_domain_searcher", - properties=keyvault.SecretPropertiesArgs( - value=props.password_domain_searcher - ), - resource_group_name=resource_group.name, - secret_name="password-domain-ldap-searcher", - vault_name=key_vault.name, - opts=ResourceOptions.merge(child_opts, ResourceOptions(parent=key_vault)), - tags=child_tags, - ) - # Deploy persistent data account storage_account_persistent_data = storage.StorageAccount( f"{self._name}_storage_account_persistent_data", @@ -264,7 +247,6 @@ def __init__( self.password_domain_azure_ad_connect = Output.secret( password_domain_azure_ad_connect.result ) - self.password_domain_searcher = Output.secret(props.password_domain_searcher) self.password_update_server_linux_admin = Output.secret( password_update_server_linux_admin.result ) From c84ccf2678df1263b47a0f15d6d0246ac8e907a6 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Tue, 9 Apr 2024 11:16:13 +0100 Subject: [PATCH 24/32] :coffin: Remove remaining SRE dependencies on SHM domain controller resource outputs --- data_safe_haven/commands/deploy.py | 15 --------- .../infrastructure/stacks/declarative_sre.py | 3 -- .../stacks/sre/hedgedoc_server.py | 4 +-- .../stacks/sre/user_services.py | 3 -- .../provisioning/sre_provisioning_manager.py | 31 +++++++------------ 5 files changed, 12 insertions(+), 44 deletions(-) diff --git a/data_safe_haven/commands/deploy.py b/data_safe_haven/commands/deploy.py index 3dedad8d21..5df156303f 100644 --- a/data_safe_haven/commands/deploy.py +++ b/data_safe_haven/commands/deploy.py @@ -133,21 +133,6 @@ def sre( ) stack.add_option("azure-native:tenantId", config.azure.tenant_id, replace=False) # Load SHM stack outputs - stack.add_option( - "shm-domain_controllers-ldap_root_dn", - shm_stack.output("domain_controllers")["ldap_root_dn"], - replace=True, - ) - stack.add_option( - "shm-domain_controllers-ldap_server_ip", - shm_stack.output("domain_controllers")["ldap_server_ip"], - replace=True, - ) - stack.add_option( - "shm-domain_controllers-netbios_name", - shm_stack.output("domain_controllers")["netbios_name"], - replace=True, - ) stack.add_option( "shm-firewall-private-ip-address", shm_stack.output("firewall")["private_ip_address"], diff --git a/data_safe_haven/infrastructure/stacks/declarative_sre.py b/data_safe_haven/infrastructure/stacks/declarative_sre.py index ba7e22b266..2c3715d62f 100644 --- a/data_safe_haven/infrastructure/stacks/declarative_sre.py +++ b/data_safe_haven/infrastructure/stacks/declarative_sre.py @@ -305,9 +305,6 @@ def run(self) -> None: databases=self.cfg.sre(self.sre_name).databases, dns_resource_group_name=dns.resource_group.name, dns_server_ip=dns.ip_address, - domain_netbios_name=self.pulumi_opts.require( - "shm-domain_controllers-netbios_name" - ), gitea_database_password=data.password_gitea_database_admin, hedgedoc_database_password=data.password_hedgedoc_database_admin, ldap_server_ip=identity.ip_address, diff --git a/data_safe_haven/infrastructure/stacks/sre/hedgedoc_server.py b/data_safe_haven/infrastructure/stacks/sre/hedgedoc_server.py index 42dcb0869a..836b85407c 100644 --- a/data_safe_haven/infrastructure/stacks/sre/hedgedoc_server.py +++ b/data_safe_haven/infrastructure/stacks/sre/hedgedoc_server.py @@ -29,7 +29,6 @@ def __init__( database_subnet_id: Input[str], dns_resource_group_name: Input[str], dns_server_ip: Input[str], - domain_netbios_name: Input[str], ldap_server_ip: Input[str], ldap_server_port: Input[int], ldap_user_filter: Input[str], @@ -53,7 +52,6 @@ def __init__( ) self.dns_resource_group_name = dns_resource_group_name self.dns_server_ip = dns_server_ip - self.domain_netbios_name = domain_netbios_name self.ldap_server_ip = ldap_server_ip self.ldap_server_port = Output.from_input(ldap_server_port).apply(str) self.ldap_user_filter = ldap_user_filter @@ -206,7 +204,7 @@ def __init__( ), containerinstance.EnvironmentVariableArgs( name="CMD_LDAP_PROVIDERNAME", - value=props.domain_netbios_name, + value="Data Safe Haven", ), containerinstance.EnvironmentVariableArgs( name="CMD_LDAP_SEARCHBASE", diff --git a/data_safe_haven/infrastructure/stacks/sre/user_services.py b/data_safe_haven/infrastructure/stacks/sre/user_services.py index 0dd0364967..d54adf65b3 100644 --- a/data_safe_haven/infrastructure/stacks/sre/user_services.py +++ b/data_safe_haven/infrastructure/stacks/sre/user_services.py @@ -24,7 +24,6 @@ def __init__( databases: list[DatabaseSystem], # this must *not* be passed as an Input[T] dns_resource_group_name: Input[str], dns_server_ip: Input[str], - domain_netbios_name: Input[str], gitea_database_password: Input[str], hedgedoc_database_password: Input[str], ldap_server_ip: Input[str], @@ -50,7 +49,6 @@ def __init__( self.databases = databases self.dns_resource_group_name = dns_resource_group_name self.dns_server_ip = dns_server_ip - self.domain_netbios_name = domain_netbios_name self.gitea_database_password = gitea_database_password self.hedgedoc_database_password = hedgedoc_database_password self.ldap_server_ip = ldap_server_ip @@ -143,7 +141,6 @@ def __init__( database_subnet_id=props.subnet_containers_support_id, dns_resource_group_name=props.dns_resource_group_name, dns_server_ip=props.dns_server_ip, - domain_netbios_name=props.domain_netbios_name, ldap_server_ip=props.ldap_server_ip, ldap_server_port=props.ldap_server_port, ldap_username_attribute=props.ldap_username_attribute, diff --git a/data_safe_haven/provisioning/sre_provisioning_manager.py b/data_safe_haven/provisioning/sre_provisioning_manager.py index e7d82806c8..ea25495ec2 100644 --- a/data_safe_haven/provisioning/sre_provisioning_manager.py +++ b/data_safe_haven/provisioning/sre_provisioning_manager.py @@ -49,22 +49,15 @@ def __init__( # Construct security group parameters self.security_group_params = { - "dn_base": shm_stack.output("domain_controllers")["ldap_root_dn"], - "resource_group_name": shm_stack.output("domain_controllers")[ - "resource_group_name" + "admin_security_group_name": sre_stack.output("ldap")[ + "admin_security_group_name" + ], + "privileged_user_security_group_name": sre_stack.output("ldap")[ + "privileged_user_security_group_name" + ], + "user_security_group_name": sre_stack.output("ldap")[ + "user_security_group_name" ], - "security_group_names": { - "admin_security_group_name": sre_stack.output("ldap")[ - "admin_security_group_name" - ], - "privileged_user_security_group_name": sre_stack.output("ldap")[ - "privileged_user_security_group_name" - ], - "user_security_group_name": sre_stack.output("ldap")[ - "user_security_group_name" - ], - }, - "vm_name": shm_stack.output("domain_controllers")["vm_name"], } # Construct VM parameters @@ -91,7 +84,7 @@ def available_vm_skus(self) -> dict[str, dict[str, Any]]: def create_security_groups(self) -> None: """Create groups in AzureAD""" - for group_name in self.security_group_params["security_group_names"].values(): + for group_name in self.security_group_params.values(): self.graph_api.create_group(group_name) def restart_remote_desktop_containers(self) -> None: @@ -129,11 +122,9 @@ def update_remote_desktop_connections(self) -> None: for vm_identifier, vm_details in self.workspaces.items() ], "system_administrator_group_name": self.security_group_params[ - "security_group_names" - ]["admin_security_group_name"], - "user_group_name": self.security_group_params["security_group_names"][ - "user_security_group_name" + "admin_security_group_name" ], + "user_group_name": self.security_group_params["user_security_group_name"], } for details in connection_data["connections"]: self.logger.info( From a6b53e91ea8dc9261ca6a57b82a6becafefd8bdf Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 11 Apr 2024 11:49:15 +0100 Subject: [PATCH 25/32] :coffin: Drop unused identity support subnet --- .../infrastructure/common/ip_ranges.py | 1 - .../infrastructure/stacks/sre/networking.py | 33 ------------------- 2 files changed, 34 deletions(-) diff --git a/data_safe_haven/infrastructure/common/ip_ranges.py b/data_safe_haven/infrastructure/common/ip_ranges.py index cb605e4dfe..f1065ea0bc 100644 --- a/data_safe_haven/infrastructure/common/ip_ranges.py +++ b/data_safe_haven/infrastructure/common/ip_ranges.py @@ -17,7 +17,6 @@ def __init__(self, index: int) -> None: self.guacamole_containers = self.vnet.next_subnet(8) self.guacamole_containers_support = self.vnet.next_subnet(8) self.identity_containers = self.vnet.next_subnet(8) - self.identity_containers_support = self.vnet.next_subnet(8) self.user_services_containers = self.vnet.next_subnet(8) self.user_services_containers_support = self.vnet.next_subnet(8) self.user_services_databases = self.vnet.next_subnet(8) diff --git a/data_safe_haven/infrastructure/stacks/sre/networking.py b/data_safe_haven/infrastructure/stacks/sre/networking.py index 7f788278a2..363973617e 100644 --- a/data_safe_haven/infrastructure/stacks/sre/networking.py +++ b/data_safe_haven/infrastructure/stacks/sre/networking.py @@ -55,9 +55,6 @@ def __init__( self.subnet_identity_containers_iprange = subnet_ranges.apply( lambda s: s.identity_containers ) - self.subnet_identity_containers_support_iprange = subnet_ranges.apply( - lambda s: s.identity_containers_support - ) self.subnet_user_services_containers_iprange = subnet_ranges.apply( lambda s: s.user_services_containers ) @@ -154,9 +151,6 @@ def __init__( subnet_identity_containers_prefix = ( props.subnet_identity_containers_iprange.apply(lambda r: str(r)) ) - subnet_identity_containers_support_prefix = ( - props.subnet_identity_containers_support_iprange.apply(lambda r: str(r)) - ) subnet_user_services_containers_prefix = ( props.subnet_user_services_containers_iprange.apply(lambda r: str(r)) ) @@ -666,17 +660,6 @@ def __init__( opts=child_opts, tags=child_tags, ) - nsg_identity_containers_support = network.NetworkSecurityGroup( - f"{self._name}_nsg_identity_containers_support", - network_security_group_name=f"{stack_name}-nsg-identity-containers-support", - resource_group_name=resource_group.name, - security_rules=[ - # Inbound - # Outbound - ], - opts=child_opts, - tags=child_tags, - ) nsg_user_services_containers = network.NetworkSecurityGroup( f"{self._name}_nsg_user_services_containers", network_security_group_name=f"{stack_name}-nsg-user-services-containers", @@ -1196,7 +1179,6 @@ def __init__( subnet_guacamole_containers_name = "GuacamoleContainersSubnet" subnet_guacamole_containers_support_name = "GuacamoleContainersSupportSubnet" subnet_identity_containers_name = "IdentityContainersSubnet" - subnet_identity_containers_support_name = "IdentityContainersSupportSubnet" subnet_user_services_containers_name = "UserServicesContainersSubnet" subnet_user_services_containers_support_name = ( "UserServicesContainersSupportSubnet" @@ -1296,16 +1278,6 @@ def __init__( ), route_table=network.RouteTableArgs(id=route_table.id), ), - # Identity containers support - network.SubnetArgs( - address_prefix=subnet_identity_containers_support_prefix, - name=subnet_identity_containers_support_name, - network_security_group=network.NetworkSecurityGroupArgs( - id=nsg_identity_containers_support.id - ), - private_endpoint_network_policies=network.VirtualNetworkPrivateEndpointNetworkPolicies.ENABLED, - route_table=network.RouteTableArgs(id=route_table.id), - ), # User services containers network.SubnetArgs( address_prefix=subnet_user_services_containers_prefix, @@ -1576,11 +1548,6 @@ def __init__( resource_group_name=resource_group.name, virtual_network_name=sre_virtual_network.name, ) - self.subnet_identity_containers_support = network.get_subnet_output( - subnet_name=subnet_identity_containers_support_name, - resource_group_name=resource_group.name, - virtual_network_name=sre_virtual_network.name, - ) self.subnet_data_private = network.get_subnet_output( subnet_name=subnet_data_private_name, resource_group_name=resource_group.name, From 5bf4bf27991a760ea8f0d526a78fdd008d462ea3 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 11 Apr 2024 14:50:46 +0100 Subject: [PATCH 26/32] :bug: Construct SRE ldap_root_dn from SHM FQDN --- data_safe_haven/infrastructure/stacks/declarative_sre.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/data_safe_haven/infrastructure/stacks/declarative_sre.py b/data_safe_haven/infrastructure/stacks/declarative_sre.py index 2c3715d62f..8e22a3dbfc 100644 --- a/data_safe_haven/infrastructure/stacks/declarative_sre.py +++ b/data_safe_haven/infrastructure/stacks/declarative_sre.py @@ -65,7 +65,7 @@ def run(self) -> None: self.pulumi_opts = pulumi.Config() # Construct LDAP paths - ldap_root_dn = self.pulumi_opts.require("shm-domain_controllers-ldap_root_dn") + ldap_root_dn = f"DC={self.cfg.shm.fqdn.replace('.', ',DC=')}" ldap_group_search_base = f"OU=groups,{ldap_root_dn}" ldap_user_search_base = f"OU=users,{ldap_root_dn}" ldap_group_name_prefix = f"Data Safe Haven SRE {self.sre_name}" From 050039c2d177708e5a2667f15f45d47a550c9358 Mon Sep 17 00:00:00 2001 From: Jim Madge Date: Tue, 16 Apr 2024 13:58:14 +0100 Subject: [PATCH 27/32] Ensure NSG rule priorities are unique --- data_safe_haven/infrastructure/common/enums.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/data_safe_haven/infrastructure/common/enums.py b/data_safe_haven/infrastructure/common/enums.py index be20353f12..c4f619c727 100644 --- a/data_safe_haven/infrastructure/common/enums.py +++ b/data_safe_haven/infrastructure/common/enums.py @@ -1,6 +1,7 @@ -from enum import Enum +from enum import UNIQUE, Enum, verify +@verify(UNIQUE) class NetworkingPriorities(int, Enum): """Priorities for network security group rules.""" From 6c204e9a87b3699756efacb1253832225c7b8a70 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Tue, 16 Apr 2024 16:39:33 +0100 Subject: [PATCH 28/32] :recycle: Simplify alphanumeric function and use in GraphAPI --- data_safe_haven/external/api/graph_api.py | 3 ++- data_safe_haven/functions/strings.py | 4 +--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/data_safe_haven/external/api/graph_api.py b/data_safe_haven/external/api/graph_api.py index 394e04867d..abf930ed04 100644 --- a/data_safe_haven/external/api/graph_api.py +++ b/data_safe_haven/external/api/graph_api.py @@ -21,6 +21,7 @@ DataSafeHavenInternalError, DataSafeHavenMicrosoftGraphError, ) +from data_safe_haven.functions import alphanumeric from data_safe_haven.utility import LoggingSingleton, NonLoggingSingleton @@ -330,7 +331,7 @@ def create_group(self, group_name: str) -> None: "displayName": group_name, "groupTypes": [], "mailEnabled": False, - "mailNickname": ("".join(filter(str.isalnum, group_name))).lower(), + "mailNickname": alphanumeric(group_name).lower(), "securityEnabled": True, } self.http_post( diff --git a/data_safe_haven/functions/strings.py b/data_safe_haven/functions/strings.py index 3a9d325281..283563cc07 100644 --- a/data_safe_haven/functions/strings.py +++ b/data_safe_haven/functions/strings.py @@ -9,9 +9,7 @@ def alphanumeric(input_string: str) -> str: """Strip any characters that are not letters or numbers from a string.""" - return "".join( - filter(lambda x: x in (string.ascii_letters + string.digits), input_string) - ) + return "".join(filter(str.isalnum, input_string)) def b64decode(input_string: str) -> str: From ef7eda24765955081d40af17d251348ac1ea8197 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Tue, 16 Apr 2024 17:16:48 +0100 Subject: [PATCH 29/32] :sparkles: Use ldap-filter to construct more readable LDAP filters --- .../infrastructure/stacks/declarative_sre.py | 86 ++++++++++--------- .../provisioning/sre_provisioning_manager.py | 16 +--- pyproject.toml | 2 + 3 files changed, 51 insertions(+), 53 deletions(-) diff --git a/data_safe_haven/infrastructure/stacks/declarative_sre.py b/data_safe_haven/infrastructure/stacks/declarative_sre.py index 8e22a3dbfc..fd7a42dd20 100644 --- a/data_safe_haven/infrastructure/stacks/declarative_sre.py +++ b/data_safe_haven/infrastructure/stacks/declarative_sre.py @@ -1,6 +1,7 @@ """Pulumi declarative program""" import pulumi +from ldap_filter import Filter from data_safe_haven.config import Config from data_safe_haven.infrastructure.common import get_subscription_id_from_rg @@ -69,39 +70,51 @@ def run(self) -> None: ldap_group_search_base = f"OU=groups,{ldap_root_dn}" ldap_user_search_base = f"OU=users,{ldap_root_dn}" ldap_group_name_prefix = f"Data Safe Haven SRE {self.sre_name}" - ldap_admin_group_name = f"{ldap_group_name_prefix} Administrators" - ldap_privileged_user_group_name = f"{ldap_group_name_prefix} Privileged Users" + ldap_group_names = { + "admin_group_name": f"{ldap_group_name_prefix} Administrators", + "privileged_user_group_name": f"{ldap_group_name_prefix} Privileged Users", + "user_group_name": f"{ldap_group_name_prefix} Users", + } ldap_username_attribute = "uid" - ldap_user_group_name = f"{ldap_group_name_prefix} Users" - # Users are a posixAccount belonging to any of these groups - ldap_user_filter = "".join( - [ - "(&", - "(objectClass=posixAccount)", - "(|", - f"(memberOf=CN={ldap_admin_group_name},{ldap_group_search_base})" - f"(memberOf=CN={ldap_privileged_user_group_name},{ldap_group_search_base})" - f"(memberOf=CN={ldap_user_group_name},{ldap_group_search_base})" - ")", - ")", - ] - ) - # Groups are a posixGroup which is either a known, named group or belonging to any of these groups - ldap_group_filter = "".join( - [ - "(&", - "(objectClass=posixGroup)", - "(|", - f"(CN={ldap_admin_group_name})", - f"(CN={ldap_privileged_user_group_name})", - f"(CN={ldap_user_group_name})", - f"(memberOf=CN=Primary user groups for {ldap_admin_group_name},{ldap_group_search_base})", - f"(memberOf=CN=Primary user groups for {ldap_privileged_user_group_name},{ldap_group_search_base})", - f"(memberOf=CN=Primary user groups for {ldap_user_group_name},{ldap_group_search_base})", - ")", - ")", - ] - ) + + # Construct an LDAP filter for users of this SRE + ldap_user_filter = Filter.AND( + ( + # Users must be a posixAccount + Filter.attribute("objectClass").equal_to("posixAccount"), + # ... that belongs to one of the LDAP groups for this SRE + Filter.OR( + [ + Filter.attribute("memberOf").equal_to( + f"CN={group_name},{ldap_group_search_base}" + ) + for group_name in ldap_group_names.values() + ] + ), + ) + ).to_string() + + # Construct an LDAP filter for groups in this SRE + ldap_group_filter = Filter.AND( + ( + # Groups must be a posixGroup + Filter.attribute("objectClass").equal_to("posixGroup"), + Filter.OR( + # ... that is one of the LDAP groups for this SRE + [ + Filter.attribute("CN").equal_to(group_name) + for group_name in ldap_group_names.values() + ] + # ... or is the primary user group for a member of one of those groups + + [ + Filter.attribute("memberOf").equal_to( + f"CN=Primary user groups for {group_name},{ldap_group_search_base}" + ) + for group_name in ldap_group_names.values() + ] + ), + ) + ).to_string() # Deploy SRE DNS server dns = SREDnsServerComponent( @@ -343,13 +356,6 @@ def run(self) -> None: # Export values for later use pulumi.export("data", data.exports) - pulumi.export( - "ldap", - { - "admin_security_group_name": ldap_admin_group_name, - "privileged_user_security_group_name": ldap_privileged_user_group_name, - "user_security_group_name": ldap_user_group_name, - }, - ) + pulumi.export("ldap", ldap_group_names) pulumi.export("remote_desktop", remote_desktop.exports) pulumi.export("workspaces", workspaces.exports) diff --git a/data_safe_haven/provisioning/sre_provisioning_manager.py b/data_safe_haven/provisioning/sre_provisioning_manager.py index ea25495ec2..68884808fa 100644 --- a/data_safe_haven/provisioning/sre_provisioning_manager.py +++ b/data_safe_haven/provisioning/sre_provisioning_manager.py @@ -48,17 +48,7 @@ def __init__( self.remote_desktop_params["timezone"] = timezone # Construct security group parameters - self.security_group_params = { - "admin_security_group_name": sre_stack.output("ldap")[ - "admin_security_group_name" - ], - "privileged_user_security_group_name": sre_stack.output("ldap")[ - "privileged_user_security_group_name" - ], - "user_security_group_name": sre_stack.output("ldap")[ - "user_security_group_name" - ], - } + self.security_group_params = dict(sre_stack.output("ldap")) # Construct VM parameters self.workspaces = {} @@ -122,9 +112,9 @@ def update_remote_desktop_connections(self) -> None: for vm_identifier, vm_details in self.workspaces.items() ], "system_administrator_group_name": self.security_group_params[ - "admin_security_group_name" + "admin_group_name" ], - "user_group_name": self.security_group_params["user_security_group_name"], + "user_group_name": self.security_group_params["user_group_name"], } for details in connection_data["connections"]: self.logger.info( diff --git a/pyproject.toml b/pyproject.toml index 9deec940bb..fdd17d9bd5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -36,6 +36,7 @@ dependencies = [ "cryptography~=42.0", "dnspython~=2.3", "fqdn~=1.5", + "ldap-filter~=0.2", "msal~=1.21", "psycopg~=3.1", "pulumi~=3.80", @@ -169,6 +170,7 @@ module = [ "chili.*", "cryptography.*", "dns.*", + "ldap_filter.*", "msal.*", "numpy.*", "pandas.*", From ca5ba6c5663f4085f2aee9ae67f92668c02217d5 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Tue, 16 Apr 2024 17:31:52 +0100 Subject: [PATCH 30/32] :sparkles: Add firewall priority enum --- .../infrastructure/common/__init__.py | 3 ++- .../infrastructure/common/enums.py | 16 +++++++++++++ .../infrastructure/stacks/shm/firewall.py | 24 +++++++++++-------- 3 files changed, 32 insertions(+), 11 deletions(-) diff --git a/data_safe_haven/infrastructure/common/__init__.py b/data_safe_haven/infrastructure/common/__init__.py index a8db61e96f..a8c8f4e650 100644 --- a/data_safe_haven/infrastructure/common/__init__.py +++ b/data_safe_haven/infrastructure/common/__init__.py @@ -1,4 +1,4 @@ -from .enums import NetworkingPriorities +from .enums import FirewallPriorities, NetworkingPriorities from .ip_ranges import SREDnsIpRanges, SREIpRanges from .transformations import ( get_available_ips_from_subnet, @@ -14,6 +14,7 @@ ) __all__ = [ + "FirewallPriorities", "get_available_ips_from_subnet", "get_id_from_rg", "get_id_from_subnet", diff --git a/data_safe_haven/infrastructure/common/enums.py b/data_safe_haven/infrastructure/common/enums.py index c4f619c727..8d0373e656 100644 --- a/data_safe_haven/infrastructure/common/enums.py +++ b/data_safe_haven/infrastructure/common/enums.py @@ -1,6 +1,22 @@ from enum import UNIQUE, Enum, verify +@verify(UNIQUE) +class FirewallPriorities(int, Enum): + """Priorities for firewall rules.""" + + # All sources: 1000-1099 + ALL = 1000 + # SHM sources: 2000-2999 + SHM_IDENTITY_SERVERS = 2000 + SHM_UPDATE_SERVERS = 2100 + # SRE sources: 3000-3999 + SRE_GUACAMOLE_CONTAINERS = 3000 + SRE_IDENTITY_CONTAINERS = 3100 + SRE_USER_SERVICES_SOFTWARE_REPOSITORIES = 3200 + SRE_WORKSPACES = 3300 + + @verify(UNIQUE) class NetworkingPriorities(int, Enum): """Priorities for network security group rules.""" diff --git a/data_safe_haven/infrastructure/stacks/shm/firewall.py b/data_safe_haven/infrastructure/stacks/shm/firewall.py index 92f520e5e7..2ba2476056 100644 --- a/data_safe_haven/infrastructure/stacks/shm/firewall.py +++ b/data_safe_haven/infrastructure/stacks/shm/firewall.py @@ -5,7 +5,11 @@ from pulumi import ComponentResource, Input, Output, ResourceOptions from pulumi_azure_native import network -from data_safe_haven.infrastructure.common import SREIpRanges, get_id_from_subnet +from data_safe_haven.infrastructure.common import ( + FirewallPriorities, + SREIpRanges, + get_id_from_subnet, +) class SHMFirewallProps: @@ -106,7 +110,7 @@ def __init__( network.AzureFirewallApplicationRuleCollectionArgs( action=network.AzureFirewallRCActionArgs(type="Allow"), name=f"{stack_name}-identity-servers", - priority=1000, + priority=FirewallPriorities.SHM_IDENTITY_SERVERS, rules=[ network.AzureFirewallApplicationRuleArgs( description="Allow external operational requests from AzureAD Connect", @@ -311,7 +315,7 @@ def __init__( network.AzureFirewallApplicationRuleCollectionArgs( action=network.AzureFirewallRCActionArgs(type="Allow"), name=f"{stack_name}-any", - priority=1010, + priority=FirewallPriorities.ALL, rules=[ network.AzureFirewallApplicationRuleArgs( description="Allow external Azure Automation requests", @@ -344,7 +348,7 @@ def __init__( network.AzureFirewallApplicationRuleCollectionArgs( action=network.AzureFirewallRCActionArgs(type="Allow"), name=f"{stack_name}-update-servers", - priority=1020, + priority=FirewallPriorities.SHM_UPDATE_SERVERS, rules=[ network.AzureFirewallApplicationRuleArgs( description="Allow external Linux update requests", @@ -380,7 +384,7 @@ def __init__( network.AzureFirewallApplicationRuleCollectionArgs( action=network.AzureFirewallRCActionArgs(type="Allow"), name=f"{stack_name}-sre-identity-servers", - priority=1100, + priority=FirewallPriorities.SRE_IDENTITY_CONTAINERS, rules=[ network.AzureFirewallApplicationRuleArgs( description="Allow external OAuth login requests", @@ -402,7 +406,7 @@ def __init__( network.AzureFirewallApplicationRuleCollectionArgs( action=network.AzureFirewallRCActionArgs(type="Allow"), name=f"{stack_name}-sre-package-repositories", - priority=1110, + priority=FirewallPriorities.SRE_USER_SERVICES_SOFTWARE_REPOSITORIES, rules=[ network.AzureFirewallApplicationRuleArgs( description="Allow external CRAN package requests", @@ -433,7 +437,7 @@ def __init__( network.AzureFirewallApplicationRuleCollectionArgs( action=network.AzureFirewallRCActionArgs(type="Allow"), name=f"{stack_name}-sre-remote-desktop-gateways", - priority=1120, + priority=FirewallPriorities.SRE_GUACAMOLE_CONTAINERS, rules=[ network.AzureFirewallApplicationRuleArgs( description="Allow external OAuth login requests", @@ -452,7 +456,7 @@ def __init__( network.AzureFirewallApplicationRuleCollectionArgs( action=network.AzureFirewallRCActionArgs(type="Allow"), name=f"{stack_name}-sre-workspaces", - priority=1130, + priority=FirewallPriorities.SRE_WORKSPACES, rules=[ network.AzureFirewallApplicationRuleArgs( description="Allow external Linux ClamAV update requests", @@ -504,7 +508,7 @@ def __init__( network.AzureFirewallNetworkRuleCollectionArgs( action=network.AzureFirewallRCActionArgs(type="Allow"), name=f"{stack_name}-identity-servers", - priority=1000, + priority=FirewallPriorities.SHM_IDENTITY_SERVERS, rules=[ network.AzureFirewallNetworkRuleArgs( description="Allow external DNS resolver", @@ -522,7 +526,7 @@ def __init__( network.AzureFirewallNetworkRuleCollectionArgs( action=network.AzureFirewallRCActionArgs(type="Allow"), name=f"{stack_name}-all", - priority=1010, + priority=FirewallPriorities.ALL, rules=[ network.AzureFirewallNetworkRuleArgs( description="Allow external Azure Automation requests", From 8646b651cca7bee4d6e307daff089e7acd272169 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Tue, 16 Apr 2024 17:33:47 +0100 Subject: [PATCH 31/32] :coffin: Remove unused port 389 for SRE identity servers --- .../infrastructure/stacks/sre/networking.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/data_safe_haven/infrastructure/stacks/sre/networking.py b/data_safe_haven/infrastructure/stacks/sre/networking.py index 363973617e..49ac113875 100644 --- a/data_safe_haven/infrastructure/stacks/sre/networking.py +++ b/data_safe_haven/infrastructure/stacks/sre/networking.py @@ -506,7 +506,7 @@ def __init__( access=network.SecurityRuleAccess.ALLOW, description="Allow LDAP client requests over TCP.", destination_address_prefix=subnet_identity_containers_prefix, - destination_port_ranges=["389", "1389"], + destination_port_ranges=["1389"], direction=network.SecurityRuleDirection.OUTBOUND, name="AllowIdentityServersOutbound", priority=NetworkingPriorities.INTERNAL_SRE_IDENTITY_CONTAINERS, @@ -623,7 +623,7 @@ def __init__( access=network.SecurityRuleAccess.ALLOW, description="Allow LDAP client requests from Guacamole over TCP.", destination_address_prefix=subnet_identity_containers_prefix, - destination_port_ranges=["389", "1389"], + destination_port_ranges=["1389"], direction=network.SecurityRuleDirection.INBOUND, name="AllowGuacamoleLDAPClientTCPInbound", priority=NetworkingPriorities.INTERNAL_SRE_GUACAMOLE_CONTAINERS, @@ -635,7 +635,7 @@ def __init__( access=network.SecurityRuleAccess.ALLOW, description="Allow LDAP client requests from user services over TCP.", destination_address_prefix=subnet_identity_containers_prefix, - destination_port_ranges=["389", "1389"], + destination_port_ranges=["1389"], direction=network.SecurityRuleDirection.INBOUND, name="AllowUserServicesLDAPClientTCPInbound", priority=NetworkingPriorities.INTERNAL_SRE_USER_SERVICES_CONTAINERS, @@ -647,7 +647,7 @@ def __init__( access=network.SecurityRuleAccess.ALLOW, description="Allow LDAP client requests from workspaces over TCP.", destination_address_prefix=subnet_identity_containers_prefix, - destination_port_ranges=["389", "1389"], + destination_port_ranges=["1389"], direction=network.SecurityRuleDirection.INBOUND, name="AllowWorkspaceLDAPClientTCPInbound", priority=NetworkingPriorities.INTERNAL_SRE_WORKSPACES, @@ -731,7 +731,7 @@ def __init__( access=network.SecurityRuleAccess.ALLOW, description="Allow LDAP client requests over TCP.", destination_address_prefix=subnet_identity_containers_prefix, - destination_port_ranges=["389", "1389"], + destination_port_ranges=["1389"], direction=network.SecurityRuleDirection.OUTBOUND, name="AllowIdentityServersOutbound", priority=NetworkingPriorities.INTERNAL_SRE_IDENTITY_CONTAINERS, @@ -1051,7 +1051,7 @@ def __init__( access=network.SecurityRuleAccess.ALLOW, description="Allow LDAP client requests over TCP.", destination_address_prefix=subnet_identity_containers_prefix, - destination_port_ranges=["389", "1389"], + destination_port_ranges=["1389"], direction=network.SecurityRuleDirection.OUTBOUND, name="AllowIdentityServersOutbound", priority=NetworkingPriorities.INTERNAL_SRE_IDENTITY_CONTAINERS, From 54d85916a4bb33fd7b529fde0ceb65380aaf007b Mon Sep 17 00:00:00 2001 From: Jim Madge Date: Wed, 17 Apr 2024 10:41:34 +0100 Subject: [PATCH 32/32] Revert some changes of ef7eda247 --- .../infrastructure/stacks/declarative_sre.py | 71 +++++++++---------- pyproject.toml | 2 - 2 files changed, 33 insertions(+), 40 deletions(-) diff --git a/data_safe_haven/infrastructure/stacks/declarative_sre.py b/data_safe_haven/infrastructure/stacks/declarative_sre.py index fd7a42dd20..ea4f937dc9 100644 --- a/data_safe_haven/infrastructure/stacks/declarative_sre.py +++ b/data_safe_haven/infrastructure/stacks/declarative_sre.py @@ -1,7 +1,6 @@ """Pulumi declarative program""" import pulumi -from ldap_filter import Filter from data_safe_haven.config import Config from data_safe_haven.infrastructure.common import get_subscription_id_from_rg @@ -76,45 +75,41 @@ def run(self) -> None: "user_group_name": f"{ldap_group_name_prefix} Users", } ldap_username_attribute = "uid" - - # Construct an LDAP filter for users of this SRE - ldap_user_filter = Filter.AND( - ( - # Users must be a posixAccount - Filter.attribute("objectClass").equal_to("posixAccount"), - # ... that belongs to one of the LDAP groups for this SRE - Filter.OR( - [ - Filter.attribute("memberOf").equal_to( - f"CN={group_name},{ldap_group_search_base}" - ) - for group_name in ldap_group_names.values() - ] + # LDAP filter syntax: https://ldap.com/ldap-filters/ + # LDAP filter for users of this SRE + ldap_user_filter = "".join( + [ + "(&", + # Users are a posixAccount and + "(objectClass=posixAccount)", + # belong to any of these groups + "(|", + *( + f"(memberOf=CN={group_name},{ldap_group_search_base})" + for group_name in ldap_group_names.values() ), - ) - ).to_string() - - # Construct an LDAP filter for groups in this SRE - ldap_group_filter = Filter.AND( - ( - # Groups must be a posixGroup - Filter.attribute("objectClass").equal_to("posixGroup"), - Filter.OR( - # ... that is one of the LDAP groups for this SRE - [ - Filter.attribute("CN").equal_to(group_name) - for group_name in ldap_group_names.values() - ] - # ... or is the primary user group for a member of one of those groups - + [ - Filter.attribute("memberOf").equal_to( - f"CN=Primary user groups for {group_name},{ldap_group_search_base}" - ) - for group_name in ldap_group_names.values() - ] + ")", + ")", + ] + ) + # LDAP filter for groups in this SRE + ldap_group_filter = "".join( + [ + "(&", + # Groups are a posixGroup + "(objectClass=posixGroup)", + "(|", + # which is either one of the LDAP groups + *(f"(CN={group_name})" for group_name in ldap_group_names.values()), + # or is the primary user group for a member of one of those groups + *( + f"(memberOf=CN=Primary user groups for {group_name},{ldap_group_search_base})" + for group_name in ldap_group_names.values() ), - ) - ).to_string() + ")", + ")", + ] + ) # Deploy SRE DNS server dns = SREDnsServerComponent( diff --git a/pyproject.toml b/pyproject.toml index fdd17d9bd5..9deec940bb 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -36,7 +36,6 @@ dependencies = [ "cryptography~=42.0", "dnspython~=2.3", "fqdn~=1.5", - "ldap-filter~=0.2", "msal~=1.21", "psycopg~=3.1", "pulumi~=3.80", @@ -170,7 +169,6 @@ module = [ "chili.*", "cryptography.*", "dns.*", - "ldap_filter.*", "msal.*", "numpy.*", "pandas.*",