From f1ac92f221ac2901b5b19cc8a1dcae27bc15f0fd Mon Sep 17 00:00:00 2001 From: Marco Enrico Piras Date: Thu, 2 May 2024 09:59:21 +0200 Subject: [PATCH 01/12] chore(aai): :wrench: update LS-AAI configuration settings --- lifemonitor/auth/oauth2/client/providers/lsaai.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lifemonitor/auth/oauth2/client/providers/lsaai.py b/lifemonitor/auth/oauth2/client/providers/lsaai.py index 27681a23..e40e4a5d 100644 --- a/lifemonitor/auth/oauth2/client/providers/lsaai.py +++ b/lifemonitor/auth/oauth2/client/providers/lsaai.py @@ -65,15 +65,15 @@ class LsAAI: 'client_id': current_app.config.get('LSAAI_CLIENT_ID', None), 'client_secret': current_app.config.get('LSAAI_CLIENT_SECRET', None), 'client_name': client_name, - 'uri': 'https://proxy.aai.lifescience-ri.eu', - 'api_base_url': 'https://proxy.aai.lifescience-ri.eu', - 'access_token_url': 'https://proxy.aai.lifescience-ri.eu/OIDC/token', - 'authorize_url': 'https://proxy.aai.lifescience-ri.eu/saml2sp/OIDC/authorization', + 'uri': 'https://login.aai.lifescience-ri.eu', + 'api_base_url': 'https://login.aai.lifescience-ri.eu', + 'access_token_url': 'https://login.aai.lifescience-ri.eu/oidc/token', + 'authorize_url': 'https://login.aai.lifescience-ri.eu/oidc/authorize', 'client_kwargs': {'scope': 'openid profile email orcid eduperson_principal_name'}, - 'userinfo_endpoint': 'https://proxy.aai.lifescience-ri.eu/OIDC/userinfo', + 'userinfo_endpoint': 'https://login.aai.lifescience-ri.eu/oidc/userinfo', 'userinfo_compliance_fix': normalize_userinfo, 'user_profile_html': 'https://profile.aai.lifescience-ri.eu/profile', - 'server_metadata_url': 'https://proxy.aai.lifescience-ri.eu/.well-known/openid-configuration' + 'server_metadata_url': 'https://login.aai.lifescience-ri.eu/oidc/.well-known/openid-configuration' } def __repr__(self) -> str: From 71bcea8b69fa482636c8c983866e5dc62d696989 Mon Sep 17 00:00:00 2001 From: Marco Enrico Piras Date: Thu, 2 May 2024 12:04:20 +0200 Subject: [PATCH 02/12] build(core): :construction_worker: ensure read access to source code to unprivileged users --- docker/lifemonitor.Dockerfile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docker/lifemonitor.Dockerfile b/docker/lifemonitor.Dockerfile index ed6bfe01..55f9b0ac 100644 --- a/docker/lifemonitor.Dockerfile +++ b/docker/lifemonitor.Dockerfile @@ -94,6 +94,9 @@ COPY --chown=lm:lm lifemonitor /lm/lifemonitor COPY --chown=lm:lm migrations /lm/migrations COPY --chown=lm:lm cli /lm/cli +# Ensure read access to source code to unprivileged users +RUN find /lm/lifemonitor/ -type d -exec chmod a+r {} \; + ################################################################## ## Node Stage ################################################################## From 1850979a6f4b5236637a77f26111d21205f3a196 Mon Sep 17 00:00:00 2001 From: Marco Enrico Piras Date: Thu, 2 May 2024 14:32:16 +0200 Subject: [PATCH 03/12] fix(aai): :wrench: update list of required scopes --- lifemonitor/auth/oauth2/client/providers/lsaai.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lifemonitor/auth/oauth2/client/providers/lsaai.py b/lifemonitor/auth/oauth2/client/providers/lsaai.py index e40e4a5d..48bcaae9 100644 --- a/lifemonitor/auth/oauth2/client/providers/lsaai.py +++ b/lifemonitor/auth/oauth2/client/providers/lsaai.py @@ -69,7 +69,7 @@ class LsAAI: 'api_base_url': 'https://login.aai.lifescience-ri.eu', 'access_token_url': 'https://login.aai.lifescience-ri.eu/oidc/token', 'authorize_url': 'https://login.aai.lifescience-ri.eu/oidc/authorize', - 'client_kwargs': {'scope': 'openid profile email orcid eduperson_principal_name'}, + 'client_kwargs': {'scope': 'openid profile email'}, 'userinfo_endpoint': 'https://login.aai.lifescience-ri.eu/oidc/userinfo', 'userinfo_compliance_fix': normalize_userinfo, 'user_profile_html': 'https://profile.aai.lifescience-ri.eu/profile', From 42876e0637bbf6cb1b4b1ec9c23f60560d96dfd5 Mon Sep 17 00:00:00 2001 From: Marco Enrico Piras Date: Thu, 2 May 2024 15:26:38 +0200 Subject: [PATCH 04/12] fix: :bug: ensure that the user data id defined --- lifemonitor/auth/oauth2/client/models.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lifemonitor/auth/oauth2/client/models.py b/lifemonitor/auth/oauth2/client/models.py index 1d2f245a..1643d2ab 100644 --- a/lifemonitor/auth/oauth2/client/models.py +++ b/lifemonitor/auth/oauth2/client/models.py @@ -126,6 +126,7 @@ def to_dict(self): @staticmethod def from_dict(data: dict): + assert data, "User data from the OAuth Provider cannot be empty" profile = OAuthUserProfile() for k, v, in data.items(): setattr(profile, k, v) From 807a200c9f8838e09a011bcfcdb4fc47c5f7dcbf Mon Sep 17 00:00:00 2001 From: Marco Enrico Piras Date: Thu, 2 May 2024 15:29:43 +0200 Subject: [PATCH 05/12] feat(oauth): :sparkles: add custom exception and handler for OAuth errors --- lifemonitor/errors.py | 14 ++++++++++++++ lifemonitor/exceptions.py | 8 ++++++++ 2 files changed, 22 insertions(+) diff --git a/lifemonitor/errors.py b/lifemonitor/errors.py index 7de4d74f..0639df2d 100644 --- a/lifemonitor/errors.py +++ b/lifemonitor/errors.py @@ -78,6 +78,19 @@ def handle_400(e: Exception = None, description: str = None): ) +@blueprint.route("/401") +def handle_401(e: Exception = None, description: str = None): + return __handle_error__( + { + "title": getattr(e, 'title', None) or "Unauthorized", + "code": "401", + "description": description if description + else str(e) if e and logger.isEnabledFor(logging.DEBUG) + else "Bad request", + } + ) + + @blueprint.route("/404") def handle_404(e: Exception = None): resource = request.args.get("resource", None, type=str) @@ -187,6 +200,7 @@ def register_api(app): logger.debug("Registering errors blueprint") app.register_blueprint(blueprint) app.register_error_handler(400, handle_400) + app.register_error_handler(401, handle_401) app.register_error_handler(404, handle_404) app.register_error_handler(429, handle_429) app.register_error_handler(500, handle_500) diff --git a/lifemonitor/exceptions.py b/lifemonitor/exceptions.py index e186b9ce..77a529a9 100644 --- a/lifemonitor/exceptions.py +++ b/lifemonitor/exceptions.py @@ -221,6 +221,14 @@ def __init__(self, detail=None, detail=detail, status=status, **kwargs) +class OAuthAuthorizationException(LifeMonitorException): + + def __init__(self, detail=None, title="OAuth Authorization Exception", + type="about:blank", status=401, **kwargs): + super().__init__(title=title, + detail=detail, status=status, **kwargs) + + def handle_exception(e: Exception): """Return JSON instead of HTML for HTTP errors.""" # start with the correct headers and status code from the error From 8ab1f5960d8641d541f356247945a2bb78ea5a5d Mon Sep 17 00:00:00 2001 From: Marco Enrico Piras Date: Thu, 2 May 2024 15:33:18 +0200 Subject: [PATCH 06/12] feat(aai): :recycle: update parsing of LSAAI user data --- .../auth/oauth2/client/providers/lsaai.py | 39 ++++++------------- 1 file changed, 12 insertions(+), 27 deletions(-) diff --git a/lifemonitor/auth/oauth2/client/providers/lsaai.py b/lifemonitor/auth/oauth2/client/providers/lsaai.py index 48bcaae9..55526b44 100644 --- a/lifemonitor/auth/oauth2/client/providers/lsaai.py +++ b/lifemonitor/auth/oauth2/client/providers/lsaai.py @@ -21,7 +21,7 @@ import logging from flask import current_app - +from lifemonitor.exceptions import OAuthAuthorizationException from lifemonitor.auth.oauth2.client.models import OAuthIdentity # Config a module level logger @@ -29,33 +29,18 @@ def normalize_userinfo(client, data): - logger.debug("LSAAI Data: %r", data) - preferred_username = data.get('eduperson_principal_name')[0].replace('@lifescience-ri.eu', '') \ - if 'eduperson_principal_namex' in data and len(data['eduperson_principal_name']) > 0 \ - else data['name'].replace(' ', '') - params = { - 'sub': str(data['sub']), - 'name': data['name'], - 'email': data.get('email'), - 'preferred_username': preferred_username - # 'profile': data['html_url'], - # 'picture': data['avatar_url'], - # 'website': data.get('blog'), - } - - # The email can be be None despite the scope being 'user:email'. - # That is because a user can choose to make his/her email private. - # If that is the case we get all the users emails regardless if private or note - # and use the one he/she has marked as `primary` + info = {} try: - if params.get('email') is None: - resp = client.get('user/emails') - resp.raise_for_status() - data = resp.json() - params["email"] = next(email['email'] for email in data if email['primary']) - except Exception as e: - logger.warning("Unable to get user email. Reason: %r", str(e)) - return params + info["sub"] = data['sub'] + except KeyError: + raise OAuthAuthorizationException(title="Unable to get user data", + description="the LS ID is required") + for key in ['name', 'email', 'preferred_username']: + info[key] = data.get(key, None) + if info[key] is None: + logger.warning("User %r has no %r", info["sub"], key) + + return info class LsAAI: From 1949691235394e636a52c71656f7e54ef7f47bde Mon Sep 17 00:00:00 2001 From: Marco Enrico Piras Date: Thu, 2 May 2024 15:37:36 +0200 Subject: [PATCH 07/12] feat(oauth): :goal_net: notify an appropriate error if the authorization step fails --- lifemonitor/auth/oauth2/client/controllers.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/lifemonitor/auth/oauth2/client/controllers.py b/lifemonitor/auth/oauth2/client/controllers.py index ad0e37c2..d21273da 100644 --- a/lifemonitor/auth/oauth2/client/controllers.py +++ b/lifemonitor/auth/oauth2/client/controllers.py @@ -86,8 +86,15 @@ def authorize(name): user_info = remote.userinfo(token=token) return _handle_authorize(remote, token, user_info) except OAuthError as e: - logger.debug(e) - return e.description, 401 + if logger.isEnabledFor(logging.DEBUG): + logger.exception(e) + raise exceptions.OAuthAuthorizationException(title="Authorization Error", + detail=e.description) + except Exception as e: + if logger.isEnabledFor(logging.DEBUG): + logger.exception(e) + raise exceptions.OAuthAuthorizationException(title="Authorization Error", + detail="Unable to authorize the user") @blueprint.route('/login/') @next_route_aware From a10ab45d2a8c285ef4b27e2b7b1b39f245e86894 Mon Sep 17 00:00:00 2001 From: Marco Enrico Piras Date: Thu, 2 May 2024 15:39:39 +0200 Subject: [PATCH 08/12] fix(aai): :lipstick: use a more general greeting if the username is not available --- lifemonitor/auth/templates/auth/identity_not_found.j2 | 4 ++++ lifemonitor/auth/templates/auth/register.j2 | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/lifemonitor/auth/templates/auth/identity_not_found.j2 b/lifemonitor/auth/templates/auth/identity_not_found.j2 index 121fbe04..2f4a0be3 100644 --- a/lifemonitor/auth/templates/auth/identity_not_found.j2 +++ b/lifemonitor/auth/templates/auth/identity_not_found.j2 @@ -26,7 +26,11 @@ {{ macros.render_provider_logo(identity.provider) }}
diff --git a/lifemonitor/auth/templates/auth/register.j2 b/lifemonitor/auth/templates/auth/register.j2 index 0d495f2a..5447ceb8 100644 --- a/lifemonitor/auth/templates/auth/register.j2 +++ b/lifemonitor/auth/templates/auth/register.j2 @@ -26,7 +26,11 @@
{{ macros.render_provider_logo(identity.provider) }}
From 389572473b88c9549851a57284836ce925afa3db Mon Sep 17 00:00:00 2001 From: Marco Enrico Piras Date: Thu, 2 May 2024 15:43:06 +0200 Subject: [PATCH 09/12] style(ui): :art: remove blanks --- .../auth/templates/auth/identity_not_found.j2 | 14 ++++++------ lifemonitor/auth/templates/auth/register.j2 | 22 +++++++++---------- lifemonitor/templates/base.j2 | 2 +- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/lifemonitor/auth/templates/auth/identity_not_found.j2 b/lifemonitor/auth/templates/auth/identity_not_found.j2 index 2f4a0be3..6a518306 100644 --- a/lifemonitor/auth/templates/auth/identity_not_found.j2 +++ b/lifemonitor/auth/templates/auth/identity_not_found.j2 @@ -6,10 +6,10 @@ {% block body %}