Merge pull request DIRACGrid#7784 from andresailer/80_vomsTweaks

[8.0] IAM2CS: ForceNickname, fix duplicate accounts for users

src/DIRAC/ConfigurationSystem/Agent/VOMS2CSAgent.py

@@ -55,6 +55,7 @@ def __init__(self, *args, **kwargs):
         self.syncPluginName = None
         self.compareWithIAM = False
         self.useIAM = False
+        self.forceNickname = False
 
     def initialize(self):
         """Initialize the default parameters"""
@@ -70,6 +71,7 @@ def initialize(self):
         self.syncPluginName = self.am_getOption("SyncPluginName", self.syncPluginName)
         self.compareWithIAM = self.am_getOption("CompareWithIAM", self.compareWithIAM)
         self.useIAM = self.am_getOption("UseIAM", self.useIAM)
+        self.forceNickname = self.am_getOption("ForceNickname", self.forceNickname)
 
         self.detailedReport = self.am_getOption("DetailedReport", self.detailedReport)
         self.mailFrom = self.am_getOption("MailFrom", self.mailFrom)
@@ -127,6 +129,7 @@ def execute(self):
                 compareWithIAM=compareWithIAM,
                 useIAM=useIAM,
                 accessToken=accessToken,
+                forceNickname=self.forceNickname,
             )
 
             result = self.__syncCSWithVOMS(  # pylint: disable=unexpected-keyword-arg
@@ -145,6 +148,7 @@ def execute(self):
             csapi = resultDict.get("CSAPI")
             adminMessages = resultDict.get("AdminMessages", {"Errors": [], "Info": []})
             voChanged = resultDict.get("VOChanged", False)
+            noNickname = resultDict.get("NoNickname", [])
             self.log.info(
                 "Run user results",
                 ": new %d, modified %d, deleted %d, new/suspended %d"
@@ -194,6 +198,11 @@ def execute(self):
                 mailMsg = ""
                 if adminMessages["Errors"]:
                     mailMsg += "\nErrors list:\n  %s" % "\n  ".join(adminMessages["Errors"])
+                if self.forceNickname and noNickname:
+                    mailMsg += "There are users without nicknames in the IAM\n"
+                    for entry in noNickname:
+                        mailMsg += str(entry)
+                        mailMsg += "\n\n"
                 if adminMessages["Info"]:
                     mailMsg += "\nRun result:\n  %s" % "\n  ".join(adminMessages["Info"])
                 if self.detailedReport:

src/DIRAC/ConfigurationSystem/Client/VOMS2CSSynchronizer.py

@@ -132,6 +132,7 @@ def __init__(
         compareWithIAM=False,
         useIAM=False,
         accessToken=None,
+        forceNickname=False,
     ):
         """VOMS2CSSynchronizer class constructor
 
@@ -165,6 +166,7 @@ def __init__(
         self.compareWithIAM = compareWithIAM
         self.useIAM = useIAM
         self.accessToken = accessToken
+        self.forceNickname = forceNickname
 
         if syncPluginName:
             objLoader = ObjectLoader()
@@ -222,15 +224,15 @@ def compareUsers(self, voms_users, iam_users):
     @convertToReturnValue
     def _getUsers(self):
         if self.compareWithIAM or self.useIAM:
-            iamSrv = IAMService(self.accessToken, vo=self.vo)
+            iamSrv = IAMService(self.accessToken, vo=self.vo, forceNickname=self.forceNickname)
             iam_users = returnValueOrRaise(iamSrv.getUsers())
             if self.useIAM:
                 return iam_users
 
         vomsSrv = VOMSService(self.vo)
         voms_users = returnValueOrRaise(vomsSrv.getUsers())
         if self.compareWithIAM:
-            self.compareUsers(voms_users, iam_users)
+            self.compareUsers(voms_users.get("Users", {}), iam_users.get("Users", {}))
         return voms_users
 
     def syncCSWithVOMS(self):
@@ -257,8 +259,9 @@ def syncCSWithVOMS(self):
         if not result["OK"]:
             self.log.error("Could not retrieve user information", result["Message"])
             return result
-
-        self.vomsUserDict = result["Value"]
+        if getUserErrors := result["Value"]["Errors"]:
+            self.adminMsgs["Errors"].extend(getUserErrors)
+        self.vomsUserDict = result["Value"]["Users"]
         message = f"There are {len(self.vomsUserDict)} user entries in VOMS for VO {self.vomsVOName}"
         self.adminMsgs["Info"].append(message)
         self.log.info("VOMS user entries", message)
@@ -329,30 +332,37 @@ def syncCSWithVOMS(self):
                 # Check the nickName in the same VO to see if the user is already registered
                 # with another DN
                 nickName = self.vomsUserDict[dn].get("nickname")
+                if not nickName and self.forceNickname:
+                    resultDict["NoNickname"].append(self.vomsUserDict[dn])
+                    self.log.error("No nickname defined for", self.vomsUserDict[dn])
+                    continue
                 if nickName in diracUserDict or nickName in newAddedUserDict:
                     diracName = nickName
                     # This is a flag for adding the new DN to an already existing user
                     newDNForExistingUser = dn
 
                 # We have a real new user
                 if not diracName:
-                    if nickName:
-                        newDiracName = nickName.strip()
-                    else:
-                        newDiracName = self.getUserName(dn)
-
                     # Do not consider users with Suspended status in VOMS
                     if self.vomsUserDict[dn]["suspended"] or self.vomsUserDict[dn]["certSuspended"]:
                         resultDict["SuspendedUsers"].append(newDiracName)
                         continue
 
-                    # If the chosen user name exists already, append a distinguishing suffix
-                    ind = 1
-                    trialName = newDiracName
-                    while newDiracName in allDiracUsers:
-                        # We have a user with the same name but with a different DN
-                        newDiracName = "%s_%d" % (trialName, ind)
-                        ind += 1
+                    # if we have a nickname, we use the nickname no
+                    # matter what so we can have users from different
+                    # VOs with the same nickname / username
+                    if nickName:
+                        newDiracName = nickName.strip()
+                    else:
+                        newDiracName = self.getUserName(dn)
+
+                        # If the chosen user name exists already, append a distinguishing suffix
+                        ind = 1
+                        trialName = newDiracName
+                        while newDiracName in allDiracUsers:
+                            # We have a user with the same name but with a different DN
+                            newDiracName = "%s_%d" % (trialName, ind)
+                            ind += 1
 
                     # We now have everything to add the new user
                     userDict = {"DN": dn, "CA": self.vomsUserDict[dn]["CA"], "Email": self.vomsUserDict[dn]["mail"]}

src/DIRAC/ConfigurationSystem/ConfigTemplate.cfg

@@ -76,6 +76,8 @@ Agents
     CompareWithIAM = False
     # If set to true, will only query IAM and return the list of users from there
     UseIAM = False
+    # If set to true only users with a nickname attribute defined in the IAM are created in DIRAC
+    ForceNickname = False
   }
   ##END
   ##BEGIN GOCDB2CSAgent

src/DIRAC/Core/Security/IAMService.py

@@ -20,13 +20,15 @@ def convert_dn(inStr):
 
 
 class IAMService:
-    def __init__(self, access_token, vo=None):
+    def __init__(self, access_token, vo=None, forceNickname=False):
         """c'tor
 
-        :param str vo: name of the virtual organization (community)
         :param str access_token: the token used to talk to IAM, with the scim:read property
+        :param str vo: name of the virtual organization (community)
+        :param bool forceNickname: if enforce the presence of a nickname attribute and do not fall back to username in IAM
 
         """
+        self.log = gLogger.getSubLogger(self.__class__.__name__)
 
         if not access_token:
             raise ValueError("access_token not set")
@@ -36,6 +38,8 @@ def __init__(self, access_token, vo=None):
         if not vo:
             raise Exception("No VO name given")
 
+        self.forceNickname = forceNickname
+
         self.vo = vo
 
         self.iam_url = None
@@ -78,8 +82,7 @@ def _getIamUserDump(self):
             iam_users.extend(data["Resources"])
         return iam_users
 
-    @staticmethod
-    def convert_iam_to_voms(iam_output):
+    def convert_iam_to_voms(self, iam_output):
         """Convert an IAM entry into the voms style, i.e. DN based"""
         converted_output = {}
 
@@ -92,15 +95,16 @@ def convert_iam_to_voms(iam_output):
             # The nickname is available in the list of attributes
             # (if configured so)
             # in the form {'name': 'nickname', 'value': 'chaen'}
-            # otherwise, we take the userName
+            # otherwise, we take the userName unless we forceNickname
             try:
                 cert_dict["nickname"] = [
                     attr["value"]
                     for attr in iam_output["urn:indigo-dc:scim:schemas:IndigoUser"]["attributes"]
                     if attr["name"] == "nickname"
                 ][0]
             except (KeyError, IndexError):
-                cert_dict["nickname"] = iam_output["userName"]
+                if not self.forceNickname:
+                    cert_dict["nickname"] = iam_output["userName"]
 
             # This is not correct, we take the overall status instead of the certificate one
             # however there are no known case of cert suspended while the user isn't
@@ -125,15 +129,20 @@ def convert_iam_to_voms(iam_output):
         return converted_output
 
     def getUsers(self):
+        """Extract users from IAM user dump.
+
+        :return: dictionary of: "Users": user dictionary keyed by the user DN, "Errors": list of error messages
+        """
         self.iam_users_raw = self._getIamUserDump()
         users = {}
-        errors = 0
+        errors = []
         for user in self.iam_users_raw:
             try:
                 users.update(self.convert_iam_to_voms(user))
             except Exception as e:
-                errors += 1
-                print(f"Could not convert {user['name']} {e!r} ")
-        print(f"There were in total {errors} errors")
+                errors.append(f"{user['name']} {e!r}")
+                self.log.error("Could not convert", f"{user['name']} {e!r}")
+        self.log.error("There were in total", f"{len(errors)} errors")
         self.userDict = dict(users)
-        return S_OK(users)
+        result = S_OK({"Users": users, "Errors": errors})
+        return result

src/DIRAC/Core/Security/VOMSService.py

@@ -81,7 +81,7 @@ def attGetUserNickname(self, dn, _ca=None):
     def getUsers(self):
         """Get all the users of the VOMS VO with their detailed information
 
-        :return: user dictionary keyed by the user DN
+        :return: dictionary of: "Users": user dictionary keyed by the user DN, "Errors": empty list
         """
         if not self.urls:
             return S_ERROR(DErrno.ENOAUTH, "No VOMS server defined")
@@ -148,4 +148,5 @@ def getUsers(self):
                             resultDict[dn]["nickname"] = attribute.get("value")
 
         self.userDict = dict(resultDict)
-        return S_OK(resultDict)
+        # for consistency with IAM interface, we add Errors
+        return S_OK({"Users": resultDict, "Errors": []})