Skip to content

Commit

Permalink
Don't make allow_all and required_scopes mutually exclusive
Browse files Browse the repository at this point in the history
  • Loading branch information
yuvipanda committed Jan 25, 2024
1 parent ae734ef commit 8b65b4f
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 19 deletions.
13 changes: 4 additions & 9 deletions oauthenticator/oauth2.py
Original file line number Diff line number Diff line change
Expand Up @@ -483,9 +483,6 @@ def _required_scopes_validation(self, proposal):
# required scopes must be a subset of requested scopes
if set(proposal.value) - set(self.scope):
raise ValueError(f"Required Scopes must be a subset of Requested Scopes. {self.scope} is requested but {proposal.value} is required")
if self.allow_all:
# Can't set allow_all *and* require scopes
raise ValueError("Required Scopes is mutually exclusive with allow_all. Unset one of those configuration properties")
return proposal.value

extra_authorize_params = Dict(
Expand Down Expand Up @@ -1046,11 +1043,8 @@ async def check_allowed(self, username, auth_model):
if auth_model is None:
return True

if self.allow_all:
return True

# If we specific scope grants are required, validate that they have been granted
# If not, we explicitly raise an exception here, since they should not be allowed
# If not, we explicitly raise an exception here, since the user should not be allowed
# *regardless* of any other config allowing them access.
if self.required_scopes:
granted_scopes = auth_model.get('auth_state', {}).get('scope', [])
Expand All @@ -1059,8 +1053,9 @@ async def check_allowed(self, username, auth_model):
message = f"Denying access to user {username}. {self.scope} scopes were requested, {self.required_scopes} are required for authorization, but only {granted_scopes} were granted"
self.log.info(message)
raise web.HTTPError(403, message)
else:
return True

if self.allow_all:
return True

# allow users with admin status set to True via admin_users config or
# update_auth_model override
Expand Down
10 changes: 0 additions & 10 deletions oauthenticator/tests/test_generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -233,16 +233,6 @@ async def test_required_scopes_validation_scope_subset(
with raises(ValueError, match=re.escape("Required Scopes must be a subset of Requested Scopes. ['a'] is requested but ['a', 'b'] is required")):
get_authenticator(config=c)

async def test_required_scopes_validation_no_allow_all(
get_authenticator
):
c = Config()
# Test that we can't have allow all and required_scopes together
c.GenericOAuthenticator.required_scopes = ["a"]
c.GenericOAuthenticator.scope = ["a", "b"]
c.GenericOAuthenticator.allow_all = True
with raises(ValueError, match=re.escape("Required Scopes is mutually exclusive with allow_all. Unset one of those configuration properties")):
get_authenticator(config=c)

async def test_generic_callable_username_key(get_authenticator, generic_client):
c = Config()
Expand Down

0 comments on commit 8b65b4f

Please sign in to comment.