From 05c5f9c4b3f05239bf73ebb35f016bc5add1012b Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Fri, 11 Aug 2023 08:23:12 +0200 Subject: [PATCH 1/2] fix, google: don't include domain if hosted_domain has a single entry --- oauthenticator/google.py | 23 +++++++++++++--- oauthenticator/tests/test_google.py | 41 ++++++++++++++++++++++++++++- 2 files changed, 60 insertions(+), 4 deletions(-) diff --git a/oauthenticator/google.py b/oauthenticator/google.py index ed9cf4c0..58d44ed0 100644 --- a/oauthenticator/google.py +++ b/oauthenticator/google.py @@ -108,19 +108,28 @@ def _userdata_url_default(self): Unicode(), config=True, help=""" - Restrict sign-in to a list of email domain names, such as - `["mycollege.edu"]`. + This config has two functions. + + 1. Restrict sign-in to a list of email domain names, such as + `["mycollege.edu"]` or `["college1.edu", "college2.edu"]`. + 2. If a single domain is specified, the username will be stripped. Note that users with email domains in this list must still be allowed via another config, such as `allow_all`, `allowed_users`, or `allowed_google_groups`. + + ```{warning} Disruptive config changes + Changing this config either to or from having a single entry is a + disruptive change as the same Google user will get a new username, + either without or with a domain name included. + ``` """, ) @default('hosted_domain') def _hosted_domain_from_env(self): domains = [] - for domain in os.environ.get('HOSTED_DOMAIN', '').split(';'): + for domain in os.environ.get('HOSTED_DOMAIN', '').lower().split(';'): if domain: # check falsy to avoid trailing separators # adding empty domains @@ -162,11 +171,19 @@ async def update_auth_model(self, auth_model): configured and the user isn't part of `admin_users`. Note that leaving it at None makes users able to retain an admin status while setting it to False makes it be revoked. + + Strips the domain from the username if `hosted_domain` is configured + with a single entry. """ user_info = auth_model["auth_state"][self.user_auth_state_key] user_email = user_info["email"] user_domain = user_info["domain"] = user_email.split("@")[1].lower() + if len(self.hosted_domain) == 1 and self.hosted_domain[0] == user_domain: + # unambiguous domain, use only base name + username = user_email.split('@')[0] + auth_model["name"] = username + user_groups = set() if self.allowed_google_groups or self.admin_google_groups: user_groups = user_info["google_groups"] = self._fetch_user_groups( diff --git a/oauthenticator/tests/test_google.py b/oauthenticator/tests/test_google.py index d9e24196..7a28388d 100644 --- a/oauthenticator/tests/test_google.py +++ b/oauthenticator/tests/test_google.py @@ -185,7 +185,12 @@ async def test_google( assert auth_model == None -async def test_hosted_domain(google_client): +async def test_hosted_domain_single_entry(google_client): + """ + Tests that sign in is restricted to the listed domain and that the username + represents the part before the `@domain.com` as expected when hosted_domain + contains a single entry. + """ c = Config() c.GoogleOAuthenticator.hosted_domain = ["In-Hosted-Domain.com"] c.GoogleOAuthenticator.allow_all = True @@ -195,6 +200,40 @@ async def test_hosted_domain(google_client): handler = google_client.handler_for_user(handled_user_model) auth_model = await authenticator.get_authenticated_user(handler, None) assert auth_model + assert auth_model["name"] == "user1" + + handled_user_model = user_model("user1@not-in-hosted-domain.com") + handler = google_client.handler_for_user(handled_user_model) + with raises(HTTPError) as exc: + await authenticator.get_authenticated_user(handler, None) + assert exc.value.status_code == 403 + + +async def test_hosted_domain_multiple_entries(google_client): + """ + Tests that sign in is restricted to the listed domains and that the username + represents the full email as expected when hosted_domain contains multiple + entries. + """ + c = Config() + c.GoogleOAuthenticator.hosted_domain = [ + "In-Hosted-Domain1.com", + "In-Hosted-Domain2.com", + ] + c.GoogleOAuthenticator.allow_all = True + authenticator = GoogleOAuthenticator(config=c) + + handled_user_model = user_model("user1@iN-hosteD-domaiN1.com") + handler = google_client.handler_for_user(handled_user_model) + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model + assert auth_model["name"] == "user1@in-hosted-domain1.com" + + handled_user_model = user_model("user2@iN-hosteD-domaiN2.com") + handler = google_client.handler_for_user(handled_user_model) + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model + assert auth_model["name"] == "user2@in-hosted-domain2.com" handled_user_model = user_model("user1@not-in-hosted-domain.com") handler = google_client.handler_for_user(handled_user_model) From 7471a4e6eee1d0dbfdd8e15d38c3a512efcc1992 Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Fri, 11 Aug 2023 12:55:54 +0200 Subject: [PATCH 2/2] Apply suggestions from code review Co-authored-by: Min RK --- oauthenticator/google.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/oauthenticator/google.py b/oauthenticator/google.py index 58d44ed0..581c810a 100644 --- a/oauthenticator/google.py +++ b/oauthenticator/google.py @@ -112,7 +112,7 @@ def _userdata_url_default(self): 1. Restrict sign-in to a list of email domain names, such as `["mycollege.edu"]` or `["college1.edu", "college2.edu"]`. - 2. If a single domain is specified, the username will be stripped. + 2. If a single domain is specified, the username will be stripped to exclude the `@domain` part. Note that users with email domains in this list must still be allowed via another config, such as `allow_all`, `allowed_users`, or