set allow_all=False by default

This commit is contained in:
Min RK
2024-03-22 15:46:03 +01:00
parent e1e34a14a2
commit c3c69027fa
6 changed files with 333 additions and 140 deletions

View File

@@ -41,6 +41,11 @@ When testing, it may be helpful to use the
password unless a global password has been set. Once set, any username will password unless a global password has been set. Once set, any username will
still be accepted but the correct password will need to be provided. still be accepted but the correct password will need to be provided.
:::{versionadded} 5.0
The DummyAuthenticator's default `allow_all` is True,
unlike most other Authenticators.
:::
## Additional Authenticators ## Additional Authenticators
Additional authenticators can be found on GitHub Additional authenticators can be found on GitHub
@@ -81,6 +86,7 @@ Writing an Authenticator that looks up passwords in a dictionary
requires only overriding this one method: requires only overriding this one method:
```python ```python
from secrets import compare_digest
from traitlets import Dict from traitlets import Dict
from jupyterhub.auth import Authenticator from jupyterhub.auth import Authenticator
@@ -91,8 +97,14 @@ class DictionaryAuthenticator(Authenticator):
) )
async def authenticate(self, handler, data): async def authenticate(self, handler, data):
if self.passwords.get(data['username']) == data['password']: username = data["username"]
return data['username'] password = data["password"]
check_password = self.passwords.get(username, "")
# always call compare_digest, for timing attacks
if compare_digest(check_password, password) and username in self.passwords:
return username
else:
return None
``` ```
#### Normalize usernames #### Normalize usernames
@@ -190,10 +202,10 @@ via `jupyterhub --generate-config`.
When dealing with logging in, there are generally two _separate_ steps: When dealing with logging in, there are generally two _separate_ steps:
authentication authentication
: identifying who is logged in, and : identifying who is trying to log in, and
authorization authorization
: deciding whether an authenticated user is logged in : deciding whether an authenticated user is allowed to access your JupyterHub
{meth}`Authenticator.authenticate` is responsible for authenticating users. {meth}`Authenticator.authenticate` is responsible for authenticating users.
It is perfectly fine in the simplest cases for `Authenticator.authenticate` to be responsible for authentication _and_ authorization, It is perfectly fine in the simplest cases for `Authenticator.authenticate` to be responsible for authentication _and_ authorization,
@@ -203,71 +215,137 @@ However, Authenticators also have have two methods {meth}`~.Authenticator.check_
If `check_blocked_users()` returns False, authorization stops and the user is not allowed. If `check_blocked_users()` returns False, authorization stops and the user is not allowed.
If `check_allowed()` returns True, authorization proceeds. If `Authenticator.allow_all` is True OR `check_allowed()` returns True, authorization proceeds.
:::{versionadded} 5.0 :::{versionadded} 5.0
{attr}`Authenticator.allow_all` and {attr}`Authenticator.allow_existing_users` are new in JupyterHub 5.0. {attr}`.Authenticator.allow_all` and {attr}`.Authenticator.allow_existing_users` are new in JupyterHub 5.0.
By default, `allow_all` is True when `allowed_users` is empty, By default, `allow_all` is False,
and `allow_existing_users` is True when `allowed_users` is not empty. which is a change from pre-5.0, where `allow_all` was implicitly True if `allowed_users` was empty.
This is to ensure backward-compatibility, but subclasses are free to pick more restrictive defaults.
::: :::
### Overriding `check_allowed` ### Overriding `check_allowed`
:::{versionchanged} 5.0
`check_allowed()` is **not called** is `allow_all` is True.
:::
:::{versionchanged} 5.0
Starting with 5.0, `check_allowed()` should **NOT** return True if no allow config
is specified (`allow_all` should be used instead).
:::
The base implementation of {meth}`~.Authenticator.check_allowed` checks: The base implementation of {meth}`~.Authenticator.check_allowed` checks:
- if `allow_all` is True, return True
- if username is in the `allowed_users` set, return True - if username is in the `allowed_users` set, return True
- else return False - else return False
If a custom Authenticator defines additional sources of `allow` configuration, :::{versionchanged} 5.0
such as membership in a group or other information, Prior to 5.0, the check was
it should override `check_allowed` to account for this.
`allow_` configuration should generally be _additive_,
i.e. if permission is granted by _any_ allow configuration,
a user should be authorized.
:::{note}
For backward-compatibility, it is the responsibility of `Authenticator.check_allowed()` to check `.allow_all`.
This is to avoid the backward-compatible default values from granting permissions unexpectedly.
:::
If an Authenticator defines additional `allow` configuration, it must at least:
1. override `check_allowed`, and
2. override the default for `allow_all`
The default for `allow_all` in a custom authenticator should be one of `False` or a dynamic default matching something like `if not any allow configuration specified`.
False is recommended for authenticators which source much larger pools of users than are _typically_ allowed to access a Hub (e.g. generic OAuth providers like Google, GitHub, etc.).
For example, here is how `PAMAuthenticator` extends the base class to add `allowed_groups`:
```python ```python
from traitlets import default if (not allowed_users) or username in allowed_users:
@default("allow_all")
def _allow_all_default(self):
if self.allowed_users or self.allowed_groups:
# if any allow config is specified, default to False
return False
return True
def check_allowed(self, username, authentication=None):
if self.allow_all:
return True
if self.check_allowed_groups(username, authentication):
return True
return super().check_allowed(username, authentication)
``` ```
Important points to note: but the implicit `not allowed_users` has been replaced by explicit `allow_all`, which is checked _before_ calling `check_allowed`.
`check_allowed` **is not called** if `allow_all` is True.
- overriding the default for `allow_all` is required to avoid `allow_all` being True when `allowed_groups` is specified, but `allowed_users` is not. If your Authenticator subclass similarly returns True when no allow config is defined,
- `allow_all` must be checked inside `check_allowed` this is fully backward compatible for your users, but means `allow_all = False` has no real effect.
- `allowed_groups` strictly expands who is authorized,
it does not apply restrictions `allowed_users`. You can make your Authenticator forward-compatible with JupyterHub 5 by defining `allow_all` as a boolean config trait on your class:
This is recommended for all `allow_` configuration added by Authenticators.
```python
class MyAuthenticator(Authenticator):
# backport allow_all from JupyterHub 5
allow_all = Bool(False, config=True)
def check_allowed(self, username, authentication):
if self.allow_all:
# replaces previous "if no auth config"
return True
...
```
:::
If an Authenticator defines additional sources of `allow` configuration,
such as membership in a group or other information,
it should override `check_allowed` to account for this.
:::{note}
`allow_` configuration should generally be _additive_,
i.e. if access is granted by _any_ allow configuration,
a user should be authorized.
JupyterHub recommends that Authenticators applying _restrictive_ configuration should use names like `block_` or `require_`,
and check this during `check_blocked_users` or `authenticate`, not `check_allowed`.
:::
In general, an Authenticator's skeleton should look like:
```python
class MyAuthenticator(Authenticator):
# backport allow_all for compatibility with JupyterHub < 5
allow_all = Bool(False, config=True)
require_something = List(config=True)
allowed_something = Set()
def authenticate(self, data, handler):
...
if success:
return {"username": username, "auth_state": {...}}
else:
return None
def check_blocked_users(self, username, authentication=None):
"""Apply _restrictive_ configuration"""
if self.require_something and not has_something(username, self.request_):
return False
# repeat for each restriction
if restriction_defined and restriction_not_met:
return False
return super().check_blocked_users(self, username, authentication)
def check_allowed(self, username, authentication=None):
"""Apply _permissive_ configuration
Only called if check_blocked_users returns True
AND allow_all is False
"""
if self.allow_all:
# check here to backport allow_all behavior
# from JupyterHub 5
# this branch will never be taken with jupyterhub >=5
return True
if self.allowed_something and user_has_something(username):
return True
# repeat for each allow
if allow_config and allow_met:
return True
# should always have this at the end
if self.allowed_users and username in self.allowed_users:
return True
# do not call super!
# super().check_allowed is not safe with JupyterHub < 5.0,
# as it will return True if allowed_users is empty
return False
```
Key points:
- `allow_all` is backported from JupyterHub 5, for consistent behavior in all versions of JupyterHub (optional)
- restrictive configuration is checked in `check_blocked_users`
- if any restriction is not met, `check_blocked_users` returns False
- permissive configuration is checked in `check_allowed`
- if any `allow` condition is met, `check_allowed` returns True
So the logical expression for a user being authorized should look like:
> if ALL restrictions are met AND ANY admissions are met: user is authorized
#### Custom error messages #### Custom error messages

View File

@@ -6,30 +6,57 @@ The default Authenticator uses [PAM][] (Pluggable Authentication Module) to auth
their usernames and passwords. With the default Authenticator, any user their usernames and passwords. With the default Authenticator, any user
with an account and password on the system will be allowed to login. with an account and password on the system will be allowed to login.
## Create a set of allowed users (`allowed_users`) ## Deciding who is allowed
In the base Authenticator, there are 3 configuration options for granting users access to your Hub:
1. `allow_all` grants any user who can successfully authenticate access to the Hub
2. `allowed_users` defines a set of users who can access the Hub
3. `allow_existing_users` enables managing users via the JupyterHub API or admin page
These options should apply to all Authenticators.
Your chosen Authenticator may add additional configuration options to admit users, such as team membership, course enrollment, etc.
:::{important}
You should always specify at least one allow configuration if you want people to be able to access your Hub!
In most cases, this looks like:
```python
c.Authenticator.allow_all = True
# or
c.Authenticator.allowed_users = {"name", ...}
```
:::
:::{versionchanged} 5.0
If no allow config is specified, then by default **nobody will have access to your Hub**.
Prior to 5.0, the opposite was true; effectively `allow_all = True` if no other allow config was specified.
:::
You can restrict which users are allowed to login with a set, You can restrict which users are allowed to login with a set,
`Authenticator.allowed_users`: `Authenticator.allowed_users`:
```python ```python
c.Authenticator.allowed_users = {'mal', 'zoe', 'inara', 'kaylee'} c.Authenticator.allowed_users = {'mal', 'zoe', 'inara', 'kaylee'}
c.Authenticator.allow_all = False # c.Authenticator.allow_all = False
c.Authenticator.allow_existing_users = False c.Authenticator.allow_existing_users = False
``` ```
Users in the `allowed_users` set are added to the Hub database when the Hub is started. Users in the `allowed_users` set are added to the Hub database when the Hub is started.
```{warning} :::{versionchanged} 5.0
If `allowed_users` is not specified, then by default **all authenticated users will be allowed into your hub**, {attr}`.Authenticator.allow_all` and {attr}`.Authenticator.allow_existing_users` are new in JupyterHub 5.0
i.e. `allow_all` defaults to True if neither `allowed_users` nor `allow_all` are set. to enable explicit configuration of previously implicit behavior.
```
:::{versionadded} 5.0 Prior to 5.0, `allow_all` was implicitly True if `allowed_users` was empty.
{attr}`Authenticator.allow_all` and {attr}`Authenticator.allow_existing_users` are new in JupyterHub 5.0. Starting with 5.0, to allow all authenticated users by default,
`allow_all` must be explicitly set to True.
By default, `allow_all` is True when `allowed_users` is empty, By default, `allow_existing_users` is True when `allowed_users` is not empty,
and `allow_existing_users` is True when `allowed_users` is not empty. to ensure backward-compatibility.
This is to ensure backward-compatibility. To make the `allowed_users` set _restrictive_,
set `allow_existing_users = False`.
::: :::
## One Time Passwords ( request_otp ) ## One Time Passwords ( request_otp )
@@ -102,6 +129,11 @@ By default, only the deprecated `admin` role has global `access` permissions.
## Add or remove users from the Hub ## Add or remove users from the Hub
:::{versionadded} 5.0
`c.Authenticator.allow_existing_users` is added in 5.0 and enabled by default.
Prior to 5.0, this behavior was not optional.
:::
Users can be added to and removed from the Hub via the admin Users can be added to and removed from the Hub via the admin
panel or the REST API. panel or the REST API.

View File

@@ -2077,6 +2077,9 @@ class JupyterHub(Application):
"auth_state is enabled, but encryption is not available: %s" % e "auth_state is enabled, but encryption is not available: %s" % e
) )
# give the authenticator a chance to check its own config
self.authenticator.check_allow_config()
if self.admin_users and not self.authenticator.admin_users: if self.admin_users and not self.authenticator.admin_users:
self.log.warning( self.log.warning(
"\nJupyterHub.admin_users is deprecated since version 0.7.2." "\nJupyterHub.admin_users is deprecated since version 0.7.2."
@@ -2104,9 +2107,9 @@ class JupyterHub(Application):
new_users.append(user) new_users.append(user)
else: else:
user.admin = True user.admin = True
# the admin_users config variable will never be used after this point. # the admin_users config variable will never be used after this point.
# only the database values will be referenced. # only the database values will be referenced.
allowed_users = [ allowed_users = [
self.authenticator.normalize_username(name) self.authenticator.normalize_username(name)
for name in self.authenticator.allowed_users for name in self.authenticator.allowed_users
@@ -2116,10 +2119,10 @@ class JupyterHub(Application):
if not self.authenticator.validate_username(username): if not self.authenticator.validate_username(username):
raise ValueError("username %r is not valid" % username) raise ValueError("username %r is not valid" % username)
if not allowed_users: if self.authenticator.allowed_users and self.authenticator.admin_users:
self.log.info( # make sure admin users are in the allowed_users set, if defined,
"Not using allowed_users. Any authenticated user will be allowed." # otherwise they won't be able to login
) self.authenticator.allowed_users |= self.authenticator.admin_users
# add allowed users to the db # add allowed users to the db
for name in allowed_users: for name in allowed_users:

View File

@@ -121,6 +121,54 @@ class Authenticator(LoggingConfigurable):
""" """
).tag(config=True) ).tag(config=True)
any_allow_config = Bool(
False,
help="""Is there any allow config?
Used to show a warning if it looks like nobody can access the HUb,
which can happen when upgrading to JupyterHub 5,
now that `allow_all` defaults to False.
Deployments can set this explicitly to True to suppress
the "No allow config found" warning.
Will be True if any config tagged with `.tag(allow_config=True)`
or starts with `allow` is truthy.
.. versionadded:: 5.0
""",
).tag(config=True)
@default("any_allow_config")
def _default_any_allowed(self):
for trait_name, trait in self.traits(config=True).items():
if trait.metadata.get("allow_config", False) or trait_name.startswith(
"allow"
):
# this is only used for a helpful warning, so not the biggest deal if it's imperfect
if getattr(self, trait_name):
return True
return False
def check_allow_config(self):
"""Log a warning if no allow config can be found.
Could get a false positive if _only_ unrecognized allow config is used.
Authenticators can apply `.tag(allow_config=True)` to label this config
to make sure it is found.
Subclasses can override to perform additonal checks and warn about likely
authenticator configuration problems.
.. versionadded:: 5.0
"""
if not self.any_allow_config:
self.log.warning(
"No allow config found, it's possible that nobody can login to your Hub!\n"
"You can set `c.Authenticator.allow_all = True` to allow any user who can login to access the Hub,\n"
"or e.g. `allowed_users` to a set of users who should have access."
)
whitelist = Set( whitelist = Set(
help="Deprecated, use `Authenticator.allowed_users`", help="Deprecated, use `Authenticator.allowed_users`",
config=True, config=True,
@@ -145,50 +193,35 @@ class Authenticator(LoggingConfigurable):
).tag(config=True) ).tag(config=True)
allow_all = Bool( allow_all = Bool(
# dynamic default computed from allowed_users False,
config=True, config=True,
help=""" help="""
Any users who can successfully authenticate are allowed to access JupyterHub. Allow every user who can successfully authenticate to access JupyterHub.
For backward-compatibility, this is True by default if `allowed_users` is unspecified, False by default, which means for most Authenticators,
False if `allowed_users` is specified. _some_ allow-related configuration is required to allow any users to log in.
Authenticator subclasses may override the default with e.g.:: Authenticator subclasses may override the default with e.g.::
@default("allow_all") @default("allow_all")
def _default_allow_all(self): def _default_allow_all(self):
return False # if _any_ auth config (depends on the Authenticator)
if self.allowed_users or self.allowed_groups or self.allow_existing_users:
# or
@default("allow_all")
def _default_allow_all(self):
if self.allowed_users or self.allowed_groups:
return False return False
else: else:
return True return True
False is a good idea when the group of users who can authenticate is typically larger
than the group users who should have access (e.g. OAuth providers).
Authenticator subclasses that define additional sources of `allow` config
beyond `allowed_users` should specify a default value for allow_all,
either always False or `if not any allow_config`.
This is checked inside `check_allowed`, so subclasses that override `check_allowed`
must explicitly check `allow_all` for it to have any effect.
This is for safety, to ensure that no Authenticator subclass gets unexpected behavior from `allow_all`.
.. versionadded:: 5.0 .. versionadded:: 5.0
.. versionchanged:: 5.0
Prior to 5.0, `allow_all` wasn't defined on its own,
and was instead implicitly True when no allow config was provided,
i.e. `allowed_users` unspecified or empty on the base Authenticator class.
To preserve pre-5.0 behavior,
set `allow_all = True` if you have no other allow configuration.
""", """,
) ).tag(allow_config=True)
@default("allow_all")
def _default_allow_all(self):
if self.allowed_users:
return False
else:
return True
allow_existing_users = Bool( allow_existing_users = Bool(
# dynamic default computed from allowed_users # dynamic default computed from allowed_users
@@ -223,7 +256,7 @@ class Authenticator(LoggingConfigurable):
.. versionadded:: 5.0 .. versionadded:: 5.0
""", """,
) ).tag(allow_config=True)
@default("allow_existing_users") @default("allow_existing_users")
def _allow_existing_users_default(self): def _allow_existing_users_default(self):
@@ -616,8 +649,9 @@ class Authenticator(LoggingConfigurable):
The various stages can be overridden separately: The various stages can be overridden separately:
- `authenticate` turns formdata into a username - `authenticate` turns formdata into a username
- `normalize_username` normalizes the username - `normalize_username` normalizes the username
- `check_allowed` checks against the allowed usernames
- `check_blocked_users` check against the blocked usernames - `check_blocked_users` check against the blocked usernames
- `allow_all` is checked
- `check_allowed` checks against the allowed usernames
- `is_admin` check if a user is an admin - `is_admin` check if a user is an admin
.. versionchanged:: 0.8 .. versionchanged:: 0.8
@@ -651,7 +685,11 @@ class Authenticator(LoggingConfigurable):
self.log.warning("User %r blocked. Stop authentication", username) self.log.warning("User %r blocked. Stop authentication", username)
return return
allowed_pass = await maybe_future(self.check_allowed(username, authenticated)) allowed_pass = self.allow_all
if not allowed_pass:
allowed_pass = await maybe_future(
self.check_allowed(username, authenticated)
)
if allowed_pass: if allowed_pass:
if authenticated['admin'] is None: if authenticated['admin'] is None:
@@ -776,14 +814,14 @@ class Authenticator(LoggingConfigurable):
By default, this adds the user to the allowed_users set if By default, this adds the user to the allowed_users set if
allow_existing_users is true. allow_existing_users is true.
Subclasses may do more extensive things, such as adding actual unix users, Subclasses may do more extensive things, such as creating actual system users,
but they should call super to ensure the allowed_users set is updated. but they should call super to ensure the allowed_users set is updated.
Note that this should be idempotent, since it is called whenever the hub restarts Note that this should be idempotent, since it is called whenever the hub restarts
for all users. for all users.
.. versionchanged:: 5.0 .. versionchanged:: 5.0
Now adds users to the allowed_users set if allow_all is False, Now adds users to the allowed_users set if allow_all is False and allow_existing_users is True,
instead of if allowed_users is not empty. instead of if allowed_users is not empty.
Args: Args:
@@ -791,10 +829,7 @@ class Authenticator(LoggingConfigurable):
""" """
if not self.validate_username(user.name): if not self.validate_username(user.name):
raise ValueError("Invalid username: %s" % user.name) raise ValueError("Invalid username: %s" % user.name)
# this is unnecessary if allow_all is True, if self.allow_existing_users and not self.allow_all:
# but skipping this when allow_all is False breaks backward-compatibility
# for Authenticator subclasses that may not yet understand allow_all
if self.allow_existing_users:
self.allowed_users.add(user.name) self.allowed_users.add(user.name)
def delete_user(self, user): def delete_user(self, user):
@@ -1005,18 +1040,9 @@ class LocalAuthenticator(Authenticator):
`allowed_groups` may be specified together with allowed_users, `allowed_groups` may be specified together with allowed_users,
to grant access by group OR name. to grant access by group OR name.
""" """
).tag(config=True) ).tag(config=True, allow_config=True)
@default("allow_all")
def _allow_all_default(self):
if self.allowed_users or self.allowed_groups:
# if any allow config is specified, default to False
return False
return True
def check_allowed(self, username, authentication=None): def check_allowed(self, username, authentication=None):
if self.allow_all:
return True
if self.check_allowed_groups(username, authentication): if self.check_allowed_groups(username, authentication):
return True return True
return super().check_allowed(username, authentication) return super().check_allowed(username, authentication)
@@ -1349,8 +1375,20 @@ class DummyAuthenticator(Authenticator):
if it logs in with that password. if it logs in with that password.
.. versionadded:: 1.0 .. versionadded:: 1.0
.. versionadded:: 5.0
`allow_all` defaults to True,
preserving default behavior.
""" """
@default("allow_all")
def _allow_all_default(self):
if self.allowed_users:
return False
else:
# allow all by default
return True
password = Unicode( password = Unicode(
config=True, config=True,
help=""" help="""
@@ -1360,6 +1398,12 @@ class DummyAuthenticator(Authenticator):
""", """,
) )
def check_allow_config(self):
super().check_allow_config()
self.log.warning(
f"Using testing authenticator {self.__class__.__name__}! This is not meant for production!"
)
async def authenticate(self, handler, data): async def authenticate(self, handler, data):
"""Checks against a global password if it's been set. If not, allow any user/pass combo""" """Checks against a global password if it's been set. If not, allow any user/pass combo"""
if self.password: if self.password:

View File

@@ -243,6 +243,7 @@ class MockHub(JupyterHub):
cert_location = kwargs['internal_certs_location'] cert_location = kwargs['internal_certs_location']
kwargs['external_certs'] = ssl_setup(cert_location, 'hub-ca') kwargs['external_certs'] = ssl_setup(cert_location, 'hub-ca')
super().__init__(*args, **kwargs) super().__init__(*args, **kwargs)
self.config.Authenticator.allow_all = True
@default('subdomain_host') @default('subdomain_host')
def _subdomain_host_default(self): def _subdomain_host_default(self):

View File

@@ -19,7 +19,7 @@ from .utils import add_user, async_requests, get_page, public_url
async def test_pam_auth(): async def test_pam_auth():
authenticator = MockPAMAuthenticator() authenticator = MockPAMAuthenticator(allow_all=True)
authorized = await authenticator.get_authenticated_user( authorized = await authenticator.get_authenticated_user(
None, {'username': 'match', 'password': 'match'} None, {'username': 'match', 'password': 'match'}
) )
@@ -38,7 +38,7 @@ async def test_pam_auth():
async def test_pam_auth_account_check_disabled(): async def test_pam_auth_account_check_disabled():
authenticator = MockPAMAuthenticator(check_account=False) authenticator = MockPAMAuthenticator(allow_all=True, check_account=False)
authorized = await authenticator.get_authenticated_user( authorized = await authenticator.get_authenticated_user(
None, {'username': 'allowedmatch', 'password': 'allowedmatch'} None, {'username': 'allowedmatch', 'password': 'allowedmatch'}
) )
@@ -83,7 +83,9 @@ async def test_pam_auth_admin_groups():
return user_group_map[name] return user_group_map[name]
authenticator = MockPAMAuthenticator( authenticator = MockPAMAuthenticator(
admin_groups={'jh_admins', 'wheel'}, admin_users={'override_admin'} admin_groups={'jh_admins', 'wheel'},
admin_users={'override_admin'},
allow_all=True,
) )
# Check admin_group applies as expected # Check admin_group applies as expected
@@ -142,7 +144,10 @@ async def test_pam_auth_admin_groups():
async def test_pam_auth_allowed(): async def test_pam_auth_allowed():
authenticator = MockPAMAuthenticator(allowed_users={'wash', 'kaylee'}) authenticator = MockPAMAuthenticator(
allowed_users={'wash', 'kaylee'}, allow_all=False
)
authorized = await authenticator.get_authenticated_user( authorized = await authenticator.get_authenticated_user(
None, {'username': 'kaylee', 'password': 'kaylee'} None, {'username': 'kaylee', 'password': 'kaylee'}
) )
@@ -163,7 +168,7 @@ async def test_pam_auth_allowed_groups():
def getgrnam(name): def getgrnam(name):
return MockStructGroup('grp', ['kaylee']) return MockStructGroup('grp', ['kaylee'])
authenticator = MockPAMAuthenticator(allowed_groups={'group'}) authenticator = MockPAMAuthenticator(allowed_groups={'group'}, allow_all=False)
with mock.patch.object(authenticator, '_getgrnam', getgrnam): with mock.patch.object(authenticator, '_getgrnam', getgrnam):
authorized = await authenticator.get_authenticated_user( authorized = await authenticator.get_authenticated_user(
@@ -180,14 +185,14 @@ async def test_pam_auth_allowed_groups():
async def test_pam_auth_blocked(): async def test_pam_auth_blocked():
# Null case compared to next case # Null case compared to next case
authenticator = MockPAMAuthenticator() authenticator = MockPAMAuthenticator(allow_all=True)
authorized = await authenticator.get_authenticated_user( authorized = await authenticator.get_authenticated_user(
None, {'username': 'wash', 'password': 'wash'} None, {'username': 'wash', 'password': 'wash'}
) )
assert authorized['name'] == 'wash' assert authorized['name'] == 'wash'
# Blacklist basics # Blocklist basics
authenticator = MockPAMAuthenticator(blocked_users={'wash'}) authenticator = MockPAMAuthenticator(blocked_users={'wash'}, allow_all=True)
authorized = await authenticator.get_authenticated_user( authorized = await authenticator.get_authenticated_user(
None, {'username': 'wash', 'password': 'wash'} None, {'username': 'wash', 'password': 'wash'}
) )
@@ -195,7 +200,9 @@ async def test_pam_auth_blocked():
# User in both allowed and blocked: default deny. Make error someday? # User in both allowed and blocked: default deny. Make error someday?
authenticator = MockPAMAuthenticator( authenticator = MockPAMAuthenticator(
blocked_users={'wash'}, allowed_users={'wash', 'kaylee'} blocked_users={'wash'},
allowed_users={'wash', 'kaylee'},
allow_all=True,
) )
authorized = await authenticator.get_authenticated_user( authorized = await authenticator.get_authenticated_user(
None, {'username': 'wash', 'password': 'wash'} None, {'username': 'wash', 'password': 'wash'}
@@ -204,7 +211,8 @@ async def test_pam_auth_blocked():
# User not in blocked set can log in # User not in blocked set can log in
authenticator = MockPAMAuthenticator( authenticator = MockPAMAuthenticator(
blocked_users={'wash'}, allowed_users={'wash', 'kaylee'} blocked_users={'wash'},
allowed_users={'wash', 'kaylee'},
) )
authorized = await authenticator.get_authenticated_user( authorized = await authenticator.get_authenticated_user(
None, {'username': 'kaylee', 'password': 'kaylee'} None, {'username': 'kaylee', 'password': 'kaylee'}
@@ -222,7 +230,8 @@ async def test_pam_auth_blocked():
# User in neither list # User in neither list
authenticator = MockPAMAuthenticator( authenticator = MockPAMAuthenticator(
blocked_users={'mal'}, allowed_users={'wash', 'kaylee'} blocked_users={'mal'},
allowed_users={'wash', 'kaylee'},
) )
authorized = await authenticator.get_authenticated_user( authorized = await authenticator.get_authenticated_user(
None, {'username': 'simon', 'password': 'simon'} None, {'username': 'simon', 'password': 'simon'}
@@ -258,7 +267,9 @@ async def test_deprecated_signatures():
async def test_pam_auth_no_such_group(): async def test_pam_auth_no_such_group():
authenticator = MockPAMAuthenticator(allowed_groups={'nosuchcrazygroup'}) authenticator = MockPAMAuthenticator(
allowed_groups={'nosuchcrazygroup'},
)
authorized = await authenticator.get_authenticated_user( authorized = await authenticator.get_authenticated_user(
None, {'username': 'kaylee', 'password': 'kaylee'} None, {'username': 'kaylee', 'password': 'kaylee'}
) )
@@ -406,7 +417,7 @@ async def test_auth_state_disabled(app, auth_state_unavailable):
async def test_normalize_names(): async def test_normalize_names():
a = MockPAMAuthenticator() a = MockPAMAuthenticator(allow_all=True)
authorized = await a.get_authenticated_user( authorized = await a.get_authenticated_user(
None, {'username': 'ZOE', 'password': 'ZOE'} None, {'username': 'ZOE', 'password': 'ZOE'}
) )
@@ -429,7 +440,7 @@ async def test_normalize_names():
async def test_username_map(): async def test_username_map():
a = MockPAMAuthenticator(username_map={'wash': 'alpha'}) a = MockPAMAuthenticator(username_map={'wash': 'alpha'}, allow_all=True)
authorized = await a.get_authenticated_user( authorized = await a.get_authenticated_user(
None, {'username': 'WASH', 'password': 'WASH'} None, {'username': 'WASH', 'password': 'WASH'}
) )
@@ -459,7 +470,7 @@ async def test_post_auth_hook():
authentication['testkey'] = 'testvalue' authentication['testkey'] = 'testvalue'
return authentication return authentication
a = MockPAMAuthenticator(post_auth_hook=test_auth_hook) a = MockPAMAuthenticator(allow_all=True, post_auth_hook=test_auth_hook)
authorized = await a.get_authenticated_user( authorized = await a.get_authenticated_user(
None, {'username': 'test_user', 'password': 'test_user'} None, {'username': 'test_user', 'password': 'test_user'}
@@ -567,6 +578,7 @@ async def test_auth_managed_groups(
parent=app, parent=app,
authenticated_groups=authenticated_groups, authenticated_groups=authenticated_groups,
refresh_groups=refresh_groups, refresh_groups=refresh_groups,
allow_all=True,
) )
user.groups.append(group) user.groups.append(group)
@@ -600,15 +612,18 @@ async def test_auth_managed_groups(
"allowed_users, allow_all, allow_existing_users", "allowed_users, allow_all, allow_existing_users",
[ [
('specified', False, True), ('specified', False, True),
('', True, False), ('', False, False),
], ],
) )
def test_allow_all_defaults(app, user, allowed_users, allow_all, allow_existing_users): async def test_allow_defaults(
app, user, allowed_users, allow_all, allow_existing_users
):
if allowed_users: if allowed_users:
allowed_users = set(allowed_users.split(',')) allowed_users = set(allowed_users.split(','))
else: else:
allowed_users = set() allowed_users = set()
authenticator = auth.Authenticator(allowed_users=allowed_users) authenticator = auth.Authenticator(allowed_users=allowed_users)
authenticator.authenticate = lambda handler, data: data["username"]
assert authenticator.allow_all == allow_all assert authenticator.allow_all == allow_all
assert authenticator.allow_existing_users == allow_existing_users assert authenticator.allow_existing_users == allow_existing_users
@@ -620,8 +635,21 @@ def test_allow_all_defaults(app, user, allowed_users, allow_all, allow_existing_
else: else:
authenticator.allowed_users == set() authenticator.allowed_users == set()
assert authenticator.check_allowed("specified") specified_allowed = await authenticator.get_authenticated_user(
assert authenticator.check_allowed(user.name) None, {"username": "specified"}
)
if "specified" in allowed_users:
assert specified_allowed is not None
else:
assert specified_allowed is None
existing_allowed = await authenticator.get_authenticated_user(
None, {"username": user.name}
)
if allow_existing_users:
assert existing_allowed is not None
else:
assert existing_allowed is None
@pytest.mark.parametrize("allow_all", [None, True, False]) @pytest.mark.parametrize("allow_all", [None, True, False])
@@ -693,6 +721,9 @@ class AllowAllIgnoringAuthenticator(auth.Authenticator):
allowed_letters = Tuple(config=True, help="Initial letters to allow") allowed_letters = Tuple(config=True, help="Initial letters to allow")
def authenticate(self, handler, data):
return {"name": data["username"]}
def check_allowed(self, username, auth=None): def check_allowed(self, username, auth=None):
if not self.allowed_users and not self.allowed_letters: if not self.allowed_users and not self.allowed_letters:
# this subclass doesn't know about the JupyterHub 5 allow_all config # this subclass doesn't know about the JupyterHub 5 allow_all config
@@ -707,7 +738,7 @@ class AllowAllIgnoringAuthenticator(auth.Authenticator):
# allow_all is not recognized by Authenticator subclass # allow_all is not recognized by Authenticator subclass
# make sure it doesn't make anything more permissive, at least # make sure it doesn't make anything more permissive, at least
@pytest.mark.parametrize("allow_all", [True, False, None]) @pytest.mark.parametrize("allow_all", [True, False])
@pytest.mark.parametrize( @pytest.mark.parametrize(
"allowed_users, allowed_letters, allow_existing_users, allowed, not_allowed", "allowed_users, allowed_letters, allow_existing_users, allowed, not_allowed",
[ [
@@ -720,7 +751,7 @@ class AllowAllIgnoringAuthenticator(auth.Authenticator):
("specified", "a,b", True, "specified,alice,bebe,existing", "other"), ("specified", "a,b", True, "specified,alice,bebe,existing", "other"),
], ],
) )
def test_authenticator_without_allow_all( async def test_authenticator_without_allow_all(
app, app,
allowed_users, allowed_users,
allowed_letters, allowed_letters,
@@ -753,10 +784,14 @@ def test_authenticator_without_allow_all(
expected_allowed = sorted(allowed) expected_allowed = sorted(allowed)
expected_not_allowed = sorted(not_allowed) expected_not_allowed = sorted(not_allowed)
to_check = list(chain(expected_allowed, expected_not_allowed)) to_check = list(chain(expected_allowed, expected_not_allowed))
if allow_all:
expected_allowed = to_check
expected_not_allowed = []
are_allowed = [] are_allowed = []
are_not_allowed = [] are_not_allowed = []
for username in to_check: for username in to_check:
if authenticator.check_allowed(username): if await authenticator.get_authenticated_user(None, {"username": username}):
are_allowed.append(username) are_allowed.append(username)
else: else:
are_not_allowed.append(username) are_not_allowed.append(username)