Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LDAP: Update user names formatting on sync #903

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions cds_ils/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -193,17 +193,17 @@ def run_command(command, catch_exceptions=False, verbose=True):

# Create users
run_command("users create [email protected] -a --password=123456") # ID 1
create_userprofile_for("[email protected]", "patron1", "Yannic Vilma")
create_userprofile_for("[email protected]", "patron1", "VILMA, Yannic")
run_command("users create [email protected] -a --password=123456") # ID 2
create_userprofile_for("[email protected]", "patron2", "Diana Adi")
create_userprofile_for("[email protected]", "patron2", "ADI, Diana")
run_command("users create [email protected] -a --password=123456") # ID 3
create_userprofile_for("[email protected]", "admin", "Zeki Ryoichi")
create_userprofile_for("[email protected]", "admin", "RYOICHI, Zeki")
run_command("users create [email protected] -a --password=123456") # ID 4
create_userprofile_for("[email protected]", "librarian", "Hector Nabu")
create_userprofile_for("[email protected]", "librarian", "NABU, Hector")
run_command("users create [email protected] -a --password=123456") # ID 5
create_userprofile_for("[email protected]", "patron3", "Medrod Tara")
create_userprofile_for("[email protected]", "patron3", "TARA, Medrod")
run_command("users create [email protected] -a --password=123456") # ID 6
create_userprofile_for("[email protected]", "patron4", "Devi Cupid")
create_userprofile_for("[email protected]", "patron4", "CUPID, Devi")

