From 724c86dcc4c005955c4eefa82ff4b7d52fc73f04 Mon Sep 17 00:00:00 2001 From: Nathan Papapietro Date: Tue, 31 Jan 2023 23:30:39 -0600 Subject: [PATCH 1/2] [All] Adding overrides to user_is_authorized method --- .pre-commit-config.yaml | 2 +- oauthenticator/bitbucket.py | 19 +++++++++++---- oauthenticator/cilogon.py | 2 +- oauthenticator/generic.py | 32 ++++++++++++++++++-------- oauthenticator/github.py | 14 ++++++++--- oauthenticator/gitlab.py | 29 +++++++++++++++-------- oauthenticator/globus.py | 15 +++++++++--- oauthenticator/google.py | 17 ++++++++++++-- oauthenticator/oauth2.py | 5 ++-- oauthenticator/openshift.py | 11 +++++---- oauthenticator/tests/conftest.py | 13 +++++++++++ oauthenticator/tests/test_bitbucket.py | 29 +++++++++++++++++++++++ oauthenticator/tests/test_generic.py | 25 ++++++++++++++++++++ oauthenticator/tests/test_github.py | 11 ++++++++- oauthenticator/tests/test_gitlab.py | 19 +++++++++++++-- oauthenticator/tests/test_globus.py | 11 +++++++++ oauthenticator/tests/test_google.py | 14 +++++++++++ 17 files changed, 226 insertions(+), 42 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index bb1d5317..1ea6e76c 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -28,7 +28,7 @@ repos: # Autoformat: python imports - repo: https://github.com/pycqa/isort - rev: 5.11.4 + rev: 5.11.5 hooks: - id: isort diff --git a/oauthenticator/bitbucket.py b/oauthenticator/bitbucket.py index 27cd84e4..2958aebc 100644 --- a/oauthenticator/bitbucket.py +++ b/oauthenticator/bitbucket.py @@ -45,16 +45,24 @@ def _userdata_url_default(self): config=True, help="Automatically allow members of selected teams" ) - async def user_is_authorized(self, auth_model): + async def user_is_authorized(self, auth_model, **overrides): + """Checks if user is authorized with bitbucket OAuth. + + Overrides: + - allowed_teams: Can override default self.allowed_teams + + Returns: True if authorized + """ access_token = auth_model["auth_state"]["token_response"]["access_token"] token_type = auth_model["auth_state"]["token_response"]["token_type"] username = auth_model["name"] # Check if user is a member of any allowed teams. # This check is performed here, as the check requires `access_token`. - if self.allowed_teams: + allowed_teams = overrides.pop("allowed_teams", self.allowed_teams) + if allowed_teams: user_in_team = await self._check_membership_allowed_teams( - username, access_token, token_type + username, access_token, token_type, set(allowed_teams) ) if not user_in_team: self.log.warning(f"{username} not in team allowed list of users") @@ -62,7 +70,7 @@ async def user_is_authorized(self, auth_model): return True - async def _check_membership_allowed_teams(self, username, access_token, token_type): + async def _check_membership_allowed_teams(self, username, access_token, token_type, allowed_teams): headers = self.build_userdata_request_headers(access_token, token_type) # We verify the team membership by calling teams endpoint. next_page = url_concat( @@ -75,9 +83,10 @@ async def _check_membership_allowed_teams(self, username, access_token, token_ty user_teams = {entry["name"] for entry in resp_json["values"]} # check if any of the organizations seen thus far are in the allowed list - if len(self.allowed_teams & user_teams) > 0: + if len(allowed_teams & user_teams) > 0: return True return False + class LocalBitbucketOAuthenticator(LocalAuthenticator, BitbucketOAuthenticator): diff --git a/oauthenticator/cilogon.py b/oauthenticator/cilogon.py index 9230da25..7eb8de54 100644 --- a/oauthenticator/cilogon.py +++ b/oauthenticator/cilogon.py @@ -281,7 +281,7 @@ def user_info_to_username(self, user_info): ) raise web.HTTPError(500, "Failed to get username from CILogon") - async def user_is_authorized(self, auth_model): + async def user_is_authorized(self, auth_model, **overrides): username = auth_model["name"] # Check if selected idp was marked as allowed if self.allowed_idps: diff --git a/oauthenticator/generic.py b/oauthenticator/generic.py index b6d24dff..2a89b5a5 100644 --- a/oauthenticator/generic.py +++ b/oauthenticator/generic.py @@ -96,33 +96,44 @@ def user_info_to_username(self, user_info): return username - async def user_is_authorized(self, auth_model): + async def user_is_authorized(self, auth_model, **overrides): + """Checks if user is authorized with generic OAuth. + + Overrides: + - allowed_groups: Can override default self.allowed_groups + - claim_groups_key: Can override default self.claim_groups_key + + Returns: True if authorized + """ + + claim_groups_key = overrides.pop("claim_groups_key", self.claim_groups_key) + allowed_groups = overrides.pop("allowed_groups", self.allowed_groups) user_info = auth_model["auth_state"][self.user_auth_state_key] - if self.allowed_groups: + if allowed_groups: self.log.info( - f"Validating if user claim groups match any of {self.allowed_groups}" + f"Validating if user claim groups match any of {allowed_groups}" ) - if callable(self.claim_groups_key): - groups = self.claim_groups_key(user_info) + if callable(claim_groups_key): + groups = claim_groups_key(user_info) else: try: groups = reduce( - dict.get, self.claim_groups_key.split("."), user_info + dict.get, claim_groups_key.split("."), user_info ) except TypeError: # This happens if a nested key does not exist (reduce trying to call None.get) self.log.error( - f"The key {self.claim_groups_key} does not exist in the user token, or it is set to null" + f"The key {claim_groups_key} does not exist in the user token, or it is set to null" ) groups = None if not groups: self.log.error( - f"No claim groups found for user! Something wrong with the `claim_groups_key` {self.claim_groups_key}? {user_info}" + f"No claim groups found for user! Something wrong with the `claim_groups_key` {claim_groups_key}? {user_info}" ) return False - if not self.check_user_in_groups(groups, self.allowed_groups): + if not self.check_user_in_groups(groups, allowed_groups): return False return True @@ -142,6 +153,9 @@ async def update_auth_model(self, auth_model): ) return auth_model + async def check_user_in_allowed_groups(self, user_model, allowed_groups=None): + return await super().check_user_in_allowed_groups(user_model, allowed_groups) + class LocalGenericOAuthenticator(LocalAuthenticator, GenericOAuthenticator): """A version that mixes in local system user creation""" diff --git a/oauthenticator/github.py b/oauthenticator/github.py index acd53023..c12ace15 100644 --- a/oauthenticator/github.py +++ b/oauthenticator/github.py @@ -133,13 +133,21 @@ def _github_client_secret_changed(self, name, old, new): config=True, ) - async def user_is_authorized(self, auth_model): + async def user_is_authorized(self, auth_model, **overrides): + """Checks if user is authorized with github OAuth. + + Overrides: + - allowed_organizations: Can override default self.allowed_organizations + + Returns: True if authorized + """ # Check if user is a member of any allowed organizations. # This check is performed here, as it requires `access_token`. access_token = auth_model["auth_state"]["token_response"]["access_token"] token_type = auth_model["auth_state"]["token_response"]["token_type"] - if self.allowed_organizations: - for org in self.allowed_organizations: + allowed_organizations = overrides.pop("allowed_organizations", self.allowed_organizations) + if allowed_organizations: + for org in allowed_organizations: user_in_org = await self._check_membership_allowed_organizations( org, auth_model["name"], access_token, token_type ) diff --git a/oauthenticator/gitlab.py b/oauthenticator/gitlab.py index ac367425..238ff43a 100644 --- a/oauthenticator/gitlab.py +++ b/oauthenticator/gitlab.py @@ -117,7 +117,16 @@ def _userdata_url_default(self): gitlab_version = None - async def user_is_authorized(self, auth_model): + async def user_is_authorized(self, auth_model, **overrides): + """Checks if user is authorized with gitlab OAuth. + + Overrides: + - allowed_project_ids: Can override default self.allowed_project_ids + - allowed_gitlab_groups: Can override default self.allowed_gitlab_groups + + Returns: True if authorized + """ + access_token = auth_model["auth_state"]["token_response"]["access_token"] user_id = auth_model["auth_state"][self.user_auth_state_key]["id"] @@ -131,17 +140,19 @@ async def user_is_authorized(self, auth_model): user_in_group = user_in_project = False is_group_specified = is_project_id_specified = False - if self.allowed_gitlab_groups: + allowed_gitlab_groups = overrides.pop("allowed_gitlab_groups", self.allowed_gitlab_groups) + if allowed_gitlab_groups: is_group_specified = True user_in_group = await self._check_membership_allowed_groups( - user_id, access_token + user_id, access_token, allowed_gitlab_groups ) # We skip project_id check if user is in allowed group. - if self.allowed_project_ids and not user_in_group: + allowed_project_ids = overrides.pop("allowed_project_ids", self.allowed_project_ids) + if allowed_project_ids and not user_in_group: is_project_id_specified = True user_in_project = await self._check_membership_allowed_project_ids( - user_id, access_token + user_id, access_token, allowed_project_ids ) no_config_specified = not (is_group_specified or is_project_id_specified) @@ -171,10 +182,10 @@ async def _get_gitlab_version(self, access_token): version_ints = list(map(int, version_strings)) return version_ints - async def _check_membership_allowed_groups(self, user_id, access_token): + async def _check_membership_allowed_groups(self, user_id, access_token, allowed_gitlab_groups): headers = _api_headers(access_token) # Check if user is a member of any group in the allowed list - for group in map(url_escape, self.allowed_gitlab_groups): + for group in map(url_escape, allowed_gitlab_groups): url = "%s/groups/%s/members/%s%d" % ( self.gitlab_api, quote(group, safe=''), @@ -192,10 +203,10 @@ async def _check_membership_allowed_groups(self, user_id, access_token): return True # user _is_ in group return False - async def _check_membership_allowed_project_ids(self, user_id, access_token): + async def _check_membership_allowed_project_ids(self, user_id, access_token, allowed_project_ids): headers = _api_headers(access_token) # Check if user has developer access to any project in the allowed list - for project in self.allowed_project_ids: + for project in allowed_project_ids: url = "%s/projects/%s/members/%s%d" % ( self.gitlab_api, project, diff --git a/oauthenticator/globus.py b/oauthenticator/globus.py index 0d55d316..f016a4d7 100644 --- a/oauthenticator/globus.py +++ b/oauthenticator/globus.py @@ -254,14 +254,23 @@ async def get_users_groups_ids(self, tokens): return user_group_ids - async def user_is_authorized(self, auth_model): + async def user_is_authorized(self, auth_model, **overrides): + """Checks if user is authorized with globus OAuth. + + Overrides: + - allowed_globus_groups: Can override default self.allowed_globus_groups + + Returns: True if authorized + """ + tokens = self.get_globus_tokens(auth_model["auth_state"]["token_response"]) - if self.allowed_globus_groups or self.admin_globus_groups: + allowed_globus_groups = overrides.pop("allowed_globus_groups", self.allowed_globus_groups) + if allowed_globus_groups or self.admin_globus_groups: # If any of these configurations are set, user must be in the allowed or admin Globus Group user_group_ids = await self.get_users_groups_ids(tokens) if not self.check_user_in_groups( - user_group_ids, self.allowed_globus_groups + user_group_ids, set(allowed_globus_groups) ): if not self.check_user_in_groups( user_group_ids, self.admin_globus_groups diff --git a/oauthenticator/google.py b/oauthenticator/google.py index 75df66d9..64abfbb2 100644 --- a/oauthenticator/google.py +++ b/oauthenticator/google.py @@ -126,14 +126,21 @@ def _cast_hosted_domain(self, proposal): def _username_claim_default(self): return 'email' - async def user_is_authorized(self, auth_model): + async def user_is_authorized(self, auth_model, **overrides): + """Checks if user is authorized with google OAuth. + + Overrides: + - allowed_google_groups: Can override default self.allowed_google_groups + + Returns: True if authorized + """ user_email = auth_model["auth_state"][self.user_auth_state_key]['email'] user_email_domain = user_email.split('@')[1] if not auth_model["auth_state"][self.user_auth_state_key]['verified_email']: self.log.warning(f"Google OAuth unverified email attempt: {user_email}") raise HTTPError(403, f"Google email {user_email} not verified") - + if self.hosted_domain: if user_email_domain not in self.hosted_domain: self.log.warning( @@ -142,6 +149,12 @@ async def user_is_authorized(self, auth_model): raise HTTPError( 403, f"Google account domain @{user_email_domain} not authorized." ) + + allowed_google_groups = overrides.pop("allowed_google_groups", None) + if allowed_google_groups: + # Only check if the override is passed so original authentication logic is not altered + return (await self._add_google_groups_info(auth_model, google_groups=allowed_google_groups)) is not None + return True async def update_auth_model(self, auth_model, google_groups=None): diff --git a/oauthenticator/oauth2.py b/oauthenticator/oauth2.py index c269d9fc..85b67c9d 100644 --- a/oauthenticator/oauth2.py +++ b/oauthenticator/oauth2.py @@ -703,10 +703,11 @@ async def update_auth_model(self, auth_model, **kwargs): """ return auth_model - async def user_is_authorized(self, auth_model): + async def user_is_authorized(self, auth_model, **overrides): """ Checks if the user that is authenticating should be authorized or not and False otherwise. - Should be overridden with any relevant logic specific to each oauthenticator. + Should be overridden with any relevant logic specific to each oauthenticator. Overrides can be + passed to check user authorization against different conditions. Returns True by default. diff --git a/oauthenticator/openshift.py b/oauthenticator/openshift.py index 8ea6e796..28c858ba 100644 --- a/oauthenticator/openshift.py +++ b/oauthenticator/openshift.py @@ -108,24 +108,27 @@ async def update_auth_model(self, auth_model): return auth_model - async def user_is_authorized(self, auth_model): + async def user_is_authorized(self, auth_model, **overrides): """ Use the group info stored on the OpenShift User object to determine if a user is authorized to login. + + Overrides: + - allowed_groups: Can override default self.allowed_groups """ user_groups = set(auth_model['auth_state']['openshift_user']['groups']) username = auth_model['name'] + allowed_groups = overrides.pop("allowed_groups", self.allowed_groups) - if self.allowed_groups or self.admin_groups: + if allowed_groups or self.admin_groups: msg = f"username:{username} User not in any of the allowed/admin groups" - if not self.user_in_groups(user_groups, self.allowed_groups): + if not self.user_in_groups(user_groups, allowed_groups): if not self.user_in_groups(user_groups, self.admin_groups): self.log.warning(msg) return False return True - class LocalOpenShiftOAuthenticator(LocalAuthenticator, OpenShiftOAuthenticator): """A version that mixes in local system user creation""" diff --git a/oauthenticator/tests/conftest.py b/oauthenticator/tests/conftest.py index 59163b0d..4a977d90 100644 --- a/oauthenticator/tests/conftest.py +++ b/oauthenticator/tests/conftest.py @@ -39,3 +39,16 @@ def client(io_loop, request): c = AsyncHTTPClient() assert isinstance(c, MockAsyncHTTPClient) return c + +@fixture +def get_auth_model(): + async def mock_auth_model(authenticator, handler): + access_token_params = authenticator.build_access_tokens_request_params(handler, None) + token_info = await authenticator.get_token_info(handler, access_token_params) + user_info = await authenticator.token_to_user(token_info) + username = authenticator.user_info_to_username(user_info) + return { + "name": username, + "auth_state": authenticator.build_auth_state_dict(token_info, user_info), + } + return mock_auth_model \ No newline at end of file diff --git a/oauthenticator/tests/test_bitbucket.py b/oauthenticator/tests/test_bitbucket.py index d3a73290..4c517bbb 100644 --- a/oauthenticator/tests/test_bitbucket.py +++ b/oauthenticator/tests/test_bitbucket.py @@ -96,3 +96,32 @@ def test_deprecated_config(caplog): assert authenticator.allowed_teams == {"red"} assert authenticator.allowed_users == {"blue"} + +async def test_bitbucket_user_is_authorized_overrides(bitbucket_client, get_auth_model): + client = bitbucket_client + authenticator = BitbucketOAuthenticator() + authenticator.allowed_teams = ['blue', 'red'] + + teams = { + 'red': ['grif', 'simmons', 'donut', 'sarge', 'lopez'], + 'blue': ['tucker', 'caboose', 'burns', 'sheila', 'texas'], + } + + def list_teams(request): + token = request.headers['Authorization'].split(None, 1)[1] + username = client.access_tokens[token]['username'] + values = [] + for team, members in teams.items(): + if username in members: + values.append({'name': team}) + return {'values': values} + + client.hosts['api.bitbucket.org'].append(('/2.0/workspaces', list_teams)) + + handler = client.handler_for_user(user_model('caboose')) + auth_model = await get_auth_model(authenticator, handler) + + is_authorized = await authenticator.user_is_authorized( + auth_model, allowed_teams=['red'] # caboose is not red + ) + assert not is_authorized \ No newline at end of file diff --git a/oauthenticator/tests/test_generic.py b/oauthenticator/tests/test_generic.py index df3d4699..e101e4ef 100644 --- a/oauthenticator/tests/test_generic.py +++ b/oauthenticator/tests/test_generic.py @@ -210,3 +210,28 @@ async def test_generic_callable_groups_claim_key_with_allowed_groups_and_admin_g user_info = await authenticator.authenticate(handler) assert user_info['name'] == 'zoe' assert user_info['admin'] is True + + + +async def test_generic_is_authenticated_overrides( + get_authenticator, generic_client, get_auth_model +): + authenticator = get_authenticator( + username_key=lambda r: r['alternate_username'], + scope=['openid', 'profile', 'roles'], + claim_groups_key="roles", + allowed_groups=['user', 'public'], + admin_groups=['administrator'], + ) + handler = generic_client.handler_for_user( + user_model( + 'wash', + alternate_username='zoe', + roles=['public'], + ) + ) + auth_model = await get_auth_model(authenticator,handler) + is_authorized = await authenticator.user_is_authorized( + auth_model, allowed_groups=['auditor', 'user'] + ) + assert not is_authorized \ No newline at end of file diff --git a/oauthenticator/tests/test_github.py b/oauthenticator/tests/test_github.py index 853c6703..6ed8354f 100644 --- a/oauthenticator/tests/test_github.py +++ b/oauthenticator/tests/test_github.py @@ -59,7 +59,7 @@ def make_link_header(urlinfo, page): } -async def test_allowed_org_membership(github_client): +async def test_allowed_org_membership(github_client, get_auth_model): client = github_client authenticator = GitHubOAuthenticator() @@ -190,6 +190,15 @@ def team_membership(request): user = await authenticator.authenticate(handler) assert user is None + # test_google_user_is_authorized_overrides + handler = client.handler_for_user(user_model('grif')) + auth_model = await get_auth_model(authenticator, handler) + is_authorized = await authenticator.user_is_authorized( + auth_model, allowed_organizations=['blue:alpha'] + ) + assert not is_authorized + + client_hosts.pop() client_hosts.pop() diff --git a/oauthenticator/tests/test_gitlab.py b/oauthenticator/tests/test_gitlab.py index 74a297cb..68a09f25 100644 --- a/oauthenticator/tests/test_gitlab.py +++ b/oauthenticator/tests/test_gitlab.py @@ -74,7 +74,7 @@ def make_link_header(urlinfo, page): } -async def test_allowed_groups(gitlab_client): +async def test_allowed_groups(gitlab_client, get_auth_model): client = gitlab_client authenticator = GitLabOAuthenticator() mock_api_version(client, '12.4.0-ee') @@ -178,10 +178,17 @@ def groups_paginated(user, page, urlinfo, response): name = user_info['name'] assert name == 'grif' + handler = client.handler_for_user(group_user_model('grif')) + auth_model = await get_auth_model(authenticator, handler) + is_authorized = await authenticator.user_is_authorized( + auth_model, allowed_gitlab_groups=['blue'] + ) + assert not is_authorized + client.hosts['gitlab.com'].pop() -async def test_allowed_project_ids(gitlab_client): +async def test_allowed_project_ids(gitlab_client, get_auth_model): client = gitlab_client authenticator = GitLabOAuthenticator() mock_api_version(client, '12.4.0-pre') @@ -266,6 +273,14 @@ def is_member(request): name = user_info['name'] assert name == 'harry' + # Not authenticated since Harry doesn't have developer access to the project in the list + handler = client.handler_for_user(harry_user_model) + auth_model = await get_auth_model(authenticator, handler) + is_authorized = await authenticator.user_is_authorized( + auth_model, allowed_project_ids=[123123152543] + ) + assert not is_authorized + def test_deprecated_config(caplog): cfg = Config() diff --git a/oauthenticator/tests/test_globus.py b/oauthenticator/tests/test_globus.py index 3299e72f..8980a7ad 100644 --- a/oauthenticator/tests/test_globus.py +++ b/oauthenticator/tests/test_globus.py @@ -433,3 +433,14 @@ async def test_user_allowed_not_admin(globus_client): data = await authenticator.authenticate(handler) assert data['name'] == 'wash' assert data['admin'] == False + + +async def test_globus_user_is_authorized_overrides(globus_client, get_auth_model): + authenticator = GlobusOAuthenticator() + authenticator.allowed_globus_groups = set({'21c6bc5d-fc12-4f60-b999-76766cd596c2', 'e01df78d-0607-482b-8b4b-8c37b85fbd07', '915dcd61-c842-4ea4-97c6-57396b936016'}) + handler = globus_client.handler_for_user(user_model('wash@uflightacademy.edu')) + auth_model = await get_auth_model(authenticator, handler) + is_authorized = await authenticator.user_is_authorized( + auth_model, allowed_globus_groups=['e01df78d-0607-482b-8b4b-8c37b85fbd07'] + ) + assert not is_authorized \ No newline at end of file diff --git a/oauthenticator/tests/test_google.py b/oauthenticator/tests/test_google.py index 82db8de2..af4a9600 100644 --- a/oauthenticator/tests/test_google.py +++ b/oauthenticator/tests/test_google.py @@ -173,3 +173,17 @@ def test_deprecated_config(caplog): assert authenticator.allowed_google_groups == {'email.com': ['group']} assert authenticator.allowed_users == {"user1"} + + +async def test_google_user_is_authorized_overrides(google_client, get_auth_model): + authenticator = GoogleOAuthenticator( + hosted_domain=['email.com', 'mycollege.edu'], + allowed_google_groups={'email.com': ['fakegroup'], ',mycollege.edu': []}, + ) + handler = google_client.handler_for_user(user_model('fakeadmin@email.com')) + + auth_model = await get_auth_model(authenticator, handler) + is_authorized = await authenticator.user_is_authorized( + auth_model, allowed_google_groups=['fakegroup2'] + ) + assert not is_authorized \ No newline at end of file From fc748425e30ae04c7aaf6dfff34fedf80733be33 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 1 Feb 2023 05:57:46 +0000 Subject: [PATCH 2/2] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- oauthenticator/bitbucket.py | 7 ++++--- oauthenticator/generic.py | 6 ++---- oauthenticator/github.py | 6 ++++-- oauthenticator/gitlab.py | 18 +++++++++++++----- oauthenticator/globus.py | 6 ++++-- oauthenticator/google.py | 12 ++++++++---- oauthenticator/openshift.py | 1 + oauthenticator/tests/conftest.py | 10 +++++++--- oauthenticator/tests/test_bitbucket.py | 5 +++-- oauthenticator/tests/test_generic.py | 5 ++--- oauthenticator/tests/test_github.py | 1 - oauthenticator/tests/test_globus.py | 10 ++++++++-- oauthenticator/tests/test_google.py | 2 +- 13 files changed, 57 insertions(+), 32 deletions(-) diff --git a/oauthenticator/bitbucket.py b/oauthenticator/bitbucket.py index 2958aebc..a2b77083 100644 --- a/oauthenticator/bitbucket.py +++ b/oauthenticator/bitbucket.py @@ -47,7 +47,7 @@ def _userdata_url_default(self): async def user_is_authorized(self, auth_model, **overrides): """Checks if user is authorized with bitbucket OAuth. - + Overrides: - allowed_teams: Can override default self.allowed_teams @@ -70,7 +70,9 @@ async def user_is_authorized(self, auth_model, **overrides): return True - async def _check_membership_allowed_teams(self, username, access_token, token_type, allowed_teams): + async def _check_membership_allowed_teams( + self, username, access_token, token_type, allowed_teams + ): headers = self.build_userdata_request_headers(access_token, token_type) # We verify the team membership by calling teams endpoint. next_page = url_concat( @@ -86,7 +88,6 @@ async def _check_membership_allowed_teams(self, username, access_token, token_ty if len(allowed_teams & user_teams) > 0: return True return False - class LocalBitbucketOAuthenticator(LocalAuthenticator, BitbucketOAuthenticator): diff --git a/oauthenticator/generic.py b/oauthenticator/generic.py index 2a89b5a5..b06c3788 100644 --- a/oauthenticator/generic.py +++ b/oauthenticator/generic.py @@ -98,7 +98,7 @@ def user_info_to_username(self, user_info): async def user_is_authorized(self, auth_model, **overrides): """Checks if user is authorized with generic OAuth. - + Overrides: - allowed_groups: Can override default self.allowed_groups - claim_groups_key: Can override default self.claim_groups_key @@ -118,9 +118,7 @@ async def user_is_authorized(self, auth_model, **overrides): groups = claim_groups_key(user_info) else: try: - groups = reduce( - dict.get, claim_groups_key.split("."), user_info - ) + groups = reduce(dict.get, claim_groups_key.split("."), user_info) except TypeError: # This happens if a nested key does not exist (reduce trying to call None.get) self.log.error( diff --git a/oauthenticator/github.py b/oauthenticator/github.py index c12ace15..437e6d61 100644 --- a/oauthenticator/github.py +++ b/oauthenticator/github.py @@ -135,7 +135,7 @@ def _github_client_secret_changed(self, name, old, new): async def user_is_authorized(self, auth_model, **overrides): """Checks if user is authorized with github OAuth. - + Overrides: - allowed_organizations: Can override default self.allowed_organizations @@ -145,7 +145,9 @@ async def user_is_authorized(self, auth_model, **overrides): # This check is performed here, as it requires `access_token`. access_token = auth_model["auth_state"]["token_response"]["access_token"] token_type = auth_model["auth_state"]["token_response"]["token_type"] - allowed_organizations = overrides.pop("allowed_organizations", self.allowed_organizations) + allowed_organizations = overrides.pop( + "allowed_organizations", self.allowed_organizations + ) if allowed_organizations: for org in allowed_organizations: user_in_org = await self._check_membership_allowed_organizations( diff --git a/oauthenticator/gitlab.py b/oauthenticator/gitlab.py index 238ff43a..51d4dac7 100644 --- a/oauthenticator/gitlab.py +++ b/oauthenticator/gitlab.py @@ -119,7 +119,7 @@ def _userdata_url_default(self): async def user_is_authorized(self, auth_model, **overrides): """Checks if user is authorized with gitlab OAuth. - + Overrides: - allowed_project_ids: Can override default self.allowed_project_ids - allowed_gitlab_groups: Can override default self.allowed_gitlab_groups @@ -140,7 +140,9 @@ async def user_is_authorized(self, auth_model, **overrides): user_in_group = user_in_project = False is_group_specified = is_project_id_specified = False - allowed_gitlab_groups = overrides.pop("allowed_gitlab_groups", self.allowed_gitlab_groups) + allowed_gitlab_groups = overrides.pop( + "allowed_gitlab_groups", self.allowed_gitlab_groups + ) if allowed_gitlab_groups: is_group_specified = True user_in_group = await self._check_membership_allowed_groups( @@ -148,7 +150,9 @@ async def user_is_authorized(self, auth_model, **overrides): ) # We skip project_id check if user is in allowed group. - allowed_project_ids = overrides.pop("allowed_project_ids", self.allowed_project_ids) + allowed_project_ids = overrides.pop( + "allowed_project_ids", self.allowed_project_ids + ) if allowed_project_ids and not user_in_group: is_project_id_specified = True user_in_project = await self._check_membership_allowed_project_ids( @@ -182,7 +186,9 @@ async def _get_gitlab_version(self, access_token): version_ints = list(map(int, version_strings)) return version_ints - async def _check_membership_allowed_groups(self, user_id, access_token, allowed_gitlab_groups): + async def _check_membership_allowed_groups( + self, user_id, access_token, allowed_gitlab_groups + ): headers = _api_headers(access_token) # Check if user is a member of any group in the allowed list for group in map(url_escape, allowed_gitlab_groups): @@ -203,7 +209,9 @@ async def _check_membership_allowed_groups(self, user_id, access_token, allowed_ return True # user _is_ in group return False - async def _check_membership_allowed_project_ids(self, user_id, access_token, allowed_project_ids): + async def _check_membership_allowed_project_ids( + self, user_id, access_token, allowed_project_ids + ): headers = _api_headers(access_token) # Check if user has developer access to any project in the allowed list for project in allowed_project_ids: diff --git a/oauthenticator/globus.py b/oauthenticator/globus.py index f016a4d7..61ab9f94 100644 --- a/oauthenticator/globus.py +++ b/oauthenticator/globus.py @@ -256,7 +256,7 @@ async def get_users_groups_ids(self, tokens): async def user_is_authorized(self, auth_model, **overrides): """Checks if user is authorized with globus OAuth. - + Overrides: - allowed_globus_groups: Can override default self.allowed_globus_groups @@ -265,7 +265,9 @@ async def user_is_authorized(self, auth_model, **overrides): tokens = self.get_globus_tokens(auth_model["auth_state"]["token_response"]) - allowed_globus_groups = overrides.pop("allowed_globus_groups", self.allowed_globus_groups) + allowed_globus_groups = overrides.pop( + "allowed_globus_groups", self.allowed_globus_groups + ) if allowed_globus_groups or self.admin_globus_groups: # If any of these configurations are set, user must be in the allowed or admin Globus Group user_group_ids = await self.get_users_groups_ids(tokens) diff --git a/oauthenticator/google.py b/oauthenticator/google.py index 64abfbb2..504702d9 100644 --- a/oauthenticator/google.py +++ b/oauthenticator/google.py @@ -128,7 +128,7 @@ def _username_claim_default(self): async def user_is_authorized(self, auth_model, **overrides): """Checks if user is authorized with google OAuth. - + Overrides: - allowed_google_groups: Can override default self.allowed_google_groups @@ -140,7 +140,7 @@ async def user_is_authorized(self, auth_model, **overrides): if not auth_model["auth_state"][self.user_auth_state_key]['verified_email']: self.log.warning(f"Google OAuth unverified email attempt: {user_email}") raise HTTPError(403, f"Google email {user_email} not verified") - + if self.hosted_domain: if user_email_domain not in self.hosted_domain: self.log.warning( @@ -153,8 +153,12 @@ async def user_is_authorized(self, auth_model, **overrides): allowed_google_groups = overrides.pop("allowed_google_groups", None) if allowed_google_groups: # Only check if the override is passed so original authentication logic is not altered - return (await self._add_google_groups_info(auth_model, google_groups=allowed_google_groups)) is not None - + return ( + await self._add_google_groups_info( + auth_model, google_groups=allowed_google_groups + ) + ) is not None + return True async def update_auth_model(self, auth_model, google_groups=None): diff --git a/oauthenticator/openshift.py b/oauthenticator/openshift.py index 28c858ba..a373fe1f 100644 --- a/oauthenticator/openshift.py +++ b/oauthenticator/openshift.py @@ -129,6 +129,7 @@ async def user_is_authorized(self, auth_model, **overrides): return True + class LocalOpenShiftOAuthenticator(LocalAuthenticator, OpenShiftOAuthenticator): """A version that mixes in local system user creation""" diff --git a/oauthenticator/tests/conftest.py b/oauthenticator/tests/conftest.py index 4a977d90..bea1dff5 100644 --- a/oauthenticator/tests/conftest.py +++ b/oauthenticator/tests/conftest.py @@ -40,15 +40,19 @@ def client(io_loop, request): assert isinstance(c, MockAsyncHTTPClient) return c + @fixture def get_auth_model(): async def mock_auth_model(authenticator, handler): - access_token_params = authenticator.build_access_tokens_request_params(handler, None) + access_token_params = authenticator.build_access_tokens_request_params( + handler, None + ) token_info = await authenticator.get_token_info(handler, access_token_params) user_info = await authenticator.token_to_user(token_info) username = authenticator.user_info_to_username(user_info) - return { + return { "name": username, "auth_state": authenticator.build_auth_state_dict(token_info, user_info), } - return mock_auth_model \ No newline at end of file + + return mock_auth_model diff --git a/oauthenticator/tests/test_bitbucket.py b/oauthenticator/tests/test_bitbucket.py index 4c517bbb..8e5b0b2a 100644 --- a/oauthenticator/tests/test_bitbucket.py +++ b/oauthenticator/tests/test_bitbucket.py @@ -97,6 +97,7 @@ def test_deprecated_config(caplog): assert authenticator.allowed_teams == {"red"} assert authenticator.allowed_users == {"blue"} + async def test_bitbucket_user_is_authorized_overrides(bitbucket_client, get_auth_model): client = bitbucket_client authenticator = BitbucketOAuthenticator() @@ -122,6 +123,6 @@ def list_teams(request): auth_model = await get_auth_model(authenticator, handler) is_authorized = await authenticator.user_is_authorized( - auth_model, allowed_teams=['red'] # caboose is not red + auth_model, allowed_teams=['red'] # caboose is not red ) - assert not is_authorized \ No newline at end of file + assert not is_authorized diff --git a/oauthenticator/tests/test_generic.py b/oauthenticator/tests/test_generic.py index e101e4ef..de5ae6b4 100644 --- a/oauthenticator/tests/test_generic.py +++ b/oauthenticator/tests/test_generic.py @@ -212,7 +212,6 @@ async def test_generic_callable_groups_claim_key_with_allowed_groups_and_admin_g assert user_info['admin'] is True - async def test_generic_is_authenticated_overrides( get_authenticator, generic_client, get_auth_model ): @@ -230,8 +229,8 @@ async def test_generic_is_authenticated_overrides( roles=['public'], ) ) - auth_model = await get_auth_model(authenticator,handler) + auth_model = await get_auth_model(authenticator, handler) is_authorized = await authenticator.user_is_authorized( auth_model, allowed_groups=['auditor', 'user'] ) - assert not is_authorized \ No newline at end of file + assert not is_authorized diff --git a/oauthenticator/tests/test_github.py b/oauthenticator/tests/test_github.py index 6ed8354f..d2a689bb 100644 --- a/oauthenticator/tests/test_github.py +++ b/oauthenticator/tests/test_github.py @@ -198,7 +198,6 @@ def team_membership(request): ) assert not is_authorized - client_hosts.pop() client_hosts.pop() diff --git a/oauthenticator/tests/test_globus.py b/oauthenticator/tests/test_globus.py index 8980a7ad..9423eb74 100644 --- a/oauthenticator/tests/test_globus.py +++ b/oauthenticator/tests/test_globus.py @@ -437,10 +437,16 @@ async def test_user_allowed_not_admin(globus_client): async def test_globus_user_is_authorized_overrides(globus_client, get_auth_model): authenticator = GlobusOAuthenticator() - authenticator.allowed_globus_groups = set({'21c6bc5d-fc12-4f60-b999-76766cd596c2', 'e01df78d-0607-482b-8b4b-8c37b85fbd07', '915dcd61-c842-4ea4-97c6-57396b936016'}) + authenticator.allowed_globus_groups = set( + { + '21c6bc5d-fc12-4f60-b999-76766cd596c2', + 'e01df78d-0607-482b-8b4b-8c37b85fbd07', + '915dcd61-c842-4ea4-97c6-57396b936016', + } + ) handler = globus_client.handler_for_user(user_model('wash@uflightacademy.edu')) auth_model = await get_auth_model(authenticator, handler) is_authorized = await authenticator.user_is_authorized( auth_model, allowed_globus_groups=['e01df78d-0607-482b-8b4b-8c37b85fbd07'] ) - assert not is_authorized \ No newline at end of file + assert not is_authorized diff --git a/oauthenticator/tests/test_google.py b/oauthenticator/tests/test_google.py index af4a9600..bc9286c9 100644 --- a/oauthenticator/tests/test_google.py +++ b/oauthenticator/tests/test_google.py @@ -186,4 +186,4 @@ async def test_google_user_is_authorized_overrides(google_client, get_auth_model is_authorized = await authenticator.user_is_authorized( auth_model, allowed_google_groups=['fakegroup2'] ) - assert not is_authorized \ No newline at end of file + assert not is_authorized