Merge pull request #4701 from consideRatio/pr/add-allow-existing-users

Add Authenticator config `allow_all` and `allow_existing_users`

docs/source/reference/authenticators.md

@@ -37,14 +37,19 @@ A [generic implementation](https://github.com/jupyterhub/oauthenticator/blob/mas
 ## The Dummy Authenticator
 
 When testing, it may be helpful to use the
-{class}`jupyterhub.auth.DummyAuthenticator`. This allows for any username and
+{class}`~.jupyterhub.auth.DummyAuthenticator`. This allows for any username and
-password unless if 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.
 
+:::{versionadded} 5.0
+The DummyAuthenticator's default `allow_all` is True,
+unlike most other Authenticators.
+:::
+
 ## Additional Authenticators
 
-A partial list of other authenticators is available on the
+Additional authenticators can be found on GitHub
-[JupyterHub wiki](https://github.com/jupyterhub/jupyterhub/wiki/Authenticators).
+by searching for [topic:jupyterhub topic:authenticator](https://github.com/search?q=topic%3Ajupyterhub%20topic%3Aauthenticator&type=repositories).
 
 ## Technical Overview of Authentication
 
@@ -54,9 +59,9 @@ The base authenticator uses simple username and password authentication.
 
 The base Authenticator has one central method:
 
-#### Authenticator.authenticate method
+#### Authenticator.authenticate
 
-    Authenticator.authenticate(handler, data)
+{meth}`.Authenticator.authenticate`
 
 This method is passed the Tornado `RequestHandler` and the `POST data`
 from JupyterHub's login form. Unless the login form has been customized,
@@ -81,7 +86,8 @@ Writing an Authenticator that looks up passwords in a dictionary
 requires only overriding this one method:
 
 ```python
-from IPython.utils.traitlets import Dict
+from secrets import compare_digest
+from traitlets import Dict
 from jupyterhub.auth import Authenticator
 
 class DictionaryAuthenticator(Authenticator):
@@ -91,8 +97,14 @@ class DictionaryAuthenticator(Authenticator):
     )
 
     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
@@ -136,7 +148,7 @@ To only allow usernames that start with 'w':
 c.Authenticator.username_pattern = r'w.*'
 ```
 
-### How to write a custom authenticator
+## How to write a custom authenticator
 
 You can use custom Authenticator subclasses to enable authentication
 via other mechanisms. One such example is using [GitHub OAuth][].
@@ -148,11 +160,6 @@ and {meth}`.Authenticator.post_spawn_stop`, are hooks that can be used to do
 auth-related startup (e.g. opening PAM sessions) and cleanup
 (e.g. closing PAM sessions).
 
-See a list of custom Authenticators [on the wiki](https://github.com/jupyterhub/jupyterhub/wiki/Authenticators).
-
-If you are interested in writing a custom authenticator, you can read
-[this tutorial](http://jupyterhub-tutorial.readthedocs.io/en/latest/authenticators.html).
-
 ### Registering custom Authenticators via entry points
 
 As of JupyterHub 1.0, custom authenticators can register themselves via
@@ -188,6 +195,166 @@ Additionally, configurable attributes for your authenticator will
 appear in jupyterhub help output and auto-generated configuration files
 via `jupyterhub --generate-config`.
 
+(authenticator-allow)=
+
+### Allowing access
+
+When dealing with logging in, there are generally two _separate_ steps:
+
+authentication
+: identifying who is trying to log in, and
+
+authorization
+: deciding whether an authenticated user is allowed to access your JupyterHub
+
+{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,
+in which case `authenticate` may return `None` if the user is not authorized.
+
+However, Authenticators also have two methods, {meth}`~.Authenticator.check_allowed` and {meth}`~.Authenticator.check_blocked_users`, which are called after successful authentication to further check if the user is allowed.
+
+If `check_blocked_users()` returns False, authorization stops and the user is not allowed.
+
+If `Authenticator.allow_all` is True OR `check_allowed()` returns True, authorization proceeds.
+
+:::{versionadded} 5.0
+{attr}`.Authenticator.allow_all` and {attr}`.Authenticator.allow_existing_users` are new in JupyterHub 5.0.
+
+By default, `allow_all` is False,
+which is a change from pre-5.0, where `allow_all` was implicitly True if `allowed_users` was empty.
+:::
+
+### Overriding `check_allowed`
+
+:::{versionchanged} 5.0
+`check_allowed()` is **not called** if `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:
+
+- if username is in the `allowed_users` set, return True
+- else return False
+
+:::{versionchanged} 5.0
+Prior to 5.0, this would also return True if `allowed_users` was empty.
+
+For clarity, this is no longer the case. A new `allow_all` property (default False) has been added which is checked _before_ calling `check_allowed`.
+If `allow_all` is True, this takes priority over `check_allowed`, which will be ignored.
+
+If your Authenticator subclass similarly returns True when no allow config is defined,
+this is fully backward compatible for your users, but means `allow_all = False` has no real effect.
+
+You can make your Authenticator forward-compatible with JupyterHub 5 by defining `allow_all` as a boolean config trait on your class:
+
+```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
+
+Any of these authentication and authorization methods may raise a `web.HTTPError` Exception
+
+```python
+from tornado import web
+
+raise web.HTTPError(403, "informative message")
+```
+
+if you want to show a more informative login failure message rather than the generic one.
+
 (authenticator-auth-state)=
 
 ### Authentication state

docs/source/tutorial/getting-started/authenticators-users-basics.md

@@ -6,21 +6,58 @@ The default Authenticator uses [PAM][] (Pluggable Authentication Module) to auth
 their usernames and passwords. With the default Authenticator, any user
 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,
 `Authenticator.allowed_users`:
 
 ```python
 c.Authenticator.allowed_users = {'mal', 'zoe', 'inara', 'kaylee'}
+# c.Authenticator.allow_all = False
+c.Authenticator.allow_existing_users = False
 ```
 
-Users in the `allowed_users` set are added to the Hub database when the Hub is
+Users in the `allowed_users` set are added to the Hub database when the Hub is started.
-started.
 
-```{warning}
+:::{versionchanged} 5.0
-If this configuration value is not set, then **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
-```
+to enable explicit configuration of previously implicit behavior.
+
+Prior to 5.0, `allow_all` was implicitly True if `allowed_users` was empty.
+Starting with 5.0, to allow all authenticated users by default,
+`allow_all` must be explicitly set to True.
+
+By default, `allow_existing_users` is True when `allowed_users` is not empty,
+to ensure backward-compatibility.
+To make the `allowed_users` set _restrictive_,
+set `allow_existing_users = False`.
+:::
 
 ## One Time Passwords ( request_otp )
 
@@ -42,7 +79,7 @@ c.Authenticator.otp_prompt = 'Google Authenticator:'
 ```{note}
 As of JupyterHub 2.0, the full permissions of `admin_users`
 should not be required.
-Instead, you can assign [roles](define-role-target) to users or groups
+Instead, it is best to assign [roles](define-role-target) to users or groups
 with only the scopes they require.
 ```
 
@@ -68,26 +105,55 @@ group. For example, we can let any user in the `wheel` group be an admin:
 c.PAMAuthenticator.admin_groups = {'wheel'}
 ```
 
-## Give admin access to other users' notebook servers (`admin_access`)
+## Give some users access to other users' notebook servers
 
-Since the default `JupyterHub.admin_access` setting is `False`, the admins
+The `access:servers` scope can be granted to users to give them permission to visit other users' servers.
-do not have permission to log in to the single user notebook servers
+For example, to give members of the `teachers` group access to the servers of members of the `students` group:
-owned by _other users_. If `JupyterHub.admin_access` is set to `True`,
+
-then admins have permission to log in _as other users_ on their
+```python
-respective machines for debugging. **As a courtesy, you should make
+c.JupyterHub.load_roles = [
-sure your users know if admin_access is enabled.**
+  {
+    "name": "teachers",
+    "scopes": [
+      "admin-ui",
+      "list:users",
+      "access:servers!group=students",
+    ],
+    "groups": ["teachers"],
+  }
+]
+```
+
+By default, only the deprecated `admin` role has global `access` permissions.
+**As a courtesy, you should make sure your users know if admin access is enabled.**
 
 ## Add or remove users from the Hub
 
+:::{versionadded} 5.0
+`c.Authenticator.allow_existing_users` is added in 5.0 and True by default _if_ any `allowed_users` are specified.
+
+Prior to 5.0, this behavior was not optional.
+:::
+
 Users can be added to and removed from the Hub via the admin
-panel or the REST API. When a user is **added**, the user will be
+panel or the REST API.
-automatically added to the `allowed_users` set and database. Restarting the Hub
+
-will not require manually updating the `allowed_users` set in your config file,
+To enable this behavior, set:
+
+```python
+c.Authenticator.allow_existing_users = True
+```
+
+When a user is **added**, the user will be
+automatically added to the `allowed_users` set and database.
+If `allow_existing_users` is True, restarting the Hub will not require manually updating the `allowed_users` set in your config file,
 as the users will be loaded from the database.
+If `allow_existing_users` is False, users not granted access by configuration such as `allowed_users` will not be permitted to login,
+even if they are present in the database.
 
 After starting the Hub once, it is not sufficient to **remove** a user
 from the allowed users set in your config file. You must also remove the user
-from the Hub's database, either by deleting the user from JupyterHub's
+from the Hub's database, either by deleting the user via JupyterHub's
 admin page, or you can clear the `jupyterhub.sqlite` database and start
 fresh.
 

jupyterhub/app.py

@@ -2098,6 +2098,9 @@ class JupyterHub(Application):
                     "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:
             self.log.warning(
                 "\nJupyterHub.admin_users is deprecated since version 0.7.2."
@@ -2125,9 +2128,9 @@ class JupyterHub(Application):
                 new_users.append(user)
             else:
                 user.admin = True
+
         # the admin_users config variable will never be used after this point.
         # only the database values will be referenced.
-
         allowed_users = [
             self.authenticator.normalize_username(name)
             for name in self.authenticator.allowed_users
@@ -2137,10 +2140,10 @@ class JupyterHub(Application):
             if not self.authenticator.validate_username(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
         for name in allowed_users:

jupyterhub/auth.py

@@ -121,6 +121,55 @@ class Authenticator(LoggingConfigurable):
         """
     ).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.\n"
+                "You may suppress this warning by setting c.Authenticator.any_allow_config = True."
+            )
+
     whitelist = Set(
         help="Deprecated, use `Authenticator.allowed_users`",
         config=True,
@@ -132,7 +181,7 @@ class Authenticator(LoggingConfigurable):
 
         Use this to limit which authenticated users may login.
         Default behavior: only users in this set are allowed.
-        
+
         If empty, does not perform any restriction,
         in which case any authenticated user is allowed.
 
@@ -144,6 +193,83 @@ class Authenticator(LoggingConfigurable):
         """
     ).tag(config=True)
 
+    allow_all = Bool(
+        False,
+        config=True,
+        help="""
+        Allow every user who can successfully authenticate to access JupyterHub.
+        
+        False by default, which means for most Authenticators,
+        _some_ allow-related configuration is required to allow users to log in.
+
+        Authenticator subclasses may override the default with e.g.::
+
+            @default("allow_all")
+            def _default_allow_all(self):
+                # if _any_ auth config (depends on the Authenticator)
+                if self.allowed_users or self.allowed_groups or self.allow_existing_users:
+                    return False
+                else:
+                    return True
+
+        .. 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)
+
+    allow_existing_users = Bool(
+        # dynamic default computed from allowed_users
+        config=True,
+        help="""
+        Allow existing users to login.
+
+        Defaults to True if `allowed_users` is set for historical reasons, and
+        False otherwise.
+
+        With this enabled, all users present in the JupyterHub database are allowed to login.
+        This has the effect of any user who has _previously_ been allowed to login
+        via any means will continue to be allowed until the user is deleted via the /hub/admin page
+        or REST API.
+
+        .. warning::
+
+           Before enabling this you should review the existing users in the
+           JupyterHub admin panel at `/hub/admin`. You may find users existing
+           there because they have previously been declared in config such as
+           `allowed_users` or allowed to sign in.
+
+        .. warning::
+
+           When this is enabled and you wish to remove access for one or more
+           users previously allowed, you must make sure that they
+           are removed from the jupyterhub database. This can be tricky to do
+           if you stop allowing an externally managed group of users for example.
+
+        With this enabled, JupyterHub admin users can visit `/hub/admin` or use
+        JupyterHub's REST API to add and remove users to manage who can login.
+
+        .. versionadded:: 5.0
+        """,
+    ).tag(allow_config=True)
+
+    @default("allow_existing_users")
+    def _allow_existing_users_default(self):
+        """
+        Computes the default value of allow_existing_users based on if
+        allowed_users to align with original behavior not introduce a breaking
+        change.
+        """
+        if self.allowed_users:
+            return True
+        return False
+
     blocked_users = Set(
         help="""
         Set of usernames that are not allowed to log in.
@@ -472,8 +598,7 @@ class Authenticator(LoggingConfigurable):
             web.HTTPError(403):
                 Raising HTTPErrors directly allows customizing the message shown to the user.
         """
-        if not self.allowed_users:
+        if self.allow_all:
-            # No allowed set means any name is allowed
             return True
         return username in self.allowed_users
 
@@ -525,8 +650,9 @@ class Authenticator(LoggingConfigurable):
         The various stages can be overridden separately:
          - `authenticate` turns formdata into a username
          - `normalize_username` normalizes the username
-         - `check_allowed` checks against the allowed 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
 
         .. versionchanged:: 0.8
@@ -560,7 +686,11 @@ class Authenticator(LoggingConfigurable):
             self.log.warning("User %r blocked. Stop authentication", username)
             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 authenticated['admin'] is None:
@@ -677,25 +807,31 @@ class Authenticator(LoggingConfigurable):
         """Hook called when a user is added to JupyterHub
 
         This is called:
-         - When a user first authenticates
+         - When a user first authenticates, _after_ all allow and block checks have passed
-         - When the hub restarts, for all users.
+         - When the hub restarts, for all users in the database (i.e. users previously allowed)
+         - When a user is added to the database, either via configuration or REST API
 
         This method may be a coroutine.
 
-        By default, this just adds the user to the allowed_users set.
+        By default, this adds the user to the allowed_users set if
+        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.
 
         Note that this should be idempotent, since it is called whenever the hub restarts
         for all users.
 
+        .. versionchanged:: 5.0
+           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.
+
         Args:
             user (User): The User wrapper object
         """
         if not self.validate_username(user.name):
             raise ValueError("Invalid username: %s" % user.name)
-        if self.allowed_users:
+        if self.allow_existing_users and not self.allow_all:
             self.allowed_users.add(user.name)
 
     def delete_user(self, user):
@@ -902,23 +1038,16 @@ class LocalAuthenticator(Authenticator):
         help="""
         Allow login from all users in these UNIX groups.
 
-        If set, allowed username set is ignored.
+        .. versionchanged:: 5.0
+            `allowed_groups` may be specified together with allowed_users,
+            to grant access by group OR name.
         """
-    ).tag(config=True)
+    ).tag(config=True, allow_config=True)
-
-    @observe('allowed_groups')
-    def _allowed_groups_changed(self, change):
-        """Log a warning if mutually exclusive user and group allowed sets are specified."""
-        if self.allowed_users:
-            self.log.warning(
-                "Ignoring Authenticator.allowed_users set because Authenticator.allowed_groups supplied!"
-            )
 
     def check_allowed(self, username, authentication=None):
-        if self.allowed_groups:
+        if self.check_allowed_groups(username, authentication):
-            return self.check_allowed_groups(username, authentication)
+            return True
-        else:
+        return super().check_allowed(username, authentication)
-            return super().check_allowed(username, authentication)
 
     def check_allowed_groups(self, username, authentication=None):
         """
@@ -1248,8 +1377,20 @@ class DummyAuthenticator(Authenticator):
     if it logs in with that password.
 
     .. 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(
         config=True,
         help="""
@@ -1259,6 +1400,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):
         """Checks against a global password if it's been set. If not, allow any user/pass combo"""
         if self.password:

jupyterhub/tests/mocking.py

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

jupyterhub/tests/test_app.py

@@ -475,6 +475,7 @@ async def test_user_creation(tmpdir, request):
     ]
 
     cfg = Config()
+    cfg.Authenticator.allow_all = False
     cfg.Authenticator.allowed_users = allowed_users
     cfg.JupyterHub.load_groups = groups
     cfg.JupyterHub.load_roles = roles

jupyterhub/tests/test_auth.py

@@ -3,12 +3,13 @@
 # Copyright (c) Jupyter Development Team.
 # Distributed under the terms of the Modified BSD License.
 import logging
+from itertools import chain
 from unittest import mock
 from urllib.parse import urlparse
 
 import pytest
 from requests import HTTPError
-from traitlets import Any
+from traitlets import Any, Tuple
 from traitlets.config import Config
 
 from jupyterhub import auth, crypto, orm
@@ -18,7 +19,7 @@ from .utils import add_user, async_requests, get_page, public_url
 
 
 async def test_pam_auth():
-    authenticator = MockPAMAuthenticator()
+    authenticator = MockPAMAuthenticator(allow_all=True)
     authorized = await authenticator.get_authenticated_user(
         None, {'username': 'match', 'password': 'match'}
     )
@@ -37,7 +38,7 @@ async def test_pam_auth():
 
 
 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(
         None, {'username': 'allowedmatch', 'password': 'allowedmatch'}
     )
@@ -82,7 +83,9 @@ async def test_pam_auth_admin_groups():
         return user_group_map[name]
 
     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
@@ -141,7 +144,10 @@ async def test_pam_auth_admin_groups():
 
 
 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(
         None, {'username': 'kaylee', 'password': 'kaylee'}
     )
@@ -162,7 +168,7 @@ async def test_pam_auth_allowed_groups():
     def getgrnam(name):
         return MockStructGroup('grp', ['kaylee'])
 
-    authenticator = MockPAMAuthenticator(allowed_groups={'group'})
+    authenticator = MockPAMAuthenticator(allowed_groups={'group'}, allow_all=False)
 
     with mock.patch.object(authenticator, '_getgrnam', getgrnam):
         authorized = await authenticator.get_authenticated_user(
@@ -179,14 +185,14 @@ async def test_pam_auth_allowed_groups():
 
 async def test_pam_auth_blocked():
     # Null case compared to next case
-    authenticator = MockPAMAuthenticator()
+    authenticator = MockPAMAuthenticator(allow_all=True)
     authorized = await authenticator.get_authenticated_user(
         None, {'username': 'wash', 'password': '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(
         None, {'username': 'wash', 'password': 'wash'}
     )
@@ -194,7 +200,9 @@ async def test_pam_auth_blocked():
 
     # User in both allowed and blocked: default deny.  Make error someday?
     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(
         None, {'username': 'wash', 'password': 'wash'}
@@ -203,7 +211,8 @@ async def test_pam_auth_blocked():
 
     # User not in blocked set can log in
     authenticator = MockPAMAuthenticator(
-        blocked_users={'wash'}, allowed_users={'wash', 'kaylee'}
+        blocked_users={'wash'},
+        allowed_users={'wash', 'kaylee'},
     )
     authorized = await authenticator.get_authenticated_user(
         None, {'username': 'kaylee', 'password': 'kaylee'}
@@ -221,7 +230,8 @@ async def test_pam_auth_blocked():
 
     # User in neither list
     authenticator = MockPAMAuthenticator(
-        blocked_users={'mal'}, allowed_users={'wash', 'kaylee'}
+        blocked_users={'mal'},
+        allowed_users={'wash', 'kaylee'},
     )
     authorized = await authenticator.get_authenticated_user(
         None, {'username': 'simon', 'password': 'simon'}
@@ -257,7 +267,9 @@ async def test_deprecated_signatures():
 
 
 async def test_pam_auth_no_such_group():
-    authenticator = MockPAMAuthenticator(allowed_groups={'nosuchcrazygroup'})
+    authenticator = MockPAMAuthenticator(
+        allowed_groups={'nosuchcrazygroup'},
+    )
     authorized = await authenticator.get_authenticated_user(
         None, {'username': 'kaylee', 'password': 'kaylee'}
     )
@@ -405,7 +417,7 @@ async def test_auth_state_disabled(app, auth_state_unavailable):
 
 
 async def test_normalize_names():
-    a = MockPAMAuthenticator()
+    a = MockPAMAuthenticator(allow_all=True)
     authorized = await a.get_authenticated_user(
         None, {'username': 'ZOE', 'password': 'ZOE'}
     )
@@ -428,7 +440,7 @@ async def test_normalize_names():
 
 
 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(
         None, {'username': 'WASH', 'password': 'WASH'}
     )
@@ -458,7 +470,7 @@ async def test_post_auth_hook():
         authentication['testkey'] = 'testvalue'
         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(
         None, {'username': 'test_user', 'password': 'test_user'}
@@ -566,6 +578,7 @@ async def test_auth_managed_groups(
         parent=app,
         authenticated_groups=authenticated_groups,
         refresh_groups=refresh_groups,
+        allow_all=True,
     )
 
     user.groups.append(group)
@@ -593,3 +606,193 @@ async def test_auth_managed_groups(
         assert not app.db.dirty
         groups = sorted(g.name for g in user.groups)
         assert groups == expected_refresh_groups
+
+
+@pytest.mark.parametrize(
+    "allowed_users,  allow_existing_users",
+    [
+        ('specified', True),
+        ('', False),
+    ],
+)
+async def test_allow_defaults(app, user, allowed_users, allow_existing_users):
+    if allowed_users:
+        allowed_users = set(allowed_users.split(','))
+    else:
+        allowed_users = set()
+    authenticator = auth.Authenticator(allowed_users=allowed_users)
+    authenticator.authenticate = lambda handler, data: data["username"]
+    assert authenticator.allow_all is False
+    assert authenticator.allow_existing_users == allow_existing_users
+
+    # user was already in the database
+    # this happens during hub startup
+    authenticator.add_user(user)
+    if allowed_users:
+        assert user.name in authenticator.allowed_users
+    else:
+        authenticator.allowed_users == set()
+
+    specified_allowed = await authenticator.get_authenticated_user(
+        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_existing_users", [None, True, False])
+@pytest.mark.parametrize("allowed_users", ["existing", ""])
+def test_allow_existing_users(
+    app, user, allowed_users, allow_all, allow_existing_users
+):
+    if allowed_users:
+        allowed_users = set(allowed_users.split(','))
+    else:
+        allowed_users = set()
+    authenticator = auth.Authenticator(
+        allowed_users=allowed_users,
+    )
+    if allow_all is None:
+        # default allow_all
+        allow_all = authenticator.allow_all
+    else:
+        authenticator.allow_all = allow_all
+    if allow_existing_users is None:
+        # default allow_all
+        allow_existing_users = authenticator.allow_existing_users
+    else:
+        authenticator.allow_existing_users = allow_existing_users
+
+    # first, nobody in the database
+    assert authenticator.check_allowed("newuser") == allow_all
+
+    # user was already in the database
+    # this happens during hub startup
+    authenticator.add_user(user)
+    if allow_existing_users or allow_all:
+        assert authenticator.check_allowed(user.name)
+    else:
+        assert not authenticator.check_allowed(user.name)
+    for username in allowed_users:
+        assert authenticator.check_allowed(username)
+
+    assert authenticator.check_allowed("newuser") == allow_all
+
+
+@pytest.mark.parametrize("allow_all", [True, False])
+@pytest.mark.parametrize("allow_existing_users", [True, False])
+def test_allow_existing_users_first_time(user, allow_all, allow_existing_users):
+    # make sure that calling add_user doesn't change results
+    authenticator = auth.Authenticator(
+        allow_all=allow_all,
+        allow_existing_users=allow_existing_users,
+    )
+    allowed_before_one = authenticator.check_allowed(user.name)
+    allowed_before_two = authenticator.check_allowed("newuser")
+    # add_user is called after successful login
+    # it shouldn't change results (e.g. by switching .allowed_users from empty to non-empty)
+    if allowed_before_one:
+        authenticator.add_user(user)
+    assert authenticator.check_allowed(user.name) == allowed_before_one
+    assert authenticator.check_allowed("newuser") == allowed_before_two
+
+
+class AllowAllIgnoringAuthenticator(auth.Authenticator):
+    """Test authenticator with custom check_allowed
+
+    not updated for allow_all, allow_existing_users
+
+    Make sure new config doesn't break backward-compatibility
+    or grant unintended access for Authenticators written before JupyterHub 5.
+    """
+
+    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):
+        if not self.allowed_users and not self.allowed_letters:
+            # this subclass doesn't know about the JupyterHub 5 allow_all config
+            # no allow config, allow all!
+            return True
+        if self.allowed_users and username in self.allowed_users:
+            return True
+        if self.allowed_letters and username.startswith(self.allowed_letters):
+            return True
+        return False
+
+
+# allow_all is not recognized by Authenticator subclass
+# make sure it doesn't make anything more permissive, at least
+@pytest.mark.parametrize("allow_all", [True, False])
+@pytest.mark.parametrize(
+    "allowed_users, allowed_letters, allow_existing_users, allowed, not_allowed",
+    [
+        ("", "", None, "anyone,should-be,allowed,existing", ""),
+        ("", "a,b", None, "alice,bebe", "existing,other"),
+        ("", "a,b", False, "alice,bebe", "existing,other"),
+        ("", "a,b", True, "alice,bebe,existing", "other"),
+        ("specified", "a,b", None, "specified,alice,bebe,existing", "other"),
+        ("specified", "a,b", False, "specified,alice,bebe", "existing,other"),
+        ("specified", "a,b", True, "specified,alice,bebe,existing", "other"),
+    ],
+)
+async def test_authenticator_without_allow_all(
+    app,
+    allowed_users,
+    allowed_letters,
+    allow_existing_users,
+    allowed,
+    not_allowed,
+    allow_all,
+):
+    kwargs = {}
+    if allow_all is not None:
+        kwargs["allow_all"] = allow_all
+    if allow_existing_users is not None:
+        kwargs["allow_existing_users"] = allow_existing_users
+    if allowed_users:
+        kwargs["allowed_users"] = set(allowed_users.split(','))
+    if allowed_letters:
+        kwargs["allowed_letters"] = tuple(allowed_letters.split(','))
+
+    authenticator = AllowAllIgnoringAuthenticator(**kwargs)
+
+    # load one user from db
+    existing_user = add_user(app.db, app, name="existing")
+    authenticator.add_user(existing_user)
+
+    if allowed:
+        allowed = allowed.split(",")
+    if not_allowed:
+        not_allowed = not_allowed.split(",")
+
+    expected_allowed = sorted(allowed)
+    expected_not_allowed = sorted(not_allowed)
+    to_check = list(chain(expected_allowed, expected_not_allowed))
+    if allow_all:
+        expected_allowed = to_check
+        expected_not_allowed = []
+
+    are_allowed = []
+    are_not_allowed = []
+    for username in to_check:
+        if await authenticator.get_authenticated_user(None, {"username": username}):
+            are_allowed.append(username)
+        else:
+            are_not_allowed.append(username)
+
+    assert are_allowed == expected_allowed
+    assert are_not_allowed == expected_not_allowed