`allow_all` doc string is incomplete / wrong

[yuvipanda]

Bug description

The allow_all config currently is documented as:

Allow all authenticated users to login.

However, this is not true because in many cases other authorization steps do step in. But not all authorization steps.

Currently, based on the code in https://github.com/jupyterhub/oauthenticator/blob/5667bf15f8e1dead29c32cc0886146d6ddb835ec/oauthenticator/oauth2.py#L1015, setting allow_all to True ignores the following other config:

1. allowed_users is fully ignored
2. Setting admin in the auth_model is ignored

But anything in subclasses is not ignored:

1. GitHubOAuthenticator's allowed_organizations is not ignored
2. Generic's allowed_groups is not ignored
3. etc

I think this is pretty confusing.

Suggested change

1. We should augment the currently sparse allow_all doc with more information on what exactly it is doing.
2. If it's intentional that allowed_users is ignored if allowed_all is set, we should add a validation that marks them as mutually exclusive.

[yuvipanda]

I just tested this config:

        GitHubOAuthenticator:
          allow_all: true
          allowed_organizations:
            - test:test

And despite the fact I'm not a member of that org, I was allowed in. That's actually what I was expecting - all other variants are ignored.

So in this case, it actually doesn't matter that allowed_organizations is set. So I think it's the same that allowed_users may be set, and just as ignored.

I understand this better now, and it actually does work as I think it should.

[yuvipanda]

I think the problem was just me misunderstanding something https://github.com/jupyterhub/oauthenticator/pull/719#issuecomment-1911166511. Sorry for the noise everyone.

[manics]

I'm reopening this since you've brought up a valid point. Although the docstring for allow_all may be technically correct it's unclear how allow_all fits in to the whole authorisation chain. For example, JupyterHub has blocked_users so clearly there has to be a priority amongst these different conditions.

I think we should

• document the current behaviour, (perhaps as a diagram?)
• consider whether this is the behaviour we want. My intuition is that allow_all comes at the end of the allow/block chain, kind of like an Iptables default policy, but I don't know if that's actually the case

[consideRatio]

There are several config options allowing a user access, but they are independent from each other so that the ordering is irrelevant.

If a user is allowed by being listed in allowed_users, admin_users, being an existing user and allow_existing_users is true, or by being part of a allowed_organization, or by simply being any authenticated user via allow_all.

From https://oauthenticator.readthedocs.io/en/stable/reference/changelog.html and the 16.0.0 entry:

Its quite simple to describe how things are in oauthenticator 16 i think:

• some explicitly configured "allow config" must have made each authenticated user allowed, otherwise the authenticated user isn't authorized no matter what
• beyond this, the user must not be part of blocked_users either as its a separate requirement
• (if required_scopes would be added, it would be like blocked_users - a requirement to meet besides being listed as allowed via some allow config)

So briefly to be authorized, you have to:

• get authenticated
• meet requirements like not being a blocked_user
• be allowed via one of several configs describing what users are allowed

With this said, i don't think allow_all should be updated specifically as this issues title suggests - but yes, clarifying the requirement of being authorized further could be relevant somewhere if this config isn't intuitive enough.

[consideRatio]

I'll go for a close on this issue, I've opened https://github.com/jupyterhub/oauthenticator/issues/727 to track what I think is what you are asking for @manics.