Merge pull request #1772 from jemrobinson/1570-replace-domain-control…

…ler-with-apricot

Use Apricot for authentication/identity

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:

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()
@@ -34,7 +32,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 +49,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 +61,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 +71,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
@@ -79,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:
@@ -134,7 +130,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 +143,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 +189,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 +214,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

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

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"],

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:

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