From c1d492d877b1ac62c86bf7cc2a35e3da625ddd24 Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Mon, 24 Mar 2025 12:51:02 +0000
Subject: [PATCH 01/44] [minor] initial drop of user utils + script for saas

https://jsw.ibm.com/browse/MASCORE-6072
---
 bin/mas-devops-create-initial-users-for-saas |  84 ++
 setup.py                                     |   3 +-
 src/mas/devops/users.py                      | 790 +++++++++++++++++++
 3 files changed, 876 insertions(+), 1 deletion(-)
 create mode 100644 bin/mas-devops-create-initial-users-for-saas
 create mode 100644 src/mas/devops/users.py

diff --git a/bin/mas-devops-create-initial-users-for-saas b/bin/mas-devops-create-initial-users-for-saas
new file mode 100644
index 00000000..ec0f1097
--- /dev/null
+++ b/bin/mas-devops-create-initial-users-for-saas
@@ -0,0 +1,84 @@
+#!/usr/bin/env python3
+
+# *****************************************************************************
+# Copyright (c) 2024 IBM Corporation and other Contributors.
+#
+# All rights reserved. This program and the accompanying materials
+# are made available under the terms of the Eclipse Public License v1.0
+# which accompanies this distribution, and is available at
+# http://www.eclipse.org/legal/epl-v10.html
+#
+# *****************************************************************************
+
+from kubernetes import client, config
+from kubernetes.config.config_exception import ConfigException
+import argparse
+import logging
+import urllib3
+urllib3.disable_warnings()
+import os
+import sys
+import json
+import re
+import yaml
+
+from mas.devops.users import MASUserUtils
+
+
+
+if __name__ == "__main__":
+    parser = argparse.ArgumentParser()
+
+    # Primary Options
+    parser.add_argument("--mas-instance-id", required=True)
+    parser.add_argument("--mas-workspace-id", required=True)
+    parser.add_argument("--initial-users-yaml-file", required=True)
+    parser.add_argument("--log-level", required=False, choices=["DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL"], default="WARNING")
+    parser.add_argument("--dry-run", required=False, help="When specified, nothing will actually be deleted from the cluster", action="store_true")
+
+    args, unknown = parser.parse_known_args()
+
+    log_level = getattr(logging, args.log_level)
+
+    logger = logging.getLogger()
+    logger.setLevel(log_level)
+
+    ch = logging.StreamHandler()
+    ch.setLevel(log_level)
+    chFormatter = logging.Formatter(
+        "%(asctime)-25s %(name)-50s [%(threadName)s] %(levelname)-8s %(message)s"
+    )
+    ch.setFormatter(chFormatter)
+    logger.addHandler(ch)
+
+
+    mas_instance_id = args.mas_instance_id
+    mas_workspace_id = args.mas_workspace_id
+    initial_users_yaml_file = args.initial_users_yaml_file
+    dry_run = args.dry_run
+
+    logger.info("Configuration:")
+    logger.info("--------------")
+    logger.info(f"mas_instance_id:         {mas_instance_id}")
+    logger.info(f"mas_workspace_id:        {mas_workspace_id}")
+    logger.info(f"initial_users_yaml_file: {initial_users_yaml_file}")
+    logger.info(f"log_level:               {log_level}")
+    logger.info(f"dry_run:                 {dry_run}")
+    logger.info("")
+
+    try:
+        # Try to load in-cluster configuration
+        config.load_incluster_config()
+        logger.info("Loaded in-cluster configuration")
+    except ConfigException:
+        # If that fails, fall back to kubeconfig file
+        config.load_kube_config()
+        logger.info("Loaded kubeconfig file")
+
+    user_utils = MASUserUtils(mas_instance_id, mas_workspace_id, client.api_client.ApiClient())
+
+    with open(initial_users_yaml_file, 'r') as file:
+       initial_users_yaml = yaml.safe_load(file)
+
+    user_utils.create_initial_users_for_saas(initial_users_yaml)
+
diff --git a/setup.py b/setup.py
index a0643b2f..4818aaf5 100644
--- a/setup.py
+++ b/setup.py
@@ -84,6 +84,7 @@ def get_version(rel_path):
         'Topic :: Software Development :: Libraries :: Python Modules'
     ],
     scripts=[
-        'bin/mas-devops-db2-validate-config'
+        'bin/mas-devops-db2-validate-config',
+        'bin/mas-devops-create-initial-users-for-saas'
     ]
 )
diff --git a/src/mas/devops/users.py b/src/mas/devops/users.py
new file mode 100644
index 00000000..5018fc89
--- /dev/null
+++ b/src/mas/devops/users.py
@@ -0,0 +1,790 @@
+# *****************************************************************************
+# Copyright (c) 2025 IBM Corporation and other Contributors.
+#
+# All rights reserved. This program and the accompanying materials
+# are made available under the terms of the Eclipse Public License v1.0
+# which accompanies this distribution, and is available at
+# http://www.eclipse.org/legal/epl-v10.html
+#
+# *****************************************************************************
+
+import requests
+import logging
+from kubernetes import client
+from openshift.dynamic import DynamicClient
+import base64
+import atexit
+import certifi
+import tempfile
+import os
+import time
+import re
+
+
+'''
+TODO:
+    idempotency
+    handle user deletion if removed from config?
+    MVI / other apps?
+    unit tests
+
+    where are we going to run this from? Needs to run in cluster, so a Job in an app
+    which app though? Manage?
+    Perhaps we do the core bits in a core app (suite workspace?)
+    And app-specific bits in the apps themselves
+    but if we create a user before manage is ready, sync fails, and no way to trigger resync in MAS < 9.1.0?
+
+    # could do it all from a core app, but check for readiness of apps
+
+'''
+
+
+class MASUserUtils():
+
+    MAXADMIN = "MAXADMIN"
+
+    def __init__(self, mas_instance_id: str, mas_workspace_id: str, k8s_client: client.api_client.ApiClient):
+        self.mas_instance_id = mas_instance_id
+        self.mas_workspace_id = mas_workspace_id
+        self.k8s_client = k8s_client
+        self.logger = logging.getLogger(f"{__name__}.{self.__class__.__name__}")
+
+        self.mas_core_namespace = f"mas-{self.mas_instance_id}-core"
+        self.manage_namespace = f"mas-{self.mas_instance_id}-manage"
+        self.dyn_client = DynamicClient(self.k8s_client)
+        self.v1_routes = self.dyn_client.resources.get(api_version="route.openshift.io/v1", kind="Route")
+        self.v1_secrets = self.dyn_client.resources.get(api_version="v1", kind="Secret")
+
+        self._mas_superuser_credentials = None
+
+        self._mas_admin_route = None
+        self._mas_admin_url = None
+        self._mas_admin_url_ca_chain_file_path = None
+
+        self._mas_api_route = None
+        self._mas_api_url = None
+        self._mas_api_url_ca_chain_file_path = None
+
+        self._superuser_auth_token = None
+
+        self._manage_api_route = None
+        self._manage_api_url_internal = None
+        self._manage_internal_tls_secret = None
+        self._manage_internal_ca_pem_file_path = None
+        self._manage_internal_client_pem_file_path = None
+
+        self._manage_maxadmin_api_key = None
+
+    @staticmethod
+    def ca_chain_file_path_from_route(route):
+        # TODO: this just "happens" to work for the MAS api and admin routes (in no frontend, manual_cert_mgmt: false mode)
+        #       should try and come up with a more robust approach here (see ntoes for manage API urls below)
+        # maybe switch to calling via internal service?
+        try:
+            # This may include CA certificates that we need in order to trust the certificates presented by the endpoint
+            cert = route["spec"]["tls"]["certificate"]
+        except KeyError:
+            pass
+        try:
+            # This may include CA certificates that we need in order to trust the certificates presented by the endpoint
+            ca = route["spec"]["tls"]["caCertificate"]
+        except KeyError:
+            pass
+        try:
+            # This may include CA certificates that we need in order to trust the certificates presented by the endpoint
+            dest_ca = route["spec"]["tls"]["destinationCACertificate"]
+        except KeyError:
+            pass
+
+        # Load default CA bundle. This will include certs for well-known CAs. This ensures that we will
+        # trust the certificates presented by the endpoint when MAS is configured to use
+        # an external frontend like CIS.
+        with open(certifi.where(), 'rb') as default_ca:
+            default_ca_content = default_ca.read()
+
+        with tempfile.NamedTemporaryFile(delete=False) as chain_file:
+            if cert:
+                chain_file.write(cert.encode())
+            if ca:
+                chain_file.write(ca.encode())
+            if dest_ca:
+                chain_file.write(dest_ca.encode())
+
+            chain_file.write(default_ca_content)
+
+            chain_file.flush()
+            chain_file.close()
+            atexit.register(os.remove, chain_file.name)
+
+            return chain_file.name
+
+    @property
+    def mas_superuser_credentials(self):
+        if self._mas_superuser_credentials is None:
+            k8s_secret = self.v1_secrets.get(name=f"{self.mas_instance_id}-credentials-superuser", namespace=self.mas_core_namespace)
+            self._mas_superuser_credentials = dict(
+                username=base64.b64decode(str(k8s_secret.data.username)).decode("utf-8"),
+                password=base64.b64decode(str(k8s_secret.data.password)).decode("utf-8"),
+            )
+        return self._mas_superuser_credentials
+
+    @property
+    def mas_admin_route(self):
+        if self._mas_admin_route is None:
+            self._mas_admin_route = self.v1_routes.get(name=f"{self.mas_instance_id}-admin", namespace=self.mas_core_namespace)
+        return self._mas_admin_route
+
+    @property
+    def mas_admin_url(self):
+        if self._mas_admin_url is None:
+            self._mas_admin_url = f"https://{self.mas_admin_route.spec.host}{self.mas_admin_route.spec.path}"
+        return self._mas_admin_url
+
+    @property
+    def mas_admin_url_ca_chain_file_path(self):
+        if self._mas_admin_url_ca_chain_file_path is None:
+            self._mas_admin_url_ca_chain_file_path = MASUserUtils.ca_chain_file_path_from_route(self.mas_admin_route)
+        return self._mas_admin_url_ca_chain_file_path
+
+    @property
+    def mas_api_route(self):
+        if self._mas_api_route is None:
+            self._mas_api_route = self.v1_routes.get(name=f"{self.mas_instance_id}-api", namespace=self.mas_core_namespace)
+        return self._mas_api_route
+
+    @property
+    def mas_api_url(self):
+        if self._mas_api_url is None:
+            self._mas_api_url = f"https://{self.mas_api_route.spec.host}"
+        return self._mas_api_url
+
+    @property
+    def mas_api_url_ca_chain_file_path(self):
+        if self._mas_api_url_ca_chain_file_path is None:
+            self._mas_api_url_ca_chain_file_path = MASUserUtils.ca_chain_file_path_from_route(self.mas_api_route)
+        return self._mas_api_url_ca_chain_file_path
+
+    @property
+    def manage_api_route(self):
+        if self._manage_api_route is None:
+            self._manage_api_route = self.v1_routes.get(name=f"{self.mas_instance_id}-manage-{self.mas_workspace_id}", namespace=self.manage_namespace)
+        return self._manage_api_route
+
+    @property
+    def manage_api_url_internal(self):
+        if self._manage_api_url_internal is None:
+            self.logger.info("Getting Manage internal API URL")
+            # for local testing:
+            # add to /etc/hosts:
+            #    127.0.0.1               tgk01-masdev.mas-tgk01-manage.svc.cluster.local
+
+            # oc port-forward service/tgk01-masdev 8443:443 -n mas-tgk01-manage
+
+            self._manage_api_url_internal = f'https://{self.mas_instance_id}-{self.mas_workspace_id}.{self.manage_namespace}.svc.cluster.local'
+
+            # uncomment for local testing
+            self._manage_api_url_internal = f"{self._manage_api_url_internal}:8443"
+        return self._manage_api_url_internal
+
+    @property
+    def superuser_auth_token(self):
+        if self._superuser_auth_token is None:
+            self.logger.info("Getting superuser auth token")
+            url = f"{self.mas_admin_url}/logininitial"
+            headers = {
+                "Content-Type": "application/json"
+            }
+            querystring = {
+                "verify": False
+            }
+            payload = self.mas_superuser_credentials
+            response = requests.post(
+                url,
+                json=payload,
+                headers=headers,
+                params=querystring,
+                verify=self.mas_admin_url_ca_chain_file_path
+            )
+            self._superuser_auth_token = response.json()["token"]
+        return self._superuser_auth_token
+
+    @property
+    def manage_internal_tls_secret(self):
+        if self._manage_internal_tls_secret is None:
+            self._manage_internal_tls_secret = self.v1_secrets.get(name=f"{self.mas_instance_id}-internal-manage-tls", namespace=self.manage_namespace)
+        return self._manage_internal_tls_secret
+
+    @property
+    def manage_internal_client_pem_file_path(self):
+        if self._manage_internal_client_pem_file_path is None:
+            cert = base64.b64decode(self.manage_internal_tls_secret.data["tls.crt"]).decode('utf-8')
+            key = base64.b64decode(self.manage_internal_tls_secret.data["tls.key"]).decode('utf-8')
+            with tempfile.NamedTemporaryFile(delete=False, suffix=".pem") as pem_file:
+                pem_file.write(key.encode())
+                pem_file.write(cert.encode())
+                pem_file.flush()
+                pem_file.close()
+                atexit.register(os.remove, pem_file.name)
+                self._manage_internal_client_pem_file_path = pem_file.name
+        return self._manage_internal_client_pem_file_path
+
+    @property
+    def manage_internal_ca_pem_file_path(self):
+        if self._manage_internal_ca_pem_file_path is None:
+            ca = base64.b64decode(self.manage_internal_tls_secret.data["ca.crt"]).decode('utf-8')
+            with tempfile.NamedTemporaryFile(delete=False, suffix=".pem") as pem_file:
+                pem_file.write(ca.encode())
+                pem_file.flush()
+                pem_file.close()
+                atexit.register(os.remove, pem_file.name)
+                self._manage_internal_ca_pem_file_path = pem_file.name
+        return self._manage_internal_ca_pem_file_path
+
+    @property
+    def manage_maxadmin_api_key(self):
+        if self._manage_maxadmin_api_key is None:
+            self._manage_maxadmin_api_key = self.create_or_get_manage_api_key_for_user(MASUserUtils.MAXADMIN)
+        return self._manage_maxadmin_api_key
+
+    def get_or_create_user(self, payload):
+        '''
+        User is identified by payload["id"] field
+        If user already exists, return their record. No attempt will be made to update the user if other fields of the payload differ from the existing user.
+        Otherwise, the user will be created.
+
+        Example payload:
+            {
+                "id": user_id,
+                "status": {"active": True},
+                "username": username,
+                "token": password,
+                "owner": "local",
+                "emails": [
+                    {
+                        "value": email,
+                        "type": "Work",
+                        "primary": True
+                    }
+                ],
+                "displayName": display_name,
+                "issuer": "local",
+                "permissions": {
+                    "systemAdmin": True,
+                    "userAdmin": True,
+                    "apikeyAdmin": True
+                },
+                "entitlement": {
+                    "application": "PREMIUM",
+                    "admin": "ADMIN_PREMIUM",
+                    "alwaysReserveLicense": True
+                },
+                "title": title,
+                "givenName": given_name,
+                "familyName": family_name
+            }
+        '''
+        self.logger.info(f"Upserting user {payload["id"]}")
+        existing_user = self.get_user(payload["id"])
+
+        if existing_user is not None:
+            self.logger.info(f"Existing user {existing_user['id']} found")
+            return existing_user
+
+        self.logger.info(f"Creating new user {payload["id"]}")
+
+        url = f"{self.mas_api_url}/v3/users"
+        querystring = {}
+        payload = payload
+        headers = {
+            "Content-Type": "application/json",
+            "x-access-token": self.superuser_auth_token
+        }
+        response = requests.post(
+            url,
+            json=payload,
+            headers=headers,
+            params=querystring,
+            verify=self.mas_api_url_ca_chain_file_path
+        )
+        if response.status_code == 201:
+            return response.json()
+
+        # if response.status_code == 409:
+        #     json = response.json()
+        #     if "exception" in json and "message" in json["exception"] and json["exception"]["message"] == "AIUCO1005E":
+        #         return None
+
+        raise Exception(f"{response.status_code} {response.text}")
+
+    def link_user_to_local_idp(self, user_id, email_password=False):
+        '''
+        TODO: idempotency
+        '''
+        self.logger.info(f"Linking user {user_id} to local IDP (email_password: {email_password})")
+        url = f"{self.mas_api_url}/v3/users/{user_id}/idps/local"
+        querystring = {
+            "emailPassword": email_password
+        }
+        payload = {
+            "idpUserId": user_id
+        }
+        headers = {
+            "Content-Type": "application/json",
+            "x-access-token": self.superuser_auth_token
+        }
+        response = requests.put(
+            url,
+            json=payload,
+            headers=headers,
+            params=querystring,
+            verify=self.mas_api_url_ca_chain_file_path
+        )
+        if response.status_code != 200:
+            raise Exception(response.text)
+
+        return response.json()
+
+    def get_user(self, user_id):
+        self.logger.info(f"Getting user {user_id}")
+        url = f"{self.mas_api_url}/v3/users/{user_id}"
+        headers = {
+            "Accept": "application/json",
+            "x-access-token": self.superuser_auth_token
+        }
+        response = requests.get(
+            url,
+            headers=headers,
+            verify=self.mas_api_url_ca_chain_file_path
+        )
+
+        if response.status_code == 404:
+            return None
+
+        if response.status_code == 200:
+            return response.json()
+
+        raise Exception(f"{response.status_code} {response.text}")
+
+    def add_user_to_workspace(self, user_id, is_workspace_admin=False):
+        '''
+        TODO: idempotency
+        '''
+        self.logger.info(f"Adding user {user_id} to {self.mas_workspace_id} (is_workspace_admin: {is_workspace_admin})")
+        url = f"{self.mas_api_url}/workspaces/{self.mas_workspace_id}/users/{user_id}"
+        querystring = {}
+        payload = {
+            "permissions": {
+                "workspaceAdmin": is_workspace_admin
+            }
+        }
+        headers = {
+            "Content-Type": "application/json",
+            "x-access-token": self.superuser_auth_token
+        }
+        response = requests.put(
+            url,
+            json=payload,
+            headers=headers,
+            params=querystring,
+            verify=self.mas_api_url_ca_chain_file_path
+        )
+        return response.json()
+
+    def set_user_application_permission(self, user_id, application_id, role):
+        '''
+        TODO: idempotency
+        '''
+        url = f"{self.mas_api_url}/workspaces/{self.mas_workspace_id}/applications/{application_id}/users/{user_id}"
+        querystring = {}
+        payload = {
+            "role": role
+        }
+        headers = {
+            "Content-Type": "application/json",
+            "x-access-token": self.superuser_auth_token
+        }
+        response = requests.put(
+            url,
+            json=payload,
+            headers=headers,
+            params=querystring,
+            verify=self.mas_api_url_ca_chain_file_path
+        )
+        return response.json()
+
+    def check_user_sync(self, user_id, application_id, timeout_secs=60 * 10):
+        t_end = time.time() + timeout_secs
+        self.logger.info(f"Awaiting user {user_id} sync status \"SUCCESS\" for app {application_id}: {t_end - time.time():.2f} seconds remaining")
+        while time.time() < t_end:
+            user = self.get_user(user_id)
+            sync_state = user["applications"][application_id]["sync"]["state"]
+            self.logger.info(f"User sync {user_id} status is currently {sync_state}")
+            if sync_state == "SUCCESS":
+                return
+            elif sync_state == "ERROR":
+                # coreapi >= 25.2.3, not in mas 9.0.9, mas >= 9.1 only?
+                # self.resync_users([user_id])
+                # time.sleep(8)
+                # alternative mechanism to kick off a user resync?
+                # if not, bomb out here since we'll never get SUCCESS?
+                pass
+            else:
+                self.logger.info(f"User {user_id} sync has not been completed yet for app {application_id}: {t_end - time.time():.2f} seconds remaining")
+                time.sleep(5)
+        raise Exception(f"User {user_id} sync failed to complete for app within {timeout_secs} seconds")
+
+    # coreapi >= 25.2.3, not in mas 9.0.9, mas >= 9.1 only?
+
+    def resync_users(self, user_ids):
+        self.logger.info(f"Issuing resync request for user(s) {user_ids}")
+        url = f"{self.mas_api_url}/v3/users/utils/resync"
+        querystring = {}
+        payload = {
+            "users": user_ids
+        }
+        headers = {
+            "Content-Type": "application/json",
+            "x-access-token": self.superuser_auth_token
+        }
+        response = requests.put(
+            url,
+            json=payload,
+            headers=headers,
+            params=querystring,
+            verify=self.mas_api_url_ca_chain_file_path
+        )
+        if response.status_code != 204:
+            raise Exception(response.text)
+
+    def create_or_get_manage_api_key_for_user(self, user_id):
+        self.logger.info(f"Attempting to create Manage API Key for user {user_id}")
+        url = f"{self.manage_api_url_internal}/maximo/api/os/mxapiapikey"
+        querystring = {
+            "ccm": 1,
+            "lean": 1,
+        }
+
+        # TODO: is this just a temporary API Key for this script?
+        #       if so, add cleanup logic (atext) and perhaps set an expiration date in case cleanup fails
+
+        #
+        payload = {
+            "expiration": -1,
+            "userid": user_id
+        }
+        headers = {
+            "Content-Type": "application/json",
+        }
+        response = requests.post(
+            url,
+            json=payload,
+            headers=headers,
+            params=querystring,
+            verify=self.manage_internal_ca_pem_file_path,
+            cert=self.manage_internal_client_pem_file_path,
+        )
+
+        if response.status_code == 400:
+            error_json = response.json()
+            if "Error" in error_json and "reasonCode" in error_json["Error"] and error_json["Error"]["reasonCode"] == "BMXAA10051E":
+                # BMXAA10051E - Only one API key allowed per user.
+                self.logger.info(f"Reusing existing Manage API Key for user {user_id}")
+                pass
+            else:
+                # any other 400 error is unexpected
+                raise Exception(response.text)
+
+        elif response.status_code != 201:
+            # any other status code is unexpected
+            raise Exception(response.text)
+
+        # otherwise, retrieve the apikey (either it already existed, or we just created it)
+
+        apikey = self.get_manage_api_key_for_user(user_id)
+        if apikey is None:
+            # either create call reported that apikey already exists, or we created the api key
+            # so we expect the get call to find it
+            raise Exception("API key was unexpectedly not found")
+
+        if response.status_code == 201:
+            # We created this api key, register a handler to delete it when we're done
+            # TODO: is there a danger of some other process adopting this singleton api key
+            #       which may then fail if we subsequently delete it?
+            #       NOTE: the manage sanity test seems to create maxadmin apikey and not clean it up
+            atexit.register(self.delete_manage_api_key, apikey)
+
+            pass
+
+        return apikey
+
+    def get_manage_api_key_for_user(self, user_id):
+        self.logger.info(f"Getting Manage API Key for user {user_id}")
+        url = f"{self.manage_api_url_internal}/maximo/api/os/mxapiapikey"
+        querystring = {
+            "ccm": 1,
+            "lean": 1,
+            "oslc.select": "*",
+            "oslc.where": f"userid=\"{user_id}\"",
+        }
+        headers = {
+            "Accept": "application/json",
+        }
+        self.logger.debug(f"  > {url} {querystring}")
+
+        response = requests.get(
+            url,
+            headers=headers,
+            params=querystring,
+            verify=self.manage_internal_ca_pem_file_path,
+            cert=self.manage_internal_client_pem_file_path
+        )
+        self.logger.debug(f"  < {response.status_code}")
+        if response.status_code != 200:
+            raise Exception(response.text)
+
+        json = response.json()
+
+        if "member" in json and len(json["member"]) > 0:
+            return json["member"][0]
+
+        return None
+
+    def delete_manage_api_key(self, manage_api_key):
+        self.logger.info(f"Deleting Manage API Key for user {manage_api_key['userid']}")
+
+        # extract the apikey's identifier from the href
+        match = re.search(r'\/maximo\/api\/os\/mxapiapikey\/(.*)', manage_api_key['href'])
+        id = match.group(1)
+
+        url = f"{self.manage_api_url_internal}/maximo/api/os/mxapiapikey/{id}"
+        querystring = {
+            "ccm": 1,
+            "lean": 1,
+        }
+        headers = {
+            "Accept": "application/json",
+        }
+        response = requests.delete(
+            url,
+            headers=headers,
+            params=querystring,
+            verify=self.manage_internal_ca_pem_file_path,
+            cert=self.manage_internal_client_pem_file_path,
+        )
+
+        if response.status_code != 204 and response.status_code != 404:
+            raise Exception(response.text)
+        # {"Error":{"extendedError":{"moreInfo":{"href":"https:\/\/masdev.manage.tgk01.apps.noble4.cp.fyre.ibm.com\/maximo\/api\/error\/messages\/BMXAA8727E"}},"reasonCode":"BMXAA8727E","message":"The OSLC resource MXAPIAPIKEY with the ID _WmxvZlZLNVl2V3dGa1FseUJoKzJ4ZzQzSEd1bmRUamdWcTFiV1hWMGQ5QnAyNHQxQm53TmVFRWtVbmN4YkI2alZSTlp3eElsQko2bElNSCJzcCJ1M3hiNlE9PQ-- was not found as it does not exist in the system. In the database, verify whether the resource for the ID exists.","statusCode":"404"}}
+
+        if manage_api_key["userid"] == MASUserUtils.MAXADMIN:
+            # clear any cached _manage_maxadmin_api_key if necessary
+            self._manage_maxadmin_api_key = None
+
+    def get_manage_group_id(self, group_name):
+        self.logger.info(f"Getting ID for Manage group {group_name}")
+        url = f"{self.manage_api_url_internal}/maximo/api/os/mxapigroup"
+        querystring = {
+            "ccm": 1,
+            "lean": 1,
+            "oslc.select": "*",
+            "oslc.where": f"groupname=\"{group_name}\"",
+        }
+        headers = {
+            "Accept": "application/json",
+            "apikey": self.manage_maxadmin_api_key["apikey"],  # <--- careful, don't log headers as-is (apikey is sensitive)
+        }
+        self.logger.debug(f"  > {url} {querystring}")
+
+        response = requests.get(
+            url,
+            headers=headers,
+            params=querystring,
+            verify=self.manage_internal_ca_pem_file_path,
+        )
+        self.logger.debug(f"  < {response.status_code}")
+        if response.status_code != 200:
+            raise Exception(response.text)
+
+        json = response.json()
+
+        if "member" in json and len(json["member"]) > 0 and "maxgroupid" in json["member"][0]:
+            return json["member"][0]['maxgroupid']
+
+        return None
+
+    def add_user_to_manage_group(self, user_id, group_name):
+        '''
+        TODO: idempotency
+        '''
+        self.logger.info(f"Adding user {user_id} to Manage group {group_name}")
+
+        group_id = self.get_manage_group_id(group_name)
+
+        url = f"{self.manage_api_url_internal}/maximo/api/os/mxapigroup/{group_id}"
+        querystring = {
+            "lean": 1,
+        }
+        headers = {
+            "Content-Type": "application/json",
+            "Accept": "application/json",
+            "x-method-override": "PATCH",
+            "patchtype": "MERGE",
+            "apikey": self.manage_maxadmin_api_key["apikey"],  # <--- careful, don't log headers as-is (apikey is sensitive)
+        }
+        payload = {
+            "groupuser": [
+                {
+                    "userid": f"{user_id}"
+                }
+            ]
+        }
+        self.logger.debug(f"  > {url} {querystring}")
+
+        response = requests.post(
+            url,
+            headers=headers,
+            params=querystring,
+            json=payload,
+            verify=self.manage_internal_ca_pem_file_path,
+        )
+        self.logger.debug(f"  < {response.status_code}")
+        if response.status_code != 204:
+            raise Exception(response.text)
+
+    def get_groups(self):
+        self.logger.info("Getting groups")
+        url = f"{self.mas_api_url}/groups"
+        headers = {
+            "Accept": "application/json",
+            "x-access-token": self.superuser_auth_token
+        }
+        response = requests.get(
+            url,
+            headers=headers,
+            verify=self.mas_api_url_ca_chain_file_path
+        )
+        return response.json()
+
+    def get_user_groups(self, user_id):
+        self.logger.info(f"Getting groups for user {user_id}")
+        url = f"{self.mas_api_url}/v3/users/{user_id}/groups"
+        headers = {
+            "Accept": "application/json",
+            "x-access-token": self.superuser_auth_token
+        }
+        response = requests.get(
+            url,
+            headers=headers,
+            verify=self.mas_api_url_ca_chain_file_path
+        )
+        return response.json()
+
+    def create_initial_users_for_saas(self, initial_users):
+
+        # Validate input
+        if "users" not in initial_users:
+            raise Exception("expected top-level key 'users' not found")
+        users = initial_users["users"]
+        if "primary" not in users:
+            raise Exception("expected key 'users.primary' not found")
+        primary_users = users["primary"]
+        if type(primary_users) is not list:
+            raise Exception("'users.primary' is not a list")
+        if "secondary" not in users:
+            raise Exception("expected key 'users.secondary' not found")
+        secondary_users = users["secondary"]
+        if type(secondary_users) is not list:
+            raise Exception("'users.secondary' is not a list")
+
+        for primary_user in primary_users:
+            self.create_initial_user_for_saas(primary_user, "PRIMARY")
+
+        for secondary_user in secondary_users:
+            self.create_initial_user_for_saas(secondary_user, "SECONDARY")
+
+    def create_initial_user_for_saas(self, user, user_type):
+        if "email" not in user:
+            raise Exception("'email' not found in at least one of the user defs")
+        if "given_name" not in user:
+            raise Exception("'given_name' not found in at least one of the user defs")
+        if "family_name" not in user:
+            raise Exception("'family_name' not found in at least one of the user defs")
+
+        user_email = user["email"]
+        user_given_name = user["given_name"]
+        user_family_name = user["family_name"]
+
+        user_id = user_email
+        username = user_email
+        # display_name = re.search('^([^@]+)@', user_email).group(1) # local part of the email
+        display_name = f"{user_given_name} {user_family_name}"
+
+        # Set user permissions and entitlements based on requested user_type
+        if user_type == "PRIMARY":
+            permissions = {
+                "systemAdmin": False,
+                "userAdmin": True,
+                "apikeyAdmin": False
+            }
+            entitlement = {
+                "application": "PREMIUM",
+                "admin": "ADMIN_BASE",
+                "alwaysReserveLicense": True
+            }
+            is_workspace_admin = True
+            # application_role = "ADMINISTRATOR"
+        elif user_type == "SECONDARY":
+            permissions = {
+                "systemAdmin": False,
+                "userAdmin": False,
+                "apikeyAdmin": False
+            }
+            entitlement = {
+                "application": "BASE",
+                "admin": "NONE",
+                "alwaysReserveLicense": True
+            }
+            is_workspace_admin = False
+            # application_role = "USER"
+        else:
+            raise Exception(f"Unsupported user_type: {user_type}")
+
+        # manage_role = "MANAGEUSER"
+
+        user_def = {
+            "id": user_id,
+            "status": {"active": True},
+            "username": username,
+            "owner": "local",
+            "emails": [
+                {
+                    "value": user_email,
+                    "type": "Work",
+                    "primary": True
+                }
+            ],
+            "displayName": display_name,
+            "issuer": "local",
+            "permissions": permissions,
+            "entitlement": entitlement,
+            "givenName": user_given_name,
+            "familyName": user_family_name
+        }
+
+        self.get_or_create_user(user_def)
+        self.link_user_to_local_idp(user_id)
+        self.add_user_to_workspace(user_id, is_workspace_admin=is_workspace_admin)
+
+        # for mas_application in mas_applications:
+        #     if mas_application == "manage":
+        #         role = manage_role
+        #     else:
+        #         role = application_role
+        #     user_utils.set_user_application_permission(user_id, mas_application, role)
+
+        # for mas_application in mas_applications:
+        #     user_utils.check_user_sync(user_id, mas_application)
+
+        # if "manage" in mas_applications:
+        #     maxadmin_group_id = user_utils.get_manage_group_id("MAXADMIN")
+        #     user_utils.add_user_to_manage_group(user_id, "MAXADMIN")

From 3f247d0acac983103c036c25f68de8a20f97b2ea Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Mon, 24 Mar 2025 12:56:39 +0000
Subject: [PATCH 02/44] make link_user_to_local_idp idempotent

---
 src/mas/devops/users.py | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/src/mas/devops/users.py b/src/mas/devops/users.py
index 5018fc89..71e94e14 100644
--- a/src/mas/devops/users.py
+++ b/src/mas/devops/users.py
@@ -318,8 +318,16 @@ def get_or_create_user(self, payload):
 
     def link_user_to_local_idp(self, user_id, email_password=False):
         '''
-        TODO: idempotency
+        Checks if user already has a local identity, no-op if so.
+        Assumes user exists, raises if not
         '''
