From 8cddc646aaa9d968c8efe1a9070d9ed50e9ee08e Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Thu, 5 Oct 2023 20:04:40 +0200 Subject: [PATCH 1/3] callysto: fix auth logic for use with oauthenticator v16 --- config/clusters/callysto/common.values.yaml | 120 ++++++++++++-------- 1 file changed, 70 insertions(+), 50 deletions(-) diff --git a/config/clusters/callysto/common.values.yaml b/config/clusters/callysto/common.values.yaml index 01bd567c25..204eb1263d 100644 --- a/config/clusters/callysto/common.values.yaml +++ b/config/clusters/callysto/common.values.yaml @@ -65,67 +65,68 @@ jupyterhub: hub: extraConfig: 001-cilogon-email-auth: | - # Custom override of CILogonOAuthenticator to allow access control via one - # property (email) while the username itself is a different property (oidc). - # This allows us to restrict access based on the *email* of the authenticated - # user without having to store the actual email in our servers. - from oauthenticator.cilogon import CILogonOAuthenticator from fnmatch import fnmatch - from traitlets import List, Unicode + + from oauthenticator.cilogon import CILogonOAuthenticator + class EmailAuthenticatingCILogonOAuthenticator(CILogonOAuthenticator): - allowed_domains = List( - Unicode, - default=None, - # This must always be set! - allow_none=False, - help=""" - Allow access to users with verified emails belonging to these domains. + """ + Custom override of CILogonOAuthenticator to allow access control via + one property (email) while the username itself is a different + property (oidc), and to allow `allowed_domains` to involve wildcards + like *. + + This allows us to restrict access based on the *email* of the + authenticated user without having to store the actual email in our + servers or use it as a username. + """ - Allows glob patterns but not regexes. So you can allow access to - *.edu, for example. See https://docs.python.org/3/library/fnmatch.html for - list of allowed patterns. - """, - config=True - ) + async def check_allowed(self, username, auth_model): + """ + Replaces the default implementation of check_allowed as it + otherwise would error, not finding a `@` symbol in the non-email + username. - async def authenticate(self, *args, **kwargs): - authenticated_user = await super().authenticate(*args, **kwargs) + Mostly a copy of OAuthenticator 16.1, for comparison see: + https://github.com/jupyterhub/oauthenticator/blob/16.1.0/oauthenticator/cilogon.py#L349-L373 + """ + if await super(CILogonOAuthenticator, self).check_allowed(username, auth_model): + return True - email = authenticated_user['auth_state']['cilogon_user']['email'] + user_info = auth_model["auth_state"][self.user_auth_state_key] + user_idp = user_info["idp"] - domain = email.rsplit('@', 1)[1] - for ad in self.allowed_domains: - # fnmatch will allow us to use wildcards like * and ?, but not the full - # regex. For simple domain matching this is good enough. If we were to - # use regexes instead, people will have to escape all their '.'s, and since - # that is actually going to match 'any character' it is a possible security hole. - if fnmatch(domain, ad): - return authenticated_user + idp_allow_all = self.allowed_idps[user_idp].get("allow_all") + if idp_allow_all: + return True + + user_info = auth_model["auth_state"][self.user_auth_state_key] + user_idp = user_info["idp"] + + idp_allowed_domains = self.allowed_idps[user_idp].get("allowed_domains") + if idp_allowed_domains: + # this was the part we wanted to replace + email = auth_model["auth_state"]["cilogon_user"]["email"] + email_domain = email.split("@", 1)[1].lower() + for ad in idp_allowed_domains: + # fnmatch allow us to use wildcards like * and ?, but + # not the full regex. For simple domain matching this is + # good enough. If we were to use regexes instead, people + # will have to escape all their '.'s, and since that is + # actually going to match 'any character' it is a + # possible security hole. For details see + # https://docs.python.org/3/library/fnmatch.html. + if fnmatch(email_domain, ad): + return True + + # users should be explicitly allowed via config, otherwise they aren't + return False - return None c.JupyterHub.authenticator_class = EmailAuthenticatingCILogonOAuthenticator + config: - EmailAuthenticatingCILogonOAuthenticator: - allowed_domains: - - 2i2c.org - - btps.ca - - callysto.ca - - cssd.ab.ca - - cybera.ca - - eics.ab.ca - - eips.ca - - epsb.ca - - fmpsd.ab.ca - - fmcsd.ab.ca - - rvschools.ab.ca - - spschools.org - - wsrd.ca - - ucalgary.ca - - ualberta.ca - - cfis.com - - "*.ca" CILogonOAuthenticator: # Usernames are based on a unique "oidc" claim and not email, so we need # to reference these names when declaring admin_users. The custom @@ -143,6 +144,25 @@ jupyterhub: http://google.com/accounts/o8/id: username_derivation: username_claim: "oidc" + allowed_domains: &allowed_domains + - 2i2c.org + - btps.ca + - callysto.ca + - cssd.ab.ca + - cybera.ca + - eics.ab.ca + - eips.ca + - epsb.ca + - fmpsd.ab.ca + - fmcsd.ab.ca + - rvschools.ab.ca + - spschools.org + - wsrd.ca + - ucalgary.ca + - ualberta.ca + - cfis.com + - "*.ca" http://login.microsoftonline.com/common/oauth2/v2.0/authorize: username_derivation: username_claim: "oidc" + allowed_domains: *allowed_domains From 14d4b5edd52cb25f83120265c786b7c5c16b1099 Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Mon, 9 Oct 2023 13:58:08 +0200 Subject: [PATCH 2/3] callysto: avoid hardcoding key in auth_state for the user info Co-authored-by: Georgiana --- config/clusters/callysto/common.values.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/clusters/callysto/common.values.yaml b/config/clusters/callysto/common.values.yaml index 204eb1263d..a49e2c958b 100644 --- a/config/clusters/callysto/common.values.yaml +++ b/config/clusters/callysto/common.values.yaml @@ -107,7 +107,7 @@ jupyterhub: idp_allowed_domains = self.allowed_idps[user_idp].get("allowed_domains") if idp_allowed_domains: # this was the part we wanted to replace - email = auth_model["auth_state"]["cilogon_user"]["email"] + email = auth_model["auth_state"][self.user_auth_state_key]["email"] email_domain = email.split("@", 1)[1].lower() for ad in idp_allowed_domains: # fnmatch allow us to use wildcards like * and ?, but From 46d9e30c841f3f0f3f2b5bc8cfbef4b1399d82ff Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Mon, 9 Oct 2023 21:09:51 +0200 Subject: [PATCH 3/3] callysto: add fixme note to cleanup custom authenticator class --- config/clusters/callysto/common.values.yaml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/config/clusters/callysto/common.values.yaml b/config/clusters/callysto/common.values.yaml index a49e2c958b..ac6e48f701 100644 --- a/config/clusters/callysto/common.values.yaml +++ b/config/clusters/callysto/common.values.yaml @@ -64,6 +64,13 @@ jupyterhub: {% endblock %} hub: extraConfig: + # FIXME: If the following issues are resolved, we should remove this + # custom authenticator class and use CILogonOAuthenticator + # directly instead. + # + # https://github.com/jupyterhub/oauthenticator/issues/547 + # https://github.com/jupyterhub/oauthenticator/issues/692 + # 001-cilogon-email-auth: | from fnmatch import fnmatch