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

[Google] Fix regression in v16 of no longer stripping username's domain if hosted_domain has a single entry #661

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
23 changes: 20 additions & 3 deletions oauthenticator/google.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
consideRatio marked this conversation as resolved.
Show resolved Hide resolved

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
Expand Down Expand Up @@ -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(
Expand Down
41 changes: 40 additions & 1 deletion oauthenticator/tests/test_google.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down