# Assign roles
run_command("roles add [email protected] admin")
Expand Down
2 changes: 2 additions & 0 deletions cds_ils/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -292,12 +292,14 @@ def _parse_env_bool(var_name, default=None):
# non-REST
OAUTH_REMOTE_APP = copy.deepcopy(cern_openid.REMOTE_APP)
OAUTH_REMOTE_APP["params"].update(_OAUTH_REMOTE_APP_COMMON)
OAUTH_REMOTE_APP["signup_handler"]["info"] = "cds_ils.oauth:account_info"
OAUTHCLIENT_REMOTE_APPS = dict(cern_openid=OAUTH_REMOTE_APP)
###############################################################################
# REST
logout_redirect_url = os.environ.get("INVENIO_SPA_HOST", "https://127.0.0.1:3000/")
OAUTH_REMOTE_REST_APP = copy.deepcopy(cern_openid.REMOTE_REST_APP)
OAUTH_REMOTE_REST_APP["params"].update(_OAUTH_REMOTE_APP_COMMON)
OAUTH_REMOTE_REST_APP["signup_handler"]["info"] = "cds_ils.oauth:account_info_rest"
OAUTH_REMOTE_REST_APP["logout_url"] = os.environ.get(
"OAUTH_CERN_OPENID_LOGOUT_URL",
"https://keycloak-qa.cern.ch/auth/realms/cern/"
Expand Down
6 changes: 4 additions & 2 deletions cds_ils/ldap/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ class LdapClient(object):

Response example:
[
{'displayName': [b'Joe Foe'],
{'givenName': [b'Joe'],
'sn': [b'FOE'],
'department': [b'IT/CDA'],
'uidNumber': [b'100000'],
'mail': [b'[email protected]'],
Expand All @@ -32,7 +33,8 @@ class LdapClient(object):

LDAP_USER_RESP_FIELDS = [
"mail",
"displayName",
"givenName",
"sn",
"department",
"cernAccountType",
"employeeID",
Expand Down
7 changes: 5 additions & 2 deletions cds_ils/ldap/user_importer.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ class LdapUserImporter:

Expected input format for ldap users:
[
{'displayName': [b'Joe Foe'],
{'givenName': [b'Joe'],
'sn': [b'FOE'],
'department': [b'IT/CDA'],
'uidNumber': [b'100000'],
'mail': [b'[email protected]'],
Expand Down Expand Up @@ -54,7 +55,9 @@ def create_invenio_user_identity(self, user_id, ldap_user):

def create_invenio_user_profile(self, user_id, ldap_user):
"""Return new user profile."""
display_name = ldap_user["user_profile_full_name"]
display_name = "{0}, {1}".format(
ldap_user["user_profile_last_name"], ldap_user["user_profile_first_name"]
)
return UserProfile(
user_id=user_id,
_displayname="id_{}".format(user_id),
Expand Down
11 changes: 8 additions & 3 deletions cds_ils/ldap/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ def serialize(ldap_user_data):
decoded_data[key] = value[0].decode("utf8")
serialized_data = dict(
user_email=decoded_data["mail"].lower(),
user_profile_full_name=decoded_data["displayName"],
user_profile_first_name=decoded_data["givenName"],
user_profile_last_name=decoded_data["sn"].upper(),
user_identity_id=decoded_data["uidNumber"],
cern_account_type=decoded_data["cernAccountType"],
remote_account_person_id=str(decoded_data["employeeID"]),
Expand Down Expand Up @@ -103,8 +104,10 @@ def __init__(self, remote_account):

def _get_full_user_info(self):
"""Serialize data from user db models."""
last_name, first_name = self.user_profile.full_name.split(",")
user_info = dict(
user_profile_full_name=self.user_profile.full_name,
user_profile_first_name=first_name.strip(),
user_profile_last_name=last_name.strip(),
user_email=self.user.email,
user_identity_id=self.user_identity.id,
remote_account_id=self.remote_account.id,
Expand All @@ -120,7 +123,9 @@ def update(self, ldap_user):
ra.extra_data["department"] = ldap_user["remote_account_department"]
ra.extra_data["mailbox"] = ldap_user["remote_account_mailbox"]
self.user.email = ldap_user["user_email"]
self.user_profile.full_name = ldap_user["user_profile_full_name"]
self.user_profile.full_name = "{0}, {1}".format(
ldap_user["user_profile_last_name"], ldap_user["user_profile_first_name"]
)

def mark_for_deletion(self):
"""Mark user for deletion."""
Expand Down
63 changes: 63 additions & 0 deletions cds_ils/oauth.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
# Copyright (C) 2024 CERN.
#
# CDS-ILS is free software; you can redistribute it and/or modify it under
# the terms of the MIT License; see LICENSE file for more details.

"""CDS-ILS oauth overrides."""
from flask import current_app, flash, g, redirect
from invenio_oauthclient.contrib.cern_openid import get_resource
from invenio_oauthclient.errors import OAuthCERNRejectedAccountError
from invenio_oauthclient.handlers.rest import response_handler


def _account_info(remote, resp):
"""Retrieve remote account information used to find local user."""
g.oauth_logged_in_with_remote = remote
resource = get_resource(remote, resp)

valid_roles = current_app.config.get("OAUTHCLIENT_CERN_OPENID_ALLOWED_ROLES")
cern_roles = resource.get("cern_roles")
if cern_roles is None or not set(cern_roles).issubset(valid_roles):
raise OAuthCERNRejectedAccountError(
"User roles {0} are not one of {1}".format(cern_roles, valid_roles),
remote,
resp,
)

email = resource["email"]
external_id = resource["cern_upn"]
nice = resource["preferred_username"]
name = "{0}, {1}".format(resource["family_name"].upper(), resource["given_name"])

return dict(
user=dict(email=email.lower(), profile=dict(username=nice, full_name=name)),
external_id=external_id,
external_method="cern_openid",
active=True,
)


def account_info(remote, resp):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't we only need some of these functions? even better, can we have configurable callable to inject our own _account_info? it would require adding this variable in the oauthclient (you will need to make sure the default callable is the one defined in oauthclient)

"""Retrieve remote account information used to find local user."""
try:
return _account_info(remote, resp)
except OAuthCERNRejectedAccountError as e:
current_app.logger.warning(e.message, exc_info=True)
flash(_("CERN account not allowed."), category="danger")
return redirect("/")


def account_info_rest(remote, resp):
"""Retrieve remote account information used to find local user."""
try:
return _account_info(remote, resp)
except OAuthCERNRejectedAccountError as e:
current_app.logger.warning(e.message, exc_info=True)
remote_app_config = current_app.config["OAUTHCLIENT_REST_REMOTE_APPS"][
remote.name
]
return response_handler(
remote,
remote_app_config["error_redirect_url"],
payload=dict(message="CERN account not allowed.", code=400),
)
4 changes: 2 additions & 2 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ def patrons(app, db):
**dict(
user_id=user_id,
_displayname="id_" + str(user_id),
full_name="System User",
full_name="USER, System",
)
)
db.session.add(profile)
Expand All @@ -147,7 +147,7 @@ def patrons(app, db):
**dict(
user_id=user2_id,
_displayname="id_" + str(user2_id),
full_name="System User",
full_name="USER, System",
)
)
db.session.add(profile)
Expand Down
Loading
Loading