Skip to content
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
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ be relevant to read more about in the configuration reference:

- {attr}`.GoogleOAuthenticator.allowed_google_groups`
- {attr}`.GoogleOAuthenticator.admin_google_groups`
- {attr}`.GoogleOAuthenticator.hosted_domain`
- {attr}`.GoogleOAuthenticator.allowed_hosted_domains`
- {attr}`.GoogleOAuthenticator.restrict_hosted_domains`

If you configure `allowed_google_groups` or `admin_google_groups`, you are
required to also configure:
Expand Down
123 changes: 98 additions & 25 deletions oauthenticator/google.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from jupyterhub.auth import LocalAuthenticator
from tornado.auth import GoogleOAuth2Mixin
from tornado.web import HTTPError
from traitlets import Bool, Dict, List, Set, Unicode, default, validate
from traitlets import Bool, Dict, List, Set, Unicode, default, observe, validate

from .oauth2 import OAuthenticator

Expand Down Expand Up @@ -125,55 +125,95 @@ def _userdata_url_default(self):
strip_domain = Bool(
config=True,
help="""
Strip the username to exclude the `@domain` part.
This happens by default when there is only one hosted domain specified
Strip the username to exclude the `@domain` part.

.. warning::
Should be combined with `restrict_hosted_domains` containing a single domain.

.. deprecated:: 17.5
strip_domain is enabled by default when the deprecated `hosted_domain` is
set to a single domain.
If the new `restrict_hosted_domains` is used instead,
`strip_domain` is strictly opt-in, not implicitly enabled.

If domains are stripped from usernames and multiple `hosted_domains` are specified,
there is a chance of clashing usernames.
.. versionchanged:: 17.5
strip_domain is always applied if True, not just for accounts in `hosted_domain`.

.. warning::
If domains are stripped from usernames and multiple domains are allowed,
multiple Google accounts may access the same JupyterHub account.
""",
)

@default('strip_domain')
def _strip_if_single_domain(self):
return len(self.hosted_domain) <= 1
if (
len(self.hosted_domain) == 1
and self.hosted_domain == self.restrict_hosted_domains
):
# deprecate this implicit default
self.log.warning(
f"Implicitly setting strip_domain=True due to a single-value in deprecated `hosted_domain={self.hosted_domain}`."
" This implicit configuration is deprecated in OAuthenticator 17.5,"
" and not inherited by the new `restrict_hosted_domains` config."
" Set `GoogleOAuthenticator.strip_domain = True|False` explicitly to avoid this warning."
)
return True
else:
return False

@validate('strip_domain')
def _check_multiple_hosted_domain(self, strip_domain):
if len(self.hosted_domain) > 1 and strip_domain:
@observe('strip_domain')
def _check_multiple_hosted_domain(self, change):
if len(self.restrict_hosted_domains) != 1 and change.value:
self.log.warning(
"User names are stripped of `@domain`, but multiple domains are specified."
" This can lead to clashing usernames"
"User names are stripped of `@domain`, but more than one domain may be allowed."
" This can lead to multiple Google accounts accessing the same JupyterHub user."
)
return strip_domain.value

hosted_domain = List(
Unicode(),
config=True,
help="""
This config has two functions.
.. deprecated:: 17.5
The ambiguous `hosted_domain` is deprecated in favor of clearer and simpler :attr:`~.GoogleOAuthenticator.restrict_hosted_domains`.

This config does one or two things.

1. Restrict sign-in to users part of Google organizations/workspaces
managing domains, such as `["mycollege.edu"]` or `["college1.edu",
"college2.edu"]`.
2. If a single domain is specified, usernames with that domain will be
stripped to exclude the `@domain` part.
2. If a single domain is specified, usernames will be stripped to exclude the `@domain` part by default.
You can opt-out of this behavior by setting `GoogleOAuthenticator.strip_domain = False`,
which preserves the email address as the account name (default behavior in most configurations).

Users not restricted by this configuration must still be explicitly
allowed by a configuration intended to allow users, like `allow_all`,
`allowed_users`, `allowed_hosted_domains`, or `allowed_google_groups`.
This config only **restricts** access, it does not **grant** any users access.
Users in these domains must still be explicitly
allowed by additional configuration intended to allow users,
such as
`allow_all`, `allowed_users`, `allowed_hosted_domains`, or `allowed_google_groups`, etc.

Users not in these hosted domains **cannot be granted access** via `allowed_users`, etc..
**Only users in these domains** are considered for authentication with JupyterHub.

.. warning::

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.
Changing this config either to or from having a single entry
will change the default value of `strip_domain`,
(True when `hosted_domain` has exactly one domain, False otherwise).
changing the resulting usernames.
You can set `strip_domain` explicitly to avoid any implicit changes.
Suggestion: use `restrict_hosted_domains`, which does not imply any setting
for `strip_domain`.

.. versionchanged:: 16.1

Now restricts sign-in based on the hd claim, not the domain in the
user's email.

.. seealso::

- :attr:`~.GoogleOAuthenticator.strip_domain`
- :attr:`~.GoogleOAuthenticator.allowed_hosted_domains` for granting access
to members of a domain without *excluding* accounts from other sources.
""",
)