+
+        # For the sake of idempotency, check if the user already has a local identity
+        user = self.get_user(user_id)
+        if "identities" in user and "_local" in user["identities"]:
+            self.logger.info(f"User {user_id} already has a local identity")
+            return None
+
         self.logger.info(f"Linking user {user_id} to local IDP (email_password: {email_password})")
         url = f"{self.mas_api_url}/v3/users/{user_id}/idps/local"
         querystring = {
@@ -342,7 +350,7 @@ def link_user_to_local_idp(self, user_id, email_password=False):
         if response.status_code != 200:
             raise Exception(response.text)
 
-        return response.json()
+        return None
 
     def get_user(self, user_id):
         self.logger.info(f"Getting user {user_id}")

From b01614355b3703589e0651f81d8835ce2dfa5dc5 Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Mon, 24 Mar 2025 13:33:21 +0000
Subject: [PATCH 03/44] make add_user_to_workspace idempotent

---
 src/mas/devops/users.py | 42 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 40 insertions(+), 2 deletions(-)

diff --git a/src/mas/devops/users.py b/src/mas/devops/users.py
index 71e94e14..1de92b52 100644
--- a/src/mas/devops/users.py
+++ b/src/mas/devops/users.py
@@ -40,6 +40,10 @@
 
 
 class MASUserUtils():
+    '''
+    A collection of utilities for interacting with the MAS Core V3 User APIs and related APIs.
+    Each instance of this class is tied to a specific MAS instance and workspace ID.
+    '''
 
     MAXADMIN = "MAXADMIN"
 
@@ -373,10 +377,40 @@ def get_user(self, user_id):
 
         raise Exception(f"{response.status_code} {response.text}")
 
+    def get_user_workspaces(self, user_id):
+        '''
+        Assumes user exists, raises if not.
+        '''
+        self.logger.info(f"Getting workspaces for user {user_id}")
+        url = f"{self.mas_api_url}/v3/users/{user_id}/workspaces"
+        headers = {
+            "Accept": "application/json",
+            "x-access-token": self.superuser_auth_token
+        }
+        response = requests.get(
+            url,
+            headers=headers,
+            verify=self.mas_api_url_ca_chain_file_path
+        )
+
+        if response.status_code == 404:
+            raise Exception(f"User {user_id} does not exist")
+
+        if response.status_code == 200:
+            return response.json()
+
+        raise Exception(f"{response.status_code} {response.text}")
+
     def add_user_to_workspace(self, user_id, is_workspace_admin=False):
         '''
-        TODO: idempotency
+        No-op if user is already a member of the workspace. No attempt will be made to update their existing is_workspace_admin flag if it differs.
         '''
+        workspaces = self.get_user_workspaces(user_id)
+        for workspace in workspaces:
+            if "id" in workspace and workspace["id"] == self.mas_workspace_id:
+                self.logger.info(f"User {user_id} is already a member of workspace {self.mas_workspace_id}")
+                return None
+
         self.logger.info(f"Adding user {user_id} to {self.mas_workspace_id} (is_workspace_admin: {is_workspace_admin})")
         url = f"{self.mas_api_url}/workspaces/{self.mas_workspace_id}/users/{user_id}"
         querystring = {}
@@ -396,7 +430,11 @@ def add_user_to_workspace(self, user_id, is_workspace_admin=False):
             params=querystring,
             verify=self.mas_api_url_ca_chain_file_path
         )
-        return response.json()
+
+        if response.status_code == 200:
+            return None
+
+        raise Exception(f"{response.status_code} {response.text}")
 
     def set_user_application_permission(self, user_id, application_id, role):
         '''

From af1dbf57c8602d7640083446b06e09a665b6b4ad Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Mon, 24 Mar 2025 15:36:00 +0000
Subject: [PATCH 04/44] fetcg MAS apps, wait for them to be ready, set user
 permissions and check user sync completes

https://jsw.ibm.com/browse/MASCORE-6072
---
 bin/mas-devops-create-initial-users-for-saas |   1 +
 src/mas/devops/users.py                      | 127 ++++++++++++++-----
 2 files changed, 99 insertions(+), 29 deletions(-)

diff --git a/bin/mas-devops-create-initial-users-for-saas b/bin/mas-devops-create-initial-users-for-saas
index ec0f1097..9dfc6609 100644
--- a/bin/mas-devops-create-initial-users-for-saas
+++ b/bin/mas-devops-create-initial-users-for-saas
@@ -80,5 +80,6 @@ if __name__ == "__main__":
     with open(initial_users_yaml_file, 'r') as file:
        initial_users_yaml = yaml.safe_load(file)
 
+
     user_utils.create_initial_users_for_saas(initial_users_yaml)
 
diff --git a/src/mas/devops/users.py b/src/mas/devops/users.py
index 1de92b52..143ecf76 100644
--- a/src/mas/devops/users.py
+++ b/src/mas/devops/users.py
@@ -193,7 +193,7 @@ def manage_api_url_internal(self):
     @property
     def superuser_auth_token(self):
         if self._superuser_auth_token is None:
-            self.logger.info("Getting superuser auth token")
+            self.logger.debug("Getting superuser auth token")
             url = f"{self.mas_admin_url}/logininitial"
             headers = {
                 "Content-Type": "application/json"
@@ -287,7 +287,7 @@ def get_or_create_user(self, payload):
                 "familyName": family_name
             }
         '''
-        self.logger.info(f"Upserting user {payload["id"]}")
+        self.logger.info(f"Creating (or getting) user {payload["id"]}")
         existing_user = self.get_user(payload["id"])
 
         if existing_user is not None:
@@ -357,7 +357,7 @@ def link_user_to_local_idp(self, user_id, email_password=False):
         return None
 
     def get_user(self, user_id):
