Skip to content

Commit

Permalink
sweep: #7784 IAM2CS: ForceNickname, fix duplicate accounts for users
Browse files Browse the repository at this point in the history
  • Loading branch information
andresailer committed Sep 17, 2024
1 parent 41c93fa commit dd5f83d
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 31 deletions.
9 changes: 9 additions & 0 deletions src/DIRAC/ConfigurationSystem/Agent/VOMS2CSAgent.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"""
Expand All @@ -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)
Expand Down Expand Up @@ -127,6 +129,7 @@ def execute(self):
compareWithIAM=compareWithIAM,
useIAM=useIAM,
accessToken=accessToken,
forceNickname=self.forceNickname,
)

result = self.__syncCSWithVOMS( # pylint: disable=unexpected-keyword-arg
Expand All @@ -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"
Expand Down Expand Up @@ -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:
Expand Down
42 changes: 26 additions & 16 deletions src/DIRAC/ConfigurationSystem/Client/VOMS2CSSynchronizer.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ def __init__(
compareWithIAM=False,
useIAM=False,
accessToken=None,
forceNickname=False,
):
"""VOMS2CSSynchronizer class constructor
Expand Down Expand Up @@ -167,6 +168,7 @@ def __init__(
self.compareWithIAM = compareWithIAM
self.useIAM = useIAM
self.accessToken = accessToken
self.forceNickname = forceNickname

if syncPluginName:
objLoader = ObjectLoader()
Expand Down Expand Up @@ -224,15 +226,15 @@ def compareUsers(self, voms_users, iam_users):
@convertToReturnValue
def _getUsers(self):
if self.compareWithIAM or self.useIAM:
self.iamSrv = IAMService(self.accessToken, vo=self.vo)
self.iamSrv = IAMService(self.accessToken, vo=self.vo, forceNickname=self.forceNickname)
iam_users = returnValueOrRaise(self.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):
Expand All @@ -259,8 +261,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)
Expand Down Expand Up @@ -331,30 +334,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"]}
Expand Down
2 changes: 2 additions & 0 deletions src/DIRAC/ConfigurationSystem/ConfigTemplate.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,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
Expand Down
35 changes: 22 additions & 13 deletions src/DIRAC/Core/Security/IAMService.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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
Expand Down Expand Up @@ -79,8 +83,7 @@ def _getIamUserDump(self):
self.iam_users_raw.extend(data["Resources"])
return self.iam_users_raw

@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 = {}

Expand All @@ -93,15 +96,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
Expand All @@ -126,18 +130,23 @@ def convert_iam_to_voms(iam_output):
return converted_output

def getUsers(self):
iam_users_raw = self._getIamUserDump()
"""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
for user in iam_users_raw:
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

def getUsersSub(self) -> dict[str, str]:
"""
Expand Down
5 changes: 3 additions & 2 deletions src/DIRAC/Core/Security/VOMSService.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ def __init__(self, vo=None, compareWithIAM=False, useIAM=False):
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")
Expand Down Expand Up @@ -126,4 +126,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": []})

0 comments on commit dd5f83d

Please sign in to comment.