Expand All @@ -199,6 +239,36 @@ def _cast_hosted_domain(self, proposal):
return [proposal.value.lower()]
return [hd.lower() for hd in proposal.value]

restrict_hosted_domains = List(
Unicode(),
config=True,
help="""
Restrict sign-in to users part of Google organizations/workspaces
managing domains, such as `["mycollege.edu"]` or `["college1.edu",
"college2.edu"]`.

This config only **restricts** access, it does not **grant** any users access.
Users in these domains must still be explicitly
allowed by additional configuration intended to allow users,
such as `allow_all`, `allowed_users`, `allowed_hosted_domains`, or `allowed_google_groups`, etc.

Users not in these hosted domains **cannot be granted access** via `allowed_users`, etc..
**Only users in these domains** are considered for authentication with JupyterHub.

.. versionadded:: 17.5

.. seealso::

- :attr:`~.GoogleOAuthenticator.strip_domain`
- :attr:`~.GoogleOAuthenticator.allowed_hosted_domains` for granting access
to members of a domain without *excluding* accounts from other sources.
""",
)

@validate('restrict_hosted_domains')
def _cast_restrict_hosted_domains(self, proposal):
return [hd.lower() for hd in proposal.value]

allowed_hosted_domains = List(
Unicode(),
config=True,
Expand All @@ -221,6 +291,7 @@ def _cast_allowed_hosted_domains(self, proposal):
# _deprecated_oauth_aliases is used by deprecation logic in OAuthenticator
_deprecated_oauth_aliases = {
"google_group_whitelist": ("allowed_google_groups", "0.12.0"),
"hosted_domain": ("restrict_hosted_domains", "17.5.0"),
**OAuthenticator._deprecated_oauth_aliases,
}
google_group_whitelist = Dict(
Expand All @@ -247,7 +318,7 @@ def user_info_to_username(self, user_info):
# derivation. Decoupling hosted_domain from this is considered in
# https://github.com/jupyterhub/oauthenticator/issues/733.

if self.strip_domain and user_info["domain"] in self.hosted_domain:
if self.strip_domain:
username = username.split("@")[0]

return username
Expand Down Expand Up @@ -302,8 +373,10 @@ def check_blocked_users(self, username, auth_model):
# hd ref: https://developers.google.com/identity/openid-connect/openid-connect#id_token-hd
hd = user_info.get("hd", "")

if self.hosted_domain and hd not in self.hosted_domain:
self.log.warning(f"Blocked {username} with 'hd={hd}' not in hosted_domain")
if self.restrict_hosted_domains and hd not in self.restrict_hosted_domains:
self.log.warning(
f"Blocked {username} with 'hd={hd}' not in restrict_hosted_domains={self.restrict_hosted_domains}"
)
return False

return super().check_blocked_users(username, auth_model)
Expand Down
16 changes: 6 additions & 10 deletions oauthenticator/tests/test_google.py
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ async def test_google(
if authenticator.allowed_google_groups or authenticator.admin_google_groups:
assert user_info["google_groups"] == ["group1"]
else:
assert auth_model == None
assert auth_model is None


@mark.parametrize(
Expand All @@ -244,7 +244,7 @@ async def test_google(
("05", "user1@not-ok.org", "", None, False, None),
# Test variation 06 below isn't believed to be possible, but since we
# aren't sure this test clarifies what we expect to happen.
("06", "user1@other.org", "ok-hd.org", "user1@other.org", True, None),
("06", "user2@other.org", "ok-hd.org", "user2", True, None),
],
)
async def test_hosted_domain_single_entry(
Expand All @@ -267,6 +267,7 @@ async def test_hosted_domain_single_entry(
c.GoogleOAuthenticator.allowed_users = {"user2", "blocked", "user1@other.org"}
c.GoogleOAuthenticator.blocked_users = {"blocked"}
authenticator = GoogleOAuthenticator(config=c)
assert authenticator.restrict_hosted_domains == c.GoogleOAuthenticator.hosted_domain

handled_user_model = user_model(user_email, hd=user_hd)
handler = google_client.handler_for_user(handled_user_model)
Expand All @@ -276,7 +277,7 @@ async def test_hosted_domain_single_entry(
assert auth_model["name"] == expect_username
assert auth_model["admin"] == expect_admin
else:
assert auth_model == None
assert auth_model is None


@mark.parametrize(
Expand Down Expand Up @@ -321,12 +322,7 @@ async def test_hosted_domain_multiple_entries(
entries.
"""
c = Config()
c.GoogleOAuthenticator.hosted_domain = [
"ok-hd1.org",
"ok-hd2.ORG",
]

c.GoogleOAuthenticator.allowed_hosted_domains = [
c.GoogleOAuthenticator.restrict_hosted_domains = [
"ok-hd1.org",
"ok-hd2.ORG",
]
Expand All @@ -342,7 +338,7 @@ async def test_hosted_domain_multiple_entries(
assert auth_model
assert auth_model["name"] == expect_username
else:
assert auth_model == None
assert auth_model is None


@mark.parametrize(
Expand Down
Loading