-        self.logger.info(f"Getting user {user_id}")
+        self.logger.debug(f"Getting user {user_id}")
         url = f"{self.mas_api_url}/v3/users/{user_id}"
         headers = {
             "Accept": "application/json",
@@ -381,7 +381,7 @@ def get_user_workspaces(self, user_id):
         '''
         Assumes user exists, raises if not.
         '''
-        self.logger.info(f"Getting workspaces for user {user_id}")
+        self.logger.debug(f"Getting workspaces for user {user_id}")
         url = f"{self.mas_api_url}/v3/users/{user_id}/workspaces"
         headers = {
             "Accept": "application/json",
@@ -440,6 +440,7 @@ def set_user_application_permission(self, user_id, application_id, role):
         '''
         TODO: idempotency
         '''
+        self.logger.info(f"Setting user {user_id} role for {application_id} to {role}")
         url = f"{self.mas_api_url}/workspaces/{self.mas_workspace_id}/applications/{application_id}/users/{user_id}"
         querystring = {}
         payload = {
@@ -464,7 +465,6 @@ def check_user_sync(self, user_id, application_id, timeout_secs=60 * 10):
         while time.time() < t_end:
             user = self.get_user(user_id)
             sync_state = user["applications"][application_id]["sync"]["state"]
-            self.logger.info(f"User sync {user_id} status is currently {sync_state}")
             if sync_state == "SUCCESS":
                 return
             elif sync_state == "ERROR":
@@ -473,9 +473,10 @@ def check_user_sync(self, user_id, application_id, timeout_secs=60 * 10):
                 # time.sleep(8)
                 # alternative mechanism to kick off a user resync?
                 # if not, bomb out here since we'll never get SUCCESS?
-                pass
+                # TODO: I think you can just set user roles against to retrigger user sync
+                raise Exception(f"User {user_id} sync failed, aborting")
             else:
-                self.logger.info(f"User {user_id} sync has not been completed yet for app {application_id}: {t_end - time.time():.2f} seconds remaining")
+                self.logger.info(f"User {user_id} sync has not been completed yet for app {application_id} (currrently {sync_state}): {t_end - time.time():.2f} seconds remaining")
                 time.sleep(5)
         raise Exception(f"User {user_id} sync failed to complete for app within {timeout_secs} seconds")
 
@@ -564,7 +565,7 @@ def create_or_get_manage_api_key_for_user(self, user_id):
         return apikey
 
     def get_manage_api_key_for_user(self, user_id):
-        self.logger.info(f"Getting Manage API Key for user {user_id}")
+        self.logger.debug(f"Getting Manage API Key for user {user_id}")
         url = f"{self.manage_api_url_internal}/maximo/api/os/mxapiapikey"
         querystring = {
             "ccm": 1,
@@ -627,7 +628,7 @@ def delete_manage_api_key(self, manage_api_key):
             self._manage_maxadmin_api_key = None
 
     def get_manage_group_id(self, group_name):
-        self.logger.info(f"Getting ID for Manage group {group_name}")
+        self.logger.debug(f"Getting ID for Manage group {group_name}")
         url = f"{self.manage_api_url_internal}/maximo/api/os/mxapigroup"
         querystring = {
             "ccm": 1,
@@ -694,11 +695,14 @@ def add_user_to_manage_group(self, user_id, group_name):
             verify=self.manage_internal_ca_pem_file_path,
         )
         self.logger.debug(f"  < {response.status_code}")
-        if response.status_code != 204:
-            raise Exception(response.text)
+
+        if response.status_code == 204:
+            return None
+
+        raise Exception(f"{response.status_code} {response.text}")
 
     def get_groups(self):
-        self.logger.info("Getting groups")
+        self.logger.debug("Getting groups")
         url = f"{self.mas_api_url}/groups"
         headers = {
             "Accept": "application/json",
@@ -709,7 +713,9 @@ def get_groups(self):
             headers=headers,
             verify=self.mas_api_url_ca_chain_file_path
         )
-        return response.json()
+        if response.status_code == 200:
+            return response.json()
+        raise Exception(f"{response.status_code} {response.text}")
 
     def get_user_groups(self, user_id):
         self.logger.info(f"Getting groups for user {user_id}")
@@ -723,7 +729,70 @@ def get_user_groups(self, user_id):
             headers=headers,
             verify=self.mas_api_url_ca_chain_file_path
         )
-        return response.json()
+        if response.status_code == 200:
+            return response.json()
+        raise Exception(f"{response.status_code} {response.text}")
+
+    def get_installed_mas_applications(self):
+        self.logger.debug("Getting installed MAS Applications")
+        url = f"{self.mas_api_url}/applications"
+        headers = {
+            "Accept": "application/json",
+            "x-access-token": self.superuser_auth_token
+        }
+        response = requests.get(
+            url,
+            headers=headers,
+            verify=self.mas_api_url_ca_chain_file_path
+        )
+        if response.status_code == 200:
+            return response.json()
+        raise Exception(f"{response.status_code} {response.text}")
+
+    def get_mas_applications_in_workspace(self):
+        self.logger.debug(f"Getting MAS Applications in workspace {self.mas_workspace_id}")
+        url = f"{self.mas_api_url}/workspaces/{self.mas_workspace_id}/applications"
+        headers = {
+            "Accept": "application/json",
+            "x-access-token": self.superuser_auth_token
+        }
+        response = requests.get(
+            url,
+            headers=headers,
+            verify=self.mas_api_url_ca_chain_file_path
+        )
+        if response.status_code == 200:
+            return response.json()
+        raise Exception(f"{response.status_code} {response.text}")
+
+    def get_mas_application_availability(self, mas_application_id):
+        self.logger.debug(f"Getting availability of MAS Application {mas_application_id} in workspace {self.mas_workspace_id}")
+        url = f"{self.mas_api_url}/workspaces/{self.mas_workspace_id}/applications/{mas_application_id}"
+        headers = {
+            "Accept": "application/json",
+            "x-access-token": self.superuser_auth_token
+        }
+        response = requests.get(
+            url,
+            headers=headers,
+            verify=self.mas_api_url_ca_chain_file_path
+        )
+        if response.status_code == 200:
+            return response.json()
+        raise Exception(f"{response.status_code} {response.text}")
+
+    def await_mas_application_availability(self, mas_application_id, timeout_secs=60 * 10):
+        t_end = time.time() + timeout_secs
+        self.logger.info(f"Waiting for {mas_application_id} to become ready and available: {t_end - time.time():.2f} seconds remaining")
+        while time.time() < t_end:
+            app = self.get_mas_application_availability(mas_application_id)
+            if "available" in app and "ready" in app and app["ready"] and app["available"]:
+                return
+            # TODO: error state?
+            else:
+                self.logger.info(f"{mas_application_id} is not ready or available, retry in 5 seconds: {t_end - time.time():.2f} seconds remaining")
+                time.sleep(5)
+        raise Exception(f"{mas_application_id} did not become ready and available in time, aborting")
 
     def create_initial_users_for_saas(self, initial_users):
 
@@ -778,7 +847,7 @@ def create_initial_user_for_saas(self, user, user_type):
                 "alwaysReserveLicense": True
             }
             is_workspace_admin = True
-            # application_role = "ADMINISTRATOR"
+            application_role = "ADMINISTRATOR"
         elif user_type == "SECONDARY":
             permissions = {
                 "systemAdmin": False,
@@ -791,12 +860,10 @@ def create_initial_user_for_saas(self, user, user_type):
                 "alwaysReserveLicense": True
             }
             is_workspace_admin = False
-            # application_role = "USER"
+            application_role = "USER"
         else:
             raise Exception(f"Unsupported user_type: {user_type}")
 
-        # manage_role = "MANAGEUSER"
-
         user_def = {
             "id": user_id,
             "status": {"active": True},
@@ -821,16 +888,18 @@ def create_initial_user_for_saas(self, user, user_type):
         self.link_user_to_local_idp(user_id)
         self.add_user_to_workspace(user_id, is_workspace_admin=is_workspace_admin)
 
-        # for mas_application in mas_applications:
-        #     if mas_application == "manage":
-        #         role = manage_role
-        #     else:
-        #         role = application_role
-        #     user_utils.set_user_application_permission(user_id, mas_application, role)
+        mas_application_ids = list(map(lambda ma: ma["id"], self.get_mas_applications_in_workspace()))
+
+        for mas_application_id in mas_application_ids:
+            self.await_mas_application_availability(mas_application_id)
+            if mas_application_id == "manage":
+                role = "MANAGEUSER"
+            else:
+                role = application_role
+            self.set_user_application_permission(user_id, mas_application_id, role)
 
-        # for mas_application in mas_applications:
-        #     user_utils.check_user_sync(user_id, mas_application)
+        for mas_application_id in mas_application_ids:
+            self.check_user_sync(user_id, mas_application_id)
 
-        # if "manage" in mas_applications:
-        #     maxadmin_group_id = user_utils.get_manage_group_id("MAXADMIN")
-        #     user_utils.add_user_to_manage_group(user_id, "MAXADMIN")
+        # if "manage" in mas_application_ids:
+        #     self.add_user_to_manage_group(user_id, "MAXADMIN")

From 88423bcda868253129dcd1538339bb23fc76c364 Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Mon, 24 Mar 2025 15:47:27 +0000
Subject: [PATCH 05/44] Make set_user_application_permission idempotent

https://jsw.ibm.com/browse/MASCORE-6072
---
 bin/mas-devops-create-initial-users-for-saas |  4 +--
 src/mas/devops/users.py                      | 37 ++++++++++++++++++--
 2 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/bin/mas-devops-create-initial-users-for-saas b/bin/mas-devops-create-initial-users-for-saas
index 9dfc6609..62925ee4 100644
--- a/bin/mas-devops-create-initial-users-for-saas
+++ b/bin/mas-devops-create-initial-users-for-saas
@@ -69,11 +69,11 @@ if __name__ == "__main__":
     try:
         # Try to load in-cluster configuration
         config.load_incluster_config()
-        logger.info("Loaded in-cluster configuration")
+        logger.debug("Loaded in-cluster configuration")
     except ConfigException:
         # If that fails, fall back to kubeconfig file
         config.load_kube_config()
-        logger.info("Loaded kubeconfig file")
+        logger.debug("Loaded kubeconfig file")
 
     user_utils = MASUserUtils(mas_instance_id, mas_workspace_id, client.api_client.ApiClient())
 
diff --git a/src/mas/devops/users.py b/src/mas/devops/users.py
index 143ecf76..90a33e62 100644
--- a/src/mas/devops/users.py
+++ b/src/mas/devops/users.py
@@ -287,7 +287,6 @@ def get_or_create_user(self, payload):
                 "familyName": family_name
             }
         '''
-        self.logger.info(f"Creating (or getting) user {payload["id"]}")
         existing_user = self.get_user(payload["id"])
 
         if existing_user is not None:
@@ -436,10 +435,38 @@ def add_user_to_workspace(self, user_id, is_workspace_admin=False):
 
         raise Exception(f"{response.status_code} {response.text}")
 
+    def get_user_application_permissions(self, user_id, application_id):
+        self.logger.debug(f"Getting user {user_id} permissions for application {application_id}")
+        url = f"{self.mas_api_url}/workspaces/{self.mas_workspace_id}/applications/{application_id}/users/{user_id}"
+        headers = {
+            "Accept": "application/json",
+            "x-access-token": self.superuser_auth_token
+        }
+        response = requests.get(
+            url,
+            headers=headers,
+            verify=self.mas_api_url_ca_chain_file_path
+        )
+
+        if response.status_code == 200:
+            return response.json()
+
+        if response.status_code == 404:
+            return None
+
+        raise Exception(f"{response.status_code} {response.text}")
+
     def set_user_application_permission(self, user_id, application_id, role):
         '''
-        TODO: idempotency
+        No-op if user already has a role established for the application. No attempt will be made to update the role if it differs.
         '''
+
+        existing_permissions = self.get_user_application_permissions(user_id, application_id)
+
+        if existing_permissions is not None:
+            self.logger.info(f"User {user_id} already has permissions set for application {application_id}")
+            return None
+
         self.logger.info(f"Setting user {user_id} role for {application_id} to {role}")
         url = f"{self.mas_api_url}/workspaces/{self.mas_workspace_id}/applications/{application_id}/users/{user_id}"
         querystring = {}
@@ -457,7 +484,11 @@ def set_user_application_permission(self, user_id, application_id, role):
             params=querystring,
             verify=self.mas_api_url_ca_chain_file_path
         )
-        return response.json()
+
+        if response.status_code == 200:
+            return None
+
+        raise Exception(f"{response.status_code} {response.text}")
 
     def check_user_sync(self, user_id, application_id, timeout_secs=60 * 10):
         t_end = time.time() + timeout_secs

From f928bbeab0fa7f72c85ae3833e3436607a25f0c2 Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Mon, 24 Mar 2025 16:40:48 +0000
Subject: [PATCH 06/44] use internal services for calling MAS APIs

---
 src/mas/devops/users.py | 197 ++++++++++++++++++----------------------
 1 file changed, 88 insertions(+), 109 deletions(-)

diff --git a/src/mas/devops/users.py b/src/mas/devops/users.py
index 90a33e62..c11d3c2b 100644
--- a/src/mas/devops/users.py
+++ b/src/mas/devops/users.py
@@ -14,7 +14,6 @@
 from openshift.dynamic import DynamicClient
 import base64
 import atexit
-import certifi
 import tempfile
 import os
 import time
@@ -61,17 +60,16 @@ def __init__(self, mas_instance_id: str, mas_workspace_id: str, k8s_client: clie
 
         self._mas_superuser_credentials = None
 
-        self._mas_admin_route = None
-        self._mas_admin_url = None
-        self._mas_admin_url_ca_chain_file_path = None
+        self._mas_admin_url_internal = None
+        self._admin_internal_tls_secret = None
+        self._admin_internal_ca_pem_file_path = None
 
-        self._mas_api_route = None
-        self._mas_api_url = None
-        self._mas_api_url_ca_chain_file_path = None
+        self._mas_api_url_internal = None
+        self._core_internal_tls_secret = None
+        self._core_internal_ca_pem_file_path = None
 
         self._superuser_auth_token = None
 
-        self._manage_api_route = None
         self._manage_api_url_internal = None
         self._manage_internal_tls_secret = None
         self._manage_internal_ca_pem_file_path = None
@@ -79,49 +77,6 @@ def __init__(self, mas_instance_id: str, mas_workspace_id: str, k8s_client: clie
 
         self._manage_maxadmin_api_key = None
 
-    @staticmethod
-    def ca_chain_file_path_from_route(route):
-        # TODO: this just "happens" to work for the MAS api and admin routes (in no frontend, manual_cert_mgmt: false mode)
-        #       should try and come up with a more robust approach here (see ntoes for manage API urls below)
-        # maybe switch to calling via internal service?
-        try:
-            # This may include CA certificates that we need in order to trust the certificates presented by the endpoint
-            cert = route["spec"]["tls"]["certificate"]
-        except KeyError:
-            pass
-        try:
-            # This may include CA certificates that we need in order to trust the certificates presented by the endpoint
-            ca = route["spec"]["tls"]["caCertificate"]
-        except KeyError:
-            pass
-        try:
-            # This may include CA certificates that we need in order to trust the certificates presented by the endpoint
-            dest_ca = route["spec"]["tls"]["destinationCACertificate"]
-        except KeyError:
-            pass
-
-        # Load default CA bundle. This will include certs for well-known CAs. This ensures that we will
-        # trust the certificates presented by the endpoint when MAS is configured to use
-        # an external frontend like CIS.
-        with open(certifi.where(), 'rb') as default_ca:
-            default_ca_content = default_ca.read()
-
-        with tempfile.NamedTemporaryFile(delete=False) as chain_file:
-            if cert:
-                chain_file.write(cert.encode())
-            if ca:
-                chain_file.write(ca.encode())
-            if dest_ca:
-                chain_file.write(dest_ca.encode())
-
-            chain_file.write(default_ca_content)
-
-            chain_file.flush()
-            chain_file.close()
-            atexit.register(os.remove, chain_file.name)
-
-            return chain_file.name
-
     @property
     def mas_superuser_credentials(self):
         if self._mas_superuser_credentials is None:
@@ -133,51 +88,74 @@ def mas_superuser_credentials(self):
         return self._mas_superuser_credentials
 
     @property
-    def mas_admin_route(self):
-        if self._mas_admin_route is None:
-            self._mas_admin_route = self.v1_routes.get(name=f"{self.mas_instance_id}-admin", namespace=self.mas_core_namespace)
-        return self._mas_admin_route
+    def mas_admin_url_internal(self):
+        if self._mas_admin_url_internal is None:
+            self._mas_admin_url_internal = f'https://admin-dashboard.{self.mas_core_namespace}.svc.cluster.local'
 
-    @property
-    def mas_admin_url(self):
-        if self._mas_admin_url is None:
-            self._mas_admin_url = f"https://{self.mas_admin_route.spec.host}{self.mas_admin_route.spec.path}"
-        return self._mas_admin_url
+            # for local testing:
+            # add to /etc/hosts:
+            #    127.0.0.1               admin-dashboard.mas-tgk01-core.svc.cluster.local
+            # oc port-forward service/admin-dashboard 8445:443 -n mas-tgk01-core
+
+            # uncomment for local testing
+            # TODO: make configurable
+            self._mas_admin_url_internal = f"{self._mas_admin_url_internal}:8445"
+        return self._mas_admin_url_internal
 
     @property
-    def mas_admin_url_ca_chain_file_path(self):
-        if self._mas_admin_url_ca_chain_file_path is None:
-            self._mas_admin_url_ca_chain_file_path = MASUserUtils.ca_chain_file_path_from_route(self.mas_admin_route)
-        return self._mas_admin_url_ca_chain_file_path
+    def admin_internal_tls_secret(self):
+        if self._admin_internal_tls_secret is None:
+            self._admin_internal_tls_secret = self.v1_secrets.get(name=f"{self.mas_instance_id}-admindashboard-cert-internal", namespace=self.mas_core_namespace)
+        return self._admin_internal_tls_secret
 
     @property
-    def mas_api_route(self):
-        if self._mas_api_route is None:
-            self._mas_api_route = self.v1_routes.get(name=f"{self.mas_instance_id}-api", namespace=self.mas_core_namespace)
-        return self._mas_api_route
+    def admin_internal_ca_pem_file_path(self):
+        if self._admin_internal_ca_pem_file_path is None:
+            ca = base64.b64decode(self.core_internal_tls_secret.data["ca.crt"]).decode('utf-8')
+            with tempfile.NamedTemporaryFile(delete=False, suffix=".pem") as pem_file:
+                pem_file.write(ca.encode())
+                pem_file.flush()
+                pem_file.close()
+                atexit.register(os.remove, pem_file.name)
+                self._admin_internal_ca_pem_file_path = pem_file.name
+        return self._admin_internal_ca_pem_file_path
 
     @property
-    def mas_api_url(self):
-        if self._mas_api_url is None:
-            self._mas_api_url = f"https://{self.mas_api_route.spec.host}"
-        return self._mas_api_url
+    def mas_api_url_internal(self):
+        if self._mas_api_url_internal is None:
+            self._mas_api_url_internal = f'https://coreapi.{self.mas_core_namespace}.svc.cluster.local'
+
+            # for local testing:
+            # add to /etc/hosts:
+            #    127.0.0.1               coreapi.mas-tgk01-core.svc.cluster.local
+            # oc port-forward service/coreapi 8444:443 -n mas-tgk01-core
+
+            # uncomment for local testing
+            # TODO: make configurable
+            self._mas_api_url_internal = f"{self._mas_api_url_internal}:8444"
+        return self._mas_api_url_internal
 
     @property
-    def mas_api_url_ca_chain_file_path(self):
-        if self._mas_api_url_ca_chain_file_path is None:
-            self._mas_api_url_ca_chain_file_path = MASUserUtils.ca_chain_file_path_from_route(self.mas_api_route)
-        return self._mas_api_url_ca_chain_file_path
+    def core_internal_tls_secret(self):
+        if self._core_internal_tls_secret is None:
+            self._core_internal_tls_secret = self.v1_secrets.get(name=f"{self.mas_instance_id}-coreapi-cert-internal", namespace=self.mas_core_namespace)
+        return self._core_internal_tls_secret
 
     @property
-    def manage_api_route(self):
-        if self._manage_api_route is None:
-            self._manage_api_route = self.v1_routes.get(name=f"{self.mas_instance_id}-manage-{self.mas_workspace_id}", namespace=self.manage_namespace)
-        return self._manage_api_route
+    def core_internal_ca_pem_file_path(self):
+        if self._core_internal_ca_pem_file_path is None:
+            ca = base64.b64decode(self.core_internal_tls_secret.data["ca.crt"]).decode('utf-8')
+            with tempfile.NamedTemporaryFile(delete=False, suffix=".pem") as pem_file:
+                pem_file.write(ca.encode())
+                pem_file.flush()
+                pem_file.close()
+                atexit.register(os.remove, pem_file.name)
+                self._core_internal_ca_pem_file_path = pem_file.name
+        return self._core_internal_ca_pem_file_path
 
     @property
     def manage_api_url_internal(self):
         if self._manage_api_url_internal is None:
-            self.logger.info("Getting Manage internal API URL")
             # for local testing:
             # add to /etc/hosts:
             #    127.0.0.1               tgk01-masdev.mas-tgk01-manage.svc.cluster.local
@@ -187,6 +165,7 @@ def manage_api_url_internal(self):
             self._manage_api_url_internal = f'https://{self.mas_instance_id}-{self.mas_workspace_id}.{self.manage_namespace}.svc.cluster.local'
 
             # uncomment for local testing
+            # TODO: make configurable
             self._manage_api_url_internal = f"{self._manage_api_url_internal}:8443"
         return self._manage_api_url_internal
 
@@ -194,7 +173,7 @@ def manage_api_url_internal(self):
     def superuser_auth_token(self):
         if self._superuser_auth_token is None:
             self.logger.debug("Getting superuser auth token")
-            url = f"{self.mas_admin_url}/logininitial"
+            url = f"{self.mas_admin_url_internal}/logininitial"
             headers = {
                 "Content-Type": "application/json"
             }
@@ -207,7 +186,7 @@ def superuser_auth_token(self):
                 json=payload,
                 headers=headers,
                 params=querystring,
-                verify=self.mas_admin_url_ca_chain_file_path
+                verify=self.admin_internal_ca_pem_file_path
             )
             self._superuser_auth_token = response.json()["token"]
         return self._superuser_auth_token
@@ -295,7 +274,7 @@ def get_or_create_user(self, payload):
 
         self.logger.info(f"Creating new user {payload["id"]}")
 
-        url = f"{self.mas_api_url}/v3/users"
+        url = f"{self.mas_api_url_internal}/v3/users"
         querystring = {}
         payload = payload
         headers = {
@@ -307,7 +286,7 @@ def get_or_create_user(self, payload):
             json=payload,
             headers=headers,
             params=querystring,
-            verify=self.mas_api_url_ca_chain_file_path
+            verify=self.core_internal_ca_pem_file_path
         )
         if response.status_code == 201:
             return response.json()
@@ -332,7 +311,7 @@ def link_user_to_local_idp(self, user_id, email_password=False):
             return None
 
         self.logger.info(f"Linking user {user_id} to local IDP (email_password: {email_password})")
-        url = f"{self.mas_api_url}/v3/users/{user_id}/idps/local"
+        url = f"{self.mas_api_url_internal}/v3/users/{user_id}/idps/local"
         querystring = {
             "emailPassword": email_password
         }
@@ -348,7 +327,7 @@ def link_user_to_local_idp(self, user_id, email_password=False):
             json=payload,
             headers=headers,
             params=querystring,
-            verify=self.mas_api_url_ca_chain_file_path
+            verify=self.core_internal_ca_pem_file_path
         )
         if response.status_code != 200:
             raise Exception(response.text)
@@ -357,7 +336,7 @@ def link_user_to_local_idp(self, user_id, email_password=False):
 
     def get_user(self, user_id):
         self.logger.debug(f"Getting user {user_id}")
-        url = f"{self.mas_api_url}/v3/users/{user_id}"
+        url = f"{self.mas_api_url_internal}/v3/users/{user_id}"
         headers = {
             "Accept": "application/json",
             "x-access-token": self.superuser_auth_token
@@ -365,7 +344,7 @@ def get_user(self, user_id):
         response = requests.get(
             url,
             headers=headers,
-            verify=self.mas_api_url_ca_chain_file_path
+            verify=self.core_internal_ca_pem_file_path
         )
 
         if response.status_code == 404:
@@ -381,7 +360,7 @@ def get_user_workspaces(self, user_id):
         Assumes user exists, raises if not.
         '''
         self.logger.debug(f"Getting workspaces for user {user_id}")
-        url = f"{self.mas_api_url}/v3/users/{user_id}/workspaces"
+        url = f"{self.mas_api_url_internal}/v3/users/{user_id}/workspaces"
         headers = {
             "Accept": "application/json",
             "x-access-token": self.superuser_auth_token
@@ -389,7 +368,7 @@ def get_user_workspaces(self, user_id):
         response = requests.get(
             url,
             headers=headers,
-            verify=self.mas_api_url_ca_chain_file_path
+            verify=self.core_internal_ca_pem_file_path
         )
 
         if response.status_code == 404:
@@ -411,7 +390,7 @@ def add_user_to_workspace(self, user_id, is_workspace_admin=False):
                 return None
 
         self.logger.info(f"Adding user {user_id} to {self.mas_workspace_id} (is_workspace_admin: {is_workspace_admin})")
-        url = f"{self.mas_api_url}/workspaces/{self.mas_workspace_id}/users/{user_id}"
+        url = f"{self.mas_api_url_internal}/workspaces/{self.mas_workspace_id}/users/{user_id}"
         querystring = {}
         payload = {
             "permissions": {
@@ -427,7 +406,7 @@ def add_user_to_workspace(self, user_id, is_workspace_admin=False):
             json=payload,
             headers=headers,
             params=querystring,
-            verify=self.mas_api_url_ca_chain_file_path
+            verify=self.core_internal_ca_pem_file_path
         )
 
         if response.status_code == 200:
@@ -437,7 +416,7 @@ def add_user_to_workspace(self, user_id, is_workspace_admin=False):
 
     def get_user_application_permissions(self, user_id, application_id):
         self.logger.debug(f"Getting user {user_id} permissions for application {application_id}")
-        url = f"{self.mas_api_url}/workspaces/{self.mas_workspace_id}/applications/{application_id}/users/{user_id}"
+        url = f"{self.mas_api_url_internal}/workspaces/{self.mas_workspace_id}/applications/{application_id}/users/{user_id}"
         headers = {
             "Accept": "application/json",
             "x-access-token": self.superuser_auth_token
@@ -445,7 +424,7 @@ def get_user_application_permissions(self, user_id, application_id):
         response = requests.get(
             url,
             headers=headers,
-            verify=self.mas_api_url_ca_chain_file_path
+            verify=self.core_internal_ca_pem_file_path
         )
 
         if response.status_code == 200:
@@ -468,7 +447,7 @@ def set_user_application_permission(self, user_id, application_id, role):
             return None
 
         self.logger.info(f"Setting user {user_id} role for {application_id} to {role}")
-        url = f"{self.mas_api_url}/workspaces/{self.mas_workspace_id}/applications/{application_id}/users/{user_id}"
+        url = f"{self.mas_api_url_internal}/workspaces/{self.mas_workspace_id}/applications/{application_id}/users/{user_id}"
         querystring = {}
         payload = {
             "role": role
@@ -482,7 +461,7 @@ def set_user_application_permission(self, user_id, application_id, role):
             json=payload,
             headers=headers,
             params=querystring,
-            verify=self.mas_api_url_ca_chain_file_path
+            verify=self.core_internal_ca_pem_file_path
         )
 
         if response.status_code == 200:
@@ -515,7 +494,7 @@ def check_user_sync(self, user_id, application_id, timeout_secs=60 * 10):
 
     def resync_users(self, user_ids):
         self.logger.info(f"Issuing resync request for user(s) {user_ids}")
-        url = f"{self.mas_api_url}/v3/users/utils/resync"
+        url = f"{self.mas_api_url_internal}/v3/users/utils/resync"
         querystring = {}
         payload = {
             "users": user_ids
@@ -529,7 +508,7 @@ def resync_users(self, user_ids):
             json=payload,
             headers=headers,
             params=querystring,
-            verify=self.mas_api_url_ca_chain_file_path
+            verify=self.core_internal_ca_pem_file_path
         )
         if response.status_code != 204:
             raise Exception(response.text)
@@ -734,7 +713,7 @@ def add_user_to_manage_group(self, user_id, group_name):
 
     def get_groups(self):
         self.logger.debug("Getting groups")
-        url = f"{self.mas_api_url}/groups"
+        url = f"{self.mas_api_url_internal}/groups"
         headers = {
             "Accept": "application/json",
             "x-access-token": self.superuser_auth_token
@@ -742,7 +721,7 @@ def get_groups(self):
         response = requests.get(
             url,
             headers=headers,
-            verify=self.mas_api_url_ca_chain_file_path
+            verify=self.core_internal_ca_pem_file_path
         )
         if response.status_code == 200:
             return response.json()
@@ -750,7 +729,7 @@ def get_groups(self):
 
     def get_user_groups(self, user_id):
         self.logger.info(f"Getting groups for user {user_id}")
-        url = f"{self.mas_api_url}/v3/users/{user_id}/groups"
+        url = f"{self.mas_api_url_internal}/v3/users/{user_id}/groups"
         headers = {
             "Accept": "application/json",
             "x-access-token": self.superuser_auth_token
@@ -758,7 +737,7 @@ def get_user_groups(self, user_id):
         response = requests.get(
             url,
             headers=headers,
-            verify=self.mas_api_url_ca_chain_file_path
+            verify=self.core_internal_ca_pem_file_path
         )
         if response.status_code == 200:
             return response.json()
@@ -766,7 +745,7 @@ def get_user_groups(self, user_id):
 
     def get_installed_mas_applications(self):
         self.logger.debug("Getting installed MAS Applications")
-        url = f"{self.mas_api_url}/applications"
+        url = f"{self.mas_api_url_internal}/applications"
         headers = {
             "Accept": "application/json",
             "x-access-token": self.superuser_auth_token
@@ -774,7 +753,7 @@ def get_installed_mas_applications(self):
         response = requests.get(
             url,
             headers=headers,
-            verify=self.mas_api_url_ca_chain_file_path
+            verify=self.core_internal_ca_pem_file_path
         )
         if response.status_code == 200:
             return response.json()
@@ -782,7 +761,7 @@ def get_installed_mas_applications(self):
 
     def get_mas_applications_in_workspace(self):
         self.logger.debug(f"Getting MAS Applications in workspace {self.mas_workspace_id}")
-        url = f"{self.mas_api_url}/workspaces/{self.mas_workspace_id}/applications"
+        url = f"{self.mas_api_url_internal}/workspaces/{self.mas_workspace_id}/applications"
         headers = {
             "Accept": "application/json",
             "x-access-token": self.superuser_auth_token
@@ -790,7 +769,7 @@ def get_mas_applications_in_workspace(self):
         response = requests.get(
             url,
             headers=headers,
-            verify=self.mas_api_url_ca_chain_file_path
+            verify=self.core_internal_ca_pem_file_path
         )
         if response.status_code == 200:
             return response.json()
@@ -798,7 +777,7 @@ def get_mas_applications_in_workspace(self):
 
     def get_mas_application_availability(self, mas_application_id):
         self.logger.debug(f"Getting availability of MAS Application {mas_application_id} in workspace {self.mas_workspace_id}")
-        url = f"{self.mas_api_url}/workspaces/{self.mas_workspace_id}/applications/{mas_application_id}"
+        url = f"{self.mas_api_url_internal}/workspaces/{self.mas_workspace_id}/applications/{mas_application_id}"
         headers = {
             "Accept": "application/json",
             "x-access-token": self.superuser_auth_token
@@ -806,7 +785,7 @@ def get_mas_application_availability(self, mas_application_id):
         response = requests.get(
             url,
             headers=headers,
-            verify=self.mas_api_url_ca_chain_file_path
+            verify=self.core_internal_ca_pem_file_path
         )
         if response.status_code == 200:
             return response.json()

From c8ac382efd5172cda9983f62d4b1ccc54a51c5f5 Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Mon, 24 Mar 2025 17:14:46 +0000
Subject: [PATCH 07/44] make ports configurable for local testing

---
 bin/mas-devops-create-initial-users-for-saas | 15 ++++++++++-
 src/mas/devops/users.py                      | 26 ++++++++------------
 2 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/bin/mas-devops-create-initial-users-for-saas b/bin/mas-devops-create-initial-users-for-saas
index 62925ee4..cd4727e7 100644
--- a/bin/mas-devops-create-initial-users-for-saas
+++ b/bin/mas-devops-create-initial-users-for-saas
@@ -36,6 +36,12 @@ if __name__ == "__main__":
     parser.add_argument("--log-level", required=False, choices=["DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL"], default="WARNING")
     parser.add_argument("--dry-run", required=False, help="When specified, nothing will actually be deleted from the cluster", action="store_true")
 
+
+    parser.add_argument("--coreapi-port", required=False, default=443)
+    parser.add_argument("--admin-dashboard-port", required=False, default=443)
+    parser.add_argument("--manage-api-port", required=False, default=443)
+
+
     args, unknown = parser.parse_known_args()
 
     log_level = getattr(logging, args.log_level)
@@ -56,6 +62,10 @@ if __name__ == "__main__":
     mas_workspace_id = args.mas_workspace_id
     initial_users_yaml_file = args.initial_users_yaml_file
     dry_run = args.dry_run
+    coreapi_port = args.coreapi_port
+    admin_dashboard_port = args.admin_dashboard_port
+    manage_api_port = args.manage_api_port
+
 
     logger.info("Configuration:")
     logger.info("--------------")
@@ -64,6 +74,9 @@ if __name__ == "__main__":
     logger.info(f"initial_users_yaml_file: {initial_users_yaml_file}")
     logger.info(f"log_level:               {log_level}")
     logger.info(f"dry_run:                 {dry_run}")
+    logger.info(f"coreapi_port:            {coreapi_port}")
+    logger.info(f"admin_dashboard_port:    {admin_dashboard_port}")
+    logger.info(f"manage_api_port:         {manage_api_port}")
     logger.info("")
 
     try:
@@ -75,7 +88,7 @@ if __name__ == "__main__":
         config.load_kube_config()
         logger.debug("Loaded kubeconfig file")
 
-    user_utils = MASUserUtils(mas_instance_id, mas_workspace_id, client.api_client.ApiClient())
+    user_utils = MASUserUtils(mas_instance_id, mas_workspace_id, client.api_client.ApiClient(), coreapi_port=coreapi_port, admin_dashboard_port=admin_dashboard_port, manage_api_port=manage_api_port)
 
     with open(initial_users_yaml_file, 'r') as file:
        initial_users_yaml = yaml.safe_load(file)
diff --git a/src/mas/devops/users.py b/src/mas/devops/users.py
index c11d3c2b..be053fa8 100644
--- a/src/mas/devops/users.py
+++ b/src/mas/devops/users.py
@@ -27,6 +27,8 @@
     MVI / other apps?
     unit tests
 
+    dry-run support
+
     where are we going to run this from? Needs to run in cluster, so a Job in an app
     which app though? Manage?
     Perhaps we do the core bits in a core app (suite workspace?)
@@ -46,12 +48,16 @@ class MASUserUtils():
 
     MAXADMIN = "MAXADMIN"
 
-    def __init__(self, mas_instance_id: str, mas_workspace_id: str, k8s_client: client.api_client.ApiClient):
+    def __init__(self, mas_instance_id: str, mas_workspace_id: str, k8s_client: client.api_client.ApiClient, coreapi_port: int = 443, admin_dashboard_port: int = 443, manage_api_port: int = 443):
         self.mas_instance_id = mas_instance_id
         self.mas_workspace_id = mas_workspace_id
         self.k8s_client = k8s_client
         self.logger = logging.getLogger(f"{__name__}.{self.__class__.__name__}")
 
+        self.coreapi_port = coreapi_port
+        self.admin_dashboard_port = admin_dashboard_port
+        self.manage_api_port = manage_api_port
+
         self.mas_core_namespace = f"mas-{self.mas_instance_id}-core"
         self.manage_namespace = f"mas-{self.mas_instance_id}-manage"
         self.dyn_client = DynamicClient(self.k8s_client)
@@ -90,16 +96,12 @@ def mas_superuser_credentials(self):
     @property
     def mas_admin_url_internal(self):
         if self._mas_admin_url_internal is None:
-            self._mas_admin_url_internal = f'https://admin-dashboard.{self.mas_core_namespace}.svc.cluster.local'
+            self._mas_admin_url_internal = f'https://admin-dashboard.{self.mas_core_namespace}.svc.cluster.local:{self.admin_dashboard_port}'
 
             # for local testing:
             # add to /etc/hosts:
             #    127.0.0.1               admin-dashboard.mas-tgk01-core.svc.cluster.local
             # oc port-forward service/admin-dashboard 8445:443 -n mas-tgk01-core
-
-            # uncomment for local testing
-            # TODO: make configurable
-            self._mas_admin_url_internal = f"{self._mas_admin_url_internal}:8445"
         return self._mas_admin_url_internal
 
     @property
@@ -123,16 +125,12 @@ def admin_internal_ca_pem_file_path(self):
     @property
     def mas_api_url_internal(self):
         if self._mas_api_url_internal is None:
-            self._mas_api_url_internal = f'https://coreapi.{self.mas_core_namespace}.svc.cluster.local'
+            self._mas_api_url_internal = f'https://coreapi.{self.mas_core_namespace}.svc.cluster.local:{self.coreapi_port}'
 
             # for local testing:
             # add to /etc/hosts:
             #    127.0.0.1               coreapi.mas-tgk01-core.svc.cluster.local
             # oc port-forward service/coreapi 8444:443 -n mas-tgk01-core
-
-            # uncomment for local testing
-            # TODO: make configurable
-            self._mas_api_url_internal = f"{self._mas_api_url_internal}:8444"
         return self._mas_api_url_internal
 
     @property
@@ -162,11 +160,7 @@ def manage_api_url_internal(self):
 
             # oc port-forward service/tgk01-masdev 8443:443 -n mas-tgk01-manage
 
-            self._manage_api_url_internal = f'https://{self.mas_instance_id}-{self.mas_workspace_id}.{self.manage_namespace}.svc.cluster.local'
-
-            # uncomment for local testing
-            # TODO: make configurable
-            self._manage_api_url_internal = f"{self._manage_api_url_internal}:8443"
+            self._manage_api_url_internal = f'https://{self.mas_instance_id}-{self.mas_workspace_id}.{self.manage_namespace}.svc.cluster.local:{self.manage_api_port}'
         return self._manage_api_url_internal
 
     @property

From f4558931494d8b3d8c69fe55e260cea36a5e32a4 Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Wed, 26 Mar 2025 11:57:53 +0000
Subject: [PATCH 08/44] make add_user_to_manage_group idempotent.

---
 src/mas/devops/users.py | 61 +++++++++++++++++++++++++++++++----------
 1 file changed, 47 insertions(+), 14 deletions(-)

diff --git a/src/mas/devops/users.py b/src/mas/devops/users.py
index be053fa8..37d084be 100644
--- a/src/mas/devops/users.py
+++ b/src/mas/devops/users.py
@@ -508,7 +508,7 @@ def resync_users(self, user_ids):
             raise Exception(response.text)
 
     def create_or_get_manage_api_key_for_user(self, user_id):
-        self.logger.info(f"Attempting to create Manage API Key for user {user_id}")
+        self.logger.debug(f"Attempting to create Manage API Key for user {user_id}")
         url = f"{self.manage_api_url_internal}/maximo/api/os/mxapiapikey"
         querystring = {
             "ccm": 1,
@@ -545,7 +545,9 @@ def create_or_get_manage_api_key_for_user(self, user_id):
                 # any other 400 error is unexpected
                 raise Exception(response.text)
 
-        elif response.status_code != 201:
+        elif response.status_code == 201:
+            self.logger.info(f"Creating new Manage API Key for user {user_id}")
+        else:
             # any other status code is unexpected
             raise Exception(response.text)
 
@@ -580,7 +582,6 @@ def get_manage_api_key_for_user(self, user_id):
         headers = {
             "Accept": "application/json",
         }
-        self.logger.debug(f"  > {url} {querystring}")
 
         response = requests.get(
             url,
@@ -589,16 +590,16 @@ def get_manage_api_key_for_user(self, user_id):
             verify=self.manage_internal_ca_pem_file_path,
             cert=self.manage_internal_client_pem_file_path
         )
-        self.logger.debug(f"  < {response.status_code}")
-        if response.status_code != 200:
-            raise Exception(response.text)
 
-        json = response.json()
+        if response.status_code == 200:
+            json = response.json()
 
-        if "member" in json and len(json["member"]) > 0:
-            return json["member"][0]
+            if "member" in json and len(json["member"]) > 0:
+                return json["member"][0]
 
-        return None
+            return None
+
+        raise Exception(response.text)
 
     def delete_manage_api_key(self, manage_api_key):
         self.logger.info(f"Deleting Manage API Key for user {manage_api_key['userid']}")
@@ -637,7 +638,7 @@ def get_manage_group_id(self, group_name):
         querystring = {
             "ccm": 1,
             "lean": 1,
-            "oslc.select": "*",
+            "oslc.select": "maxgroupid",
             "oslc.where": f"groupname=\"{group_name}\"",
         }
         headers = {
@@ -663,10 +664,42 @@ def get_manage_group_id(self, group_name):
 
         return None
 
+    def is_user_in_manage_group(self, group_name, user_id):
+
+        group_id = self.get_manage_group_id(group_name)
+
+        url = f"{self.manage_api_url_internal}/maximo/api/os/mxapigroup/{group_id}/groupuser"
+        querystring = {
+            "lean": 1,
+            "oslc.where": f"userid=\"{user_id}\"",
+        }
+        headers = {
+            "Accept": "application/json",
+            "apikey": self.manage_maxadmin_api_key["apikey"],  # <--- careful, don't log headers as-is (apikey is sensitive)
+        }
+
+        response = requests.get(
+            url,
+            headers=headers,
+            params=querystring,
+            verify=self.manage_internal_ca_pem_file_path,
+        )
+
+        if response.status_code == 200:
+            json = response.json()
+            return "member" in json and len(json["member"]) > 0
+
+        raise Exception(f"{response.status_code} {response.text}")
+
     def add_user_to_manage_group(self, user_id, group_name):
         '''
-        TODO: idempotency
+        No-op if user_id is already a member of the manage security group
         '''
+
+        if self.is_user_in_manage_group(group_name, user_id):
+            self.logger.info(f"User {user_id} is already a member of Manage Security Group {group_name}")
+            return None
+
         self.logger.info(f"Adding user {user_id} to Manage group {group_name}")
 
         group_id = self.get_manage_group_id(group_name)
@@ -905,5 +938,5 @@ def create_initial_user_for_saas(self, user, user_type):
         for mas_application_id in mas_application_ids:
             self.check_user_sync(user_id, mas_application_id)
 
-        # if "manage" in mas_application_ids:
-        #     self.add_user_to_manage_group(user_id, "MAXADMIN")
+        if "manage" in mas_application_ids:
+            self.add_user_to_manage_group(user_id, "MAXADMIN")

From 141a8071112ea2b7489ab49381fdcb5853421f00 Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Wed, 26 Mar 2025 12:35:34 +0000
Subject: [PATCH 09/44] set different manage security groups based on user type

---
 src/mas/devops/users.py | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/src/mas/devops/users.py b/src/mas/devops/users.py
index 37d084be..1be72310 100644
--- a/src/mas/devops/users.py
+++ b/src/mas/devops/users.py
@@ -326,6 +326,8 @@ def link_user_to_local_idp(self, user_id, email_password=False):
         if response.status_code != 200:
             raise Exception(response.text)
 
+        # Important: HTTP 200 output will contain generated user token; DO NOT LOG
+
         return None
 
     def get_user(self, user_id):
@@ -645,15 +647,12 @@ def get_manage_group_id(self, group_name):
             "Accept": "application/json",
             "apikey": self.manage_maxadmin_api_key["apikey"],  # <--- careful, don't log headers as-is (apikey is sensitive)
         }
-        self.logger.debug(f"  > {url} {querystring}")
-
         response = requests.get(
             url,
             headers=headers,
             params=querystring,
             verify=self.manage_internal_ca_pem_file_path,
         )
-        self.logger.debug(f"  < {response.status_code}")
         if response.status_code != 200:
             raise Exception(response.text)
 
@@ -885,6 +884,8 @@ def create_initial_user_for_saas(self, user, user_type):
             }
             is_workspace_admin = True
             application_role = "ADMINISTRATOR"
+            # TODO: check which security groups primary users should be members of
+            manage_security_groups = ["MAXADMIN"]
         elif user_type == "SECONDARY":
             permissions = {
                 "systemAdmin": False,
@@ -898,6 +899,8 @@ def create_initial_user_for_saas(self, user, user_type):
             }
             is_workspace_admin = False
             application_role = "USER"
+            # TODO: check which security groups secondary users should be members of
+            manage_security_groups = []
         else:
             raise Exception(f"Unsupported user_type: {user_type}")
 
@@ -930,8 +933,10 @@ def create_initial_user_for_saas(self, user, user_type):
         for mas_application_id in mas_application_ids:
             self.await_mas_application_availability(mas_application_id)
             if mas_application_id == "manage":
+                # special case for manage; role is always "MANAGEUSER"
                 role = "MANAGEUSER"
             else:
+                # otherwise grant the user the appropriate role for their user_type
                 role = application_role
             self.set_user_application_permission(user_id, mas_application_id, role)
 
@@ -939,4 +944,5 @@ def create_initial_user_for_saas(self, user, user_type):
             self.check_user_sync(user_id, mas_application_id)
 
         if "manage" in mas_application_ids:
-            self.add_user_to_manage_group(user_id, "MAXADMIN")
+            for manage_security_group in manage_security_groups:
+                self.add_user_to_manage_group(user_id, manage_security_group)

From f828dac511282087bec050a8456b1f381d78cb89 Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Thu, 27 Mar 2025 13:07:14 +0000
Subject: [PATCH 10/44] Add ability to source inital user configuration from
 AWS SM

---
 README.md                                    | 41 +++++++++++
 bin/mas-devops-create-initial-users-for-saas | 71 +++++++++++++-------
 setup.py                                     |  3 +-
 src/mas/devops/users.py                      | 31 ++++++++-
 4 files changed, 120 insertions(+), 26 deletions(-)

diff --git a/README.md b/README.md
index 94f7a830..b43a0e03 100644
--- a/README.md
+++ b/README.md
@@ -37,3 +37,44 @@ updateTektonDefinitions(pipelinesNamespace, "/mascli/templates/ibm-mas-tekton.ya
 pipelineURL = launchUpgradePipeline(self.dynamicClient, instanceId)
 print(pipelineURL)
 ```
+
+
+mas-devops-create-initial-users
+---------------------------------------------
+
+
+```bash
+SM_AWS_REGION=""
+SM_AWS_ACCESS_KEY_ID=""
+SM_AWS_SECRET_ACCESS_KEY=""
+
+aws configure set default.region ${SM_AWS_REGION}
+aws configure set aws_access_key_id ${SM_AWS_ACCESS_KEY_ID}
+aws configure set aws_secret_access_key ${SM_AWS_SECRET_ACCESS_KEY}
+
+
+oc login --token=sha256~xxx --server=https://xxx:6443
+
+oc port-forward service/admin-dashboard 8445:443 -n mas-tgk01-core
+oc port-forward service/coreapi 8444:443 -n mas-tgk01-core
+oc port-forward service/tgk01-masdev 8443:443 -n mas-tgk01-manage
+
+mas-devops-create-initial-users-for-saas \
+    --mas-instance-id tgk01 \
+    --mas-workspace-id masdev \
+    --log-level INFO \
+    --initial-users-secret-name "aws-dev/noble4/tgk01/initial_users" \
+    --manage-api-port 8443 \
+    --coreapi-port 8444 \
+    --admin-dashboard-port 8445
+    
+
+mas-devops-create-initial-users-for-saas \
+    --mas-instance-id tgk01 \
+    --mas-workspace-id masdev \
+    --log-level INFO \
+    --initial-users-yaml-file /home/tom/workspaces/notes/mascore3423/example-users-single.yaml \
+    --manage-api-port 8443 \
+    --coreapi-port 8444 \
+    --admin-dashboard-port 8445
+```
\ No newline at end of file
diff --git a/bin/mas-devops-create-initial-users-for-saas b/bin/mas-devops-create-initial-users-for-saas
index cd4727e7..11e4fa7d 100644
--- a/bin/mas-devops-create-initial-users-for-saas
+++ b/bin/mas-devops-create-initial-users-for-saas
@@ -16,11 +16,12 @@ import argparse
 import logging
 import urllib3
 urllib3.disable_warnings()
-import os
-import sys
-import json
-import re
 import yaml
+import json
+import sys
+
+import boto3
+from botocore.exceptions import ClientError
 
 from mas.devops.users import MASUserUtils
 
@@ -30,18 +31,20 @@ if __name__ == "__main__":
     parser = argparse.ArgumentParser()
 
     # Primary Options
+    parser.add_argument("--mas-account-id", required=False) # TODO: remove if unused
+    parser.add_argument("--mas-cluster-id", required=False) # TODO: remove if unused
     parser.add_argument("--mas-instance-id", required=True)
     parser.add_argument("--mas-workspace-id", required=True)
-    parser.add_argument("--initial-users-yaml-file", required=True)
     parser.add_argument("--log-level", required=False, choices=["DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL"], default="WARNING")
-    parser.add_argument("--dry-run", required=False, help="When specified, nothing will actually be deleted from the cluster", action="store_true")
-
-
     parser.add_argument("--coreapi-port", required=False, default=443)
     parser.add_argument("--admin-dashboard-port", required=False, default=443)
     parser.add_argument("--manage-api-port", required=False, default=443)
 
 
+    group = parser.add_mutually_exclusive_group(required=True)
+    group.add_argument("--initial-users-yaml-file")
+    group.add_argument("--initial-users-secret-name")
+
     args, unknown = parser.parse_known_args()
 
     log_level = getattr(logging, args.log_level)
@@ -57,11 +60,12 @@ if __name__ == "__main__":
     ch.setFormatter(chFormatter)
     logger.addHandler(ch)
 
-
+    mas_account_id = args.mas_account_id
+    mas_cluster_id = args.mas_cluster_id
     mas_instance_id = args.mas_instance_id
     mas_workspace_id = args.mas_workspace_id
     initial_users_yaml_file = args.initial_users_yaml_file
-    dry_run = args.dry_run
+    initial_users_secret_name = args.initial_users_secret_name
     coreapi_port = args.coreapi_port
     admin_dashboard_port = args.admin_dashboard_port
     manage_api_port = args.manage_api_port
@@ -69,14 +73,16 @@ if __name__ == "__main__":
 
     logger.info("Configuration:")
     logger.info("--------------")
-    logger.info(f"mas_instance_id:         {mas_instance_id}")
-    logger.info(f"mas_workspace_id:        {mas_workspace_id}")
-    logger.info(f"initial_users_yaml_file: {initial_users_yaml_file}")
-    logger.info(f"log_level:               {log_level}")
-    logger.info(f"dry_run:                 {dry_run}")
-    logger.info(f"coreapi_port:            {coreapi_port}")
-    logger.info(f"admin_dashboard_port:    {admin_dashboard_port}")
-    logger.info(f"manage_api_port:         {manage_api_port}")
+    logger.info(f"mas_account_id:            {mas_account_id}")
+    logger.info(f"mas_cluster_id:            {mas_cluster_id}")
+    logger.info(f"mas_instance_id:           {mas_instance_id}")
+    logger.info(f"mas_workspace_id:          {mas_workspace_id}")
+    logger.info(f"initial_users_yaml_file:   {initial_users_yaml_file}")
+    logger.info(f"initial_users_secret_name: {initial_users_secret_name}")
+    logger.info(f"log_level:                 {log_level}")
+    logger.info(f"coreapi_port:              {coreapi_port}")
+    logger.info(f"admin_dashboard_port:      {admin_dashboard_port}")
+    logger.info(f"manage_api_port:           {manage_api_port}")
     logger.info("")
 
     try:
@@ -88,11 +94,30 @@ if __name__ == "__main__":
         config.load_kube_config()
         logger.debug("Loaded kubeconfig file")
 
-    user_utils = MASUserUtils(mas_instance_id, mas_workspace_id, client.api_client.ApiClient(), coreapi_port=coreapi_port, admin_dashboard_port=admin_dashboard_port, manage_api_port=manage_api_port)
-
-    with open(initial_users_yaml_file, 'r') as file:
-       initial_users_yaml = yaml.safe_load(file)
 
+    user_utils = MASUserUtils(mas_instance_id, mas_workspace_id, client.api_client.ApiClient(), coreapi_port=coreapi_port, admin_dashboard_port=admin_dashboard_port, manage_api_port=manage_api_port)
 
-    user_utils.create_initial_users_for_saas(initial_users_yaml)
+    if initial_users_secret_name is not None:
+
+        session = boto3.session.Session()
+        aws_sm_client = session.client(
+            service_name='secretsmanager',
+        )
+        try:
+            initial_users_secret = aws_sm_client.get_secret_value( # pragma: allowlist secret
+                SecretId=initial_users_secret_name
+            )
+        except ClientError as e:
+            raise Exception(f"Failed to fetch secret {initial_users_secret_name}: {str(e)}")
+        
+        secret_json = json.loads(initial_users_secret['SecretString'])
+        initial_users = user_utils.parse_initial_users_from_aws_secret_json(secret_json)
+    elif initial_users_yaml_file is not None:
+        with open(initial_users_yaml_file, 'r') as file:
+            initial_users = yaml.safe_load(file)
+    else:
+        raise Exception("Something unexpected happened")
+    
+    user_utils.create_initial_users_for_saas(initial_users)
+   
 
diff --git a/setup.py b/setup.py
index 4818aaf5..0c3364c2 100644
--- a/setup.py
+++ b/setup.py
@@ -60,7 +60,8 @@ def get_version(rel_path):
         'kubernetes',              # Apache Software License
         'kubeconfig',              # BSD License
         'jinja2',                  # BSD License
-        'jinja2-base64-filters'    # MIT License
+        'jinja2-base64-filters',    # MIT License
+        'boto3'
     ],
     extras_require={
         'dev': [
diff --git a/src/mas/devops/users.py b/src/mas/devops/users.py
index 1be72310..ce464c98 100644
--- a/src/mas/devops/users.py
+++ b/src/mas/devops/users.py
@@ -27,8 +27,6 @@
     MVI / other apps?
     unit tests
 
-    dry-run support
-
     where are we going to run this from? Needs to run in cluster, so a Job in an app
     which app though? Manage?
     Perhaps we do the core bits in a core app (suite workspace?)
@@ -830,6 +828,35 @@ def await_mas_application_availability(self, mas_application_id, timeout_secs=60
                 time.sleep(5)
         raise Exception(f"{mas_application_id} did not become ready and available in time, aborting")
 
+    def parse_initial_users_from_aws_secret_json(self, secret_json):
+        primary = []
+        secondary = []
+        for (email, csv) in secret_json.items():
+            values = csv.split(",")
+            user_type = values[0].strip()
+            given_name = values[1].strip()
+            family_name = values[2].strip()
+
+            user = {
+                "email": email,
+                "given_name": given_name,
+                "family_name": family_name
+            }
+            if user_type == "primary":
+                primary.append(user)
+            elif user_type == "secondary":
+                secondary.append(user)
+            else:
+                raise Exception(f"Unknown user type for {email}: {user_type}")
+
+        initial_users = {
+            "users": {
+                "primary": primary,
+                "secondary": secondary
+            }
+        }
+        return initial_users
+
     def create_initial_users_for_saas(self, initial_users):
 
         # Validate input

From f2f364e17a2df2d188af665cdcc4e2a91dd2b70d Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Thu, 27 Mar 2025 16:23:18 +0000
Subject: [PATCH 11/44] verify application availability up front

---
 src/mas/devops/users.py | 119 ++++++++++++++++++++++------------------
 1 file changed, 66 insertions(+), 53 deletions(-)

diff --git a/src/mas/devops/users.py b/src/mas/devops/users.py
index ce464c98..fac157e0 100644
--- a/src/mas/devops/users.py
+++ b/src/mas/devops/users.py
@@ -81,6 +81,8 @@ def __init__(self, mas_instance_id: str, mas_workspace_id: str, k8s_client: clie
 
         self._manage_maxadmin_api_key = None
 
+        self._mas_workspace_application_ids = None
+
     @property
     def mas_superuser_credentials(self):
         if self._mas_superuser_credentials is None:
@@ -221,6 +223,12 @@ def manage_maxadmin_api_key(self):
             self._manage_maxadmin_api_key = self.create_or_get_manage_api_key_for_user(MASUserUtils.MAXADMIN)
         return self._manage_maxadmin_api_key
 
+    @property
+    def mas_workspace_application_ids(self):
+        if self._mas_workspace_application_ids is None:
+            self._mas_workspace_application_ids = list(map(lambda ma: ma["id"], self.get_mas_applications_in_workspace()))
+        return self._mas_workspace_application_ids
+
     def get_or_create_user(self, payload):
         '''
         User is identified by payload["id"] field
@@ -735,54 +743,6 @@ def add_user_to_manage_group(self, user_id, group_name):
 
         raise Exception(f"{response.status_code} {response.text}")
 
-    def get_groups(self):
-        self.logger.debug("Getting groups")
-        url = f"{self.mas_api_url_internal}/groups"
-        headers = {
-            "Accept": "application/json",
-            "x-access-token": self.superuser_auth_token
-        }
-        response = requests.get(
-            url,
-            headers=headers,
-            verify=self.core_internal_ca_pem_file_path
-        )
-        if response.status_code == 200:
-            return response.json()
-        raise Exception(f"{response.status_code} {response.text}")
-
-    def get_user_groups(self, user_id):
-        self.logger.info(f"Getting groups for user {user_id}")
-        url = f"{self.mas_api_url_internal}/v3/users/{user_id}/groups"
-        headers = {
-            "Accept": "application/json",
-            "x-access-token": self.superuser_auth_token
-        }
-        response = requests.get(
-            url,
-            headers=headers,
-            verify=self.core_internal_ca_pem_file_path
-        )
-        if response.status_code == 200:
-            return response.json()
-        raise Exception(f"{response.status_code} {response.text}")
-
-    def get_installed_mas_applications(self):
-        self.logger.debug("Getting installed MAS Applications")
-        url = f"{self.mas_api_url_internal}/applications"
-        headers = {
-            "Accept": "application/json",
-            "x-access-token": self.superuser_auth_token
-        }
-        response = requests.get(
-            url,
-            headers=headers,
-            verify=self.core_internal_ca_pem_file_path
-        )
-        if response.status_code == 200:
-            return response.json()
-        raise Exception(f"{response.status_code} {response.text}")
-
     def get_mas_applications_in_workspace(self):
         self.logger.debug(f"Getting MAS Applications in workspace {self.mas_workspace_id}")
         url = f"{self.mas_api_url_internal}/workspaces/{self.mas_workspace_id}/applications"
@@ -874,6 +834,10 @@ def create_initial_users_for_saas(self, initial_users):
         if type(secondary_users) is not list:
             raise Exception("'users.secondary' is not a list")
 
+        # before we do anything, let's check all MAS applications are ready
+        for mas_application_id in self.mas_workspace_application_ids:
+            self.await_mas_application_availability(mas_application_id)
+
         for primary_user in primary_users:
             self.create_initial_user_for_saas(primary_user, "PRIMARY")
 
@@ -955,9 +919,7 @@ def create_initial_user_for_saas(self, user, user_type):
         self.link_user_to_local_idp(user_id)
         self.add_user_to_workspace(user_id, is_workspace_admin=is_workspace_admin)
 
-        mas_application_ids = list(map(lambda ma: ma["id"], self.get_mas_applications_in_workspace()))
-
-        for mas_application_id in mas_application_ids:
+        for mas_application_id in self.mas_workspace_application_ids:
             self.await_mas_application_availability(mas_application_id)
             if mas_application_id == "manage":
                 # special case for manage; role is always "MANAGEUSER"
@@ -967,9 +929,60 @@ def create_initial_user_for_saas(self, user, user_type):
                 role = application_role
             self.set_user_application_permission(user_id, mas_application_id, role)
 
-        for mas_application_id in mas_application_ids:
+        for mas_application_id in self.mas_workspace_application_ids:
             self.check_user_sync(user_id, mas_application_id)
 
-        if "manage" in mas_application_ids:
+        if "manage" in self.mas_workspace_application_ids:
             for manage_security_group in manage_security_groups:
                 self.add_user_to_manage_group(user_id, manage_security_group)
+
+    # Unused (but potentially useful) methods
+    # ----------------------------------------
+
+    def get_groups(self):
+        self.logger.debug("Getting groups")
+        url = f"{self.mas_api_url_internal}/groups"
+        headers = {
+            "Accept": "application/json",
+            "x-access-token": self.superuser_auth_token
+        }
+        response = requests.get(
+            url,
+            headers=headers,
+            verify=self.core_internal_ca_pem_file_path
+        )
+        if response.status_code == 200:
+            return response.json()
+        raise Exception(f"{response.status_code} {response.text}")
+
+    def get_installed_mas_applications(self):
+        self.logger.debug("Getting installed MAS Applications")
+        url = f"{self.mas_api_url_internal}/applications"
+        headers = {
+            "Accept": "application/json",
+            "x-access-token": self.superuser_auth_token
+        }
+        response = requests.get(
+            url,
+            headers=headers,
+            verify=self.core_internal_ca_pem_file_path
+        )
+        if response.status_code == 200:
+            return response.json()
+        raise Exception(f"{response.status_code} {response.text}")
+
+    def get_user_groups(self, user_id):
+        self.logger.info(f"Getting groups for user {user_id}")
+        url = f"{self.mas_api_url_internal}/v3/users/{user_id}/groups"
+        headers = {
+            "Accept": "application/json",
+            "x-access-token": self.superuser_auth_token
+        }
+        response = requests.get(
+            url,
+            headers=headers,
+            verify=self.core_internal_ca_pem_file_path
+        )
+        if response.status_code == 200:
+            return response.json()
+        raise Exception(f"{response.status_code} {response.text}")

From 28fbe6cf8c4858d30360dbe7a1ee39d1e597861e Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Thu, 27 Mar 2025 16:58:01 +0000
Subject: [PATCH 12/44] remove completed users from initial_users secret

so that they are never resynced a second time (potentially undoing a customer update)
---
 README.md                                    |  5 +++
 bin/mas-devops-create-initial-users-for-saas | 24 +++++++++++-
 src/mas/devops/users.py                      | 39 +++++++++++++++++++-
 3 files changed, 64 insertions(+), 4 deletions(-)

diff --git a/README.md b/README.md
index b43a0e03..1894c137 100644
--- a/README.md
+++ b/README.md
@@ -77,4 +77,9 @@ mas-devops-create-initial-users-for-saas \
     --manage-api-port 8443 \
     --coreapi-port 8444 \
     --admin-dashboard-port 8445
+```
+
+Example of initial_users secret:
+```json
+{"john.smith1@example.com":"primary,john1,smith1","john.smith2@example.com":"primary,john2,smith2","john.smith3@example.com":"secondary,john3,smith3"}
 ```
\ No newline at end of file
diff --git a/bin/mas-devops-create-initial-users-for-saas b/bin/mas-devops-create-initial-users-for-saas
index 11e4fa7d..92aa5b7f 100644
--- a/bin/mas-devops-create-initial-users-for-saas
+++ b/bin/mas-devops-create-initial-users-for-saas
@@ -99,6 +99,8 @@ if __name__ == "__main__":
 
     if initial_users_secret_name is not None:
 
+        logger.info(f"Loading initial_users configuration from secret {initial_users_secret_name}")
+
         session = boto3.session.Session()
         aws_sm_client = session.client(
             service_name='secretsmanager',
@@ -118,6 +120,24 @@ if __name__ == "__main__":
     else:
         raise Exception("Something unexpected happened")
     
-    user_utils.create_initial_users_for_saas(initial_users)
-   
+    
+    result = user_utils.create_initial_users_for_saas(initial_users)
 
+    # if user details were sourced from an AWS SM secret, remove the completed entries from the secret
+    # so we don't try and resync them the next time round (and potentially undo an update made by a customer)
+    if initial_users_secret_name is not None:
+        has_updates = False
+        for completed_user in result["completed"]:
+            logger.info(f"Removing synced user {completed_user['email']} from {initial_users_secret_name} secret")
+            secret_json.pop(completed_user["email"])
+            has_updates = True
+
+        if has_updates:
+            logger.info(f"Updating secret {initial_users_secret_name}")
+            try:
+                aws_sm_client.update_secret( # pragma: allowlist secret
+                    SecretId=initial_users_secret_name,
+                    SecretString=json.dumps(secret_json)
+                )
+            except ClientError as e:
+                raise Exception(f"Failed to update secret {initial_users_secret_name}: {str(e)}")
\ No newline at end of file
diff --git a/src/mas/devops/users.py b/src/mas/devops/users.py
index fac157e0..7f9514db 100644
--- a/src/mas/devops/users.py
+++ b/src/mas/devops/users.py
@@ -793,6 +793,10 @@ def parse_initial_users_from_aws_secret_json(self, secret_json):
         secondary = []
         for (email, csv) in secret_json.items():
             values = csv.split(",")
+
+            if len(values) != 3:
+                raise Exception(f"Wrong number of CSV values for {email} (expected 3 but got {len(values)})")
+
             user_type = values[0].strip()
             given_name = values[1].strip()
             family_name = values[2].strip()
@@ -834,15 +838,46 @@ def create_initial_users_for_saas(self, initial_users):
         if type(secondary_users) is not list:
             raise Exception("'users.secondary' is not a list")
 
+        if len(primary_users) == 0 and len(secondary_users) == 0:
+            self.logger.info("No users left to sync, nothing to do")
+            return {"completed": [], "failed": []}
+
         # before we do anything, let's check all MAS applications are ready
         for mas_application_id in self.mas_workspace_application_ids:
             self.await_mas_application_availability(mas_application_id)
 
+        completed = []
+        failed = []
+
         for primary_user in primary_users:
-            self.create_initial_user_for_saas(primary_user, "PRIMARY")
+            self.logger.info("")
+            try:
+                self.logger.info(f"Syncing primary user {primary_user['email']}")
+                self.create_initial_user_for_saas(primary_user, "PRIMARY")
+                completed.append(primary_user)
+                self.logger.info(f"Completed sync of primary user {primary_user['email']}")
+            except Exception as e:
+                self.logger.error(f"Sync of primary user {primary_user['email']} failed: {str(e)}")
+                failed.append(primary_user)
+            self.logger.info("")
 
         for secondary_user in secondary_users:
-            self.create_initial_user_for_saas(secondary_user, "SECONDARY")
+            self.logger.info("")
+            try:
+                self.logger.info("")
+                self.logger.info(f"Syncing secondary user {secondary_user['email']}")
+                self.create_initial_user_for_saas(secondary_user, "SECONDARY")
+                completed.append(secondary_user)
+                self.logger.info(f"Completed sync of secondary user {secondary_user['email']}")
+            except Exception as e:
+                self.logger.error(f"Sync of secondary user {secondary_user['email']} failed: {str(e)}")
+                failed.append(secondary_user)
+            self.logger.info("")
+
+        return {
+            "completed": completed,
+            "failed": failed
+        }
 
     def create_initial_user_for_saas(self, user, user_type):
         if "email" not in user:

From 6272517c97d153406dbc9ee24be91645a2d240d6 Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Thu, 27 Mar 2025 16:59:49 +0000
Subject: [PATCH 13/44] email_password=true

---
 src/mas/devops/users.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mas/devops/users.py b/src/mas/devops/users.py
index 7f9514db..bd32895d 100644
--- a/src/mas/devops/users.py
+++ b/src/mas/devops/users.py
@@ -951,7 +951,7 @@ def create_initial_user_for_saas(self, user, user_type):
         }
 
         self.get_or_create_user(user_def)
-        self.link_user_to_local_idp(user_id)
+        self.link_user_to_local_idp(user_id, email_password=True)
         self.add_user_to_workspace(user_id, is_workspace_admin=is_workspace_admin)
 
         for mas_application_id in self.mas_workspace_application_ids:

From 53e1ac5c311190c26d63d1043df532cc44d7c94a Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Fri, 28 Mar 2025 12:53:34 +0000
Subject: [PATCH 14/44] tweaks

---
 README.md               | 7 +++++++
 src/mas/devops/users.py | 6 ++++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/README.md b/README.md
index 1894c137..869f56cb 100644
--- a/README.md
+++ b/README.md
@@ -43,6 +43,13 @@ mas-devops-create-initial-users
 ---------------------------------------------
 
 
+Add to /etc/hosts
+```
+127.0.0.1               tgk01-masdev.mas-tgk01-manage.svc.cluster.local
+127.0.0.1               coreapi.mas-tgk01-core.svc.cluster.local
+127.0.0.1               admin-dashboard.mas-tgk01-core.svc.cluster.local
+```
+
 ```bash
 SM_AWS_REGION=""
 SM_AWS_ACCESS_KEY_ID=""
diff --git a/src/mas/devops/users.py b/src/mas/devops/users.py
index bd32895d..a4a87f1d 100644
--- a/src/mas/devops/users.py
+++ b/src/mas/devops/users.py
@@ -32,8 +32,11 @@
     Perhaps we do the core bits in a core app (suite workspace?)
     And app-specific bits in the apps themselves
     but if we create a user before manage is ready, sync fails, and no way to trigger resync in MAS < 9.1.0?
-
     # could do it all from a core app, but check for readiness of apps
+    Run in a dedicated instance-root app in the final syncwave
+
+    how ot cope with users that have been soft-deleted? tolerate / skip - ensure they are removed from secret so
+    we don't get caught in an infinite loop?
 
 '''
 
@@ -59,7 +62,6 @@ def __init__(self, mas_instance_id: str, mas_workspace_id: str, k8s_client: clie
         self.mas_core_namespace = f"mas-{self.mas_instance_id}-core"
         self.manage_namespace = f"mas-{self.mas_instance_id}-manage"
         self.dyn_client = DynamicClient(self.k8s_client)
-        self.v1_routes = self.dyn_client.resources.get(api_version="route.openshift.io/v1", kind="Route")
         self.v1_secrets = self.dyn_client.resources.get(api_version="v1", kind="Secret")
 
         self._mas_superuser_credentials = None

From 601746a8a56f1a50a2037bc5befe52865b642b2e Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Fri, 28 Mar 2025 14:59:41 +0000
Subject: [PATCH 15/44] fix

---
 src/mas/devops/users.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mas/devops/users.py b/src/mas/devops/users.py
index a4a87f1d..d94d0fe7 100644
--- a/src/mas/devops/users.py
+++ b/src/mas/devops/users.py
@@ -274,7 +274,7 @@ def get_or_create_user(self, payload):
             self.logger.info(f"Existing user {existing_user['id']} found")
             return existing_user
 
-        self.logger.info(f"Creating new user {payload["id"]}")
+        self.logger.info(f"Creating new user {payload['id']}")
 
         url = f"{self.mas_api_url_internal}/v3/users"
         querystring = {}

From ac4b69f38ebfbcf92bae39b36f6c24f4a490999b Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Fri, 28 Mar 2025 16:27:16 +0000
Subject: [PATCH 16/44] ensure rc 1 reported if >0 failures

---
 .gitignore                                   | 1 +
 bin/mas-devops-create-initial-users-for-saas | 7 ++++++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/.gitignore b/.gitignore
index ed45b735..6ca3502c 100644
--- a/.gitignore
+++ b/.gitignore
@@ -14,6 +14,7 @@ README.rst
 # Environments
 .venv/
 venv/
+.venv39/
 
 # Other
 kubectl.exe
diff --git a/bin/mas-devops-create-initial-users-for-saas b/bin/mas-devops-create-initial-users-for-saas
index 92aa5b7f..98d1eaff 100644
--- a/bin/mas-devops-create-initial-users-for-saas
+++ b/bin/mas-devops-create-initial-users-for-saas
@@ -140,4 +140,9 @@ if __name__ == "__main__":
                     SecretString=json.dumps(secret_json)
                 )
             except ClientError as e:
-                raise Exception(f"Failed to update secret {initial_users_secret_name}: {str(e)}")
\ No newline at end of file
+                raise Exception(f"Failed to update secret {initial_users_secret_name}: {str(e)}")
+            
+
+        if len(result["failed"]) > 0:
+            failed_user_ids = list(map(lambda u : u["email"], result["failed"]))
+            raise Exception(f"Sync failed for the following user IDs {failed_user_ids}")
\ No newline at end of file

From 9b47eb97131933eacc74d75c73d9e5d37bb68d70 Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Fri, 28 Mar 2025 16:43:29 +0000
Subject: [PATCH 17/44] set INFO log level default

---
 bin/mas-devops-create-initial-users-for-saas | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bin/mas-devops-create-initial-users-for-saas b/bin/mas-devops-create-initial-users-for-saas
index 98d1eaff..5628d0e0 100644
--- a/bin/mas-devops-create-initial-users-for-saas
+++ b/bin/mas-devops-create-initial-users-for-saas
@@ -35,7 +35,7 @@ if __name__ == "__main__":
     parser.add_argument("--mas-cluster-id", required=False) # TODO: remove if unused
     parser.add_argument("--mas-instance-id", required=True)
     parser.add_argument("--mas-workspace-id", required=True)
-    parser.add_argument("--log-level", required=False, choices=["DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL"], default="WARNING")
+    parser.add_argument("--log-level", required=False, choices=["DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL"], default="INFO")
     parser.add_argument("--coreapi-port", required=False, default=443)
     parser.add_argument("--admin-dashboard-port", required=False, default=443)
     parser.add_argument("--manage-api-port", required=False, default=443)

From 9113aaabbed88d773ad1058f1440a3c4e34ac47f Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Mon, 31 Mar 2025 09:35:31 +0100
Subject: [PATCH 18/44] also report failures in yaml-file mode

---
 bin/mas-devops-create-initial-users-for-saas | 6 +++---
 src/mas/devops/users.py                      | 1 -
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/bin/mas-devops-create-initial-users-for-saas b/bin/mas-devops-create-initial-users-for-saas
index 5628d0e0..4c7a8fd2 100644
--- a/bin/mas-devops-create-initial-users-for-saas
+++ b/bin/mas-devops-create-initial-users-for-saas
@@ -143,6 +143,6 @@ if __name__ == "__main__":
                 raise Exception(f"Failed to update secret {initial_users_secret_name}: {str(e)}")
             
 
-        if len(result["failed"]) > 0:
-            failed_user_ids = list(map(lambda u : u["email"], result["failed"]))
-            raise Exception(f"Sync failed for the following user IDs {failed_user_ids}")
\ No newline at end of file
+    if len(result["failed"]) > 0:
+        failed_user_ids = list(map(lambda u : u["email"], result["failed"]))
+        raise Exception(f"Sync failed for the following user IDs {failed_user_ids}")
\ No newline at end of file
diff --git a/src/mas/devops/users.py b/src/mas/devops/users.py
index d94d0fe7..4bf04aa8 100644
--- a/src/mas/devops/users.py
+++ b/src/mas/devops/users.py
@@ -861,7 +861,6 @@ def create_initial_users_for_saas(self, initial_users):
             except Exception as e:
                 self.logger.error(f"Sync of primary user {primary_user['email']} failed: {str(e)}")
                 failed.append(primary_user)
-            self.logger.info("")
 
         for secondary_user in secondary_users:
             self.logger.info("")

From db3c76b389927f3c57acda8e7915c4154e1214ae Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Mon, 31 Mar 2025 16:27:55 +0100
Subject: [PATCH 19/44] WIP: unit tests

---
 setup.py                |  13 +-
 src/mas/devops/users.py | 104 +++++--------
 test/src/test_users.py  | 325 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 366 insertions(+), 76 deletions(-)
 create mode 100644 test/src/test_users.py

diff --git a/setup.py b/setup.py
index 0c3364c2..6e10b471 100644
--- a/setup.py
+++ b/setup.py
@@ -60,15 +60,16 @@ def get_version(rel_path):
         'kubernetes',              # Apache Software License
         'kubeconfig',              # BSD License
         'jinja2',                  # BSD License
-        'jinja2-base64-filters',    # MIT License
-        'boto3'
+        'jinja2-base64-filters',   # MIT License
+        'boto3'                    # Apache Software License
     ],
     extras_require={
         'dev': [
-            'build',       # MIT License
-            'flake8',      # MIT License
-            'pytest',      # MIT License
-            'pytest-mock'  # MIT License
+            'build',        # MIT License
+            'flake8',       # MIT License
+            'pytest',       # MIT License
+            'pytest-mock',  # MIT License
+            'requests-mock'  # Apache Software License
         ]
     },
     classifiers=[
diff --git a/src/mas/devops/users.py b/src/mas/devops/users.py
index 4bf04aa8..75c3d188 100644
--- a/src/mas/devops/users.py
+++ b/src/mas/devops/users.py
@@ -52,31 +52,26 @@ class MASUserUtils():
     def __init__(self, mas_instance_id: str, mas_workspace_id: str, k8s_client: client.api_client.ApiClient, coreapi_port: int = 443, admin_dashboard_port: int = 443, manage_api_port: int = 443):
         self.mas_instance_id = mas_instance_id
         self.mas_workspace_id = mas_workspace_id
-        self.k8s_client = k8s_client
         self.logger = logging.getLogger(f"{__name__}.{self.__class__.__name__}")
 
-        self.coreapi_port = coreapi_port
-        self.admin_dashboard_port = admin_dashboard_port
-        self.manage_api_port = manage_api_port
-
         self.mas_core_namespace = f"mas-{self.mas_instance_id}-core"
         self.manage_namespace = f"mas-{self.mas_instance_id}-manage"
-        self.dyn_client = DynamicClient(self.k8s_client)
-        self.v1_secrets = self.dyn_client.resources.get(api_version="v1", kind="Secret")
+
+        dyn_client = DynamicClient(k8s_client)
+        self.v1_secrets = dyn_client.resources.get(api_version="v1", kind="Secret")
 
         self._mas_superuser_credentials = None
+        self._superuser_auth_token = None
 
-        self._mas_admin_url_internal = None
+        self.mas_admin_url_internal = f'https://admin-dashboard.{self.mas_core_namespace}.svc.cluster.local:{admin_dashboard_port}'
         self._admin_internal_tls_secret = None
         self._admin_internal_ca_pem_file_path = None
 
-        self._mas_api_url_internal = None
+        self.mas_api_url_internal = f'https://coreapi.{self.mas_core_namespace}.svc.cluster.local:{coreapi_port}'
         self._core_internal_tls_secret = None
         self._core_internal_ca_pem_file_path = None
 
-        self._superuser_auth_token = None
-
-        self._manage_api_url_internal = None
+        self.manage_api_url_internal = f'https://{self.mas_instance_id}-{self.mas_workspace_id}.{self.manage_namespace}.svc.cluster.local:{manage_api_port}'
         self._manage_internal_tls_secret = None
         self._manage_internal_ca_pem_file_path = None
         self._manage_internal_client_pem_file_path = None
@@ -90,22 +85,11 @@ def mas_superuser_credentials(self):
         if self._mas_superuser_credentials is None:
             k8s_secret = self.v1_secrets.get(name=f"{self.mas_instance_id}-credentials-superuser", namespace=self.mas_core_namespace)
             self._mas_superuser_credentials = dict(
-                username=base64.b64decode(str(k8s_secret.data.username)).decode("utf-8"),
-                password=base64.b64decode(str(k8s_secret.data.password)).decode("utf-8"),
+                username=base64.b64decode(k8s_secret.data["username"]).decode("utf-8"),
+                password=base64.b64decode(k8s_secret.data["password"]).decode("utf-8"),
             )
         return self._mas_superuser_credentials
 
-    @property
-    def mas_admin_url_internal(self):
-        if self._mas_admin_url_internal is None:
-            self._mas_admin_url_internal = f'https://admin-dashboard.{self.mas_core_namespace}.svc.cluster.local:{self.admin_dashboard_port}'
-
-            # for local testing:
-            # add to /etc/hosts:
-            #    127.0.0.1               admin-dashboard.mas-tgk01-core.svc.cluster.local
-            # oc port-forward service/admin-dashboard 8445:443 -n mas-tgk01-core
-        return self._mas_admin_url_internal
-
     @property
     def admin_internal_tls_secret(self):
         if self._admin_internal_tls_secret is None:
@@ -115,7 +99,7 @@ def admin_internal_tls_secret(self):
     @property
     def admin_internal_ca_pem_file_path(self):
         if self._admin_internal_ca_pem_file_path is None:
-            ca = base64.b64decode(self.core_internal_tls_secret.data["ca.crt"]).decode('utf-8')
+            ca = base64.b64decode(self.admin_internal_tls_secret.data["ca.crt"]).decode('utf-8')
             with tempfile.NamedTemporaryFile(delete=False, suffix=".pem") as pem_file:
                 pem_file.write(ca.encode())
                 pem_file.flush()
@@ -124,17 +108,6 @@ def admin_internal_ca_pem_file_path(self):
                 self._admin_internal_ca_pem_file_path = pem_file.name
         return self._admin_internal_ca_pem_file_path
 
-    @property
-    def mas_api_url_internal(self):
-        if self._mas_api_url_internal is None:
-            self._mas_api_url_internal = f'https://coreapi.{self.mas_core_namespace}.svc.cluster.local:{self.coreapi_port}'
-
-            # for local testing:
-            # add to /etc/hosts:
-            #    127.0.0.1               coreapi.mas-tgk01-core.svc.cluster.local
-            # oc port-forward service/coreapi 8444:443 -n mas-tgk01-core
-        return self._mas_api_url_internal
-
     @property
     def core_internal_tls_secret(self):
         if self._core_internal_tls_secret is None:
@@ -153,18 +126,6 @@ def core_internal_ca_pem_file_path(self):
                 self._core_internal_ca_pem_file_path = pem_file.name
         return self._core_internal_ca_pem_file_path
 
-    @property
-    def manage_api_url_internal(self):
-        if self._manage_api_url_internal is None:
-            # for local testing:
-            # add to /etc/hosts:
-            #    127.0.0.1               tgk01-masdev.mas-tgk01-manage.svc.cluster.local
-
-            # oc port-forward service/tgk01-masdev 8443:443 -n mas-tgk01-manage
-
-            self._manage_api_url_internal = f'https://{self.mas_instance_id}-{self.mas_workspace_id}.{self.manage_namespace}.svc.cluster.local:{self.manage_api_port}'
-        return self._manage_api_url_internal
-
     @property
     def superuser_auth_token(self):
         if self._superuser_auth_token is None:
@@ -231,6 +192,27 @@ def mas_workspace_application_ids(self):
             self._mas_workspace_application_ids = list(map(lambda ma: ma["id"], self.get_mas_applications_in_workspace()))
         return self._mas_workspace_application_ids
 
+    def get_user(self, user_id):
+        self.logger.debug(f"Getting user {user_id}")
+        url = f"{self.mas_api_url_internal}/v3/users/{user_id}"
+        headers = {
+            "Accept": "application/json",
+            "x-access-token": self.superuser_auth_token
+        }
+        response = requests.get(
+            url,
+            headers=headers,
+            verify=self.core_internal_ca_pem_file_path
+        )
+
+        if response.status_code == 404:
+            return None
+
+        if response.status_code == 200:
+            return response.json()
+
+        raise Exception(f"{response.status_code} {response.text}")
+
     def get_or_create_user(self, payload):
         '''
         User is identified by payload["id"] field
@@ -308,6 +290,9 @@ def link_user_to_local_idp(self, user_id, email_password=False):
 
         # For the sake of idempotency, check if the user already has a local identity
         user = self.get_user(user_id)
+        if user is None:
+            raise Exception(f"User {user_id} was not found")
+
         if "identities" in user and "_local" in user["identities"]:
             self.logger.info(f"User {user_id} already has a local identity")
             return None
@@ -338,27 +323,6 @@ def link_user_to_local_idp(self, user_id, email_password=False):
 
         return None
 
-    def get_user(self, user_id):
-        self.logger.debug(f"Getting user {user_id}")
-        url = f"{self.mas_api_url_internal}/v3/users/{user_id}"
-        headers = {
-            "Accept": "application/json",
-            "x-access-token": self.superuser_auth_token
-        }
-        response = requests.get(
-            url,
-            headers=headers,
-            verify=self.core_internal_ca_pem_file_path
-        )
-
-        if response.status_code == 404:
-            return None
-
-        if response.status_code == 200:
-            return response.json()
-
-        raise Exception(f"{response.status_code} {response.text}")
-
     def get_user_workspaces(self, user_id):
         '''
         Assumes user exists, raises if not.
diff --git a/test/src/test_users.py b/test/src/test_users.py
new file mode 100644
index 00000000..e78827e4
--- /dev/null
+++ b/test/src/test_users.py
@@ -0,0 +1,325 @@
+# *****************************************************************************
+# Copyright (c) 2024 IBM Corporation and other Contributors.
+#
+# All rights reserved. This program and the accompanying materials
+# are made available under the terms of the Eclipse Public License v1.0
+# which accompanies this distribution, and is available at
+# http://www.eclipse.org/legal/epl-v10.html
+#
+# *****************************************************************************
+
+import pytest
+import base64
+from unittest.mock import MagicMock, patch
+from pytest import fixture
+
+from mas.devops.users import MASUserUtils
+
+
+TOKEN = "TOKEN"
+MAS_INSTANCE_ID = "inst1"
+MAS_WORKSPACE_ID = "masdev"
+
+MAS_CORE_NAMESPACE = f"mas-{MAS_INSTANCE_ID}-core"
+MANAGE_NAMESPACE = f"mas-{MAS_INSTANCE_ID}-manage"
+
+ADMIN_DASHBOARD_PORT = 1
+COREAPI_PORT = 2
+
+MAS_ADMIN_URL = f"https://admin-dashboard.{MAS_CORE_NAMESPACE}.svc.cluster.local:{ADMIN_DASHBOARD_PORT}"
+MAS_API_URL = f'https://coreapi.{MAS_CORE_NAMESPACE}.svc.cluster.local:{COREAPI_PORT}'
+
+
+def get_secret(name, namespace):
+    if name.endswith("-credentials-superuser"):
+        data = {
+            "username": base64.b64encode("hello".encode("utf-8")),
+            "password": base64.b64encode("world".encode("utf-8")),
+        }
+
+    if name.endswith("-admindashboard-cert-internal"):
+        data = {
+            "ca.crt": base64.b64encode("admindashboard-ca".encode("utf-8"))
+        }
+
+    if name.endswith("-coreapi-cert-internal"):
+        data = {
+            "ca.crt": base64.b64encode("coreapi-ca".encode("utf-8"))
+        }
+
+    return MagicMock(
+        data=data
+    )
+
+
+@fixture
+def mock_v1_secrets():
+    with patch('mas.devops.users.DynamicClient') as mock_DynamicClientCls:
+        mock_DynamicClient = mock_DynamicClientCls.return_value
+        mock_v1_secrets = mock_DynamicClient.resources.get.return_value
+        mock_v1_secrets.get.side_effect = get_secret
+        yield
+
+
+@fixture
+def user_utils(mock_v1_secrets, requests_mock):
+    user_utils = MASUserUtils(
+        MAS_INSTANCE_ID,
+        MAS_WORKSPACE_ID,
+        None,
+        coreapi_port=COREAPI_PORT,
+        admin_dashboard_port=ADMIN_DASHBOARD_PORT
+    )
+    get_token = requests_mock.post(f"{MAS_ADMIN_URL}/logininitial", json=dict(token=TOKEN))
+    assert get_token.call_count == 0
+    yield user_utils
+
+    # assuming the test calls any MAS Core API (all do)
+    # we expect the token endpoint to have been called exactly once (and its response cached)
+    assert get_token.call_count == 1
+
+
+def mock_get_user(requests_mock, user_id, json, status_code):
+    return requests_mock.get(
+        f"{MAS_API_URL}/v3/users/{user_id}",
+        request_headers={"x-access-token": TOKEN},
+        json=json,
+        status_code=status_code
+    )
+
+
+def mock_get_user_200(requests_mock, user_id):
+    return mock_get_user(
+        requests_mock, user_id, {"id": user_id}, 200
+    )
+
+
+def mock_get_user_404(requests_mock, user_id):
+    return mock_get_user(
+        requests_mock, user_id, {"error": "notfound"}, 404
+    )
+
+
+def mock_get_user_500(requests_mock, user_id):
+    return mock_get_user(
+        requests_mock, user_id, {"error": "internal"}, 500
+    )
+
+
+def test_get_user_exists(user_utils, requests_mock):
+    user_id = "user1"
+    get = mock_get_user_200(requests_mock, user_id)
+    assert user_utils.get_user(user_id) == {"id": user_id}
+    assert get.call_count == 1
+
+
+def test_get_user_notfound(user_utils, requests_mock):
+    user_id = "user1"
+    get = mock_get_user_404(requests_mock, user_id)
+    assert user_utils.get_user(user_id) is None
+    assert get.call_count == 1
+
+
+def test_get_user_error(user_utils, requests_mock):
+    user_id = "user1"
+    get = mock_get_user_500(requests_mock, user_id)
+    with pytest.raises(Exception):
+        user_utils.get_user(user_id)
+    assert get.call_count == 1
+
+
+def test_get_or_create_user_exists(user_utils, requests_mock):
+    user_id = "user1"
+    get = mock_get_user_200(requests_mock, user_id)
+
+    post = requests_mock.post(
+        f"{MAS_API_URL}/v3/users",
+        request_headers={"x-access-token": TOKEN},
+        json={"id": user_id},
+        status_code=201
+    )
+
+    assert user_utils.get_or_create_user({"id": user_id}) == {"id": user_id}
+    assert get.call_count == 1
+    assert post.call_count == 0
+
+
+def test_get_or_create_user_notfound(user_utils, requests_mock):
+    user_id = "user1"
+    get = mock_get_user_404(requests_mock, user_id)
+
+    post = requests_mock.post(
+        f"{MAS_API_URL}/v3/users",
+        request_headers={"x-access-token": TOKEN},
+        json={"id": user_id},
+        status_code=201
+    )
+
+    assert user_utils.get_or_create_user({"id": user_id}) == {"id": user_id}
+    assert get.call_count == 1
+    assert post.call_count == 1
+
+
+def test_get_or_create_user_error(user_utils, requests_mock):
+    user_id = "user1"
+    get = mock_get_user_404(requests_mock, user_id)
+    post = requests_mock.post(
+        f"{MAS_API_URL}/v3/users",
+        request_headers={"x-access-token": TOKEN},
+        json={"error": "unknown"},
+        status_code=500
+    )
+
+    with pytest.raises(Exception):
+        user_utils.get_or_create_user({"id": user_id})
+    assert get.call_count == 1
+    assert post.call_count == 1
+
+
+def test_link_user_to_local_idp(user_utils, requests_mock):
+    user_id = "user1"
+    email_password = True
+    get = mock_get_user_200(requests_mock, user_id)
+
+    put = requests_mock.put(
+        f"{MAS_API_URL}/v3/users/{user_id}/idps/local?emailPassword={email_password}",
+        request_headers={"x-access-token": TOKEN},
+        json={"id": user_id},
+        status_code=200
+    )
+
+    user_utils.link_user_to_local_idp(user_id, email_password=email_password)
+
+    assert get.call_count == 1
+    assert put.call_count == 1
+
+
+def test_link_user_to_local_idp_usernotfound(user_utils, requests_mock):
+    user_id = "user1"
+    get = mock_get_user_404(requests_mock, user_id)
+    put = requests_mock.put(
+        f"{MAS_API_URL}/v3/users/{user_id}/idps/local",
+    )
+
+    with pytest.raises(Exception):
+        user_utils.link_user_to_local_idp(user_id)
+
+    assert get.call_count == 1
+    assert put.call_count == 0
+
+
+def test_link_user_to_local_idp_already_linked(user_utils, requests_mock):
+    user_id = "user1"
+    email_password = True
+    get = mock_get_user(
+        requests_mock, user_id, {"id": user_id, "identities": {"_local": {}}}, 200
+    )
+
+    put = requests_mock.put(
+        f"{MAS_API_URL}/v3/users/{user_id}/idps/local?emailPassword={email_password}",
+        request_headers={"x-access-token": TOKEN},
+        json={"identities": {}},
+        status_code=200
+    )
+
+    user_utils.link_user_to_local_idp(user_id, email_password=email_password)
+
+    assert get.call_count == 1
+    assert put.call_count == 0
+
+
+def test_get_user_workspaces(user_utils, requests_mock):
+    user_id = "user1"
+    get = requests_mock.get(
+        f"{MAS_API_URL}/v3/users/{user_id}/workspaces",
+        request_headers={"x-access-token": TOKEN},
+        json=[{"id": "masdev"}],
+        status_code=200
+    )
+    workspaces = user_utils.get_user_workspaces(user_id)
+    assert workspaces == [{"id": "masdev"}]
+    assert get.call_count == 1
+
+
+def test_get_user_workspaces_usernotfound(user_utils, requests_mock):
+    user_id = "user1"
+    get = requests_mock.get(
+        f"{MAS_API_URL}/v3/users/{user_id}/workspaces",
+        request_headers={"x-access-token": TOKEN},
+        json={},
+        status_code=404
+    )
+    with pytest.raises(Exception):
+        user_utils.get_user_workspaces(user_id)
+    assert get.call_count == 1
+
+
+def test_get_user_workspaces_error(user_utils, requests_mock):
+    user_id = "user1"
+    get = requests_mock.get(
+        f"{MAS_API_URL}/v3/users/{user_id}/workspaces",
+        request_headers={"x-access-token": TOKEN},
+        json={"error": "internal"},
+        status_code=500
+    )
+    with pytest.raises(Exception):
+        user_utils.get_user_workspaces(user_id)
+    assert get.call_count == 1
+
+
+def test_add_user_to_workspace_already_a_member(user_utils, requests_mock):
+    user_id = "user1"
+    get = requests_mock.get(
+        f"{MAS_API_URL}/v3/users/{user_id}/workspaces",
+        request_headers={"x-access-token": TOKEN},
+        json=[{"id": "someotherworkspace"}, {"id": MAS_WORKSPACE_ID}],
+        status_code=200
+    )
+    put = requests_mock.get(
+        f"{MAS_API_URL}/workspaces/{MAS_WORKSPACE_ID}/users/{user_id}",
+        request_headers={"x-access-token": TOKEN},
+        json=[{"id": "masdev"}],
+        status_code=200
+    )
+    user_utils.add_user_to_workspace(user_id, is_workspace_admin=True)
+    assert get.call_count == 1
+    assert put.call_count == 0
+
+
+def test_add_user_to_workspace(user_utils, requests_mock):
+    user_id = "user1"
+    get = requests_mock.get(
+        f"{MAS_API_URL}/v3/users/{user_id}/workspaces",
+        request_headers={"x-access-token": TOKEN},
+        json=[{"id": "someotherworkspace"}],
+        status_code=200
+    )
+    put = requests_mock.put(
+        f"{MAS_API_URL}/workspaces/{MAS_WORKSPACE_ID}/users/{user_id}",
+        request_headers={"x-access-token": TOKEN},
+        json={},
+        status_code=200
+    )
+    user_utils.add_user_to_workspace(user_id, is_workspace_admin=True)
+    assert get.call_count == 1
+    assert put.call_count == 1
+
+
+def test_add_user_to_workspace_error(user_utils, requests_mock):
+    user_id = "user1"
+    get = requests_mock.get(
+        f"{MAS_API_URL}/v3/users/{user_id}/workspaces",
+        request_headers={"x-access-token": TOKEN},
+        json=[{"id": "someotherworkspace"}],
+        status_code=200
+    )
+    put = requests_mock.put(
+        f"{MAS_API_URL}/workspaces/{MAS_WORKSPACE_ID}/users/{user_id}",
+        request_headers={"x-access-token": TOKEN},
+        json={"error": "internal"},
+        status_code=500
+    )
+    with pytest.raises(Exception):
+        user_utils.add_user_to_workspace(user_id, is_workspace_admin=True)
+    assert get.call_count == 1
+    assert put.call_count == 1

From 395ea4287e0aef80367bd84d39e3bb80f2316a21 Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Tue, 1 Apr 2025 09:54:49 +0100
Subject: [PATCH 20/44] more unit tests

---
 test/src/test_users.py | 94 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 94 insertions(+)

diff --git a/test/src/test_users.py b/test/src/test_users.py
index e78827e4..875e8760 100644
--- a/test/src/test_users.py
+++ b/test/src/test_users.py
@@ -323,3 +323,97 @@ def test_add_user_to_workspace_error(user_utils, requests_mock):
         user_utils.add_user_to_workspace(user_id, is_workspace_admin=True)
     assert get.call_count == 1
     assert put.call_count == 1
+
+
+def test_get_user_application_permissions(user_utils, requests_mock):
+    user_id = "user1"
+    application_id = "manage"
+    response_json = {
+        "role": "USER",
+        "userId": user_id,
+        "workspaceId": MAS_WORKSPACE_ID,
+        "userUrl": "https://api.yourmasdomain.com/users/joebloggs",
+        "workspaceUrl": "https://api.yourmasdomain.com/workspaces/myworkspace1"
+    }
+    get = requests_mock.get(
+        f"{MAS_API_URL}/workspaces/{MAS_WORKSPACE_ID}/applications/{application_id}/users/{user_id}",
+        request_headers={"x-access-token": TOKEN},
+        json=response_json,
+        status_code=200
+    )
+    assert user_utils.get_user_application_permissions(user_id, application_id) == response_json
+    assert get.call_count == 1
+
+
+def test_get_user_application_permissions_notfound(user_utils, requests_mock):
+    user_id = "user1"
+    application_id = "manage"
+    get = requests_mock.get(
+        f"{MAS_API_URL}/workspaces/{MAS_WORKSPACE_ID}/applications/{application_id}/users/{user_id}",
+        request_headers={"x-access-token": TOKEN},
+        json={"error": "notfound"},
+        status_code=404
+    )
+    assert user_utils.get_user_application_permissions(user_id, application_id) is None
+    assert get.call_count == 1
+
+
+def test_get_user_application_permissions_error(user_utils, requests_mock):
+    user_id = "user1"
+    application_id = "manage"
+    get = requests_mock.get(
+        f"{MAS_API_URL}/workspaces/{MAS_WORKSPACE_ID}/applications/{application_id}/users/{user_id}",
+        request_headers={"x-access-token": TOKEN},
+        json={"error": "internal"},
+        status_code=500
+    )
+    with pytest.raises(Exception):
+        user_utils.get_user_application_permissions(user_id, application_id)
+    assert get.call_count == 1
+
+
+def test_set_user_application_permissions(user_utils, requests_mock):
+    user_id = "user1"
+    application_id = "manage"
+    get = requests_mock.get(
+        f"{MAS_API_URL}/workspaces/{MAS_WORKSPACE_ID}/applications/{application_id}/users/{user_id}",
+        request_headers={"x-access-token": TOKEN},
+        json={"error": "notfound"},
+        status_code=404
+    )
+    put = requests_mock.put(
+        f"{MAS_API_URL}/workspaces/{MAS_WORKSPACE_ID}/applications/{application_id}/users/{user_id}",
+        request_headers={"x-access-token": TOKEN},
+        json={},
+        status_code=200
+    )
+    user_utils.set_user_application_permission(user_id, application_id, "USER")
+    assert get.call_count == 1
+    assert put.call_count == 1
+
+
+def test_set_user_application_permissions_alreadyset(user_utils, requests_mock):
+    user_id = "user1"
+    application_id = "manage"
+    get_response_json = {
+        "role": "ADMINISTRATOR",
+        "userId": user_id,
+        "workspaceId": MAS_WORKSPACE_ID,
+        "userUrl": "https://api.yourmasdomain.com/users/joebloggs",
+        "workspaceUrl": "https://api.yourmasdomain.com/workspaces/myworkspace1"
+    }
+    get = requests_mock.get(
+        f"{MAS_API_URL}/workspaces/{MAS_WORKSPACE_ID}/applications/{application_id}/users/{user_id}",
+        request_headers={"x-access-token": TOKEN},
+        json=get_response_json,
+        status_code=200
+    )
+    put = requests_mock.put(
+        f"{MAS_API_URL}/workspaces/{MAS_WORKSPACE_ID}/applications/{application_id}/users/{user_id}",
+        request_headers={"x-access-token": TOKEN},
+        json={},
+        status_code=200
+    )
+    user_utils.set_user_application_permission(user_id, application_id, "USER")
+    assert get.call_count == 1
+    assert put.call_count == 0

From 76264f82b9990e889ee7f0e7cf3b353140f9dd39 Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Tue, 1 Apr 2025 11:23:21 +0100
Subject: [PATCH 21/44] unit tests + user resync logic

---
 src/mas/devops/users.py | 116 ++++++++++++++++++++++++----------------
 test/src/test_users.py  |  98 ++++++++++++++++++++++++++++++---
 2 files changed, 161 insertions(+), 53 deletions(-)

diff --git a/src/mas/devops/users.py b/src/mas/devops/users.py
index 75c3d188..be241b7c 100644
--- a/src/mas/devops/users.py
+++ b/src/mas/devops/users.py
@@ -19,23 +19,12 @@
 import time
 import re
 
-
 '''
 TODO:
-    idempotency
-    handle user deletion if removed from config?
     MVI / other apps?
     unit tests
 
-    where are we going to run this from? Needs to run in cluster, so a Job in an app
-    which app though? Manage?
-    Perhaps we do the core bits in a core app (suite workspace?)
-    And app-specific bits in the apps themselves
-    but if we create a user before manage is ready, sync fails, and no way to trigger resync in MAS < 9.1.0?
-    # could do it all from a core app, but check for readiness of apps
-    Run in a dedicated instance-root app in the final syncwave
-
-    how ot cope with users that have been soft-deleted? tolerate / skip - ensure they are removed from secret so
+    how to cope with users that have been soft-deleted? tolerate / skip - ensure they are removed from secret so
     we don't get caught in an infinite loop?
 
 '''
@@ -282,6 +271,47 @@ def get_or_create_user(self, payload):
 
         raise Exception(f"{response.status_code} {response.text}")
 
+    def update_user(self, payload):
+        user_id = payload["id"]
+        self.logger.debug(f"Updating user {user_id}")
+        url = f"{self.mas_api_url_internal}/v3/users/{user_id}"
+        headers = {
+            "Accept": "application/json",
+            "x-access-token": self.superuser_auth_token
+        }
+        response = requests.put(
+            url,
+            headers=headers,
+            json=payload,
+            verify=self.core_internal_ca_pem_file_path
+        )
+
+        if response.status_code == 200:
+            return response.json()
+
+        raise Exception(f"{response.status_code} {response.text}")
+
+    def update_user_display_name(self, user_id, display_name):
+        self.logger.debug(f"Updating user display name {user_id}")
+        url = f"{self.mas_api_url_internal}/v3/users/{user_id}"
+        headers = {
+            "Accept": "application/json",
+            "x-access-token": self.superuser_auth_token
+        }
+        response = requests.patch(
+            url,
+            headers=headers,
+            json={
+                "displayName": display_name
+            },
+            verify=self.core_internal_ca_pem_file_path
+        )
+
+        if response.status_code == 200:
+            return response.json()
+
+        raise Exception(f"{response.status_code} {response.text}")
+
     def link_user_to_local_idp(self, user_id, email_password=False):
         '''
         Checks if user already has a local identity, no-op if so.
@@ -437,49 +467,41 @@ def set_user_application_permission(self, user_id, application_id, role):
 
         raise Exception(f"{response.status_code} {response.text}")
 
-    def check_user_sync(self, user_id, application_id, timeout_secs=60 * 10):
+    def check_user_sync(self, user_id, application_id, timeout_secs=60 * 10, retry_interval_secs=5):
         t_end = time.time() + timeout_secs
         self.logger.info(f"Awaiting user {user_id} sync status \"SUCCESS\" for app {application_id}: {t_end - time.time():.2f} seconds remaining")
         while time.time() < t_end:
             user = self.get_user(user_id)
-            sync_state = user["applications"][application_id]["sync"]["state"]
-            if sync_state == "SUCCESS":
-                return
-            elif sync_state == "ERROR":
-                # coreapi >= 25.2.3, not in mas 9.0.9, mas >= 9.1 only?
-                # self.resync_users([user_id])
-                # time.sleep(8)
-                # alternative mechanism to kick off a user resync?
-                # if not, bomb out here since we'll never get SUCCESS?
-                # TODO: I think you can just set user roles against to retrigger user sync
-                raise Exception(f"User {user_id} sync failed, aborting")
+
+            if "applications" not in user or application_id not in user["applications"] or "sync" not in user["applications"][application_id] or "state" not in user["applications"][application_id]["sync"]:
+                self.logger.warning(f"User {user_id} does not have any sync state for application {application_id}, triggering resync")
+                self.resync_users([user_id])
+                time.sleep(retry_interval_secs)
             else:
-                self.logger.info(f"User {user_id} sync has not been completed yet for app {application_id} (currrently {sync_state}): {t_end - time.time():.2f} seconds remaining")
-                time.sleep(5)
+                sync_state = user["applications"][application_id]["sync"]["state"]
+                if sync_state == "SUCCESS":
+                    return
+                elif sync_state == "ERROR":
+                    self.logger.warning(f"User {user_id} sync state for {application_id} was {sync_state}, triggering resync")
+                    self.resync_users([user_id])
+                    time.sleep(retry_interval_secs)
+                else:
+                    self.logger.info(f"User {user_id} sync has not been completed yet for app {application_id} (currrently {sync_state}): {t_end - time.time():.2f} seconds remaining")
+                time.sleep(retry_interval_secs)
         raise Exception(f"User {user_id} sync failed to complete for app within {timeout_secs} seconds")
 
-    # coreapi >= 25.2.3, not in mas 9.0.9, mas >= 9.1 only?
+    def resync_user(self, user_ids):
+        self.logger.info(f"Issuing resync request(s) for user(s) {user_ids}")
 
-    def resync_users(self, user_ids):
-        self.logger.info(f"Issuing resync request for user(s) {user_ids}")
-        url = f"{self.mas_api_url_internal}/v3/users/utils/resync"
-        querystring = {}
-        payload = {
-            "users": user_ids
-        }
-        headers = {
-            "Content-Type": "application/json",
-            "x-access-token": self.superuser_auth_token
-        }
-        response = requests.put(
-            url,
-            json=payload,
-            headers=headers,
-            params=querystring,
-            verify=self.core_internal_ca_pem_file_path
-        )
-        if response.status_code != 204:
-            raise Exception(response.text)
+        # The "/v3/users/utils/resync" API is only available in MAS Core >= 9.1 (coreapi >= 25.2.3)
+        # Until it is available in all supported versions of MAS,
+        # we instead perform a no-op update to the user to achieve the same effect
+        # (the "update user profile" API is used as this is this allows us to isolate the displayName field,
+        # which reduces the impact of concurrent updates leading to race conditions)
+
+        for user_id in user_ids:
+            user = self.get_user(user_id)
+            self.update_user_display_name(user_id, user["displayName"])
 
     def create_or_get_manage_api_key_for_user(self, user_id):
         self.logger.debug(f"Attempting to create Manage API Key for user {user_id}")
diff --git a/test/src/test_users.py b/test/src/test_users.py
index 875e8760..104516aa 100644
--- a/test/src/test_users.py
+++ b/test/src/test_users.py
@@ -70,14 +70,9 @@ def user_utils(mock_v1_secrets, requests_mock):
         coreapi_port=COREAPI_PORT,
         admin_dashboard_port=ADMIN_DASHBOARD_PORT
     )
-    get_token = requests_mock.post(f"{MAS_ADMIN_URL}/logininitial", json=dict(token=TOKEN))
-    assert get_token.call_count == 0
+    requests_mock.post(f"{MAS_ADMIN_URL}/logininitial", json=dict(token=TOKEN))
     yield user_utils
 
-    # assuming the test calls any MAS Core API (all do)
-    # we expect the token endpoint to have been called exactly once (and its response cached)
-    assert get_token.call_count == 1
-
 
 def mock_get_user(requests_mock, user_id, json, status_code):
     return requests_mock.get(
@@ -113,6 +108,11 @@ def test_get_user_exists(user_utils, requests_mock):
     assert get.call_count == 1
 
 
+def test_mas_superuser_credentials():
+    pass
+    # TODO this and tests for other properties
+
+
 def test_get_user_notfound(user_utils, requests_mock):
     user_id = "user1"
     get = mock_get_user_404(requests_mock, user_id)
@@ -417,3 +417,89 @@ def test_set_user_application_permissions_alreadyset(user_utils, requests_mock):
     user_utils.set_user_application_permission(user_id, application_id, "USER")
     assert get.call_count == 1
     assert put.call_count == 0
+
+
+def test_check_user_sync(user_utils, requests_mock):
+    user_id = "user1"
+    application_id = "manage"
+
+    # transitions from PENDING -> SUCCESS on the third call
+    attempts = 0
+
+    def json_callback(request, context):
+        nonlocal attempts
+        if attempts >= 2:
+            state = "SUCCESS"
+        else:
+            state = "PENDING"
+        attempts = attempts + 1
+        return {
+            "id": user_id,
+            "applications": {
+                "other": {
+                    "sync": {
+                        "state": "ERROR"
+                    }
+                },
+                "manage": {
+                    "sync": {
+                        "state": state
+                    }
+                }
+            }
+        }
+
+    get = mock_get_user(
+        requests_mock,
+        user_id,
+        json_callback,
+        200
+    )
+
+    user_utils.check_user_sync(user_id, application_id, timeout_secs=8, retry_interval_secs=0)
+    assert get.call_count == 3
+
+
+def test_check_user_sync_timeout(user_utils, requests_mock):
+    user_id = "user1"
+    application_id = "manage"
+
+    get = mock_get_user(
+        requests_mock,
+        user_id,
+        {
+            "id": user_id,
+            "applications": {
+                "other": {
+                    "sync": {
+                        "state": "ERROR"
+                    }
+                },
+                "manage": {
+                    "sync": {
+                        "state": "PENDING"
+                    }
+                }
+            }
+        },
+        200
+    )
+    with pytest.raises(Exception) as excinfo:
+        user_utils.check_user_sync(user_id, application_id, timeout_secs=0.3, retry_interval_secs=0.05)
+    assert str(excinfo.value) == f"User {user_id} sync failed to complete for app within {0.3} seconds"
+    assert get.call_count > 1
+
+
+def test_check_user_sync_appstate_notfound(user_utils, requests_mock):
+    pass
+    # TODO
+
+
+def test_check_user_sync_appstate_transient_error(user_utils, requests_mock):
+    pass
+    # TODO
+
+
+def test_check_user_sync_appstate_persistent_error(user_utils, requests_mock):
+    pass
+    # TODO

From 5435b2bb5a63a39ac9ae93ec710be11a0b993c0e Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Tue, 1 Apr 2025 12:20:07 +0100
Subject: [PATCH 22/44] more unit tests

---
 test/src/test_users.py | 50 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/test/src/test_users.py b/test/src/test_users.py
index 104516aa..4d6f35f0 100644
--- a/test/src/test_users.py
+++ b/test/src/test_users.py
@@ -176,6 +176,56 @@ def test_get_or_create_user_error(user_utils, requests_mock):
     assert post.call_count == 1
 
 
+def test_update_user(user_utils, requests_mock):
+    user_id = "user1"
+    put = requests_mock.put(
+        f"{MAS_API_URL}/v3/users/{user_id}",
+        request_headers={"x-access-token": TOKEN},
+        json={"id": user_id},
+        status_code=200
+    )
+    user_utils.update_user({"id": user_id})
+    assert put.call_count == 1
+
+
+def test_update_user_error(user_utils, requests_mock):
+    user_id = "user1"
+    put = requests_mock.put(
+        f"{MAS_API_URL}/v3/users/{user_id}",
+        request_headers={"x-access-token": TOKEN},
+        json={"error": "nofound"},
+        status_code=404
+    )
+    with pytest.raises(Exception):
+        user_utils.update_user({"id": user_id})
+    assert put.call_count == 1
+
+
+def test_update_user_display_name(user_utils, requests_mock):
+    user_id = "user1"
+    patch = requests_mock.patch(
+        f"{MAS_API_URL}/v3/users/{user_id}",
+        request_headers={"x-access-token": TOKEN},
+        json={"id": user_id},
+        status_code=200
+    )
+    user_utils.update_user_display_name(user_id, "display_name")
+    assert patch.call_count == 1
+
+
+def test_update_user_display_name_error(user_utils, requests_mock):
+    user_id = "user1"
+    patch = requests_mock.patch(
+        f"{MAS_API_URL}/v3/users/{user_id}",
+        request_headers={"x-access-token": TOKEN},
+        json={"error": "notfound"},
+        status_code=404
+    )
+    with pytest.raises(Exception):
+        user_utils.update_user_display_name(user_id, "display_name")
+    assert patch.call_count == 1
+
+
 def test_link_user_to_local_idp(user_utils, requests_mock):
     user_id = "user1"
     email_password = True

From 45fad67b7f315418d6eb907e8363d69448f915d7 Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Tue, 1 Apr 2025 12:45:33 +0100
Subject: [PATCH 23/44] unit tests

---
 src/mas/devops/users.py |   2 +-
 test/src/test_users.py  | 197 +++++++++++++++++++++++++++++++++++++---
 2 files changed, 185 insertions(+), 14 deletions(-)

diff --git a/src/mas/devops/users.py b/src/mas/devops/users.py
index be241b7c..de391b98 100644
--- a/src/mas/devops/users.py
+++ b/src/mas/devops/users.py
@@ -490,7 +490,7 @@ def check_user_sync(self, user_id, application_id, timeout_secs=60 * 10, retry_i
                 time.sleep(retry_interval_secs)
         raise Exception(f"User {user_id} sync failed to complete for app within {timeout_secs} seconds")
 
-    def resync_user(self, user_ids):
+    def resync_users(self, user_ids):
         self.logger.info(f"Issuing resync request(s) for user(s) {user_ids}")
 
         # The "/v3/users/utils/resync" API is only available in MAS Core >= 9.1 (coreapi >= 25.2.3)
diff --git a/test/src/test_users.py b/test/src/test_users.py
index 4d6f35f0..2f2e2ae6 100644
--- a/test/src/test_users.py
+++ b/test/src/test_users.py
@@ -85,7 +85,7 @@ def mock_get_user(requests_mock, user_id, json, status_code):
 
 def mock_get_user_200(requests_mock, user_id):
     return mock_get_user(
-        requests_mock, user_id, {"id": user_id}, 200
+        requests_mock, user_id, {"id": user_id, "displayName": user_id}, 200
     )
 
 
@@ -104,7 +104,7 @@ def mock_get_user_500(requests_mock, user_id):
 def test_get_user_exists(user_utils, requests_mock):
     user_id = "user1"
     get = mock_get_user_200(requests_mock, user_id)
-    assert user_utils.get_user(user_id) == {"id": user_id}
+    assert user_utils.get_user(user_id) == {"id": user_id, "displayName": user_id}
     assert get.call_count == 1
 
 
@@ -139,7 +139,7 @@ def test_get_or_create_user_exists(user_utils, requests_mock):
         status_code=201
     )
 
-    assert user_utils.get_or_create_user({"id": user_id}) == {"id": user_id}
+    assert user_utils.get_or_create_user({"id": user_id}) == {"id": user_id, "displayName": user_id}
     assert get.call_count == 1
     assert post.call_count == 0
 
@@ -151,11 +151,11 @@ def test_get_or_create_user_notfound(user_utils, requests_mock):
     post = requests_mock.post(
         f"{MAS_API_URL}/v3/users",
         request_headers={"x-access-token": TOKEN},
-        json={"id": user_id},
+        json={"id": user_id, "displayName": user_id},
         status_code=201
     )
 
-    assert user_utils.get_or_create_user({"id": user_id}) == {"id": user_id}
+    assert user_utils.get_or_create_user({"id": user_id}) == {"id": user_id, "displayName": user_id}
     assert get.call_count == 1
     assert post.call_count == 1
 
@@ -469,6 +469,32 @@ def test_set_user_application_permissions_alreadyset(user_utils, requests_mock):
     assert put.call_count == 0
 
 
+def test_resync_users(user_utils, requests_mock):
+    user_ids = ["user1", "user2"]
+
+    gets = []
+    patches = []
+    for user_id in user_ids:
+        gets.append(mock_get_user_200(requests_mock, user_id))
+
+        patches.append(
+            requests_mock.patch(
+                f"{MAS_API_URL}/v3/users/{user_id}",
+                request_headers={"x-access-token": TOKEN},
+                json={"id": user_id},
+                status_code=200
+            )
+        )
+
+    user_utils.resync_users(user_ids)
+
+    for get in gets:
+        assert get.call_count == 1
+
+    for patche in patches:
+        assert patche.call_count == 1
+
+
 def test_check_user_sync(user_utils, requests_mock):
     user_id = "user1"
     application_id = "manage"
@@ -491,7 +517,7 @@ def json_callback(request, context):
                         "state": "ERROR"
                     }
                 },
-                "manage": {
+                application_id: {
                     "sync": {
                         "state": state
                     }
@@ -525,7 +551,7 @@ def test_check_user_sync_timeout(user_utils, requests_mock):
                         "state": "ERROR"
                     }
                 },
-                "manage": {
+                application_id: {
                     "sync": {
                         "state": "PENDING"
                     }
@@ -541,15 +567,160 @@ def test_check_user_sync_timeout(user_utils, requests_mock):
 
 
 def test_check_user_sync_appstate_notfound(user_utils, requests_mock):
-    pass
-    # TODO
+    user_id = "user1"
+    application_id = "manage"
+
+    # first call (made bvy check_user_sync) returns user record with missing sync status for app
+    # subsequent calls will include sync status and so should succeed
+    # a single resync should have been triggered
+    attempts = 0
+
+    def json_callback(request, context):
+        nonlocal attempts
+        if attempts >= 1:
+            ret = {
+                "id": user_id,
+                "displayName": user_id,
+                "applications": {
+                    "other": {
+                        "sync": {
+                            "state": "ERROR"
+                        }
+                    },
+                    application_id: {
+                        "sync": {
+                            "state": "SUCCESS"
+                        }
+                    }
+                }
+            }
+        else:
+            ret = {
+                "id": user_id,
+                "displayName": user_id,
+                "applications": {
+                    "other": {
+                        "sync": {
+                            "state": "ERROR"
+                        }
+                    },
+                }
+            }
+        attempts = attempts + 1
+        return ret
+
+    patch = requests_mock.patch(
+        f"{MAS_API_URL}/v3/users/{user_id}",
+        request_headers={"x-access-token": TOKEN},
+        json={"id": user_id},
+        status_code=200
+    )
+
+    get = mock_get_user(
+        requests_mock,
+        user_id,
+        json_callback,
+        200
+    )
+
+    user_utils.check_user_sync(user_id, application_id, timeout_secs=8, retry_interval_secs=0)
+    assert get.call_count == 3
+
+    # a single resync should have been triggered
+    assert patch.call_count == 1
 
 
 def test_check_user_sync_appstate_transient_error(user_utils, requests_mock):
-    pass
-    # TODO
+    user_id = "user1"
+    application_id = "manage"
+
+    # first call (made bvy check_user_sync) returns user record with sync state error
+    # subsequent calls will have successful sync state and so should succeed
+    # a single resync should have been triggered
+    attempts = 0
+
+    def json_callback(request, context):
+        nonlocal attempts
+        if attempts >= 1:
+            ret = {
+                "id": user_id,
+                "displayName": user_id,
+                "applications": {
+                    application_id: {
+                        "sync": {
+                            "state": "SUCCESS"
+                        }
+                    }
+                }
+            }
+        else:
+            ret = {
+                "id": user_id,
+                "displayName": user_id,
+                "applications": {
+                    application_id: {
+                        "sync": {
+                            "state": "ERROR"
+                        }
+                    }
+                }
+            }
+        attempts = attempts + 1
+        return ret
+
+    patche = requests_mock.patch(
+        f"{MAS_API_URL}/v3/users/{user_id}",
+        request_headers={"x-access-token": TOKEN},
+        json={"id": user_id},
+        status_code=200
+    )
+
+    get = mock_get_user(
+        requests_mock,
+        user_id,
+        json_callback,
+        200
+    )
+
+    user_utils.check_user_sync(user_id, application_id, timeout_secs=8, retry_interval_secs=0)
+    assert get.call_count == 3
+
+    # a single resync should have been triggered
+    assert patche.call_count == 1
 
 
 def test_check_user_sync_appstate_persistent_error(user_utils, requests_mock):
-    pass
-    # TODO
+    user_id = "user1"
+    application_id = "manage"
+
+    patche = requests_mock.patch(
+        f"{MAS_API_URL}/v3/users/{user_id}",
+        request_headers={"x-access-token": TOKEN},
+        json={"id": user_id},
+        status_code=200
+    )
+
+    get = mock_get_user(
+        requests_mock,
+        user_id,
+        {
+            "id": user_id,
+            "displayName": user_id,
+            "applications": {
+                application_id: {
+                    "sync": {
+                        "state": "ERROR"
+                    }
+                }
+            }
+        },
+        200
+    )
+
+    with pytest.raises(Exception) as excinfo:
+        user_utils.check_user_sync(user_id, application_id, timeout_secs=0.3, retry_interval_secs=0.05)
+    assert str(excinfo.value) == f"User {user_id} sync failed to complete for app within {0.3} seconds"
+    assert get.call_count > 1
+
+    # an "update_user_display_name" should have been triggered for every 2 get calls (1 call by check_user_sync, 1 by resync)
+    assert patche.call_count == get.call_count / 2

From dfcb873367b3bc1d6ee23e56ad3895fd2d048b9e Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Tue, 1 Apr 2025 12:46:31 +0100
Subject: [PATCH 24/44] don't shadow patch with var

---
 test/src/test_users.py | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/test/src/test_users.py b/test/src/test_users.py
index 2f2e2ae6..99fa35ac 100644
--- a/test/src/test_users.py
+++ b/test/src/test_users.py
@@ -203,19 +203,19 @@ def test_update_user_error(user_utils, requests_mock):
 
 def test_update_user_display_name(user_utils, requests_mock):
     user_id = "user1"
-    patch = requests_mock.patch(
+    patche = requests_mock.patch(
         f"{MAS_API_URL}/v3/users/{user_id}",
         request_headers={"x-access-token": TOKEN},
         json={"id": user_id},
         status_code=200
     )
     user_utils.update_user_display_name(user_id, "display_name")
-    assert patch.call_count == 1
+    assert patche.call_count == 1
 
 
 def test_update_user_display_name_error(user_utils, requests_mock):
     user_id = "user1"
-    patch = requests_mock.patch(
+    patche = requests_mock.patch(
         f"{MAS_API_URL}/v3/users/{user_id}",
         request_headers={"x-access-token": TOKEN},
         json={"error": "notfound"},
@@ -223,7 +223,7 @@ def test_update_user_display_name_error(user_utils, requests_mock):
     )
     with pytest.raises(Exception):
         user_utils.update_user_display_name(user_id, "display_name")
-    assert patch.call_count == 1
+    assert patche.call_count == 1
 
 
 def test_link_user_to_local_idp(user_utils, requests_mock):
@@ -609,7 +609,7 @@ def json_callback(request, context):
         attempts = attempts + 1
         return ret
 
-    patch = requests_mock.patch(
+    patche = requests_mock.patch(
         f"{MAS_API_URL}/v3/users/{user_id}",
         request_headers={"x-access-token": TOKEN},
         json={"id": user_id},
@@ -627,7 +627,7 @@ def json_callback(request, context):
     assert get.call_count == 3
 
     # a single resync should have been triggered
-    assert patch.call_count == 1
+    assert patche.call_count == 1
 
 
 def test_check_user_sync_appstate_transient_error(user_utils, requests_mock):

From f6c2cb9f7d1f1fa2e50b62a92468fe22a325d97b Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Tue, 1 Apr 2025 14:13:14 +0100
Subject: [PATCH 25/44] unit tests

---
 src/mas/devops/users.py |  11 ++--
 test/src/test_users.py  | 142 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 146 insertions(+), 7 deletions(-)

diff --git a/src/mas/devops/users.py b/src/mas/devops/users.py
index de391b98..243a73fa 100644
--- a/src/mas/devops/users.py
+++ b/src/mas/devops/users.py
@@ -545,7 +545,7 @@ def create_or_get_manage_api_key_for_user(self, user_id):
             self.logger.info(f"Creating new Manage API Key for user {user_id}")
         else:
             # any other status code is unexpected
-            raise Exception(response.text)
+            raise Exception(f"{response.status_code} {response.text}")
 
         # otherwise, retrieve the apikey (either it already existed, or we just created it)
 
@@ -562,8 +562,6 @@ def create_or_get_manage_api_key_for_user(self, user_id):
             #       NOTE: the manage sanity test seems to create maxadmin apikey and not clean it up
             atexit.register(self.delete_manage_api_key, apikey)
 
-            pass
-
         return apikey
 
     def get_manage_api_key_for_user(self, user_id):
@@ -763,17 +761,16 @@ def get_mas_application_availability(self, mas_application_id):
             return response.json()
         raise Exception(f"{response.status_code} {response.text}")
 
-    def await_mas_application_availability(self, mas_application_id, timeout_secs=60 * 10):
+    def await_mas_application_availability(self, mas_application_id, timeout_secs=60 * 10, retry_interval_secs=5):
         t_end = time.time() + timeout_secs
         self.logger.info(f"Waiting for {mas_application_id} to become ready and available: {t_end - time.time():.2f} seconds remaining")
         while time.time() < t_end:
             app = self.get_mas_application_availability(mas_application_id)
             if "available" in app and "ready" in app and app["ready"] and app["available"]:
                 return
-            # TODO: error state?
             else:
-                self.logger.info(f"{mas_application_id} is not ready or available, retry in 5 seconds: {t_end - time.time():.2f} seconds remaining")
-                time.sleep(5)
+                self.logger.info(f"{mas_application_id} is not ready or available, retry in {retry_interval_secs} seconds: {t_end - time.time():.2f} seconds remaining")
+                time.sleep(retry_interval_secs)
         raise Exception(f"{mas_application_id} did not become ready and available in time, aborting")
 
     def parse_initial_users_from_aws_secret_json(self, secret_json):
diff --git a/test/src/test_users.py b/test/src/test_users.py
index 99fa35ac..43d29326 100644
--- a/test/src/test_users.py
+++ b/test/src/test_users.py
@@ -724,3 +724,145 @@ def test_check_user_sync_appstate_persistent_error(user_utils, requests_mock):
 
     # an "update_user_display_name" should have been triggered for every 2 get calls (1 call by check_user_sync, 1 by resync)
     assert patche.call_count == get.call_count / 2
+
+
+def test_create_or_get_manage_api_key_for_user(user_utils, requests_mock):
+    pass
+    # TODO
+
+
+def test_get_manage_api_key_for_user(user_utils, requests_mock):
+    pass
+    # TODO
+
+
+def test_delete_manage_api_key(user_utils, requests_mock):
+    pass
+    # TODO
+
+
+def test_get_manage_group_id(user_utils, requests_mock):
+    pass
+    # TODO
+
+
+def test_is_user_in_manage_group(user_utils, requests_mock):
+    pass
+    # TODO
+
+
+def test_add_user_to_manage_group(user_utils, requests_mock):
+    pass
+    # TODO
+
+
+def test_get_mas_applications_in_workspace(user_utils, requests_mock):
+    get = requests_mock.get(
+        f"{MAS_API_URL}/workspaces/{MAS_WORKSPACE_ID}/applications",
+        request_headers={"x-access-token": TOKEN},
+        json=[{"id": "manage"}],
+        status_code=200
+    )
+    assert user_utils.get_mas_applications_in_workspace() == [{"id": "manage"}]
+    assert get.call_count == 1
+
+
+def test_get_mas_applications_in_workspace_error(user_utils, requests_mock):
+    get = requests_mock.get(
+        f"{MAS_API_URL}/workspaces/{MAS_WORKSPACE_ID}/applications",
+        request_headers={"x-access-token": TOKEN},
+        json={"error": "internal"},
+        status_code=500
+    )
+    with pytest.raises(Exception) as excinfo:
+        user_utils.get_mas_applications_in_workspace()
+    assert get.call_count == 1
+    assert str(excinfo.value) == '500 {"error": "internal"}'
+
+
+def test_get_mas_application_availability(user_utils, requests_mock):
+    application_id = "manage"
+    get = requests_mock.get(
+        f"{MAS_API_URL}/workspaces/{MAS_WORKSPACE_ID}/applications/{application_id}",
+        request_headers={"x-access-token": TOKEN},
+        json={"id": "manage"},
+        status_code=200
+    )
+    assert user_utils.get_mas_application_availability(application_id) == {"id": "manage"}
+    assert get.call_count == 1
+
+
+def test_get_mas_application_availability_error(user_utils, requests_mock):
+    application_id = "manage"
+    get = requests_mock.get(
+        f"{MAS_API_URL}/workspaces/{MAS_WORKSPACE_ID}/applications/{application_id}",
+        request_headers={"x-access-token": TOKEN},
+        json={"error": "internal"},
+        status_code=500
+    )
+    with pytest.raises(Exception) as excinfo:
+        user_utils.get_mas_application_availability(application_id)
+    assert get.call_count == 1
+    assert str(excinfo.value) == '500 {"error": "internal"}'
+
+
+def test_await_mas_application_availability(user_utils, requests_mock):
+    pass
+    # TODO
+
+
+def test_parse_initial_users_from_aws_secret_json(user_utils):
+
+    actual_initial_users = user_utils.parse_initial_users_from_aws_secret_json(
+        {
+            "user1@example.com": "primary,joe,bloggs",
+            "user2@example.com": "  primary     ,   ben     ,   bob  ",
+            "user3@example.com": "secondary     ,bill,  bibb"
+        }
+    )
+
+    expected_initial_users = {
+        "users": {
+            "primary": [
+                {
+                    "email": "user1@example.com",
+                    "given_name": "joe",
+                    "family_name": "bloggs"
+                },
+                {
+                    "email": "user2@example.com",
+                    "given_name": "ben",
+                    "family_name": "bob"
+                }
+            ],
+            "secondary": [
+                {
+                    "email": "user3@example.com",
+                    "given_name": "bill",
+                    "family_name": "bibb"
+                }
+            ]
+        }
+    }
+
+    assert actual_initial_users == expected_initial_users
+
+    with pytest.raises(Exception) as excinfo:
+        user_utils.parse_initial_users_from_aws_secret_json({
+            "user1@example.com": "primary"
+        })
+    assert "Wrong number of CSV values for user1@example.com (expected 3 but got 1)" == str(excinfo.value)
+
+    with pytest.raises(Exception) as excinfo:
+        user_utils.parse_initial_users_from_aws_secret_json({
+            "user1@example.com": "unknown,x,y"
+        })
+    assert "Unknown user type for user1@example.com: unknown" == str(excinfo.value)
+
+
+def test_create_initial_user_for_saas(user_utils, requests_mock):
+    pass
+
+
+def test_create_initial_users_for_saas(user_utils, requests_mock):
+    pass

From f9ff747075eb7df7eefb930efd36426bf7682d36 Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Tue, 1 Apr 2025 14:22:02 +0100
Subject: [PATCH 26/44] more unit tests

---
 test/src/test_users.py | 76 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 74 insertions(+), 2 deletions(-)

diff --git a/test/src/test_users.py b/test/src/test_users.py
index 43d29326..12aed11e 100644
--- a/test/src/test_users.py
+++ b/test/src/test_users.py
@@ -807,8 +807,80 @@ def test_get_mas_application_availability_error(user_utils, requests_mock):
 
 
 def test_await_mas_application_availability(user_utils, requests_mock):
-    pass
-    # TODO
+    application_id = "manage"
+
+    # returns all possible permutations of the endpoint, until finally returning the
+    # response that should cause the retry logic to exit
+    return_values = [
+        {
+            "id": application_id,
+        },
+        {
+            "available": False,
+        },
+        {
+            "available": True,
+        },
+        {
+            "ready": False,
+        },
+        {
+            "ready": True,
+        },
+        {
+            "available": False,
+            "ready": False,
+        },
+        {
+            "available": True,
+            "ready": False,
+        },
+        {
+            "available": False,
+            "ready": True,
+        },
+        {
+            "available": True,
+            "ready": True,
+        },
+    ]
+    attempt = 0
+
+    def json_callback(request, context):
+        nonlocal attempt
+        nonlocal return_values
+        ret = return_values[attempt]
+        attempt = attempt + 1
+        return ret
+
+    get = requests_mock.get(
+        f"{MAS_API_URL}/workspaces/{MAS_WORKSPACE_ID}/applications/{application_id}",
+        request_headers={"x-access-token": TOKEN},
+        json=json_callback,
+        status_code=200
+    )
+
+    user_utils.await_mas_application_availability(application_id, timeout_secs=5, retry_interval_secs=0)
+    assert get.call_count == len(return_values)
+
+
+def test_await_mas_application_availability_timeout(user_utils, requests_mock):
+    application_id = "manage"
+
+    get = requests_mock.get(
+        f"{MAS_API_URL}/workspaces/{MAS_WORKSPACE_ID}/applications/{application_id}",
+        request_headers={"x-access-token": TOKEN},
+        json={
+            "available": False,
+            "ready": False,
+        },
+        status_code=200
+    )
+
+    with pytest.raises(Exception) as excinfo:
+        user_utils.await_mas_application_availability(application_id, timeout_secs=1, retry_interval_secs=0.1)
+    assert get.call_count > 1
+    assert str(excinfo.value) == f"{application_id} did not become ready and available in time, aborting"
 
 
 def test_parse_initial_users_from_aws_secret_json(user_utils):

From fcfba5b8a3703e5cb5e035307f908969d27dec5d Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Tue, 1 Apr 2025 15:47:56 +0100
Subject: [PATCH 27/44] unit tests

---
 test/src/test_users.py | 109 ++++++++++++++++++++++++++++++++++++-----
 1 file changed, 96 insertions(+), 13 deletions(-)

diff --git a/test/src/test_users.py b/test/src/test_users.py
index 12aed11e..edb341a5 100644
--- a/test/src/test_users.py
+++ b/test/src/test_users.py
@@ -15,6 +15,15 @@
 
 from mas.devops.users import MASUserUtils
 
+SUPERUSER_USERNAME = "superuser_username"
+SUPERUSER_PASSWORD = "superuser_password"  # pragma: allowlist secret
+
+
+ADMINDASHBOARD_CA_CRT = "admindashboard-ca"
+COREAPI_CA_CRT = "coreapi-ca"
+MANAGE_CA_CRT = "manage-ca"
+MANAGE_TLS_CRT = "manage-tls-crt"
+MANAGE_TLS_KEY = "manage-tls-key"
 
 TOKEN = "TOKEN"
 MAS_INSTANCE_ID = "inst1"
@@ -31,20 +40,27 @@
 
 
 def get_secret(name, namespace):
-    if name.endswith("-credentials-superuser"):
+    if name == f"{MAS_INSTANCE_ID}-credentials-superuser":
+        data = {
+            "username": base64.b64encode(SUPERUSER_USERNAME.encode("utf-8")),
+            "password": base64.b64encode(SUPERUSER_PASSWORD.encode("utf-8")),
+        }
+
+    if name == f"{MAS_INSTANCE_ID}-admindashboard-cert-internal":
         data = {
-            "username": base64.b64encode("hello".encode("utf-8")),
-            "password": base64.b64encode("world".encode("utf-8")),
+            "ca.crt": base64.b64encode(ADMINDASHBOARD_CA_CRT.encode("utf-8"))
         }
 
-    if name.endswith("-admindashboard-cert-internal"):
+    if name == f"{MAS_INSTANCE_ID}-coreapi-cert-internal":
         data = {
-            "ca.crt": base64.b64encode("admindashboard-ca".encode("utf-8"))
+            "ca.crt": base64.b64encode(COREAPI_CA_CRT.encode("utf-8"))
         }
 
-    if name.endswith("-coreapi-cert-internal"):
+    if name == f"{MAS_INSTANCE_ID}-internal-manage-tls":
         data = {
-            "ca.crt": base64.b64encode("coreapi-ca".encode("utf-8"))
+            "ca.crt": base64.b64encode(MANAGE_CA_CRT.encode("utf-8")),
+            "tls.crt": base64.b64encode(MANAGE_TLS_CRT.encode("utf-8")),
+            "tls.key": base64.b64encode(MANAGE_TLS_KEY.encode("utf-8")),
         }
 
     return MagicMock(
@@ -58,7 +74,7 @@ def mock_v1_secrets():
         mock_DynamicClient = mock_DynamicClientCls.return_value
         mock_v1_secrets = mock_DynamicClient.resources.get.return_value
         mock_v1_secrets.get.side_effect = get_secret
-        yield
+        yield mock_v1_secrets
 
 
 @fixture
@@ -101,6 +117,78 @@ def mock_get_user_500(requests_mock, user_id):
     )
 
 
+def test_mas_superuser_credentials(user_utils, mock_v1_secrets):
+    assert mock_v1_secrets.get.call_count == 0
+    assert user_utils.mas_superuser_credentials == {"username": SUPERUSER_USERNAME, "password": SUPERUSER_PASSWORD}
+    assert mock_v1_secrets.get.call_count == 1
+    # verify caching is working
+    assert user_utils.mas_superuser_credentials == {"username": SUPERUSER_USERNAME, "password": SUPERUSER_PASSWORD}
+    assert mock_v1_secrets.get.call_count == 1
+
+
+def test_admin_internal_tls_secret(user_utils, mock_v1_secrets):
+    assert mock_v1_secrets.get.call_count == 0
+    assert user_utils.admin_internal_tls_secret.data["ca.crt"] == base64.b64encode(ADMINDASHBOARD_CA_CRT.encode('utf-8'))
+    assert mock_v1_secrets.get.call_count == 1
+    assert user_utils.admin_internal_tls_secret.data["ca.crt"] == base64.b64encode(ADMINDASHBOARD_CA_CRT.encode('utf-8'))
+    assert mock_v1_secrets.get.call_count == 1
+
+
+def test_admin_internal_ca_pem_file_path():
+    pass
+    # TODO
+
+
+def test_core_internal_tls_secret(user_utils, mock_v1_secrets):
+    assert mock_v1_secrets.get.call_count == 0
+    assert user_utils.core_internal_tls_secret.data["ca.crt"] == base64.b64encode(COREAPI_CA_CRT.encode('utf-8'))
+    assert mock_v1_secrets.get.call_count == 1
+    assert user_utils.core_internal_tls_secret.data["ca.crt"] == base64.b64encode(COREAPI_CA_CRT.encode('utf-8'))
+    assert mock_v1_secrets.get.call_count == 1
+
+
+def test_core_internal_ca_pem_file_path():
+    pass
+    # TODO
+
+
+def test_superuser_auth_token():
+    pass
+    # TODO
+
+
+def test_manage_internal_tls_secret(user_utils, mock_v1_secrets):
+    assert mock_v1_secrets.get.call_count == 0
+    assert user_utils.manage_internal_tls_secret.data["ca.crt"] == base64.b64encode(MANAGE_CA_CRT.encode('utf-8'))
+    assert user_utils.manage_internal_tls_secret.data["tls.crt"] == base64.b64encode(MANAGE_TLS_CRT.encode('utf-8'))
+    assert user_utils.manage_internal_tls_secret.data["tls.key"] == base64.b64encode(MANAGE_TLS_KEY.encode('utf-8'))
+    assert mock_v1_secrets.get.call_count == 1
+    assert user_utils.manage_internal_tls_secret.data["ca.crt"] == base64.b64encode(MANAGE_CA_CRT.encode('utf-8'))
+    assert user_utils.manage_internal_tls_secret.data["tls.crt"] == base64.b64encode(MANAGE_TLS_CRT.encode('utf-8'))
+    assert user_utils.manage_internal_tls_secret.data["tls.key"] == base64.b64encode(MANAGE_TLS_KEY.encode('utf-8'))
+    assert mock_v1_secrets.get.call_count == 1
+
+
+def test_manage_internal_client_pem_file_path():
+    pass
+    # TODO
+
+
+def test_manage_internal_ca_pem_file_path():
+    pass
+    # TODO
+
+
+def test_manage_maxadmin_api_key():
+    pass
+    # TODO
+
+
+def test_mas_workspace_application_ids():
+    pass
+    # TODO
+
+
 def test_get_user_exists(user_utils, requests_mock):
     user_id = "user1"
     get = mock_get_user_200(requests_mock, user_id)
@@ -108,11 +196,6 @@ def test_get_user_exists(user_utils, requests_mock):
     assert get.call_count == 1
 
 
-def test_mas_superuser_credentials():
-    pass
-    # TODO this and tests for other properties
-
-
 def test_get_user_notfound(user_utils, requests_mock):
     user_id = "user1"
     get = mock_get_user_404(requests_mock, user_id)

From 84c222fa41d5fe23a719cc9c4c6ca50b58c7a6af Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Tue, 1 Apr 2025 17:11:44 +0100
Subject: [PATCH 28/44] unit tests

---
 test/src/test_users.py | 79 +++++++++++++++++++++++++++++++++---------
 1 file changed, 63 insertions(+), 16 deletions(-)

diff --git a/test/src/test_users.py b/test/src/test_users.py
index edb341a5..24f49427 100644
--- a/test/src/test_users.py
+++ b/test/src/test_users.py
@@ -10,9 +10,11 @@
 
 import pytest
 import base64
-from unittest.mock import MagicMock, patch
+from unittest.mock import MagicMock, patch, call
 from pytest import fixture
 
+import os
+
 from mas.devops.users import MASUserUtils
 
 SUPERUSER_USERNAME = "superuser_username"
@@ -38,6 +40,8 @@
 MAS_ADMIN_URL = f"https://admin-dashboard.{MAS_CORE_NAMESPACE}.svc.cluster.local:{ADMIN_DASHBOARD_PORT}"
 MAS_API_URL = f'https://coreapi.{MAS_CORE_NAMESPACE}.svc.cluster.local:{COREAPI_PORT}'
 
+PEM_PATH = "pempath"
+
 
 def get_secret(name, namespace):
     if name == f"{MAS_INSTANCE_ID}-credentials-superuser":
@@ -68,6 +72,21 @@ def get_secret(name, namespace):
     )
 
 
+@fixture
+def mock_atexit():
+    with patch('atexit.register') as mock_atexit:
+        yield mock_atexit
+
+
+@fixture
+def mock_named_temporary_file(mock_atexit):
+    with patch('tempfile.NamedTemporaryFile') as mock_named_temporary_file:
+        mock_file = MagicMock()
+        mock_file.name = PEM_PATH
+        mock_named_temporary_file.return_value.__enter__.return_value = mock_file
+        yield mock_file
+
+
 @fixture
 def mock_v1_secrets():
     with patch('mas.devops.users.DynamicClient') as mock_DynamicClientCls:
@@ -78,7 +97,7 @@ def mock_v1_secrets():
 
 
 @fixture
-def user_utils(mock_v1_secrets, requests_mock):
+def user_utils(mock_v1_secrets, requests_mock, mock_named_temporary_file, mock_atexit):
     user_utils = MASUserUtils(
         MAS_INSTANCE_ID,
         MAS_WORKSPACE_ID,
@@ -90,6 +109,17 @@ def user_utils(mock_v1_secrets, requests_mock):
     yield user_utils
 
 
+def test_admin_internal_ca_pem_file_path(user_utils, mock_named_temporary_file, mock_atexit):
+    assert str(user_utils.admin_internal_ca_pem_file_path) == PEM_PATH
+    assert mock_named_temporary_file.mock_calls == [call.write(ADMINDASHBOARD_CA_CRT.encode()), call.flush(), call.close()]
+    assert mock_atexit.mock_calls == [call(os.remove, PEM_PATH)]
+
+    # verify caching
+    assert str(user_utils.admin_internal_ca_pem_file_path) == PEM_PATH
+    assert mock_named_temporary_file.mock_calls == [call.write(ADMINDASHBOARD_CA_CRT.encode()), call.flush(), call.close()]
+    assert mock_atexit.mock_calls == [call(os.remove, PEM_PATH)]
+
+
 def mock_get_user(requests_mock, user_id, json, status_code):
     return requests_mock.get(
         f"{MAS_API_URL}/v3/users/{user_id}",
@@ -134,11 +164,6 @@ def test_admin_internal_tls_secret(user_utils, mock_v1_secrets):
     assert mock_v1_secrets.get.call_count == 1
 
 
-def test_admin_internal_ca_pem_file_path():
-    pass
-    # TODO
-
-
 def test_core_internal_tls_secret(user_utils, mock_v1_secrets):
     assert mock_v1_secrets.get.call_count == 0
     assert user_utils.core_internal_tls_secret.data["ca.crt"] == base64.b64encode(COREAPI_CA_CRT.encode('utf-8'))
@@ -147,9 +172,19 @@ def test_core_internal_tls_secret(user_utils, mock_v1_secrets):
     assert mock_v1_secrets.get.call_count == 1
 
 
-def test_core_internal_ca_pem_file_path():
-    pass
-    # TODO
+def test_core_internal_ca_pem_file_path(user_utils, mock_named_temporary_file, mock_atexit):
+    '''
+    Check the correct content is written to core_internal_ca_pem_file_path tempfile, that an exit handler is registered to
+    delete the temp file, and that the tempfile is only written once (with its path cached)
+    '''
+    assert str(user_utils.core_internal_ca_pem_file_path) == PEM_PATH
+    assert mock_named_temporary_file.mock_calls == [call.write(COREAPI_CA_CRT.encode()), call.flush(), call.close()]
+    assert mock_atexit.mock_calls == [call(os.remove, PEM_PATH)]
+
+    # verify caching
+    assert str(user_utils.core_internal_ca_pem_file_path) == PEM_PATH
+    assert mock_named_temporary_file.mock_calls == [call.write(COREAPI_CA_CRT.encode()), call.flush(), call.close()]
+    assert mock_atexit.mock_calls == [call(os.remove, PEM_PATH)]
 
 
 def test_superuser_auth_token():
@@ -169,14 +204,26 @@ def test_manage_internal_tls_secret(user_utils, mock_v1_secrets):
     assert mock_v1_secrets.get.call_count == 1
 
 
-def test_manage_internal_client_pem_file_path():
-    pass
-    # TODO
+def test_manage_internal_client_pem_file_path(user_utils, mock_named_temporary_file, mock_atexit):
+    assert str(user_utils.manage_internal_client_pem_file_path) == PEM_PATH
+    assert mock_named_temporary_file.mock_calls == [call.write(MANAGE_TLS_KEY.encode()), call.write(MANAGE_TLS_CRT.encode()), call.flush(), call.close()]
+    assert mock_atexit.mock_calls == [call(os.remove, PEM_PATH)]
 
+    # verify caching
+    assert str(user_utils.manage_internal_client_pem_file_path) == PEM_PATH
+    assert mock_named_temporary_file.mock_calls == [call.write(MANAGE_TLS_KEY.encode()), call.write(MANAGE_TLS_CRT.encode()), call.flush(), call.close()]
+    assert mock_atexit.mock_calls == [call(os.remove, PEM_PATH)]
 
-def test_manage_internal_ca_pem_file_path():
-    pass
-    # TODO
+
+def test_manage_internal_ca_pem_file_path(user_utils, mock_named_temporary_file, mock_atexit):
+    assert str(user_utils.manage_internal_ca_pem_file_path) == PEM_PATH
+    assert mock_named_temporary_file.mock_calls == [call.write(MANAGE_CA_CRT.encode()), call.flush(), call.close()]
+    assert mock_atexit.mock_calls == [call(os.remove, PEM_PATH)]
+
+    # verify caching
+    assert str(user_utils.manage_internal_ca_pem_file_path) == PEM_PATH
+    assert mock_named_temporary_file.mock_calls == [call.write(MANAGE_CA_CRT.encode()), call.flush(), call.close()]
+    assert mock_atexit.mock_calls == [call(os.remove, PEM_PATH)]
 
 
 def test_manage_maxadmin_api_key():

From 9888627c0be1198556b523467f43813184634e1d Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Wed, 2 Apr 2025 16:38:40 +0100
Subject: [PATCH 29/44] unit tests

---
 src/mas/devops/users.py |   3 +-
 test/src/test_users.py  | 113 ++++++++++++++++++++++++++++------------
 2 files changed, 82 insertions(+), 34 deletions(-)

diff --git a/src/mas/devops/users.py b/src/mas/devops/users.py
index 243a73fa..7c201d12 100644
--- a/src/mas/devops/users.py
+++ b/src/mas/devops/users.py
@@ -249,7 +249,6 @@ def get_or_create_user(self, payload):
 
         url = f"{self.mas_api_url_internal}/v3/users"
         querystring = {}
-        payload = payload
         headers = {
             "Content-Type": "application/json",
             "x-access-token": self.superuser_auth_token
@@ -292,7 +291,7 @@ def update_user(self, payload):
         raise Exception(f"{response.status_code} {response.text}")
 
     def update_user_display_name(self, user_id, display_name):
-        self.logger.debug(f"Updating user display name {user_id}")
+        self.logger.debug(f"Updating user display name {user_id} to {display_name}")
         url = f"{self.mas_api_url_internal}/v3/users/{user_id}"
         headers = {
             "Accept": "application/json",
diff --git a/test/src/test_users.py b/test/src/test_users.py
index 24f49427..6a92750f 100644
--- a/test/src/test_users.py
+++ b/test/src/test_users.py
@@ -43,6 +43,13 @@
 PEM_PATH = "pempath"
 
 
+def additional_matcher(req, json=None, verify=PEM_PATH):
+    if json is not None:
+        assert req.json() == json
+    assert req.verify == verify
+    return True
+
+
 def get_secret(name, namespace):
     if name == f"{MAS_INSTANCE_ID}-credentials-superuser":
         data = {
@@ -97,15 +104,25 @@ def mock_v1_secrets():
 
 
 @fixture
-def user_utils(mock_v1_secrets, requests_mock, mock_named_temporary_file, mock_atexit):
+def mock_logininitial_endpoint(requests_mock):
+    yield requests_mock.post(
+        f"{MAS_ADMIN_URL}/logininitial",
+        json=dict(token=TOKEN),
+        additional_matcher=lambda req: additional_matcher(req, json={"username": SUPERUSER_USERNAME, "password": SUPERUSER_PASSWORD})
+    )
+
+
+@fixture
+def user_utils(mock_v1_secrets, mock_logininitial_endpoint, mock_named_temporary_file, mock_atexit):
+    k8s_client = MagicMock()  # DynamicClient is mocked out, no methods will be called on the k8s_client
     user_utils = MASUserUtils(
         MAS_INSTANCE_ID,
         MAS_WORKSPACE_ID,
-        None,
+        k8s_client,
         coreapi_port=COREAPI_PORT,
         admin_dashboard_port=ADMIN_DASHBOARD_PORT
     )
-    requests_mock.post(f"{MAS_ADMIN_URL}/logininitial", json=dict(token=TOKEN))
+
     yield user_utils
 
 
@@ -125,7 +142,8 @@ def mock_get_user(requests_mock, user_id, json, status_code):
         f"{MAS_API_URL}/v3/users/{user_id}",
         request_headers={"x-access-token": TOKEN},
         json=json,
-        status_code=status_code
+        status_code=status_code,
+        additional_matcher=lambda req: additional_matcher(req)
     )
 
 
@@ -187,9 +205,14 @@ def test_core_internal_ca_pem_file_path(user_utils, mock_named_temporary_file, m
     assert mock_atexit.mock_calls == [call(os.remove, PEM_PATH)]
 
 
-def test_superuser_auth_token():
-    pass
-    # TODO
+def test_superuser_auth_token(user_utils, mock_logininitial_endpoint):
+    assert mock_logininitial_endpoint.call_count == 0
+    assert user_utils.superuser_auth_token == TOKEN
+    assert mock_logininitial_endpoint.call_count == 1
+
+    # verify caching
+    user_utils.superuser_auth_token
+    assert mock_logininitial_endpoint.call_count == 1
 
 
 def test_manage_internal_tls_secret(user_utils, mock_v1_secrets):
@@ -266,7 +289,8 @@ def test_get_or_create_user_exists(user_utils, requests_mock):
         f"{MAS_API_URL}/v3/users",
         request_headers={"x-access-token": TOKEN},
         json={"id": user_id},
-        status_code=201
+        status_code=201,
+        additional_matcher=lambda req: additional_matcher(req, json={"id": user_id})
     )
 
     assert user_utils.get_or_create_user({"id": user_id}) == {"id": user_id, "displayName": user_id}
@@ -282,7 +306,8 @@ def test_get_or_create_user_notfound(user_utils, requests_mock):
         f"{MAS_API_URL}/v3/users",
         request_headers={"x-access-token": TOKEN},
         json={"id": user_id, "displayName": user_id},
-        status_code=201
+        status_code=201,
+        additional_matcher=lambda req: additional_matcher(req, json={"id": user_id})
     )
 
     assert user_utils.get_or_create_user({"id": user_id}) == {"id": user_id, "displayName": user_id}
@@ -297,7 +322,8 @@ def test_get_or_create_user_error(user_utils, requests_mock):
         f"{MAS_API_URL}/v3/users",
         request_headers={"x-access-token": TOKEN},
         json={"error": "unknown"},
-        status_code=500
+        status_code=500,
+        additional_matcher=lambda req: additional_matcher(req, json={"id": user_id})
     )
 
     with pytest.raises(Exception):
@@ -312,7 +338,8 @@ def test_update_user(user_utils, requests_mock):
         f"{MAS_API_URL}/v3/users/{user_id}",
         request_headers={"x-access-token": TOKEN},
         json={"id": user_id},
-        status_code=200
+        status_code=200,
+        additional_matcher=lambda req: additional_matcher(req, json={"id": user_id})
     )
     user_utils.update_user({"id": user_id})
     assert put.call_count == 1
@@ -324,7 +351,8 @@ def test_update_user_error(user_utils, requests_mock):
         f"{MAS_API_URL}/v3/users/{user_id}",
         request_headers={"x-access-token": TOKEN},
         json={"error": "nofound"},
-        status_code=404
+        status_code=404,
+        additional_matcher=lambda req: additional_matcher(req, json={"id": user_id})
     )
     with pytest.raises(Exception):
         user_utils.update_user({"id": user_id})
@@ -337,7 +365,8 @@ def test_update_user_display_name(user_utils, requests_mock):
         f"{MAS_API_URL}/v3/users/{user_id}",
         request_headers={"x-access-token": TOKEN},
         json={"id": user_id},
-        status_code=200
+        status_code=200,
+        additional_matcher=lambda req: additional_matcher(req, json={"displayName": "display_name"})
     )
     user_utils.update_user_display_name(user_id, "display_name")
     assert patche.call_count == 1
@@ -349,7 +378,8 @@ def test_update_user_display_name_error(user_utils, requests_mock):
         f"{MAS_API_URL}/v3/users/{user_id}",
         request_headers={"x-access-token": TOKEN},
         json={"error": "notfound"},
-        status_code=404
+        status_code=404,
+        additional_matcher=lambda req: additional_matcher(req, json={"displayName": "display_name"})
     )
     with pytest.raises(Exception):
         user_utils.update_user_display_name(user_id, "display_name")
@@ -365,7 +395,8 @@ def test_link_user_to_local_idp(user_utils, requests_mock):
         f"{MAS_API_URL}/v3/users/{user_id}/idps/local?emailPassword={email_password}",
         request_headers={"x-access-token": TOKEN},
         json={"id": user_id},
-        status_code=200
+        status_code=200,
+        additional_matcher=lambda req: additional_matcher(req, json={"idpUserId": user_id})
     )
 
     user_utils.link_user_to_local_idp(user_id, email_password=email_password)
@@ -379,6 +410,7 @@ def test_link_user_to_local_idp_usernotfound(user_utils, requests_mock):
     get = mock_get_user_404(requests_mock, user_id)
     put = requests_mock.put(
         f"{MAS_API_URL}/v3/users/{user_id}/idps/local",
+        additional_matcher=lambda req: additional_matcher(req, json={"idpUserId": user_id})
     )
 
     with pytest.raises(Exception):
@@ -399,7 +431,8 @@ def test_link_user_to_local_idp_already_linked(user_utils, requests_mock):
         f"{MAS_API_URL}/v3/users/{user_id}/idps/local?emailPassword={email_password}",
         request_headers={"x-access-token": TOKEN},
         json={"identities": {}},
-        status_code=200
+        status_code=200,
+        additional_matcher=lambda req: additional_matcher(req, json={"idpUserId": user_id})
     )
 
     user_utils.link_user_to_local_idp(user_id, email_password=email_password)
@@ -414,7 +447,8 @@ def test_get_user_workspaces(user_utils, requests_mock):
         f"{MAS_API_URL}/v3/users/{user_id}/workspaces",
         request_headers={"x-access-token": TOKEN},
         json=[{"id": "masdev"}],
-        status_code=200
+        status_code=200,
+        additional_matcher=lambda req: additional_matcher(req)
     )
     workspaces = user_utils.get_user_workspaces(user_id)
     assert workspaces == [{"id": "masdev"}]
@@ -427,7 +461,8 @@ def test_get_user_workspaces_usernotfound(user_utils, requests_mock):
         f"{MAS_API_URL}/v3/users/{user_id}/workspaces",
         request_headers={"x-access-token": TOKEN},
         json={},
-        status_code=404
+        status_code=404,
+        additional_matcher=lambda req: additional_matcher(req)
     )
     with pytest.raises(Exception):
         user_utils.get_user_workspaces(user_id)
@@ -440,7 +475,8 @@ def test_get_user_workspaces_error(user_utils, requests_mock):
         f"{MAS_API_URL}/v3/users/{user_id}/workspaces",
         request_headers={"x-access-token": TOKEN},
         json={"error": "internal"},
-        status_code=500
+        status_code=500,
+        additional_matcher=lambda req: additional_matcher(req)
     )
     with pytest.raises(Exception):
         user_utils.get_user_workspaces(user_id)
@@ -453,13 +489,15 @@ def test_add_user_to_workspace_already_a_member(user_utils, requests_mock):
         f"{MAS_API_URL}/v3/users/{user_id}/workspaces",
         request_headers={"x-access-token": TOKEN},
         json=[{"id": "someotherworkspace"}, {"id": MAS_WORKSPACE_ID}],
-        status_code=200
+        status_code=200,
+        additional_matcher=lambda req: additional_matcher(req)
     )
-    put = requests_mock.get(
+    put = requests_mock.put(
         f"{MAS_API_URL}/workspaces/{MAS_WORKSPACE_ID}/users/{user_id}",
         request_headers={"x-access-token": TOKEN},
         json=[{"id": "masdev"}],
-        status_code=200
+        status_code=200,
+        additional_matcher=lambda req: additional_matcher(req, json={"permissions": {"workspaceAdmin": True}})
     )
     user_utils.add_user_to_workspace(user_id, is_workspace_admin=True)
     assert get.call_count == 1
@@ -478,7 +516,8 @@ def test_add_user_to_workspace(user_utils, requests_mock):
         f"{MAS_API_URL}/workspaces/{MAS_WORKSPACE_ID}/users/{user_id}",
         request_headers={"x-access-token": TOKEN},
         json={},
-        status_code=200
+        status_code=200,
+        additional_matcher=lambda req: additional_matcher(req, json={"permissions": {"workspaceAdmin": True}})
     )
     user_utils.add_user_to_workspace(user_id, is_workspace_admin=True)
     assert get.call_count == 1
@@ -497,7 +536,8 @@ def test_add_user_to_workspace_error(user_utils, requests_mock):
         f"{MAS_API_URL}/workspaces/{MAS_WORKSPACE_ID}/users/{user_id}",
         request_headers={"x-access-token": TOKEN},
         json={"error": "internal"},
-        status_code=500
+        status_code=500,
+        additional_matcher=lambda req: additional_matcher(req, json={"permissions": {"workspaceAdmin": True}})
     )
     with pytest.raises(Exception):
         user_utils.add_user_to_workspace(user_id, is_workspace_admin=True)
@@ -519,7 +559,8 @@ def test_get_user_application_permissions(user_utils, requests_mock):
         f"{MAS_API_URL}/workspaces/{MAS_WORKSPACE_ID}/applications/{application_id}/users/{user_id}",
         request_headers={"x-access-token": TOKEN},
         json=response_json,
-        status_code=200
+        status_code=200,
+        additional_matcher=lambda req: additional_matcher(req)
     )
     assert user_utils.get_user_application_permissions(user_id, application_id) == response_json
     assert get.call_count == 1
@@ -532,7 +573,8 @@ def test_get_user_application_permissions_notfound(user_utils, requests_mock):
         f"{MAS_API_URL}/workspaces/{MAS_WORKSPACE_ID}/applications/{application_id}/users/{user_id}",
         request_headers={"x-access-token": TOKEN},
         json={"error": "notfound"},
-        status_code=404
+        status_code=404,
+        additional_matcher=lambda req: additional_matcher(req)
     )
     assert user_utils.get_user_application_permissions(user_id, application_id) is None
     assert get.call_count == 1
@@ -545,7 +587,8 @@ def test_get_user_application_permissions_error(user_utils, requests_mock):
         f"{MAS_API_URL}/workspaces/{MAS_WORKSPACE_ID}/applications/{application_id}/users/{user_id}",
         request_headers={"x-access-token": TOKEN},
         json={"error": "internal"},
-        status_code=500
+        status_code=500,
+        additional_matcher=lambda req: additional_matcher(req)
     )
     with pytest.raises(Exception):
         user_utils.get_user_application_permissions(user_id, application_id)
@@ -559,13 +602,15 @@ def test_set_user_application_permissions(user_utils, requests_mock):
         f"{MAS_API_URL}/workspaces/{MAS_WORKSPACE_ID}/applications/{application_id}/users/{user_id}",
         request_headers={"x-access-token": TOKEN},
         json={"error": "notfound"},
-        status_code=404
+        status_code=404,
+        additional_matcher=lambda req: additional_matcher(req)
     )
     put = requests_mock.put(
         f"{MAS_API_URL}/workspaces/{MAS_WORKSPACE_ID}/applications/{application_id}/users/{user_id}",
         request_headers={"x-access-token": TOKEN},
         json={},
-        status_code=200
+        status_code=200,
+        additional_matcher=lambda req: additional_matcher(req, json={"role": "USER"})
     )
     user_utils.set_user_application_permission(user_id, application_id, "USER")
     assert get.call_count == 1
@@ -586,13 +631,15 @@ def test_set_user_application_permissions_alreadyset(user_utils, requests_mock):
         f"{MAS_API_URL}/workspaces/{MAS_WORKSPACE_ID}/applications/{application_id}/users/{user_id}",
         request_headers={"x-access-token": TOKEN},
         json=get_response_json,
-        status_code=200
+        status_code=200,
+        additional_matcher=lambda req: additional_matcher(req)
     )
     put = requests_mock.put(
         f"{MAS_API_URL}/workspaces/{MAS_WORKSPACE_ID}/applications/{application_id}/users/{user_id}",
         request_headers={"x-access-token": TOKEN},
         json={},
-        status_code=200
+        status_code=200,
+        additional_matcher=lambda req: additional_matcher(req, json={"role": "USER"})
     )
     user_utils.set_user_application_permission(user_id, application_id, "USER")
     assert get.call_count == 1
@@ -612,7 +659,9 @@ def test_resync_users(user_utils, requests_mock):
                 f"{MAS_API_URL}/v3/users/{user_id}",
                 request_headers={"x-access-token": TOKEN},
                 json={"id": user_id},
-                status_code=200
+                status_code=200,
+                # uid=user_id captures the current value of user_id during each loop iteration, ensuring that the lambda uses the correct value when it is eventually called.
+                additional_matcher=lambda req, uid=user_id: additional_matcher(req, json={"displayName": uid})
             )
         )
 

From 9cf9fceb84efbbf37236effc41c55a4863d6e547 Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Fri, 11 Apr 2025 10:34:35 +0100
Subject: [PATCH 30/44] more unit tests

---
 src/mas/devops/users.py |  25 ++++---
 test/src/test_users.py  | 147 +++++++++++++++++++++++++++++++++++++---
 2 files changed, 154 insertions(+), 18 deletions(-)

diff --git a/src/mas/devops/users.py b/src/mas/devops/users.py
index 7c201d12..1b1385e5 100644
--- a/src/mas/devops/users.py
+++ b/src/mas/devops/users.py
@@ -172,7 +172,7 @@ def manage_internal_ca_pem_file_path(self):
     @property
     def manage_maxadmin_api_key(self):
         if self._manage_maxadmin_api_key is None:
-            self._manage_maxadmin_api_key = self.create_or_get_manage_api_key_for_user(MASUserUtils.MAXADMIN)
+            self._manage_maxadmin_api_key = self.create_or_get_manage_api_key_for_user(MASUserUtils.MAXADMIN, temporary=True)
         return self._manage_maxadmin_api_key
 
     @property
@@ -502,7 +502,11 @@ def resync_users(self, user_ids):
             user = self.get_user(user_id)
             self.update_user_display_name(user_id, user["displayName"])
 
-    def create_or_get_manage_api_key_for_user(self, user_id):
+    def create_or_get_manage_api_key_for_user(self, user_id, temporary=False):
+        '''
+        Get singleton API for user_id if it already exists, create it if not
+        if temporary is True AND we created the API key, delete it on exit
+        '''
         self.logger.debug(f"Attempting to create Manage API Key for user {user_id}")
         url = f"{self.manage_api_url_internal}/maximo/api/os/mxapiapikey"
         querystring = {
@@ -531,14 +535,19 @@ def create_or_get_manage_api_key_for_user(self, user_id):
         )
 
         if response.status_code == 400:
-            error_json = response.json()
+            # Assisted by watsonx Code Assistant
+            try:
+                error_json = response.json()
+            except ValueError:
+                raise Exception(f"{response.status_code} {response.text}")
+
             if "Error" in error_json and "reasonCode" in error_json["Error"] and error_json["Error"]["reasonCode"] == "BMXAA10051E":
                 # BMXAA10051E - Only one API key allowed per user.
                 self.logger.info(f"Reusing existing Manage API Key for user {user_id}")
                 pass
             else:
                 # any other 400 error is unexpected
-                raise Exception(response.text)
+                raise Exception(f"{response.status_code} {response.text}")
 
         elif response.status_code == 201:
             self.logger.info(f"Creating new Manage API Key for user {user_id}")
@@ -554,11 +563,7 @@ def create_or_get_manage_api_key_for_user(self, user_id):
             # so we expect the get call to find it
             raise Exception("API key was unexpectedly not found")
 
-        if response.status_code == 201:
-            # We created this api key, register a handler to delete it when we're done
-            # TODO: is there a danger of some other process adopting this singleton api key
-            #       which may then fail if we subsequently delete it?
-            #       NOTE: the manage sanity test seems to create maxadmin apikey and not clean it up
+        if temporary and response.status_code == 201:
             atexit.register(self.delete_manage_api_key, apikey)
 
         return apikey
@@ -592,7 +597,7 @@ def get_manage_api_key_for_user(self, user_id):
 
             return None
 
-        raise Exception(response.text)
+        raise Exception(f"{response.status_code} {response.text}")
 
     def delete_manage_api_key(self, manage_api_key):
         self.logger.info(f"Deleting Manage API Key for user {manage_api_key['userid']}")
diff --git a/test/src/test_users.py b/test/src/test_users.py
index 6a92750f..8512ce4f 100644
--- a/test/src/test_users.py
+++ b/test/src/test_users.py
@@ -36,17 +36,20 @@
 
 ADMIN_DASHBOARD_PORT = 1
 COREAPI_PORT = 2
+MANAGE_API_PORT = 3
 
 MAS_ADMIN_URL = f"https://admin-dashboard.{MAS_CORE_NAMESPACE}.svc.cluster.local:{ADMIN_DASHBOARD_PORT}"
 MAS_API_URL = f'https://coreapi.{MAS_CORE_NAMESPACE}.svc.cluster.local:{COREAPI_PORT}'
+MANAGE_API_URL = f'https://{MAS_INSTANCE_ID}-{MAS_WORKSPACE_ID}.{MANAGE_NAMESPACE}.svc.cluster.local:{MANAGE_API_PORT}'
 
 PEM_PATH = "pempath"
 
 
-def additional_matcher(req, json=None, verify=PEM_PATH):
+def additional_matcher(req, json=None, verify=PEM_PATH, cert=None):
     if json is not None:
         assert req.json() == json
     assert req.verify == verify
+    assert req.cert == cert
     return True
 
 
@@ -120,7 +123,8 @@ def user_utils(mock_v1_secrets, mock_logininitial_endpoint, mock_named_temporary
         MAS_WORKSPACE_ID,
         k8s_client,
         coreapi_port=COREAPI_PORT,
-        admin_dashboard_port=ADMIN_DASHBOARD_PORT
+        admin_dashboard_port=ADMIN_DASHBOARD_PORT,
+        manage_api_port=MANAGE_API_PORT
     )
 
     yield user_utils
@@ -905,14 +909,141 @@ def test_check_user_sync_appstate_persistent_error(user_utils, requests_mock):
     assert patche.call_count == get.call_count / 2
 
 
-def test_create_or_get_manage_api_key_for_user(user_utils, requests_mock):
-    pass
-    # TODO
+def test_get_manage_api_key_for_user_exists(user_utils, requests_mock):
+    user_id = "user1"
+    apikey = {"userid": user_id, "href": f"https://{MANAGE_API_URL}/maximo/api/os/mxapikey/theapikeyid"}
 
+    get = requests_mock.get(
+        f"{MANAGE_API_URL}/maximo/api/os/mxapiapikey?ccm=1&lean=1&oslc.select=*&oslc.where=userid=\"{user_id}\"",
+        request_headers={"accept": "application/json"},
+        json={"member": [apikey]},
+        status_code=200,
+        additional_matcher=lambda req: additional_matcher(req, cert=PEM_PATH)
+    )
 
-def test_get_manage_api_key_for_user(user_utils, requests_mock):
-    pass
-    # TODO
+    assert user_utils.get_manage_api_key_for_user(user_id) == apikey
+    assert get.call_count == 1
+
+
+def test_get_manage_api_key_for_user_notfound(user_utils, requests_mock):
+    user_id = "user1"
+
+    get = requests_mock.get(
+        f"{MANAGE_API_URL}/maximo/api/os/mxapiapikey?ccm=1&lean=1&oslc.select=*&oslc.where=userid=\"{user_id}\"",
+        request_headers={"accept": "application/json"},
+        json={"member": []},
+        status_code=200,
+        additional_matcher=lambda req: additional_matcher(req, cert=PEM_PATH)
+    )
+
+    assert user_utils.get_manage_api_key_for_user(user_id) is None
+    assert get.call_count == 1
+
+
+def test_get_manage_api_key_for_user_error(user_utils, requests_mock):
+    user_id = "user1"
+
+    get = requests_mock.get(
+        f"{MANAGE_API_URL}/maximo/api/os/mxapiapikey?ccm=1&lean=1&oslc.select=*&oslc.where=userid=\"{user_id}\"",
+        request_headers={"accept": "application/json"},
+        text="boom",
+        status_code=500,
+        additional_matcher=lambda req: additional_matcher(req, cert=PEM_PATH)
+    )
+
+    with pytest.raises(Exception) as excinfo:
+        user_utils.get_manage_api_key_for_user(user_id)
+    assert str(excinfo.value) == "500 boom"
+    assert get.call_count == 1
+
+
+@pytest.mark.parametrize("temporary", [(True), (False)])
+def test_create_or_get_manage_api_key_for_user_new_api_key(temporary, user_utils, requests_mock, mock_atexit):
+    user_id = "user1"
+    apikey = {"userid": user_id, "href": f"https://{MANAGE_API_URL}/maximo/api/os/mxapikey/theapikeyid"}
+
+    post = requests_mock.post(
+        f"{MANAGE_API_URL}/maximo/api/os/mxapiapikey?ccm=1&lean=1",
+        request_headers={"content-type": "application/json"},
+        json={"id": user_id},
+        status_code=201,
+        additional_matcher=lambda req: additional_matcher(req, json={"expiration": -1, "userid": user_id}, cert=PEM_PATH)
+    )
+
+    get = requests_mock.get(
+        f"{MANAGE_API_URL}/maximo/api/os/mxapiapikey?ccm=1&lean=1&oslc.select=*&oslc.where=userid=\"{user_id}\"",
+        request_headers={"accept": "application/json"},
+        json={"member": [apikey]},
+        status_code=200,
+        additional_matcher=lambda req: additional_matcher(req, cert=PEM_PATH)
+    )
+
+    assert user_utils.create_or_get_manage_api_key_for_user(user_id, temporary=temporary) == apikey
+    assert post.call_count == 1
+    assert get.call_count == 1
+
+    # if temporary, check we registered the exit hook to delete the temporary Manage API Key
+    if temporary:
+        assert call(user_utils.delete_manage_api_key, apikey) in mock_atexit.mock_calls, "delete_manage_api_key exit hook not registered for temporary api key that we created"
+    else:
+        assert call(user_utils.delete_manage_api_key, apikey) not in mock_atexit.mock_calls, "delete_manage_api_key exit hook registered unexpectedly for non-temporary api key that we created"
+
+
+@pytest.mark.parametrize("temporary", [(True), (False)])
+def test_create_or_get_manage_api_key_for_user_existing_api_key(temporary, user_utils, requests_mock, mock_atexit):
+    user_id = "user1"
+    apikey = {"userid": user_id, "href": f"https://{MANAGE_API_URL}/maximo/api/os/mxapikey/theapikeyid"}
+
+    post = requests_mock.post(
+        f"{MANAGE_API_URL}/maximo/api/os/mxapiapikey?ccm=1&lean=1",
+        request_headers={"content-type": "application/json"},
+        json={"Error": {"reasonCode": "BMXAA10051E"}},
+        status_code=400,
+        additional_matcher=lambda req: additional_matcher(req, json={"expiration": -1, "userid": user_id}, cert=PEM_PATH)
+    )
+
+    get = requests_mock.get(
+        f"{MANAGE_API_URL}/maximo/api/os/mxapiapikey?ccm=1&lean=1&oslc.select=*&oslc.where=userid=\"{user_id}\"",
+        request_headers={"accept": "application/json"},
+        json={"member": [apikey]},
+        status_code=200,
+        additional_matcher=lambda req: additional_matcher(req, cert=PEM_PATH)
+    )
+
+    assert user_utils.create_or_get_manage_api_key_for_user(user_id, temporary=temporary) == apikey
+    assert post.call_count == 1
+    assert get.call_count == 1
+
+    # even if temporary is set, because we did not create the api key, we should not registered a hook to delete it
+    assert call(user_utils.delete_manage_api_key, apikey) not in mock_atexit.mock_calls, "delete_manage_api_key exit hook registered unexpectedly for existing API Key that we did not create"
+
+
+def test_create_or_get_manage_api_key_for_user_error(user_utils, requests_mock, mock_atexit):
+    user_id = "user1"
+    apikey = {"userid": user_id, "href": f"https://{MANAGE_API_URL}/maximo/api/os/mxapikey/theapikeyid"}
+
+    post = requests_mock.post(
+        f"{MANAGE_API_URL}/maximo/api/os/mxapiapikey?ccm=1&lean=1",
+        request_headers={"content-type": "application/json"},
+        text="boom",
+        status_code=400,
+        additional_matcher=lambda req: additional_matcher(req, json={"expiration": -1, "userid": user_id}, cert=PEM_PATH)
+    )
+
+    get = requests_mock.get(
+        f"{MANAGE_API_URL}/maximo/api/os/mxapiapikey?ccm=1&lean=1&oslc.select=*&oslc.where=userid=\"{user_id}\"",
+        request_headers={"accept": "application/json"},
+        json={"member": [apikey]},
+        status_code=200,
+        additional_matcher=lambda req: additional_matcher(req, cert=PEM_PATH)
+    )
+
+    with pytest.raises(Exception) as excinfo:
+        user_utils.create_or_get_manage_api_key_for_user(user_id, temporary=True)
+    assert str(excinfo.value) == "400 boom"
+    assert post.call_count == 1
+    assert get.call_count == 0
+    assert call(user_utils.delete_manage_api_key, apikey) not in mock_atexit.mock_calls, "delete_manage_api_key exit hook not registered even though we failed to create the api key"
 
 
 def test_delete_manage_api_key(user_utils, requests_mock):

From dbdcc9b4a28a23673080de38bb658045bedca3d2 Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Fri, 11 Apr 2025 10:44:20 +0100
Subject: [PATCH 31/44] remove confusing manage_apikey computed property

---
 src/mas/devops/users.py | 33 +++++++++++----------------------
 1 file changed, 11 insertions(+), 22 deletions(-)

diff --git a/src/mas/devops/users.py b/src/mas/devops/users.py
index 1b1385e5..77d91246 100644
--- a/src/mas/devops/users.py
+++ b/src/mas/devops/users.py
@@ -65,8 +65,6 @@ def __init__(self, mas_instance_id: str, mas_workspace_id: str, k8s_client: clie
         self._manage_internal_ca_pem_file_path = None
         self._manage_internal_client_pem_file_path = None
 
-        self._manage_maxadmin_api_key = None
-
         self._mas_workspace_application_ids = None
 
     @property
@@ -169,12 +167,6 @@ def manage_internal_ca_pem_file_path(self):
                 self._manage_internal_ca_pem_file_path = pem_file.name
         return self._manage_internal_ca_pem_file_path
 
-    @property
-    def manage_maxadmin_api_key(self):
-        if self._manage_maxadmin_api_key is None:
-            self._manage_maxadmin_api_key = self.create_or_get_manage_api_key_for_user(MASUserUtils.MAXADMIN, temporary=True)
-        return self._manage_maxadmin_api_key
-
     @property
     def mas_workspace_application_ids(self):
         if self._mas_workspace_application_ids is None:
@@ -626,11 +618,7 @@ def delete_manage_api_key(self, manage_api_key):
             raise Exception(response.text)
         # {"Error":{"extendedError":{"moreInfo":{"href":"https:\/\/masdev.manage.tgk01.apps.noble4.cp.fyre.ibm.com\/maximo\/api\/error\/messages\/BMXAA8727E"}},"reasonCode":"BMXAA8727E","message":"The OSLC resource MXAPIAPIKEY with the ID _WmxvZlZLNVl2V3dGa1FseUJoKzJ4ZzQzSEd1bmRUamdWcTFiV1hWMGQ5QnAyNHQxQm53TmVFRWtVbmN4YkI2alZSTlp3eElsQko2bElNSCJzcCJ1M3hiNlE9PQ-- was not found as it does not exist in the system. In the database, verify whether the resource for the ID exists.","statusCode":"404"}}
 
-        if manage_api_key["userid"] == MASUserUtils.MAXADMIN:
-            # clear any cached _manage_maxadmin_api_key if necessary
-            self._manage_maxadmin_api_key = None
-
-    def get_manage_group_id(self, group_name):
+    def get_manage_group_id(self, group_name, manage_api_key):
         self.logger.debug(f"Getting ID for Manage group {group_name}")
         url = f"{self.manage_api_url_internal}/maximo/api/os/mxapigroup"
         querystring = {
@@ -641,7 +629,7 @@ def get_manage_group_id(self, group_name):
         }
         headers = {
             "Accept": "application/json",
-            "apikey": self.manage_maxadmin_api_key["apikey"],  # <--- careful, don't log headers as-is (apikey is sensitive)
+            "apikey": manage_api_key["apikey"],  # <--- careful, don't log headers as-is (apikey is sensitive)
         }
         response = requests.get(
             url,
@@ -659,9 +647,9 @@ def get_manage_group_id(self, group_name):
 
         return None
 
-    def is_user_in_manage_group(self, group_name, user_id):
+    def is_user_in_manage_group(self, group_name, user_id, manage_api_key):
 
-        group_id = self.get_manage_group_id(group_name)
+        group_id = self.get_manage_group_id(group_name, manage_api_key)
 
         url = f"{self.manage_api_url_internal}/maximo/api/os/mxapigroup/{group_id}/groupuser"
         querystring = {
@@ -670,7 +658,7 @@ def is_user_in_manage_group(self, group_name, user_id):
         }
         headers = {
             "Accept": "application/json",
-            "apikey": self.manage_maxadmin_api_key["apikey"],  # <--- careful, don't log headers as-is (apikey is sensitive)
+            "apikey": manage_api_key["apikey"],  # <--- careful, don't log headers as-is (apikey is sensitive)
         }
 
         response = requests.get(
@@ -686,18 +674,18 @@ def is_user_in_manage_group(self, group_name, user_id):
 
         raise Exception(f"{response.status_code} {response.text}")
 
-    def add_user_to_manage_group(self, user_id, group_name):
+    def add_user_to_manage_group(self, user_id, group_name, manage_api_key):
         '''
         No-op if user_id is already a member of the manage security group
         '''
 
-        if self.is_user_in_manage_group(group_name, user_id):
+        if self.is_user_in_manage_group(group_name, user_id, manage_api_key):
             self.logger.info(f"User {user_id} is already a member of Manage Security Group {group_name}")
             return None
 
         self.logger.info(f"Adding user {user_id} to Manage group {group_name}")
 
-        group_id = self.get_manage_group_id(group_name)
+        group_id = self.get_manage_group_id(group_name, manage_api_key)
 
         url = f"{self.manage_api_url_internal}/maximo/api/os/mxapigroup/{group_id}"
         querystring = {
@@ -708,7 +696,7 @@ def add_user_to_manage_group(self, user_id, group_name):
             "Accept": "application/json",
             "x-method-override": "PATCH",
             "patchtype": "MERGE",
-            "apikey": self.manage_maxadmin_api_key["apikey"],  # <--- careful, don't log headers as-is (apikey is sensitive)
+            "apikey": manage_api_key["apikey"],  # <--- careful, don't log headers as-is (apikey is sensitive)
         }
         payload = {
             "groupuser": [
@@ -956,8 +944,9 @@ def create_initial_user_for_saas(self, user, user_type):
             self.check_user_sync(user_id, mas_application_id)
 
         if "manage" in self.mas_workspace_application_ids:
+            maxadmin_manage_api_key = self.create_or_get_manage_api_key_for_user(MASUserUtils.MAXADMIN, temporary=True)
             for manage_security_group in manage_security_groups:
-                self.add_user_to_manage_group(user_id, manage_security_group)
+                self.add_user_to_manage_group(user_id, manage_security_group, maxadmin_manage_api_key)
 
     # Unused (but potentially useful) methods
     # ----------------------------------------

From caaad9f2decda25dc9aece3858990449312d52ca Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Fri, 11 Apr 2025 10:53:30 +0100
Subject: [PATCH 32/44] unit tests

---
 src/mas/devops/users.py |  3 +--
 test/src/test_users.py  | 18 ++++++++++++++++--
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/src/mas/devops/users.py b/src/mas/devops/users.py
index 77d91246..4c7b9fc2 100644
--- a/src/mas/devops/users.py
+++ b/src/mas/devops/users.py
@@ -615,8 +615,7 @@ def delete_manage_api_key(self, manage_api_key):
         )
 
         if response.status_code != 204 and response.status_code != 404:
-            raise Exception(response.text)
-        # {"Error":{"extendedError":{"moreInfo":{"href":"https:\/\/masdev.manage.tgk01.apps.noble4.cp.fyre.ibm.com\/maximo\/api\/error\/messages\/BMXAA8727E"}},"reasonCode":"BMXAA8727E","message":"The OSLC resource MXAPIAPIKEY with the ID _WmxvZlZLNVl2V3dGa1FseUJoKzJ4ZzQzSEd1bmRUamdWcTFiV1hWMGQ5QnAyNHQxQm53TmVFRWtVbmN4YkI2alZSTlp3eElsQko2bElNSCJzcCJ1M3hiNlE9PQ-- was not found as it does not exist in the system. In the database, verify whether the resource for the ID exists.","statusCode":"404"}}
+            raise Exception(f"{response.status_code} {response.text}")
 
     def get_manage_group_id(self, group_name, manage_api_key):
         self.logger.debug(f"Getting ID for Manage group {group_name}")
diff --git a/test/src/test_users.py b/test/src/test_users.py
index 8512ce4f..3ea31343 100644
--- a/test/src/test_users.py
+++ b/test/src/test_users.py
@@ -1047,8 +1047,22 @@ def test_create_or_get_manage_api_key_for_user_error(user_utils, requests_mock,
 
 
 def test_delete_manage_api_key(user_utils, requests_mock):
-    pass
-    # TODO
+    user_id = "user1"
+    apikey_id = "theapikeyid"
+    apikey = {"userid": user_id, "href": f"https://{MANAGE_API_URL}:{MANAGE_API_PORT}/maximo/api/os/mxapiapikey/{apikey_id}"}
+
+    delete = requests_mock.delete(
+        f"{MANAGE_API_URL}/maximo/api/os/mxapiapikey/{apikey_id}?ccm=1&lean=1",
+        request_headers={"accept": "application/json"},
+        text="notused",
+        status_code=204,
+        additional_matcher=lambda req: additional_matcher(req, cert=PEM_PATH)
+    )
+
+    user_utils.delete_manage_api_key(apikey)
+    assert delete.call_count == 1
+
+# TODO: href_does_not_parse
 
 
 def test_get_manage_group_id(user_utils, requests_mock):

From 60a50043be39880989b9eece58421fbf6b2a8f20 Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Fri, 11 Apr 2025 11:10:42 +0100
Subject: [PATCH 33/44] unit tests

---
 src/mas/devops/users.py |  8 ++++--
 test/src/test_users.py  | 55 ++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 60 insertions(+), 3 deletions(-)

diff --git a/src/mas/devops/users.py b/src/mas/devops/users.py
index 4c7b9fc2..111ff1bf 100644
--- a/src/mas/devops/users.py
+++ b/src/mas/devops/users.py
@@ -596,6 +596,9 @@ def delete_manage_api_key(self, manage_api_key):
 
         # extract the apikey's identifier from the href
         match = re.search(r'\/maximo\/api\/os\/mxapiapikey\/(.*)', manage_api_key['href'])
+        if match is None:
+            raise Exception(f"Could not parse API Key href: {manage_api_key['href']}")
+
         id = match.group(1)
 
         url = f"{self.manage_api_url_internal}/maximo/api/os/mxapiapikey/{id}"
@@ -618,7 +621,7 @@ def delete_manage_api_key(self, manage_api_key):
             raise Exception(f"{response.status_code} {response.text}")
 
     def get_manage_group_id(self, group_name, manage_api_key):
-        self.logger.debug(f"Getting ID for Manage group {group_name}")
+        self.logger.debug(f"Getting ID for Manage group with name {group_name}")
         url = f"{self.manage_api_url_internal}/maximo/api/os/mxapigroup"
         querystring = {
             "ccm": 1,
@@ -637,7 +640,7 @@ def get_manage_group_id(self, group_name, manage_api_key):
             verify=self.manage_internal_ca_pem_file_path,
         )
         if response.status_code != 200:
-            raise Exception(response.text)
+            raise Exception(f"{response.status_code} {response.text}")
 
         json = response.json()
 
@@ -647,6 +650,7 @@ def get_manage_group_id(self, group_name, manage_api_key):
         return None
 
     def is_user_in_manage_group(self, group_name, user_id, manage_api_key):
+        self.logger.debug(f"Checking if {user_id} is a member of Manage group with name {group_name}")
 
         group_id = self.get_manage_group_id(group_name, manage_api_key)
 
diff --git a/test/src/test_users.py b/test/src/test_users.py
index 3ea31343..fc6efeb4 100644
--- a/test/src/test_users.py
+++ b/test/src/test_users.py
@@ -1062,7 +1062,60 @@ def test_delete_manage_api_key(user_utils, requests_mock):
     user_utils.delete_manage_api_key(apikey)
     assert delete.call_count == 1
 
-# TODO: href_does_not_parse
+
+def test_delete_manage_api_key_notfound(user_utils, requests_mock):
+    user_id = "user1"
+    apikey_id = "theapikeyid"
+    apikey = {"userid": user_id, "href": f"https://{MANAGE_API_URL}:{MANAGE_API_PORT}/maximo/api/os/mxapiapikey/{apikey_id}"}
+
+    delete = requests_mock.delete(
+        f"{MANAGE_API_URL}/maximo/api/os/mxapiapikey/{apikey_id}?ccm=1&lean=1",
+        request_headers={"accept": "application/json"},
+        text="notused",
+        status_code=404,
+        additional_matcher=lambda req: additional_matcher(req, cert=PEM_PATH)
+    )
+
+    user_utils.delete_manage_api_key(apikey)
+    assert delete.call_count == 1
+
+
+def test_delete_manage_api_key_bad_href(user_utils, requests_mock):
+    user_id = "user1"
+    apikey_id = "theapikeyid"
+    apikey = {"userid": user_id, "href": f"notgood/{apikey_id}"}
+
+    delete = requests_mock.delete(
+        f"{MANAGE_API_URL}/maximo/api/os/mxapiapikey/{apikey_id}?ccm=1&lean=1",
+        request_headers={"accept": "application/json"},
+        text="notused",
+        status_code=204,
+        additional_matcher=lambda req: additional_matcher(req, cert=PEM_PATH)
+    )
+
+    with pytest.raises(Exception) as excinfo:
+        user_utils.delete_manage_api_key(apikey)
+    assert str(excinfo.value) == f"Could not parse API Key href: notgood/{apikey_id}"
+    assert delete.call_count == 0
+
+
+def test_delete_manage_api_key_error(user_utils, requests_mock):
+    user_id = "user1"
+    apikey_id = "theapikeyid"
+    apikey = {"userid": user_id, "href": f"https://{MANAGE_API_URL}:{MANAGE_API_PORT}/maximo/api/os/mxapiapikey/{apikey_id}"}
+
+    delete = requests_mock.delete(
+        f"{MANAGE_API_URL}/maximo/api/os/mxapiapikey/{apikey_id}?ccm=1&lean=1",
+        request_headers={"accept": "application/json"},
+        text="boom",
+        status_code=500,
+        additional_matcher=lambda req: additional_matcher(req, cert=PEM_PATH)
+    )
+
+    with pytest.raises(Exception) as excinfo:
+        user_utils.delete_manage_api_key(apikey)
+    assert str(excinfo.value) == "500 boom"
+    assert delete.call_count == 1
 
 
 def test_get_manage_group_id(user_utils, requests_mock):

From 481a76f6a861af96fada04e964aa32b91dccc4d7 Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Fri, 11 Apr 2025 11:24:48 +0100
Subject: [PATCH 34/44] unit tests

---
 src/mas/devops/users.py |  3 ++
 test/src/test_users.py  | 78 +++++++++++++++++++++++++++++++++++++++--
 2 files changed, 79 insertions(+), 2 deletions(-)

diff --git a/src/mas/devops/users.py b/src/mas/devops/users.py
index 111ff1bf..b7e1f7e1 100644
--- a/src/mas/devops/users.py
+++ b/src/mas/devops/users.py
@@ -654,6 +654,9 @@ def is_user_in_manage_group(self, group_name, user_id, manage_api_key):
 
         group_id = self.get_manage_group_id(group_name, manage_api_key)
 
+        if group_id is None:
+            raise Exception(f"No Manage group found with name {group_name}")
+
         url = f"{self.manage_api_url_internal}/maximo/api/os/mxapigroup/{group_id}/groupuser"
         querystring = {
             "lean": 1,
diff --git a/test/src/test_users.py b/test/src/test_users.py
index fc6efeb4..5d6fe640 100644
--- a/test/src/test_users.py
+++ b/test/src/test_users.py
@@ -130,6 +130,33 @@ def user_utils(mock_v1_secrets, mock_logininitial_endpoint, mock_named_temporary
     yield user_utils
 
 
+@fixture
+def mock_manage_api_key(requests_mock):
+    '''
+    Setup mock Manage APIs for setting up an API Key
+    '''
+    user_id = "user1"
+    apikey = {"userid": user_id, "href": f"https://{MANAGE_API_URL}/maximo/api/os/mxapikey/theapikeyid"}
+
+    requests_mock.post(
+        f"{MANAGE_API_URL}/maximo/api/os/mxapiapikey?ccm=1&lean=1",
+        request_headers={"content-type": "application/json"},
+        json={"id": user_id},
+        status_code=201,
+        additional_matcher=lambda req: additional_matcher(req, json={"expiration": -1, "userid": user_id}, cert=PEM_PATH)
+    )
+
+    requests_mock.get(
+        f"{MANAGE_API_URL}/maximo/api/os/mxapiapikey?ccm=1&lean=1&oslc.select=*&oslc.where=userid=\"{user_id}\"",
+        request_headers={"accept": "application/json"},
+        json={"member": [apikey]},
+        status_code=200,
+        additional_matcher=lambda req: additional_matcher(req, cert=PEM_PATH)
+    )
+
+    yield apikey
+
+
 def test_admin_internal_ca_pem_file_path(user_utils, mock_named_temporary_file, mock_atexit):
     assert str(user_utils.admin_internal_ca_pem_file_path) == PEM_PATH
     assert mock_named_temporary_file.mock_calls == [call.write(ADMINDASHBOARD_CA_CRT.encode()), call.flush(), call.close()]
@@ -1119,8 +1146,55 @@ def test_delete_manage_api_key_error(user_utils, requests_mock):
 
 
 def test_get_manage_group_id(user_utils, requests_mock):
-    pass
-    # TODO
+    user_id = "user1"
+    apikey = {"userid": user_id, "apikey": "342fwasdasd", "href": f"https://{MANAGE_API_URL}/maximo/api/os/mxapikey/theapikeyid"}  # pragma: allowlist secret
+    group_name = "thegroup"
+    group_id = "39231234"
+
+    get = requests_mock.get(
+        f"{MANAGE_API_URL}/maximo/api/os/mxapigroup?ccm=1&lean=1&oslc.select=maxgroupid&oslc.where=groupname=\"{group_name}\"",
+        request_headers={"accept": "application/json"},
+        json={"member": [{"maxgroupid": group_id}]},
+        status_code=200,
+        additional_matcher=lambda req: additional_matcher(req)
+    )
+
+    assert user_utils.get_manage_group_id(group_name, apikey) == group_id
+    assert get.call_count == 1
+
+
+def test_get_manage_group_id_error(user_utils, requests_mock):
+    user_id = "user1"
+    apikey = {"userid": user_id, "apikey": "342fwasdasd", "href": f"https://{MANAGE_API_URL}/maximo/api/os/mxapikey/theapikeyid"}  # pragma: allowlist secret
+    group_name = "thegroup"
+
+    get = requests_mock.get(
+        f"{MANAGE_API_URL}/maximo/api/os/mxapigroup?ccm=1&lean=1&oslc.select=maxgroupid&oslc.where=groupname=\"{group_name}\"",
+        request_headers={"accept": "application/json"},
+        text="boom",
+        status_code=500,
+        additional_matcher=lambda req: additional_matcher(req)
+    )
+    with pytest.raises(Exception) as excinfo:
+        user_utils.get_manage_group_id(group_name, apikey)
+    assert str(excinfo.value) == "500 boom"
+    assert get.call_count == 1
+
+
+def test_get_manage_group_id_notfound(user_utils, requests_mock):
+    user_id = "user1"
+    apikey = {"userid": user_id, "apikey": "342fwasdasd", "href": f"https://{MANAGE_API_URL}/maximo/api/os/mxapikey/theapikeyid"}  # pragma: allowlist secret
+    group_name = "thegroup"
+
+    get = requests_mock.get(
+        f"{MANAGE_API_URL}/maximo/api/os/mxapigroup?ccm=1&lean=1&oslc.select=maxgroupid&oslc.where=groupname=\"{group_name}\"",
+        request_headers={"accept": "application/json"},
+        json={"member": [{}]},
+        status_code=200,
+        additional_matcher=lambda req: additional_matcher(req)
+    )
+    assert user_utils.get_manage_group_id(group_name, apikey) is None
+    assert get.call_count == 1
 
 
 def test_is_user_in_manage_group(user_utils, requests_mock):

From ea4519749f96225217bb7998bde6cb4042e5e81d Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Fri, 11 Apr 2025 12:01:46 +0100
Subject: [PATCH 35/44] unit tests

---
 src/mas/devops/users.py |   4 -
 test/src/test_users.py  | 245 ++++++++++++++++++++++++++++++++++++++--
 2 files changed, 237 insertions(+), 12 deletions(-)

diff --git a/src/mas/devops/users.py b/src/mas/devops/users.py
index b7e1f7e1..712834ce 100644
--- a/src/mas/devops/users.py
+++ b/src/mas/devops/users.py
@@ -711,8 +711,6 @@ def add_user_to_manage_group(self, user_id, group_name, manage_api_key):
                 }
             ]
         }
-        self.logger.debug(f"  > {url} {querystring}")
-
         response = requests.post(
             url,
             headers=headers,
@@ -720,8 +718,6 @@ def add_user_to_manage_group(self, user_id, group_name, manage_api_key):
             json=payload,
             verify=self.manage_internal_ca_pem_file_path,
         )
-        self.logger.debug(f"  < {response.status_code}")
-
         if response.status_code == 204:
             return None
 
diff --git a/test/src/test_users.py b/test/src/test_users.py
index 5d6fe640..5fd3dd7f 100644
--- a/test/src/test_users.py
+++ b/test/src/test_users.py
@@ -1153,7 +1153,7 @@ def test_get_manage_group_id(user_utils, requests_mock):
 
     get = requests_mock.get(
         f"{MANAGE_API_URL}/maximo/api/os/mxapigroup?ccm=1&lean=1&oslc.select=maxgroupid&oslc.where=groupname=\"{group_name}\"",
-        request_headers={"accept": "application/json"},
+        request_headers={"accept": "application/json", "apikey": apikey["apikey"]},
         json={"member": [{"maxgroupid": group_id}]},
         status_code=200,
         additional_matcher=lambda req: additional_matcher(req)
@@ -1170,7 +1170,7 @@ def test_get_manage_group_id_error(user_utils, requests_mock):
 
     get = requests_mock.get(
         f"{MANAGE_API_URL}/maximo/api/os/mxapigroup?ccm=1&lean=1&oslc.select=maxgroupid&oslc.where=groupname=\"{group_name}\"",
-        request_headers={"accept": "application/json"},
+        request_headers={"accept": "application/json", "apikey": apikey["apikey"]},
         text="boom",
         status_code=500,
         additional_matcher=lambda req: additional_matcher(req)
@@ -1188,7 +1188,7 @@ def test_get_manage_group_id_notfound(user_utils, requests_mock):
 
     get = requests_mock.get(
         f"{MANAGE_API_URL}/maximo/api/os/mxapigroup?ccm=1&lean=1&oslc.select=maxgroupid&oslc.where=groupname=\"{group_name}\"",
-        request_headers={"accept": "application/json"},
+        request_headers={"accept": "application/json", "apikey": apikey["apikey"]},
         json={"member": [{}]},
         status_code=200,
         additional_matcher=lambda req: additional_matcher(req)
@@ -1197,14 +1197,243 @@ def test_get_manage_group_id_notfound(user_utils, requests_mock):
     assert get.call_count == 1
 
 
-def test_is_user_in_manage_group(user_utils, requests_mock):
-    pass
-    # TODO
+def test_is_user_in_manage_group_yes(user_utils, requests_mock):
+    user_id = "user1"
+    apikey = {"userid": user_id, "apikey": "342fwasdasd", "href": f"https://{MANAGE_API_URL}/maximo/api/os/mxapikey/theapikeyid"}  # pragma: allowlist secret
+    group_name = "thegroup"
+    group_id = "39231234"
+
+    get_group_id = requests_mock.get(
+        f"{MANAGE_API_URL}/maximo/api/os/mxapigroup?ccm=1&lean=1&oslc.select=maxgroupid&oslc.where=groupname=\"{group_name}\"",
+        request_headers={"accept": "application/json"},
+        json={"member": [{"maxgroupid": group_id}], "apikey": apikey["apikey"]},
+        status_code=200,
+        additional_matcher=lambda req: additional_matcher(req)
+    )
+
+    get_group_user = requests_mock.get(
+        f"{MANAGE_API_URL}/maximo/api/os/mxapigroup/{group_id}/groupuser?lean=1&oslc.where=userid=\"{user_id}\"",
+        request_headers={"accept": "application/json", "apikey": apikey["apikey"]},
+        json={"member": [{}]},  # <--- member length non-empty indicates that the user is a member of the group
+        status_code=200,
+        additional_matcher=lambda req: additional_matcher(req)
+    )
+
+    assert user_utils.is_user_in_manage_group(group_name, user_id, apikey)
+    assert get_group_id.call_count == 1
+    assert get_group_user.call_count == 1
+
+
+def test_is_user_in_manage_group_no(user_utils, requests_mock):
+    user_id = "user1"
+    apikey = {"userid": user_id, "apikey": "342fwasdasd", "href": f"https://{MANAGE_API_URL}/maximo/api/os/mxapikey/theapikeyid"}  # pragma: allowlist secret
+    group_name = "thegroup"
+    group_id = "39231234"
+
+    get_group_id = requests_mock.get(
+        f"{MANAGE_API_URL}/maximo/api/os/mxapigroup?ccm=1&lean=1&oslc.select=maxgroupid&oslc.where=groupname=\"{group_name}\"",
+        request_headers={"accept": "application/json"},
+        json={"member": [{"maxgroupid": group_id}], "apikey": apikey["apikey"]},
+        status_code=200,
+        additional_matcher=lambda req: additional_matcher(req)
+    )
+
+    get_group_user = requests_mock.get(
+        f"{MANAGE_API_URL}/maximo/api/os/mxapigroup/{group_id}/groupuser?lean=1&oslc.where=userid=\"{user_id}\"",
+        request_headers={"accept": "application/json", "apikey": apikey["apikey"]},
+        json={"member": []},
+        status_code=200,
+        additional_matcher=lambda req: additional_matcher(req)
+    )
+
+    assert not user_utils.is_user_in_manage_group(group_name, user_id, apikey)
+    assert get_group_id.call_count == 1
+    assert get_group_user.call_count == 1
+
+
+def test_is_user_in_manage_group_error(user_utils, requests_mock):
+    user_id = "user1"
+    apikey = {"userid": user_id, "apikey": "342fwasdasd", "href": f"https://{MANAGE_API_URL}/maximo/api/os/mxapikey/theapikeyid"}  # pragma: allowlist secret
+    group_name = "thegroup"
+    group_id = "39231234"
+
+    get_group_id = requests_mock.get(
+        f"{MANAGE_API_URL}/maximo/api/os/mxapigroup?ccm=1&lean=1&oslc.select=maxgroupid&oslc.where=groupname=\"{group_name}\"",
+        request_headers={"accept": "application/json"},
+        json={"member": [{"maxgroupid": group_id}], "apikey": apikey["apikey"]},
+        status_code=200,
+        additional_matcher=lambda req: additional_matcher(req)
+    )
+
+    get_group_user = requests_mock.get(
+        f"{MANAGE_API_URL}/maximo/api/os/mxapigroup/{group_id}/groupuser?lean=1&oslc.where=userid=\"{user_id}\"",
+        request_headers={"accept": "application/json", "apikey": apikey["apikey"]},
+        text="boom",
+        status_code=500,
+        additional_matcher=lambda req: additional_matcher(req)
+    )
+
+    with pytest.raises(Exception) as excinfo:
+        user_utils.is_user_in_manage_group(group_name, user_id, apikey)
+    assert str(excinfo.value) == "500 boom"
+    assert get_group_id.call_count == 1
+    assert get_group_user.call_count == 1
+
+
+def test_is_user_in_manage_group_no_group_found(user_utils, requests_mock):
+    user_id = "user1"
+    apikey = {"userid": user_id, "apikey": "342fwasdasd", "href": f"https://{MANAGE_API_URL}/maximo/api/os/mxapikey/theapikeyid"}  # pragma: allowlist secret
+    group_name = "thegroup"
+    group_id = "39231234"
+
+    get_group_id = requests_mock.get(
+        f"{MANAGE_API_URL}/maximo/api/os/mxapigroup?ccm=1&lean=1&oslc.select=maxgroupid&oslc.where=groupname=\"{group_name}\"",
+        request_headers={"accept": "application/json"},
+        json={"member": [], "apikey": apikey["apikey"]},
+        status_code=200,
+        additional_matcher=lambda req: additional_matcher(req)
+    )
+
+    get_group_user = requests_mock.get(
+        f"{MANAGE_API_URL}/maximo/api/os/mxapigroup/{group_id}/groupuser?lean=1&oslc.where=userid=\"{user_id}\"",
+        request_headers={"accept": "application/json", "apikey": apikey["apikey"]},
+        text="boom",
+        status_code=500,
+        additional_matcher=lambda req: additional_matcher(req)
+    )
+
+    with pytest.raises(Exception) as excinfo:
+        user_utils.is_user_in_manage_group(group_name, user_id, apikey)
+    assert str(excinfo.value) == f"No Manage group found with name {group_name}"
+    assert get_group_id.call_count == 1
+    assert get_group_user.call_count == 0
 
 
 def test_add_user_to_manage_group(user_utils, requests_mock):
-    pass
-    # TODO
+    user_id = "user1"
+    apikey = {"userid": user_id, "apikey": "342fwasdasd", "href": f"https://{MANAGE_API_URL}/maximo/api/os/mxapikey/theapikeyid"}  # pragma: allowlist secret
+    group_name = "thegroup"
+    group_id = "39231234"
+
+    get_group_id = requests_mock.get(
+        f"{MANAGE_API_URL}/maximo/api/os/mxapigroup?ccm=1&lean=1&oslc.select=maxgroupid&oslc.where=groupname=\"{group_name}\"",
+        request_headers={"accept": "application/json"},
+        json={"member": [{"maxgroupid": group_id}], "apikey": apikey["apikey"]},
+        status_code=200,
+        additional_matcher=lambda req: additional_matcher(req)
+    )
+
+    get_group_user = requests_mock.get(
+        f"{MANAGE_API_URL}/maximo/api/os/mxapigroup/{group_id}/groupuser?lean=1",
+        request_headers={"accept": "application/json", "apikey": apikey["apikey"]},
+        json={"member": []},
+        status_code=200,
+        additional_matcher=lambda req: additional_matcher(req)
+    )
+
+    add_group_user = requests_mock.post(
+        f"{MANAGE_API_URL}/maximo/api/os/mxapigroup/{group_id}",
+        request_headers={
+            "accept": "application/json",
+            "content-type": "application/json",
+            "x-method-override": "PATCH",
+            "patchtype": "MERGE",
+            "apikey": apikey["apikey"]
+        },
+        json={},
+        status_code=204,
+        additional_matcher=lambda req: additional_matcher(req, json={"groupuser": [{"userid": user_id}]})
+    )
+
+    assert user_utils.add_user_to_manage_group(user_id, group_name, apikey) is None
+    assert get_group_id.call_count == 2
+    assert get_group_user.call_count == 1
+    assert add_group_user.call_count == 1
+
+
+def test_add_user_to_manage_group_already_member(user_utils, requests_mock):
+    user_id = "user1"
+    apikey = {"userid": user_id, "apikey": "342fwasdasd", "href": f"https://{MANAGE_API_URL}/maximo/api/os/mxapikey/theapikeyid"}  # pragma: allowlist secret
+    group_name = "thegroup"
+    group_id = "39231234"
+
+    get_group_id = requests_mock.get(
+        f"{MANAGE_API_URL}/maximo/api/os/mxapigroup?ccm=1&lean=1&oslc.select=maxgroupid&oslc.where=groupname=\"{group_name}\"",
+        request_headers={"accept": "application/json"},
+        json={"member": [{"maxgroupid": group_id}], "apikey": apikey["apikey"]},
+        status_code=200,
+        additional_matcher=lambda req: additional_matcher(req)
+    )
+
+    get_group_user = requests_mock.get(
+        f"{MANAGE_API_URL}/maximo/api/os/mxapigroup/{group_id}/groupuser?lean=1",
+        request_headers={"accept": "application/json", "apikey": apikey["apikey"]},
+        json={"member": [{}]},  # <--- member length non-empty indicates that the user is a member of the group
+        status_code=200,
+        additional_matcher=lambda req: additional_matcher(req)
+    )
+
+    add_group_user = requests_mock.post(
+        f"{MANAGE_API_URL}/maximo/api/os/mxapigroup/{group_id}",
+        request_headers={
+            "accept": "application/json",
+            "content-type": "application/json",
+            "x-method-override": "PATCH",
+            "patchtype": "MERGE",
+            "apikey": apikey["apikey"]
+        },
+        json={},
+        status_code=204,
+        additional_matcher=lambda req: additional_matcher(req, json={"groupuser": [{"userid": user_id}]})
+    )
+
+    assert user_utils.add_user_to_manage_group(user_id, group_name, apikey) is None
+    assert get_group_id.call_count == 1
+    assert get_group_user.call_count == 1
+    assert add_group_user.call_count == 0
+
+
+def test_add_user_to_manage_group_error(user_utils, requests_mock):
+    user_id = "user1"
+    apikey = {"userid": user_id, "apikey": "342fwasdasd", "href": f"https://{MANAGE_API_URL}/maximo/api/os/mxapikey/theapikeyid"}  # pragma: allowlist secret
+    group_name = "thegroup"
+    group_id = "39231234"
+
+    get_group_id = requests_mock.get(
+        f"{MANAGE_API_URL}/maximo/api/os/mxapigroup?ccm=1&lean=1&oslc.select=maxgroupid&oslc.where=groupname=\"{group_name}\"",
+        request_headers={"accept": "application/json"},
+        json={"member": [{"maxgroupid": group_id}], "apikey": apikey["apikey"]},
+        status_code=200,
+        additional_matcher=lambda req: additional_matcher(req)
+    )
+
+    get_group_user = requests_mock.get(
+        f"{MANAGE_API_URL}/maximo/api/os/mxapigroup/{group_id}/groupuser?lean=1",
+        request_headers={"accept": "application/json", "apikey": apikey["apikey"]},
+        json={"member": []},
+        status_code=200,
+        additional_matcher=lambda req: additional_matcher(req)
+    )
+
+    add_group_user = requests_mock.post(
+        f"{MANAGE_API_URL}/maximo/api/os/mxapigroup/{group_id}",
+        request_headers={
+            "accept": "application/json",
+            "content-type": "application/json",
+            "x-method-override": "PATCH",
+            "patchtype": "MERGE",
+            "apikey": apikey["apikey"]
+        },
+        text="boom",
+        status_code=500,
+        additional_matcher=lambda req: additional_matcher(req, json={"groupuser": [{"userid": user_id}]})
+    )
+    with pytest.raises(Exception) as excinfo:
+        user_utils.add_user_to_manage_group(user_id, group_name, apikey)
+    assert str(excinfo.value) == "500 boom"
+    assert get_group_id.call_count == 2
+    assert get_group_user.call_count == 1
+    assert add_group_user.call_count == 1
 
 
 def test_get_mas_applications_in_workspace(user_utils, requests_mock):

From 626850df918cbfc31b124c22552b3f3dd738b142 Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Fri, 11 Apr 2025 12:05:51 +0100
Subject: [PATCH 36/44] unit tests

---
 test/src/test_users.py | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/test/src/test_users.py b/test/src/test_users.py
index 5fd3dd7f..49a411d6 100644
--- a/test/src/test_users.py
+++ b/test/src/test_users.py
@@ -280,14 +280,19 @@ def test_manage_internal_ca_pem_file_path(user_utils, mock_named_temporary_file,
     assert mock_atexit.mock_calls == [call(os.remove, PEM_PATH)]
 
 
-def test_manage_maxadmin_api_key():
-    pass
-    # TODO
-
+def test_mas_workspace_application_ids(user_utils, requests_mock):
+    get = requests_mock.get(
+        f"{MAS_API_URL}/workspaces/{MAS_WORKSPACE_ID}/applications",
+        request_headers={"x-access-token": TOKEN},
+        json=[{"id": "manage"}, {"id": "iot"}],
+        status_code=200
+    )
+    assert user_utils.mas_workspace_application_ids == ["manage", "iot"]
+    assert get.call_count == 1
 
-def test_mas_workspace_application_ids():
-    pass
-    # TODO
+    # verify caching
+    assert user_utils.mas_workspace_application_ids == ["manage", "iot"]
+    assert get.call_count == 1
 
 
 def test_get_user_exists(user_utils, requests_mock):

From 137eb14f646ed50c27275e9d3124ecf09721c3dd Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Fri, 11 Apr 2025 14:00:19 +0100
Subject: [PATCH 37/44] unit tests

---
 src/mas/devops/users.py |   2 +-
 test/src/test_users.py  | 116 ++++++++++++++++++++++++++++++++++++++--
 2 files changed, 113 insertions(+), 5 deletions(-)

diff --git a/src/mas/devops/users.py b/src/mas/devops/users.py
index 712834ce..8aa07ab5 100644
--- a/src/mas/devops/users.py
+++ b/src/mas/devops/users.py
@@ -945,7 +945,7 @@ def create_initial_user_for_saas(self, user, user_type):
         for mas_application_id in self.mas_workspace_application_ids:
             self.check_user_sync(user_id, mas_application_id)
 
-        if "manage" in self.mas_workspace_application_ids:
+        if len(manage_security_groups) > 0 and "manage" in self.mas_workspace_application_ids:
             maxadmin_manage_api_key = self.create_or_get_manage_api_key_for_user(MASUserUtils.MAXADMIN, temporary=True)
             for manage_security_group in manage_security_groups:
                 self.add_user_to_manage_group(user_id, manage_security_group, maxadmin_manage_api_key)
diff --git a/test/src/test_users.py b/test/src/test_users.py
index 49a411d6..501eacdd 100644
--- a/test/src/test_users.py
+++ b/test/src/test_users.py
@@ -1617,9 +1617,117 @@ def test_parse_initial_users_from_aws_secret_json(user_utils):
     assert "Unknown user type for user1@example.com: unknown" == str(excinfo.value)
 
 
-def test_create_initial_user_for_saas(user_utils, requests_mock):
-    pass
+def test_create_initial_user_for_saas_no_email(user_utils):
+    with pytest.raises(Exception) as excinfo:
+        user_utils.create_initial_user_for_saas({"given_name": "asdasd", "family_name": "sdfzsd"}, None)
+    assert str(excinfo.value) == "'email' not found in at least one of the user defs"
+
+
+def test_create_initial_user_for_saas_no_given_name(user_utils):
+    with pytest.raises(Exception) as excinfo:
+        user_utils.create_initial_user_for_saas({"email": "asda", "family_name": "sdfzsd"}, None)
+    assert str(excinfo.value) == "'given_name' not found in at least one of the user defs"
+
+
+def test_create_initial_user_for_saas_no_family_name(user_utils):
+    with pytest.raises(Exception) as excinfo:
+        user_utils.create_initial_user_for_saas({"email": "asda", "given_name": "asdasd"}, None)
+    assert str(excinfo.value) == "'family_name' not found in at least one of the user defs"
+
 
+def test_create_initial_user_for_saas_unsupported_type(user_utils):
+    with pytest.raises(Exception) as excinfo:
+        user_utils.create_initial_user_for_saas({"given_name": "asdasd", "family_name": "sdfzsd", "email": "asdasd"}, "whoknows")
+    assert str(excinfo.value) == "Unsupported user_type: whoknows"
+
+# Assisted by watsonx Code Assistant
+
+
+@pytest.mark.parametrize("user_type, permissions, entitlement, is_workspace_admin, application_role, manage_security_groups", [
+    (
+        "PRIMARY",
+        {"systemAdmin": False, "userAdmin": True, "apikeyAdmin": False},
+        {"application": "PREMIUM", "admin": "ADMIN_BASE", "alwaysReserveLicense": True},
+        True,
+        "ADMINISTRATOR",
+        ["MAXADMIN"]
+    ),
+    (
+        "SECONDARY",
+        {"systemAdmin": False, "userAdmin": False, "apikeyAdmin": False},
+        {"application": "BASE", "admin": "NONE", "alwaysReserveLicense": True},
+        False,
+        "USER",
+        []
+    )
+])
+def test_create_initial_users_for_saas(
+    user_type, permissions, entitlement, is_workspace_admin, application_role, manage_security_groups,
+    user_utils, requests_mock
+):
+    user_utils.get_or_create_user = MagicMock()
+    user_utils.link_user_to_local_idp = MagicMock()
+    user_utils.add_user_to_workspace = MagicMock()
+    mas_workspace_application_ids = ["manage", "iot"]
+    user_utils.get_mas_applications_in_workspace = MagicMock(return_value=map(lambda x: {"id": x}, mas_workspace_application_ids))
+    user_utils.await_mas_application_availability = MagicMock()
+    user_utils.set_user_application_permission = MagicMock()
+    user_utils.check_user_sync = MagicMock()
+    manage_api_key = "manage_api_key"  # pragma: allowlist secret
+    user_utils.create_or_get_manage_api_key_for_user = MagicMock(return_value=manage_api_key)
+    user_utils.add_user_to_manage_group = MagicMock()
+
+    user_email = "bill.bob@acme.com"
+    user_given_name = "billy"
+    user_family_name = "bobby"
+    user_id = user_email
+    username = user_email
+    display_name = f"{user_given_name} {user_family_name}"
+
+    user_utils.create_initial_user_for_saas({
+        "email": user_email,
+        "given_name": user_given_name,
+        "family_name": user_family_name
+    },
+        user_type
+    )
+
+    user_utils.get_or_create_user.assert_called_once_with({
+        "id": user_id,
+        "status": {"active": True},
+        "username": username,
+        "owner": "local",
+        "emails": [
+            {
+                "value": user_email,
+                "type": "Work",
+                "primary": True
+            }
+        ],
+        "displayName": display_name,
+        "issuer": "local",
+        "permissions": permissions,
+        "entitlement": entitlement,
+        "givenName": user_given_name,
+        "familyName": user_family_name
+    })
+    user_utils.link_user_to_local_idp.assert_called_once_with(user_id, email_password=True)
+    user_utils.add_user_to_workspace.assert_called_once_with(user_id, is_workspace_admin=is_workspace_admin)
+    user_utils.await_mas_application_availability.assert_has_calls([call("manage"), call("iot")])
+    user_utils.set_user_application_permission.assert_has_calls([
+        call(user_id, "manage", "MANAGEUSER"),
+        call(user_id, "iot", application_role),
+    ])
+    user_utils.check_user_sync.assert_has_calls([
+        call(user_id, "manage"),
+        call(user_id, "iot")
+    ])
+
+    if len(manage_security_groups) > 0:
+        user_utils.create_or_get_manage_api_key_for_user.assert_called_once_with("MAXADMIN", temporary=True)
+    else:
+        user_utils.create_or_get_manage_api_key_for_user.assert_not_called()
 
-def test_create_initial_users_for_saas(user_utils, requests_mock):
-    pass
+    user_utils.add_user_to_manage_group.assert_has_calls(
+        map(lambda sg: call(user_id, sg, manage_api_key), manage_security_groups)
+    )

From eaddbf708461125b782f1fa3a33132f9b4a3348a Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Fri, 11 Apr 2025 14:58:32 +0100
Subject: [PATCH 38/44] unit tests

---
 test/src/test_users.py | 71 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 70 insertions(+), 1 deletion(-)

diff --git a/test/src/test_users.py b/test/src/test_users.py
index 501eacdd..30376a5b 100644
--- a/test/src/test_users.py
+++ b/test/src/test_users.py
@@ -1661,7 +1661,7 @@ def test_create_initial_user_for_saas_unsupported_type(user_utils):
         []
     )
 ])
-def test_create_initial_users_for_saas(
+def test_create_initial_user_for_saas(
     user_type, permissions, entitlement, is_workspace_admin, application_role, manage_security_groups,
     user_utils, requests_mock
 ):
@@ -1731,3 +1731,72 @@ def test_create_initial_users_for_saas(
     user_utils.add_user_to_manage_group.assert_has_calls(
         map(lambda sg: call(user_id, sg, manage_api_key), manage_security_groups)
     )
+
+
+def test_create_initial_users_for_saas_invalid_inputs(user_utils):
+    with pytest.raises(Exception) as excinfo:
+        user_utils.create_initial_users_for_saas({})
+    assert str(excinfo.value) == "expected top-level key 'users' not found"
+
+    with pytest.raises(Exception) as excinfo:
+        user_utils.create_initial_users_for_saas({"users": {}})
+    assert str(excinfo.value) == "expected key 'users.primary' not found"
+
+    with pytest.raises(Exception) as excinfo:
+        user_utils.create_initial_users_for_saas({"users": {"primary": "nope"}})
+    assert str(excinfo.value) == "'users.primary' is not a list"
+
+    with pytest.raises(Exception) as excinfo:
+        user_utils.create_initial_users_for_saas({"users": {"primary": []}})
+    assert str(excinfo.value) == "expected key 'users.secondary' not found"
+
+    with pytest.raises(Exception) as excinfo:
+        user_utils.create_initial_users_for_saas({"users": {"primary": [], "secondary": "nope"}})
+    assert str(excinfo.value) == "'users.secondary' is not a list"
+
+
+def test_create_initial_users_for_saas_no_users(user_utils):
+    assert user_utils.create_initial_users_for_saas({"users": {"primary": [], "secondary": []}}) == {"completed": [], "failed": []}
+
+
+def test_create_initial_users_for_saas(user_utils):
+
+    mas_workspace_application_ids = ["manage", "iot"]
+    user_utils.get_mas_applications_in_workspace = MagicMock(return_value=map(lambda x: {"id": x}, mas_workspace_application_ids))
+    user_utils.await_mas_application_availability = MagicMock()
+    user_utils.create_initial_user_for_saas = MagicMock()
+
+    def fail_for_users_b_and_e(user, user_type):
+        if user["email"] in ["b", "e"]:
+            raise Exception(f"{user['email']} should fail")
+    user_utils.create_initial_user_for_saas.side_effect = fail_for_users_b_and_e
+
+    initial_users = {
+        "users": {
+            "primary": [
+                {"email": "a"},
+                {"email": "b"},
+                {"email": "c"}
+            ],
+            "secondary": [
+                {"email": "d"},
+                {"email": "e"},
+                {"email": "f"}
+            ]
+        }
+    }
+
+    assert user_utils.create_initial_users_for_saas(initial_users) == {
+        "completed": [
+            {"email": "a"},
+            {"email": "c"},
+            {"email": "d"},
+            {"email": "f"},
+        ],
+        "failed": [
+            {"email": "b"},
+            {"email": "e"},
+        ]
+    }
+
+    user_utils.await_mas_application_availability.assert_has_calls([call("manage"), call("iot")])

From 526ba15a734f0d81598358399e293c35e010650d Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Fri, 11 Apr 2025 15:01:33 +0100
Subject: [PATCH 39/44] remove unused, untested methods

---
 src/mas/devops/users.py | 51 -----------------------------------------
 1 file changed, 51 deletions(-)

diff --git a/src/mas/devops/users.py b/src/mas/devops/users.py
index 8aa07ab5..8440c0cf 100644
--- a/src/mas/devops/users.py
+++ b/src/mas/devops/users.py
@@ -949,54 +949,3 @@ def create_initial_user_for_saas(self, user, user_type):
             maxadmin_manage_api_key = self.create_or_get_manage_api_key_for_user(MASUserUtils.MAXADMIN, temporary=True)
             for manage_security_group in manage_security_groups:
                 self.add_user_to_manage_group(user_id, manage_security_group, maxadmin_manage_api_key)
-
-    # Unused (but potentially useful) methods
-    # ----------------------------------------
-
-    def get_groups(self):
-        self.logger.debug("Getting groups")
-        url = f"{self.mas_api_url_internal}/groups"
-        headers = {
-            "Accept": "application/json",
-            "x-access-token": self.superuser_auth_token
-        }
-        response = requests.get(
-            url,
-            headers=headers,
-            verify=self.core_internal_ca_pem_file_path
-        )
-        if response.status_code == 200:
-            return response.json()
-        raise Exception(f"{response.status_code} {response.text}")
-
-    def get_installed_mas_applications(self):
-        self.logger.debug("Getting installed MAS Applications")
-        url = f"{self.mas_api_url_internal}/applications"
-        headers = {
-            "Accept": "application/json",
-            "x-access-token": self.superuser_auth_token
-        }
-        response = requests.get(
-            url,
-            headers=headers,
-            verify=self.core_internal_ca_pem_file_path
-        )
-        if response.status_code == 200:
-            return response.json()
-        raise Exception(f"{response.status_code} {response.text}")
-
-    def get_user_groups(self, user_id):
-        self.logger.info(f"Getting groups for user {user_id}")
-        url = f"{self.mas_api_url_internal}/v3/users/{user_id}/groups"
-        headers = {
-            "Accept": "application/json",
-            "x-access-token": self.superuser_auth_token
-        }
-        response = requests.get(
-            url,
-            headers=headers,
-            verify=self.core_internal_ca_pem_file_path
-        )
-        if response.status_code == 200:
-            return response.json()
-        raise Exception(f"{response.status_code} {response.text}")

From 02814f694bba7cce66762dd9e6edebe5179769eb Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Thu, 17 Apr 2025 17:05:06 +0100
Subject: [PATCH 40/44] fix application role ADMIN (not ADMINISTRATOR)

---
 src/mas/devops/users.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mas/devops/users.py b/src/mas/devops/users.py
index 8440c0cf..07fdde27 100644
--- a/src/mas/devops/users.py
+++ b/src/mas/devops/users.py
@@ -887,7 +887,7 @@ def create_initial_user_for_saas(self, user, user_type):
                 "alwaysReserveLicense": True
             }
             is_workspace_admin = True
-            application_role = "ADMINISTRATOR"
+            application_role = "ADMIN"
             # TODO: check which security groups primary users should be members of
             manage_security_groups = ["MAXADMIN"]
         elif user_type == "SECONDARY":

From 2514d50bd5ec0f8ba53e9fa77143489baaa50ab9 Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Thu, 17 Apr 2025 17:08:50 +0100
Subject: [PATCH 41/44] skip user creation if no secret found

---
 bin/mas-devops-create-initial-users-for-saas | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/bin/mas-devops-create-initial-users-for-saas b/bin/mas-devops-create-initial-users-for-saas
index 4c7a8fd2..6d1a9801 100644
--- a/bin/mas-devops-create-initial-users-for-saas
+++ b/bin/mas-devops-create-initial-users-for-saas
@@ -110,6 +110,10 @@ if __name__ == "__main__":
                 SecretId=initial_users_secret_name
             )
         except ClientError as e:
+            if e.response['Error']['Code'] == 'ResourceNotFoundException':
+                logger.info(f"Secret {initial_users_secret_name} was not found, nothing to do, exiting now.")
+                sys.exit(0)
+
             raise Exception(f"Failed to fetch secret {initial_users_secret_name}: {str(e)}")
         
         secret_json = json.loads(initial_users_secret['SecretString'])

From 74a226c1dfcb8e5b1b673e8f2b899dbc2353cd3e Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Thu, 17 Apr 2025 17:10:34 +0100
Subject: [PATCH 42/44] fix unit test

---
 test/src/test_users.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/src/test_users.py b/test/src/test_users.py
index 30376a5b..7be29e60 100644
--- a/test/src/test_users.py
+++ b/test/src/test_users.py
@@ -1649,7 +1649,7 @@ def test_create_initial_user_for_saas_unsupported_type(user_utils):
         {"systemAdmin": False, "userAdmin": True, "apikeyAdmin": False},
         {"application": "PREMIUM", "admin": "ADMIN_BASE", "alwaysReserveLicense": True},
         True,
-        "ADMINISTRATOR",
+        "ADMIN",
         ["MAXADMIN"]
     ),
     (

From 85897498d1612abdc7900e0baee3bd592db41126 Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Thu, 8 May 2025 13:50:26 +0100
Subject: [PATCH 43/44] comments

---
 src/mas/devops/users.py | 16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/src/mas/devops/users.py b/src/mas/devops/users.py
index 07fdde27..9a094509 100644
--- a/src/mas/devops/users.py
+++ b/src/mas/devops/users.py
@@ -19,16 +19,6 @@
 import time
 import re
 
-'''
-TODO:
-    MVI / other apps?
-    unit tests
-
-    how to cope with users that have been soft-deleted? tolerate / skip - ensure they are removed from secret so
-    we don't get caught in an infinite loop?
-
-'''
-
 
 class MASUserUtils():
     '''
@@ -324,7 +314,7 @@ def link_user_to_local_idp(self, user_id, email_password=False):
             "emailPassword": email_password
         }
         payload = {
-            "idpUserId": user_id
+            "idpUserId": user_id,
         }
         headers = {
             "Content-Type": "application/json",
@@ -506,10 +496,6 @@ def create_or_get_manage_api_key_for_user(self, user_id, temporary=False):
             "lean": 1,
         }
 
-        # TODO: is this just a temporary API Key for this script?
-        #       if so, add cleanup logic (atext) and perhaps set an expiration date in case cleanup fails
-
-        #
         payload = {
             "expiration": -1,
             "userid": user_id

From 993701d98ff3267f1142cf81af10c8468b8bd2b6 Mon Sep 17 00:00:00 2001
From: "klapitom@uk.ibm.com" <7372253+tomklapiscak@users.noreply.github.com>
Date: Tue, 13 May 2025 13:31:32 +0100
Subject: [PATCH 44/44] requested changes

---
 bin/mas-devops-create-initial-users-for-saas | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/bin/mas-devops-create-initial-users-for-saas b/bin/mas-devops-create-initial-users-for-saas
index 6d1a9801..bac45ce9 100644
--- a/bin/mas-devops-create-initial-users-for-saas
+++ b/bin/mas-devops-create-initial-users-for-saas
@@ -1,7 +1,7 @@
 #!/usr/bin/env python3
 
 # *****************************************************************************
-# Copyright (c) 2024 IBM Corporation and other Contributors.
+# Copyright (c) 2025 IBM Corporation and other Contributors.
 #
 # All rights reserved. This program and the accompanying materials
 # are made available under the terms of the Eclipse Public License v1.0
@@ -31,8 +31,6 @@ if __name__ == "__main__":
     parser = argparse.ArgumentParser()
 
     # Primary Options
-    parser.add_argument("--mas-account-id", required=False) # TODO: remove if unused
-    parser.add_argument("--mas-cluster-id", required=False) # TODO: remove if unused
     parser.add_argument("--mas-instance-id", required=True)
     parser.add_argument("--mas-workspace-id", required=True)
     parser.add_argument("--log-level", required=False, choices=["DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL"], default="INFO")
@@ -60,8 +58,6 @@ if __name__ == "__main__":
     ch.setFormatter(chFormatter)
     logger.addHandler(ch)
 
-    mas_account_id = args.mas_account_id
-    mas_cluster_id = args.mas_cluster_id
     mas_instance_id = args.mas_instance_id
     mas_workspace_id = args.mas_workspace_id
     initial_users_yaml_file = args.initial_users_yaml_file
@@ -73,8 +69,6 @@ if __name__ == "__main__":
 
     logger.info("Configuration:")
     logger.info("--------------")
-    logger.info(f"mas_account_id:            {mas_account_id}")
-    logger.info(f"mas_cluster_id:            {mas_cluster_id}")
     logger.info(f"mas_instance_id:           {mas_instance_id}")
     logger.info(f"mas_workspace_id:          {mas_workspace_id}")
     logger.info(f"initial_users_yaml_file:   {initial_users_yaml_file}")