diff --git a/docs/source/how-to/example-oauthenticator.py b/docs/source/how-to/example-oauthenticator.py index 822aecbc..c139ee14 100644 --- a/docs/source/how-to/example-oauthenticator.py +++ b/docs/source/how-to/example-oauthenticator.py @@ -88,9 +88,9 @@ def build_auth_state_dict(self, token_info, user_info): # Updates `auth_model` dict if any fields have changed or additional information is available # or returns the unchanged `auth_model`. # Returns the model unchanged by default. - # Should be overridden to take into account additional checks such as against group/admin/team membership. + # Should be overridden to take into account additional checks such as against group/admin/team membership. # if the OAuth provider has such a concept - async def update_auth_model(self, auth_model, **kwargs): + async def update_auth_model(self, username, auth_model): pass diff --git a/docs/source/reference/changelog.md b/docs/source/reference/changelog.md index ed3ab57a..d4e7da3b 100644 --- a/docs/source/reference/changelog.md +++ b/docs/source/reference/changelog.md @@ -6,6 +6,24 @@ command line for details. ## [Unreleased] +### Breaking changes + +- [All] Users are now authorized based on _either_ being part of + `Authenticator.admin_users`, `Authenticator.allowed_users`, an Authenticator + specific allowed team/group/organization, or declared in + `JupyterHub.load_roles` or `JupyterHub.load_groups`. +- [Google] If `GoogleOAuthenticator.admin_google_groups` is configured, users + logging in not explicitly there or in `Authenticator.admin_users` will get + their admin status revoked. +- [Generic, Google] `GenericOAuthenticator.allowed_groups`, + `GenericOAuthenticator.allowed_groups` + `GoogleOAuthenticator.allowed_google_groups`, and + `GoogleOAuthenticator.admin_google_groups` are now Set based configuration + instead of List based configuration. It is still possible to set these with + lists as as they are converted to sets automatically, but anyone reading and + adding entries must now use set logic and not list logic. +- [Google] Authentication state's `google_groups` is now a set, not a list. + (changelog:version-15)= ## 15.0 diff --git a/oauthenticator/auth0.py b/oauthenticator/auth0.py index 25401779..909551e6 100644 --- a/oauthenticator/auth0.py +++ b/oauthenticator/auth0.py @@ -99,5 +99,4 @@ def _userdata_url_default(self): class LocalAuth0OAuthenticator(LocalAuthenticator, Auth0OAuthenticator): - """A version that mixes in local system user creation""" diff --git a/oauthenticator/bitbucket.py b/oauthenticator/bitbucket.py index 53e53fb6..1a22f147 100644 --- a/oauthenticator/bitbucket.py +++ b/oauthenticator/bitbucket.py @@ -40,38 +40,64 @@ def _userdata_url_default(self): config=True, help="Automatically allow members of selected teams" ) - async def user_is_authorized(self, auth_model): - 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: - user_in_team = await self._check_membership_allowed_teams( - username, access_token, token_type - ) - if not user_in_team: - self.log.warning(f"{username} not in team allowed list of users") - return False - - return True - - async def _check_membership_allowed_teams(self, username, access_token, token_type): + async def _fetch_user_teams(self, access_token, token_type): + """ + Get user's team memberships via bitbucket's API. + """ headers = self.build_userdata_request_headers(access_token, token_type) - # We verify the team membership by calling teams endpoint. next_page = url_concat( "https://api.bitbucket.org/2.0/workspaces", {'role': 'member'} ) + + user_teams = set() while next_page: resp_json = await self.httpfetch(next_page, method="GET", headers=headers) next_page = resp_json.get('next', None) - - 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: + user_teams |= {entry["name"] for entry in resp_json["values"]} + return user_teams + + async def update_auth_model(self, auth_model): + """ + Fetch and store `user_teams` in auth state if `allowed_teams` is + configured. + """ + if self.allowed_teams: + access_token = auth_model["auth_state"]["token_response"]["access_token"] + token_type = auth_model["auth_state"]["token_response"]["token_type"] + user_teams = await self._fetch_user_teams(access_token, token_type) + auth_model["auth_state"]["user_teams"] = user_teams + + return auth_model + + async def check_allowed(self, username, auth_model): + """ + Returns True for users allowed to be authorized. + + Overrides the OAuthenticator.check_allowed implementation to allow users + either part of `allowed_users` or `allowed_teams`, and not just those + part of `allowed_users`. + """ + # A workaround for JupyterHub<=4.0.1, described in + # https://github.com/jupyterhub/oauthenticator/issues/621 + if auth_model is None: + return True + + # allow admin users recognized via admin_users or update_auth_model + if auth_model["admin"]: + return True + + # if allowed_users or allowed_teams is configured, we deny users not + # part of either + if self.allowed_users or self.allowed_teams: + user_teams = auth_model["auth_state"]["user_teams"] + if username in self.allowed_users: + return True + if any(user_teams & self.allowed_teams): return True - return False + return False + + # otherwise, authorize all users + return True class LocalBitbucketOAuthenticator(LocalAuthenticator, BitbucketOAuthenticator): diff --git a/oauthenticator/cilogon.py b/oauthenticator/cilogon.py index 852f5e72..b1aee5ea 100644 --- a/oauthenticator/cilogon.py +++ b/oauthenticator/cilogon.py @@ -246,90 +246,138 @@ def _validate_allowed_idps(self, proposal): This is useful for linked identities where not all of them return the primary username_claim. """, + default_value=["email"], ) - def user_info_to_username(self, user_info): - claimlist = [self.username_claim] + def _get_final_username_claim_list(self, user_info): + """ + The username claims that will be used to determine the hub username can be set through: + - `CILogonOAutnenticator.username_claim`, that can be extended through `CILogonOAutnenticator.additional_username_claims` + or + - `CILogonOAuthenticator.allowed_idps..username_claim`, that + will overwrite any value set through CILogonOAuthenticator.username_claim + for this identity provider. + + This function returns the username claim list that will be used for the current user trying to login + based on the idp that they have selected. If no `CILogonOAutnenticator.allowed_idps` is set, then + `CILogonOAutnenticator.username_claim` will be used. + """ + username_claims = [self.username_claim] if self.additional_username_claims: - claimlist.extend(self.additional_username_claims) - + username_claims.extend(self.additional_username_claims) if self.allowed_idps: - selected_idp = user_info.get("idp") - if selected_idp: + selected_idp = user_info["idp"] + if selected_idp in self.allowed_idps.keys(): # The username_claim which should be used for this idp - claimlist = [ + return [ self.allowed_idps[selected_idp]["username_derivation"][ "username_claim" ] ] + else: + return username_claims + return username_claims - for claim in claimlist: + def _get_username_from_claim_list(self, user_info, username_claims): + username = None + for claim in username_claims: username = user_info.get(claim) if username: - return username + break + + return username + + def user_info_to_username(self, user_info): + username_claims = self._get_final_username_claim_list(user_info) + username = self._get_username_from_claim_list(user_info, username_claims) if not username: user_info_keys = sorted(user_info.keys()) self.log.error( - f"No username claim in the list at {claimlist} was found in the response {user_info_keys}" + f"No username claim in the list at {username_claims} was found in the response {user_info_keys}" ) raise web.HTTPError(500, "Failed to get username from CILogon") - async def user_is_authorized(self, auth_model): - username = auth_model["name"] - # Check if selected idp was marked as allowed + # Optionally strip idp domain or prefix the username if self.allowed_idps: - selected_idp = auth_model["auth_state"][self.user_auth_state_key].get("idp") - # Fail hard if idp wasn't allowed + selected_idp = user_info["idp"] + if selected_idp in self.allowed_idps.keys(): + username_derivation = self.allowed_idps[selected_idp][ + "username_derivation" + ] + action = username_derivation.get("action") + + if action == "strip_idp_domain": + username = username.split("@", 1)[0] + elif action == "prefix": + prefix = username_derivation["prefix"] + username = f"{prefix}:{username}" + + return username + + async def check_allowed(self, username, auth_model): + """ + Returns True for authorized users, raises errors for users + denied authorization. + + Overrides the `OAuthenticator.check_allowed` implementation to only allow users + logging in using a provider that is part of `allowed_idps`. + Following this, the user must either be part of `allowed_users` or `allowed_domains` + to be authorized if either is configured, otherwise all users are + authorized. + """ + # A workaround for JupyterHub<=4.0.1, described in + # https://github.com/jupyterhub/oauthenticator/issues/621 + if auth_model is None: + return True + + # allow admin users recognized via admin_users or update_auth_model + if auth_model["admin"]: + return True + + if self.allowed_idps: + user_info = auth_model["auth_state"][self.user_auth_state_key] + selected_idp = user_info["idp"] if selected_idp not in self.allowed_idps.keys(): self.log.error( f"Trying to login from an identity provider that was not allowed {selected_idp}", ) raise web.HTTPError( - 500, + 403, "Trying to login using an identity provider that was not allowed", ) - allowed_domains = self.allowed_idps[selected_idp].get( - "allowed_domains", None - ) + allowed_domains = self.allowed_idps[selected_idp].get("allowed_domains") + if self.allowed_users or allowed_domains: + if username in self.allowed_users: + return True - if allowed_domains: - gotten_name, gotten_domain = username.split('@') - if gotten_domain not in allowed_domains: - raise web.HTTPError( - 500, - "Trying to login using a domain that was not allowed", + if allowed_domains: + username_claims = self._get_final_username_claim_list(user_info) + username_with_domain = self._get_username_from_claim_list( + user_info, username_claims ) - + user_domain = username_with_domain.split("@", 1)[1] + if user_domain in allowed_domains: + return True + else: + raise web.HTTPError( + 403, + "Trying to login using a domain that was not allowed", + ) + + return False + # Although not recommended, it might be that `allowed_idps` is not specified + # In this case we need to make sure we still check `allowed_users` and don't assume + # everyone should be authorized + elif self.allowed_users: + if username in self.allowed_users: + return True + return False + + # otherwise, authorize all users return True - async def update_auth_model(self, auth_model): - selected_idp = auth_model["auth_state"][self.user_auth_state_key].get("idp") - - # Check if the requested username_claim exists in the response from the provider - username = auth_model["name"] - - # Check if we need to strip/prefix username - if self.allowed_idps: - username_derivation_config = self.allowed_idps[selected_idp][ - "username_derivation" - ] - action = username_derivation_config.get("action", None) - allowed_domains = self.allowed_idps[selected_idp].get( - "allowed_domains", None - ) - - if action == "strip_idp_domain": - gotten_name, gotten_domain = username.split('@') - username = gotten_name - elif action == "prefix": - prefix = username_derivation_config["prefix"] - username = f"{prefix}:{username}" - - auth_model["name"] = username - return auth_model - class LocalCILogonOAuthenticator(LocalAuthenticator, CILogonOAuthenticator): diff --git a/oauthenticator/generic.py b/oauthenticator/generic.py index 9f946f96..cae41193 100644 --- a/oauthenticator/generic.py +++ b/oauthenticator/generic.py @@ -7,7 +7,7 @@ from jupyterhub.auth import LocalAuthenticator from jupyterhub.traitlets import Callable from tornado.httpclient import AsyncHTTPClient -from traitlets import Bool, Dict, List, Unicode, Union, default +from traitlets import Bool, Dict, Set, Unicode, Union, default from .oauth2 import OAuthenticator @@ -36,13 +36,13 @@ class GenericOAuthenticator(OAuthenticator): """, ) - allowed_groups = List( + allowed_groups = Set( Unicode(), config=True, help="Automatically allow members of selected groups", ) - admin_groups = List( + admin_groups = Set( Unicode(), config=True, help="Groups whose members should have Jupyterhub admin privileges", @@ -70,7 +70,7 @@ class GenericOAuthenticator(OAuthenticator): tls_verify = Bool( os.environ.get('OAUTH2_TLS_VERIFY', 'True').lower() in {'true', '1'}, config=True, - help="Disable TLS verification on http request", + help="Require valid tls certificates in HTTP requests", ) @default("basic_auth") @@ -83,10 +83,6 @@ def _default_http_client(self): force_instance=True, defaults=dict(validate_cert=self.tls_verify) ) - @staticmethod - def check_user_in_groups(member_groups, allowed_groups): - return bool(set(member_groups) & set(allowed_groups)) - def user_info_to_username(self, user_info): if callable(self.username_claim): username = self.username_claim(user_info) @@ -99,52 +95,80 @@ def user_info_to_username(self, user_info): return username - async def user_is_authorized(self, auth_model): - user_info = auth_model["auth_state"][self.user_auth_state_key] - if self.allowed_groups: - self.log.info( - f"Validating if user claim groups match any of {self.allowed_groups}" + def get_user_groups(self, user_info): + """ + Returns a set of groups the user belongs to based on claim_groups_key + and provided user_info. + + - If claim_groups_key is a callable, it is meant to return the groups + directly. + - If claim_groups_key is a nested dictionary key like + "permissions.groups", this function returns + user_info["permissions"]["groups"]. + + Note that this method is introduced by GenericOAuthenticator and not + present in the base class. + """ + if callable(self.claim_groups_key): + return set(self.claim_groups_key(user_info)) + try: + return set(reduce(dict.get, self.claim_groups_key.split("."), user_info)) + except TypeError: + self.log.error( + f"The claim_groups_key {self.claim_groups_key} does not exist in the user token" ) - - if callable(self.claim_groups_key): - groups = self.claim_groups_key(user_info) - else: - try: - groups = reduce( - dict.get, self.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" - ) - 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}" - ) - return False - - if not self.check_user_in_groups(groups, self.allowed_groups): - return False - - return True + return set() async def update_auth_model(self, auth_model): - user_info = auth_model["auth_state"][self.user_auth_state_key] - if self.allowed_groups: - if callable(self.claim_groups_key): - groups = self.claim_groups_key(user_info) - else: - groups = user_info.get(self.claim_groups_key) - - # User has been checked to be in allowed_groups too if we're here - if groups: - auth_model['admin'] = self.check_user_in_groups( - groups, self.admin_groups - ) + """ + Sets admin status to True or False if `admin_groups` is configured and + the user isn't part of `admin_users` or `admin_groups`. Note that + leaving it at None makes users able to retain an admin status while + setting it to False makes it be revoked. + """ + if auth_model["admin"]: + # auth_model["admin"] being True means the user was in admin_users + return auth_model + + if self.admin_groups: + # admin status should in this case be True or False, not None + user_info = auth_model["auth_state"][self.user_auth_state_key] + user_groups = self.get_user_groups(user_info) + auth_model["admin"] = any(user_groups & self.admin_groups) + return auth_model + async def check_allowed(self, username, auth_model): + """ + Returns True for users allowed to be authorized. + + Overrides the OAuthenticator.check_allowed implementation to allow users + either part of `allowed_users` or `allowed_groups`, and not just those + part of `allowed_users`. + """ + # A workaround for JupyterHub<=4.0.1, described in + # https://github.com/jupyterhub/oauthenticator/issues/621 + if auth_model is None: + return True + + # allow admin users recognized via admin_users or update_auth_model + if auth_model["admin"]: + return True + + # if allowed_users or allowed_groups is configured, we deny users not + # part of either + if self.allowed_users or self.allowed_groups: + user_info = auth_model["auth_state"][self.user_auth_state_key] + user_groups = self.get_user_groups(user_info) + if username in self.allowed_users: + return True + if any(user_groups & self.allowed_groups): + return True + return False + + # otherwise, authorize all users + return True + class LocalGenericOAuthenticator(LocalAuthenticator, GenericOAuthenticator): """A version that mixes in local system user creation""" diff --git a/oauthenticator/github.py b/oauthenticator/github.py index 245f5af4..47ba4769 100644 --- a/oauthenticator/github.py +++ b/oauthenticator/github.py @@ -13,10 +13,6 @@ class GitHubOAuthenticator(OAuthenticator): - # see github_scopes.md for details about scope config - # set scopes via config, e.g. - # c.GitHubOAuthenticator.scope = ['read:org'] - _deprecated_oauth_aliases = { "github_organization_whitelist": ("allowed_organizations", "0.12.0"), **OAuthenticator._deprecated_oauth_aliases, @@ -106,7 +102,14 @@ def _github_client_secret_changed(self, name, old, new): ) allowed_organizations = Set( - config=True, help="Automatically allow members of selected organizations" + help=""" + Allow members of organizations or organizations' teams by specifying + strings like `org-a` and/or `org-b:team-1`. + + Requires `read:org` to be set in `scope` to not just allow based on + public membership. + """, + config=True, ) populate_teams_in_auth_state = Bool( @@ -128,27 +131,60 @@ def _github_client_secret_changed(self, name, old, new): config=True, ) - async def user_is_authorized(self, auth_model): - # 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: - user_in_org = await self._check_membership_allowed_organizations( - org, auth_model["name"], access_token, token_type - ) - if user_in_org: - break - else: # User not found in member list for any organisation + async def check_allowed(self, username, auth_model): + """ + Returns True for users allowed to be authorized. + + Overrides the OAuthenticator.check_allowed implementation to allow users + either part of `allowed_users` or `allowed_organizations`, and not just + those part of `allowed_users`. + """ + # A workaround for JupyterHub<=4.0.1, described in + # https://github.com/jupyterhub/oauthenticator/issues/621 + if auth_model is None: + return True + + # allow admin users recognized via admin_users or update_auth_model + if auth_model["admin"]: + return True + + # if allowed_users or allowed_organizations is configured, we deny users not + # part of either + if self.allowed_users or self.allowed_organizations: + if username in self.allowed_users: + return True + + if self.allowed_organizations: + access_token = auth_model["auth_state"]["token_response"][ + "access_token" + ] + token_type = auth_model["auth_state"]["token_response"]["token_type"] + for org in self.allowed_organizations: + user_in_org = await self._check_membership_allowed_organizations( + org, username, access_token, token_type + ) + if user_in_org: + return True self.log.warning( - f"User {auth_model['name']} is not in allowed org list", + f"User {username} is not part of allowed_organizations" ) - return False + return False + + # otherwise, authorize all users return True async def update_auth_model(self, auth_model): + """ + Fetch and store `email` in auth state if the user's only was: private, + not part of the initial response, and we was granted a scope to fetch + the private email. + + Also fetch and store `teams` in auth state if + `populate_teams_in_auth_state` is configured. + """ + user_info = auth_model["auth_state"][self.user_auth_state_key] + # If a public email is not available, an extra API call has to be made # to a /user/emails using the access token to retrieve emails. The # scopes relevant for this are checked based on this documentation: @@ -158,15 +194,12 @@ async def update_auth_model(self, auth_model): # Note that the read:user scope does not imply the user:emails scope! access_token = auth_model["auth_state"]["token_response"]["access_token"] token_type = auth_model["auth_state"]["token_response"]["token_type"] - granted_scopes = [] - if auth_model["auth_state"]["scope"]: - granted_scopes = auth_model["auth_state"]["scope"] - - if not auth_model["auth_state"]["github_user"]["email"] and ( + granted_scopes = auth_model["auth_state"].get("scope", []) + if not user_info["email"] and ( "user" in granted_scopes or "user:email" in granted_scopes ): resp_json = await self.httpfetch( - self.github_api + "/user/emails", + f"{self.github_api}/user/emails", "fetching user emails", method="GET", headers=self.build_userdata_request_headers(access_token, token_type), @@ -174,25 +207,21 @@ async def update_auth_model(self, auth_model): ) for val in resp_json: if val["primary"]: - auth_model["auth_state"]["github_user"]["email"] = val["email"] + user_info["email"] = val["email"] break if self.populate_teams_in_auth_state: if "read:org" not in self.scope: - # This means the "read:org" scope was not set, and we can"t fetch teams + # This means the "read:org" scope was not set, and we can't + # fetch teams self.log.error( "read:org scope is required for populate_teams_in_auth_state functionality to work" ) else: - # Number of teams to request per page - per_page = 100 - - # https://docs.github.com/en/rest/reference/teams#list-teams-for-the-authenticated-user - url = self.github_api + f"/user/teams?per_page={per_page}" - - auth_model["auth_state"]["teams"] = await self._paginated_fetch( - url, access_token, token_type - ) + # https://docs.github.com/en/rest/teams/teams?apiVersion=2022-11-28#list-teams-for-the-authenticated-user + url = f"{self.github_api}/user/teams?per_page=100" + user_teams = await self._paginated_fetch(url, access_token, token_type) + auth_model["auth_state"]["teams"] = user_teams return auth_model @@ -243,21 +272,33 @@ async def _paginated_fetch(self, api_url, access_token, token_type): return content async def _check_membership_allowed_organizations( - self, org, username, access_token, token_type + self, org_team, username, access_token, token_type ): + """ + Checks if a user is part of an organization or organization's team via + GitHub's REST API. The `read:org` scope is required to not only check + for public org/team membership. + + The `org_team` parameter accepts values like `org-a` or `org-b:team-1`, + and will adjust to use a the relevant REST API to check either org or + team membership. + """ headers = self.build_userdata_request_headers(access_token, token_type) - # Check membership of user `username` for organization `org` via api [check-membership](https://docs.github.com/en/rest/orgs/members#check-membership) - # With empty scope (even if authenticated by an org member), this - # will only await public org members. You want 'read:org' in order - # to be able to iterate through all members. If you would only like to - # allow certain teams within an organisation, specify - # allowed_organisations = {org_name:team_name} - check_membership_url = self._build_check_membership_url(org, username) + if ":" in org_team: + # check if user is part of an organization's team + # https://docs.github.com/en/rest/teams/members?apiVersion=2022-11-28#get-team-member-legacy + org, team = org_team.split(":") + api_url = f"{self.github_api}/orgs/{org}/teams/{team}/members/{username}" + else: + # check if user is part of an organization + # https://docs.github.com/en/rest/orgs/members?apiVersion=2022-11-28#check-organization-membership-for-a-user + org = org_team + api_url = f"{self.github_api}/orgs/{org}/members/{username}" self.log.debug(f"Checking GitHub organization membership: {username} in {org}?") resp = await self.httpfetch( - check_membership_url, + api_url, parse_json=False, raise_error=False, method="GET", @@ -265,7 +306,7 @@ async def _check_membership_allowed_organizations( validate_cert=self.validate_server_cert, ) if resp.code == 204: - self.log.info(f"Allowing {username} as member of {org}") + self.log.debug(f"Allowing {username} as member of {org_team}") return True else: try: @@ -274,18 +315,10 @@ async def _check_membership_allowed_organizations( except ValueError: message = '' self.log.debug( - f"{username} does not appear to be a member of {org} (status={resp.code}): {message}", + f"{username} does not appear to be a member of {org_team} (status={resp.code}): {message}", ) return False - def _build_check_membership_url(self, org: str, username: str) -> str: - if ":" in org: - org, team = org.split(":") - return f"{self.github_api}/orgs/{org}/teams/{team}/members/{username}" - else: - return f"{self.github_api}/orgs/{org}/members/{username}" - class LocalGitHubOAuthenticator(LocalAuthenticator, GitHubOAuthenticator): - """A version that mixes in local system user creation""" diff --git a/oauthenticator/gitlab.py b/oauthenticator/gitlab.py index 73e9228e..e2d36048 100644 --- a/oauthenticator/gitlab.py +++ b/oauthenticator/gitlab.py @@ -7,7 +7,6 @@ from jupyterhub.auth import LocalAuthenticator from tornado.escape import url_escape -from tornado.httpclient import HTTPRequest from traitlets import CUnicode, Set, Unicode, default from .oauth2 import OAuthenticator @@ -113,47 +112,58 @@ def _userdata_url_default(self): ) gitlab_version = None + member_api_variant = None - async def user_is_authorized(self, auth_model): - access_token = auth_model["auth_state"]["token_response"]["access_token"] - user_id = auth_model["auth_state"][self.user_auth_state_key]["id"] - + async def _set_gitlab_version(self, access_token): # memoize gitlab version for class lifetime if self.gitlab_version is None: self.gitlab_version = await self._get_gitlab_version(access_token) self.member_api_variant = 'all/' if self.gitlab_version >= [12, 4] else '' - # Check if user is a member of any allowed groups or projects. - # These checks are performed here, as it requires `access_token`. - user_in_group = user_in_project = False - is_group_specified = is_project_id_specified = False + async def check_allowed(self, username, auth_model): + """ + Returns True for users allowed to be authorized. + + Overrides the OAuthenticator.check_allowed implementation to allow users + either part of `allowed_users`, `allowed_gitlab_groups`, or `allowed_project_ids`, + and not just those part of `allowed_users`. + """ + # A workaround for JupyterHub<=4.0.1, described in + # https://github.com/jupyterhub/oauthenticator/issues/621 + if auth_model is None: + return True - if self.allowed_gitlab_groups: - is_group_specified = True - user_in_group = await self._check_membership_allowed_groups( - user_id, access_token - ) + # allow admin users recognized via admin_users or update_auth_model + if auth_model["admin"]: + return True - # We skip project_id check if user is in allowed group. - if self.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 - ) + # if allowed_users, allowed_gitlab_groups, or allowed_project_ids is + # configured, we deny users not part of either + if self.allowed_users or self.allowed_gitlab_groups or self.allowed_project_ids: + if username in self.allowed_users: + return True - no_config_specified = not (is_group_specified or is_project_id_specified) + access_token = auth_model["auth_state"]["token_response"]["access_token"] + user_id = auth_model["auth_state"][self.user_auth_state_key]["id"] - if ( - (is_group_specified and user_in_group) - or (is_project_id_specified and user_in_project) - or no_config_specified - ): - return True + if self.allowed_gitlab_groups: + user_in_group = await self._check_membership_allowed_groups( + user_id, access_token + ) + if user_in_group: + return True - self.log.warning( - f"{auth_model['name']} not in group or project allowed list", - ) - return False + if self.allowed_project_ids: + user_in_project = await self._check_membership_allowed_project_ids( + user_id, access_token + ) + if user_in_project: + return True + + return False + + # otherwise, authorize all users + return True async def _get_gitlab_version(self, access_token): url = f"{self.gitlab_api}/version" @@ -169,6 +179,8 @@ async def _get_gitlab_version(self, access_token): async def _check_membership_allowed_groups(self, user_id, access_token): headers = _api_headers(access_token) + await self._set_gitlab_version(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): url = "%s/groups/%s/members/%s%d" % ( @@ -177,9 +189,6 @@ async def _check_membership_allowed_groups(self, user_id, access_token): self.member_api_variant, user_id, ) - req = HTTPRequest( - url, - ) resp = await self.httpfetch( url, parse_json=False, @@ -194,6 +203,8 @@ async def _check_membership_allowed_groups(self, user_id, access_token): async def _check_membership_allowed_project_ids(self, user_id, access_token): headers = _api_headers(access_token) + await self._set_gitlab_version(access_token) + # Check if user has developer access to any project in the allowed list for project in self.allowed_project_ids: url = "%s/projects/%s/members/%s%d" % ( diff --git a/oauthenticator/globus.py b/oauthenticator/globus.py index fb182cf3..4e225685 100644 --- a/oauthenticator/globus.py +++ b/oauthenticator/globus.py @@ -165,10 +165,6 @@ def _revoke_tokens_on_logout_default(self): their UUIDs. Setting this will add the Globus Groups scope.""" ).tag(config=True) - @staticmethod - def check_user_in_groups(member_groups, allowed_groups): - return bool(set(member_groups) & set(allowed_groups)) - async def pre_spawn_start(self, user, spawner): """Add tokens to the spawner whenever the spawner starts a notebook. This will allow users to create a transfer client: @@ -230,8 +226,8 @@ def build_auth_state_dict(self, token_info, user_info): self.user_auth_state_key: user_info, } - async def get_users_groups_ids(self, tokens): - user_group_ids = set() + async def _fetch_users_groups(self, tokens): + user_groups = set() # Get Groups access token, may not be in dict headed to auth state for token_dict in tokens: if token_dict['resource_server'] == 'groups.api.globus.org': @@ -244,44 +240,81 @@ async def get_users_groups_ids(self, tokens): ) # Build set of Group IDs for group in groups_resp: - user_group_ids.add(group['id']) + user_groups.add(group['id']) - return user_group_ids + return user_groups - async def user_is_authorized(self, auth_model): - tokens = self.get_globus_tokens(auth_model["auth_state"]["token_response"]) + async def check_allowed(self, username, auth_model): + """ + Returns True for users allowed to be authorized. - if self.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 - ): - if not self.check_user_in_groups( - user_group_ids, self.admin_globus_groups - ): - username = self.user_info_to_username( - auth_model["auth_state"][self.user_auth_state_key] - ) - self.log.warning(f"{username} not in an allowed Globus Group") - return False + Overrides the OAuthenticator.check_allowed implementation to allow users + either part of `allowed_users` or `allowed_globus_groups`, and not just those + part of `allowed_users`. + """ + # A workaround for JupyterHub<=4.0.1, described in + # https://github.com/jupyterhub/oauthenticator/issues/621 + if auth_model is None: + return True + + # allow admin users recognized via admin_users or update_auth_model + if auth_model["admin"]: + return True + + if self.identity_provider: + # It's possible for identity provider domains to be namespaced + # https://docs.globus.org/api/auth/specification/#identity_provider_namespaces + user_info = auth_model["auth_state"][self.user_auth_state_key] + domain = user_info.get(self.username_claim).split('@', 1)[-1] + if domain != self.identity_provider: + self.log.warning( + f"Trying to login from an identity provider that was not allowed {domain}", + ) + raise HTTPError( + 403, + f"This site is restricted to {self.identity_provider} accounts. " + "Please link your account at app.globus.org/account.", + ) + + # if allowed_users or allowed_globus_groups is configured, we deny users + # not part of either + if self.allowed_users or self.allowed_globus_groups: + if username in self.allowed_users: + return True + if self.allowed_globus_groups: + user_groups = auth_model["auth_state"]["globus_groups"] + if any(user_groups & self.allowed_globus_groups): + return True + self.log.warning(f"{username} not in an allowed Globus Group") + + return False + # otherwise, authorize all users return True async def update_auth_model(self, auth_model): - username = self.user_info_to_username( - auth_model["auth_state"][self.user_auth_state_key] - ) - tokens = self.get_globus_tokens(auth_model["auth_state"]["token_response"]) + """ + Fetch and store `globus_groups` in auth state if `allowed_globus_groups` + or `admin_globus_groups` is configured. + + Sets admin status to True or False if `admin_globus_groups` is + configured and the user isn't part of `admin_users` or + `admin_globus_groups`. Note that leaving it at None makes users able to + retain an admin status while setting it to False makes it be revoked. + """ + user_groups = set() + if self.allowed_globus_groups or self.admin_globus_groups: + tokens = self.get_globus_tokens(auth_model["auth_state"]["token_response"]) + user_groups = await self._fetch_users_groups(tokens) + auth_model["auth_state"]["globus_groups"] = user_groups + + if auth_model["admin"]: + # auth_model["admin"] being True means the user was in admin_users + return auth_model if 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) - # Admin users are being managed via Globus Groups - # Default to False - auth_model['admin'] = False - if self.check_user_in_groups(user_group_ids, self.admin_globus_groups): - auth_model['admin'] = True + # admin status should in this case be True or False, not None + auth_model["admin"] = any(user_groups & self.admin_globus_groups) return auth_model @@ -292,16 +325,7 @@ def user_info_to_username(self, user_info): will have the 'foouser' account in Jupyterhub. """ - # It's possible for identity provider domains to be namespaced - # https://docs.globus.org/api/auth/specification/#identity_provider_namespaces # noqa - username, domain = user_info.get(self.username_claim).split('@', 1) - if self.identity_provider and domain != self.identity_provider: - raise HTTPError( - 403, - f"This site is restricted to {self.identity_provider} accounts. " - "Please link your account at app.globus.org/account.", - ) - return username + return user_info.get(self.username_claim).split('@')[0] def get_default_headers(self): return {"Accept": "application/json", "User-Agent": "JupyterHub"} diff --git a/oauthenticator/google.py b/oauthenticator/google.py index e149c5a3..f09d9634 100644 --- a/oauthenticator/google.py +++ b/oauthenticator/google.py @@ -8,19 +8,11 @@ from jupyterhub.auth import LocalAuthenticator from tornado.auth import GoogleOAuth2Mixin from tornado.web import HTTPError -from traitlets import Dict, List, Unicode, default, validate +from traitlets import Dict, List, Set, Unicode, default, validate from .oauth2 import OAuthenticator -def check_user_in_groups(member_groups, allowed_groups): - # Check if user is a member of any group in the allowed groups - if any(g in member_groups for g in allowed_groups): - return True # user _is_ in group - else: - return False - - class GoogleOAuthenticator(OAuthenticator, GoogleOAuth2Mixin): _deprecated_oauth_aliases = { "google_group_whitelist": ("allowed_google_groups", "0.12.0"), @@ -78,11 +70,11 @@ def _userdata_url_default(self): ) allowed_google_groups = Dict( - List(Unicode()), help="Automatically allow members of selected groups" + Set(Unicode()), help="Automatically allow members of selected groups" ).tag(config=True) admin_google_groups = Dict( - List(Unicode()), + Set(Unicode()), help="Groups whose members should have Jupyterhub admin privileges", ).tag(config=True) @@ -124,37 +116,94 @@ def _cast_hosted_domain(self, proposal): help="""Google Apps hosted domain string, e.g. My College""", ) - async def user_is_authorized(self, auth_model): - user_email = auth_model["auth_state"][self.user_auth_state_key]['email'] - user_email_domain = user_email.split('@')[1] + async def update_auth_model(self, auth_model): + """ + Fetch and store `google_groups` in auth state if `allowed_google_groups` + or `admin_google_groups` is configured. Also declare the user an admin + if part of `admin_google_groups`. + + Sets admin status to True or False if `admin_google_groups` is + configured and the user isn't part of `admin_users` or + `admin_google_groups`. Note that leaving it at None makes users able to + retain an admin status while setting it to False makes it be revoked. + """ + user_info = auth_model["auth_state"][self.user_auth_state_key] + user_email = user_info["email"] + user_domain = user_info["domain"] = user_email.split("@")[1] + + user_groups = set() + if self.allowed_google_groups or self.admin_google_groups: + user_groups = user_info["google_groups"] = self._fetch_user_groups( + user_email, user_domain + ) + user_info["google_groups"] = user_groups + + if auth_model["admin"]: + # auth_model["admin"] being True means the user was in admin_users + return auth_model + + if self.admin_google_groups: + # admin status should in this case be True or False, not None + admin_groups = self.admin_google_groups.get(user_domain, set()) + auth_model["admin"] = any(user_groups & admin_groups) + + return auth_model + + async def check_allowed(self, username, auth_model): + """ + Returns True for users allowed to be authorized. + + Overrides the OAuthenticator.check_allowed implementation to allow users + either part of `allowed_users` or `allowed_google_groups`, and not just those + part of `allowed_users`. + """ + # A workaround for JupyterHub<=4.0.1, described in + # https://github.com/jupyterhub/oauthenticator/issues/621 + if auth_model is None: + return True + + # allow admin users recognized via admin_users or update_auth_model + if auth_model["admin"]: + return True + + user_info = auth_model["auth_state"][self.user_auth_state_key] + user_email = user_info["email"] + user_domain = user_info["domain"] + user_groups = user_info["google_groups"] - if not auth_model["auth_state"][self.user_auth_state_key]['verified_email']: + if not user_info["verified_email"]: self.log.warning(f"Google OAuth unverified email attempt: {user_email}") raise HTTPError(403, f"Google email {user_email} not verified") + # NOTE: If hosted_domain is configured as ["a.com", "b.com"], and + # allowed_google_groups is declared as {"a.com": {"a-group"}}, a + # "b.com" user won't be authorized unless allowed in another way. + # + # This means that its not possible to allow all users of a given + # domain if one wants to restrict another. + # if self.hosted_domain: - if user_email_domain not in self.hosted_domain: + if user_domain not in self.hosted_domain: self.log.warning( f"Google OAuth unauthorized domain attempt: {user_email}" ) raise HTTPError( - 403, f"Google account domain @{user_email_domain} not authorized." + 403, f"Google account domain @{user_domain} not authorized" ) - return True - - async def update_auth_model(self, auth_model, google_groups=None): - username = auth_model["name"] - user_email = auth_model["auth_state"][self.user_auth_state_key]['email'] - - if len(self.hosted_domain) == 1 and user_email == username: - # unambiguous domain, use only base name - username = user_email.split('@')[0] - auth_model["name"] = username - - if self.admin_google_groups or self.allowed_google_groups: - auth_model = await self._add_google_groups_info(auth_model, google_groups) - return auth_model + # if allowed_users or allowed_google_groups is configured, we deny users + # not part of either + if self.allowed_users or self.allowed_google_groups: + if username in self.allowed_users: + return True + if self.allowed_google_groups: + allowed_groups = self.allowed_google_groups.get(user_domain, set()) + if any(user_groups & allowed_groups): + return True + return False + + # otherwise, authorize all users + return True def _service_client_credentials(self, scopes, user_email_domain): """ @@ -204,10 +253,19 @@ def _service_client(self, service_name, service_version, credentials, http=None) http=http, ) - async def _google_groups_for_user(self, user_email, credentials, http=None): + def _fetch_user_groups(self, user_email, user_email_domain, http=None): """ - Return google groups a given user is a member of + Return a set with the google groups a given user is a member of """ + # FIXME: When this function is used and waiting for web request + # responses, JupyterHub gets blocked from doing other things. + # Ideally the web requests should be made using an async client + # that can be awaited while JupyterHub handles other things. + # + credentials = self._service_client_credentials( + scopes=[f"{self.google_api_url}/auth/admin.directory.group.readonly"], + user_email_domain=user_email_domain, + ) service = self._service_client( service_name='admin', service_version='directory_v1', @@ -215,52 +273,12 @@ async def _google_groups_for_user(self, user_email, credentials, http=None): http=http, ) - results = service.groups().list(userKey=user_email).execute() - results = [ - g['email'].split('@')[0] for g in results.get('groups', [{'email': None}]) - ] - self.log.debug(f"user_email {user_email} is a member of {results}") - return results - - async def _add_google_groups_info(self, user_info, google_groups=None): - user_email_domain = user_info['auth_state']['google_user']['hd'] - user_email = user_info['auth_state']['google_user']['email'] - if google_groups is None: - credentials = self._service_client_credentials( - scopes=[f"{self.google_api_url}/auth/admin.directory.group.readonly"], - user_email_domain=user_email_domain, - ) - google_groups = await self._google_groups_for_user( - user_email=user_email, credentials=credentials - ) - user_info['auth_state']['google_user']['google_groups'] = google_groups - - # Check if user is a member of any admin groups. - if self.admin_google_groups: - is_admin = check_user_in_groups( - google_groups, self.admin_google_groups[user_email_domain] - ) - - # Check if user is a member of any allowed groups. - allowed_groups = self.allowed_google_groups - - if allowed_groups: - if user_email_domain in allowed_groups: - user_in_group = check_user_in_groups( - google_groups, allowed_groups[user_email_domain] - ) - else: - return None - else: - user_in_group = True - - if self.admin_google_groups and (is_admin or user_in_group): - user_info['admin'] = is_admin - return user_info - elif user_in_group: - return user_info - else: - return None + resp = service.groups().list(userKey=user_email).execute() + user_groups = { + g['email'].split('@')[0] for g in resp.get('groups', [{'email': None}]) + } + self.log.debug(f"user_email {user_email} is a member of {user_groups}") + return user_groups class LocalGoogleOAuthenticator(LocalAuthenticator, GoogleOAuthenticator): diff --git a/oauthenticator/mediawiki.py b/oauthenticator/mediawiki.py index c4d64e34..49caa90b 100644 --- a/oauthenticator/mediawiki.py +++ b/oauthenticator/mediawiki.py @@ -102,6 +102,7 @@ def normalize_username(self, username): """ Override normalize_username to avoid lowercasing usernames """ + username = username.replace(' ', '_') return username def _executor_default(self): @@ -137,12 +138,7 @@ async def token_to_user(self, token_info): self.executor.submit(handshaker.identify, token_info["access_token"]) ) - async def update_auth_model(self, auth_model): - auth_model['name'] = auth_model['name'].replace(' ', '_') - return auth_model - def build_auth_state_dict(self, token_info, user_info): - username = self.user_info_to_username(user_info) # this shouldn't be necessary anymore, # but keep for backward-compatibility return { diff --git a/oauthenticator/oauth2.py b/oauthenticator/oauth2.py index 3d892b6a..564cb2eb 100644 --- a/oauthenticator/oauth2.py +++ b/oauthenticator/oauth2.py @@ -555,7 +555,6 @@ def build_token_info_request_headers(self): def user_info_to_username(self, user_info): """ Gets the self.username_claim key's value from the user_info dictionary. - This is equivalent to the JupyterHub username. Should be overridden by the authenticators for which the hub username cannot be extracted this way and needs extra processing. @@ -733,44 +732,45 @@ def build_auth_state_dict(self, token_info, user_info): self.user_auth_state_key: user_info, } - async def update_auth_model(self, auth_model, **kwargs): + async def update_auth_model(self, auth_model): """ - Updates `auth_model` dict if any fields have changed or additional information is available - or returns the unchanged `auth_model`. + Updates and returns the `auth_model` dict. - Returns the model unchanged by default. + Should be overridden to collect information required for check_allowed. - Should be overridden to take into account changes like group/admin membership. - - Args: auth_model - the auth model dictionary dict instead, containing: - - the `name` key holding the username - - the `auth_state` key, the dictionary of of auth state + Args: auth_model - the auth model dictionary, containing: + - `name`: the normalized username + - `admin`: the admin status (True/False/None), where None means it + should be unchanged. + - `auth_state`: the dictionary of of auth state returned by :meth:`oauthenticator.OAuthenticator.build_auth_state_dict` Called by the :meth:`oauthenticator.OAuthenticator.authenticate` """ return auth_model - async def user_is_authorized(self, auth_model): + async def authenticate(self, handler, data=None, **kwargs): """ - 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. + A JupyterHub Authenticator's authenticate method's job is: - Returns True by default. + - return None if the user isn't successfully authenticated + - return a dictionary if authentication is successful with name, admin + (optional), and auth_state (optional) - Called by the :meth:`oauthenticator.OAuthenticator.authenticate` - """ - return True + Subclasses should not override this method. - async def authenticate(self, handler, data=None, **kwargs): + ref: https://jupyterhub.readthedocs.io/en/stable/reference/authenticators.html#authenticator-authenticate-method + ref: https://github.com/jupyterhub/jupyterhub/blob/4.0.0/jupyterhub/auth.py#L581-L611 + """ # build the parameters to be used in the request exchanging the oauth code for the access token access_token_params = self.build_access_tokens_request_params(handler, data) # exchange the oauth code for an access token and get the JSON with info about it token_info = await self.get_token_info(handler, access_token_params) # use the access_token to get userdata info user_info = await self.token_to_user(token_info) - # extract the username out of the user_info dict + # extract the username out of the user_info dict and normalize it username = self.user_info_to_username(user_info) + username = self.normalize_username(username) # check if there any refresh_token in the token_info dict refresh_token = token_info.get("refresh_token", None) @@ -782,19 +782,45 @@ async def authenticate(self, handler, data=None, **kwargs): if refresh_token: token_info["refresh_token"] = refresh_token - # build the auth model to be persisted if authentication goes right + # build the auth model to be read if authentication goes right auth_model = { "name": username, + "admin": True if username in self.admin_users else None, "auth_state": self.build_auth_state_dict(token_info, user_info), } - # check if the username that's authenticating should be authorized - authorized = await self.user_is_authorized(auth_model) - if not authorized: - return None + # update the auth_model with info to later authorize the user in + # check_allowed, such as admin status and group memberships + return await self.update_auth_model(auth_model) + + async def check_allowed(self, username, auth_model): + """ + Returns True for users allowed to be authorized - # update the auth model with any info if available - return await self.update_auth_model(auth_model, **kwargs) + Overrides Authenticator.check_allowed that is called from + `Authenticator.get_authenticated_user` after + `OAuthenticator.authenticate` has been called, and therefore also after + `update_auth_model` has been called. + + Subclasses with authorization logic involving allowed groups should + override this. + """ + # A workaround for JupyterHub<=4.0.1, described in + # https://github.com/jupyterhub/oauthenticator/issues/621 + if auth_model is None: + return True + + # authorize users to become admins by admin_users or logic in + # update_auth_model + if auth_model["admin"]: + return True + + # if allowed_users is configured, authorize/unauthorize based on that + if self.allowed_users: + return username in self.allowed_users + + # otherwise, authorize all users + return True _deprecated_oauth_aliases = {} diff --git a/oauthenticator/openshift.py b/oauthenticator/openshift.py index 3e256073..c876a7cf 100644 --- a/oauthenticator/openshift.py +++ b/oauthenticator/openshift.py @@ -86,40 +86,53 @@ def _token_url_default(self): def _userdata_url_default(self): return f"{self.openshift_rest_api_url}/apis/user.openshift.io/v1/users/~" - @staticmethod - def user_in_groups(user_groups: set, allowed_groups: set): - return any(user_groups.intersection(allowed_groups)) - def user_info_to_username(self, user_info): return user_info['metadata']['name'] async def update_auth_model(self, auth_model): """ - Use the group info stored on the OpenShift User object to determine if a user - is an admin and update the auth_model with this info. + Update admin status based on `admin_groups` if its configured. """ - user_groups = set(auth_model['auth_state']['openshift_user']['groups']) - if self.admin_groups: - auth_model['admin'] = self.user_in_groups(user_groups, self.admin_groups) + # if admin_groups is configured and the user wasn't part of + # admin_users, we must set the admin status to True or False, + # otherwise removing a user from the admin_groups won't have an + # effect + user_info = auth_model["auth_state"][self.user_auth_state_key] + user_groups = set(user_info["groups"]) + auth_model["admin"] = any(user_groups & self.admin_groups) return auth_model - async def user_is_authorized(self, auth_model): - """ - Use the group info stored on the OpenShift User object to determine if a user - is authorized to login. + async def check_allowed(self, username, auth_model): """ - user_groups = set(auth_model['auth_state']['openshift_user']['groups']) - username = auth_model['name'] - - if self.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, self.admin_groups): - self.log.warning(msg) - return False + Returns True for users allowed to be authorized. + Overrides the OAuthenticator.check_allowed implementation to allow users + either part of `allowed_users` or `allowed_groups`, and not just those + part of `allowed_users`. + """ + # A workaround for JupyterHub<=4.0.1, described in + # https://github.com/jupyterhub/oauthenticator/issues/621 + if auth_model is None: + return True + + # allow admin users recognized via admin_users or update_auth_model + if auth_model["admin"]: + return True + + # if allowed_users or allowed_groups is configured, we deny users not + # part of either + if self.allowed_users or self.allowed_groups: + if username in self.allowed_users: + return True + user_info = auth_model["auth_state"][self.user_auth_state_key] + user_groups = set(user_info["groups"]) + if any(user_groups & self.allowed_groups): + return True + return False + + # otherwise, authorize all users return True diff --git a/oauthenticator/tests/mocks.py b/oauthenticator/tests/mocks.py index f02870ed..b896c4db 100644 --- a/oauthenticator/tests/mocks.py +++ b/oauthenticator/tests/mocks.py @@ -253,5 +253,5 @@ async def no_code_test(authenticator): handler = Mock(spec=web.RequestHandler) handler.get_argument = Mock(return_value=None) with pytest.raises(web.HTTPError) as exc: - name = await authenticator.authenticate(handler) + auth_model = await authenticator.get_authenticated_user(handler, None) assert exc.value.status_code == 400 diff --git a/oauthenticator/tests/test_auth0.py b/oauthenticator/tests/test_auth0.py index 53146318..7fb28ef4 100644 --- a/oauthenticator/tests/test_auth0.py +++ b/oauthenticator/tests/test_auth0.py @@ -42,11 +42,10 @@ async def test_auth0(config, auth0_client): authenticator = Auth0OAuthenticator(config=cfg) handler = auth0_client.handler_for_user(user_model('kaylee@serenity.now')) - user_info = await authenticator.authenticate(handler) - assert sorted(user_info) == ['auth_state', 'name'] - name = user_info['name'] - assert name == 'kaylee@serenity.now' - auth_state = user_info['auth_state'] + auth_model = await authenticator.get_authenticated_user(handler, None) + assert sorted(auth_model) == ['admin', 'auth_state', 'name'] + assert auth_model['name'] == 'kaylee@serenity.now' + auth_state = auth_model['auth_state'] assert 'access_token' in auth_state assert 'auth0_user' in auth_state @@ -60,9 +59,8 @@ async def test_username_key(config, auth0_client): authenticator = Auth0OAuthenticator(config=cfg) authenticator.username_key = 'nickname' handler = auth0_client.handler_for_user(user_model('kaylee@serenity.now', 'kayle')) - user_info = await authenticator.authenticate(handler) - - assert user_info['name'] == 'kayle' + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model['name'] == 'kayle' async def test_custom_logout(monkeypatch): diff --git a/oauthenticator/tests/test_azuread.py b/oauthenticator/tests/test_azuread.py index d8c9ca77..0070540a 100644 --- a/oauthenticator/tests/test_azuread.py +++ b/oauthenticator/tests/test_azuread.py @@ -92,17 +92,17 @@ async def test_azuread(username_claim, azure_client): ) ) - user_info = await authenticator.authenticate(handler) - assert sorted(user_info) == ['auth_state', 'name'] + auth_model = await authenticator.authenticate(handler) + assert sorted(auth_model) == ['admin', 'auth_state', 'name'] - auth_state = user_info['auth_state'] + auth_state = auth_model['auth_state'] assert 'access_token' in auth_state assert 'user' in auth_state auth_state_user_info = auth_state['user'] assert auth_state_user_info['aud'] == authenticator.client_id - username = user_info['name'] + username = auth_model['name'] if username_claim: assert username == auth_state_user_info[username_claim] else: diff --git a/oauthenticator/tests/test_bitbucket.py b/oauthenticator/tests/test_bitbucket.py index dfcc4c9e..1cf24b4f 100644 --- a/oauthenticator/tests/test_bitbucket.py +++ b/oauthenticator/tests/test_bitbucket.py @@ -28,11 +28,10 @@ def bitbucket_client(client): async def test_bitbucket(bitbucket_client): authenticator = BitbucketOAuthenticator() handler = bitbucket_client.handler_for_user(user_model('yorba')) - user_info = await authenticator.authenticate(handler) - assert sorted(user_info) == ['auth_state', 'name'] - name = user_info['name'] - assert name == 'yorba' - auth_state = user_info['auth_state'] + auth_model = await authenticator.get_authenticated_user(handler, None) + assert sorted(auth_model) == ['admin', 'auth_state', 'name'] + assert auth_model['name'] == 'yorba' + auth_state = auth_model['auth_state'] assert 'access_token' in auth_state assert 'bitbucket_user' in auth_state @@ -59,25 +58,23 @@ def list_teams(request): client.hosts['api.bitbucket.org'].append(('/2.0/workspaces', list_teams)) handler = client.handler_for_user(user_model('caboose')) - user_info = await authenticator.authenticate(handler) - name = user_info['name'] - assert name == 'caboose' + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model['name'] == 'caboose' handler = client.handler_for_user(user_model('donut')) - name = await authenticator.authenticate(handler) - assert name is None + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model is None # reverse it, just to be safe authenticator.allowed_teams = ['red'] handler = client.handler_for_user(user_model('caboose')) - name = await authenticator.authenticate(handler) - assert name is None + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model is None handler = client.handler_for_user(user_model('donut')) - user_info = await authenticator.authenticate(handler) - name = user_info['name'] - assert name == 'donut' + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model['name'] == 'donut' async def test_deprecated_config(caplog): diff --git a/oauthenticator/tests/test_cilogon.py b/oauthenticator/tests/test_cilogon.py index eca7a442..5e0c44a6 100644 --- a/oauthenticator/tests/test_cilogon.py +++ b/oauthenticator/tests/test_cilogon.py @@ -38,11 +38,10 @@ def cilogon_client(client): async def test_cilogon(cilogon_client): authenticator = CILogonOAuthenticator() handler = cilogon_client.handler_for_user(user_model('wash')) - user_info = await authenticator.authenticate(handler) - print(json.dumps(user_info, sort_keys=True, indent=4)) - name = user_info['name'] - assert name == 'wash@serenity.space' - auth_state = user_info['auth_state'] + auth_model = await authenticator.get_authenticated_user(handler, None) + print(json.dumps(auth_model, sort_keys=True, indent=4)) + assert auth_model['name'] == 'wash@serenity.space' + auth_state = auth_model['auth_state'] assert 'access_token' in auth_state assert 'token_response' in auth_state assert auth_state["cilogon_user"] == user_model('wash') @@ -53,11 +52,10 @@ async def test_cilogon_alternate_claim(cilogon_client): handler = cilogon_client.handler_for_user( alternative_user_model('jtkirk@ufp.gov', 'uid') ) - user_info = await authenticator.authenticate(handler) - print(json.dumps(user_info, sort_keys=True, indent=4)) - name = user_info['name'] - assert name == 'jtkirk@ufp.gov' - auth_state = user_info['auth_state'] + auth_model = await authenticator.get_authenticated_user(handler, None) + print(json.dumps(auth_model, sort_keys=True, indent=4)) + assert auth_model['name'] == 'jtkirk@ufp.gov' + auth_state = auth_model['auth_state'] assert 'access_token' in auth_state assert 'token_response' in auth_state assert auth_state["cilogon_user"] == alternative_user_model('jtkirk@ufp.gov', 'uid') @@ -68,11 +66,10 @@ async def test_cilogon_additional_claim(cilogon_client): handler = cilogon_client.handler_for_user( alternative_user_model('jtkirk@ufp.gov', 'uid') ) - user_info = await authenticator.authenticate(handler) - print(json.dumps(user_info, sort_keys=True, indent=4)) - name = user_info['name'] - assert name == 'jtkirk@ufp.gov' - auth_state = user_info['auth_state'] + auth_model = await authenticator.get_authenticated_user(handler, None) + print(json.dumps(auth_model, sort_keys=True, indent=4)) + assert auth_model['name'] == 'jtkirk@ufp.gov' + auth_state = auth_model['auth_state'] assert 'access_token' in auth_state assert 'token_response' in auth_state assert auth_state["cilogon_user"] == alternative_user_model('jtkirk@ufp.gov', 'uid') @@ -84,7 +81,7 @@ async def test_cilogon_missing_alternate_claim(cilogon_client): alternative_user_model('jtkirk@ufp.gov', 'uid') ) with raises(HTTPError): - user_info = await authenticator.authenticate(handler) + auth_model = await authenticator.get_authenticated_user(handler, None) async def test_deprecated_config(caplog): @@ -273,10 +270,9 @@ async def test_strip_and_prefix_username(cilogon_client): 'jtkirk@uni.edu', 'email', idp='https://some-idp.com/login/oauth/authorize' ) ) - user_info = await authenticator.authenticate(handler) - print(json.dumps(user_info, sort_keys=True, indent=4)) - name = user_info['name'] - assert name == 'jtkirk' + auth_model = await authenticator.get_authenticated_user(handler, None) + print(json.dumps(auth_model, sort_keys=True, indent=4)) + assert auth_model['name'] == 'jtkirk' # Test appending prefixes handler = cilogon_client.handler_for_user( @@ -284,10 +280,9 @@ async def test_strip_and_prefix_username(cilogon_client): 'jtkirk', 'nickname', idp='https://another-idp.com/login/oauth/authorize' ) ) - user_info = await authenticator.authenticate(handler) - print(json.dumps(user_info, sort_keys=True, indent=4)) - name = user_info['name'] - assert name == 'idp:jtkirk' + auth_model = await authenticator.get_authenticated_user(handler, None) + print(json.dumps(auth_model, sort_keys=True, indent=4)) + assert auth_model['name'] == 'idp:jtkirk' async def test_no_action_specified(cilogon_client): @@ -308,9 +303,8 @@ async def test_no_action_specified(cilogon_client): 'jtkirk@uni.edu', 'email', idp='https://some-idp.com/login/oauth/authorize' ) ) - user_info = await authenticator.authenticate(handler) - name = user_info['name'] - assert name == 'jtkirk@uni.edu' + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model['name'] == 'jtkirk@uni.edu' async def test_not_allowed_domains_and_stripping(cilogon_client): @@ -337,7 +331,7 @@ async def test_not_allowed_domains_and_stripping(cilogon_client): # The domain to be stripped isn't allowed, so it should fail with raises(HTTPError): - user_info = await authenticator.authenticate(handler) + auth_model = await authenticator.get_authenticated_user(handler, None) async def test_allowed_domains_and_stripping(cilogon_client): @@ -363,9 +357,8 @@ async def test_allowed_domains_and_stripping(cilogon_client): ) # The domain to be stripped is allowed, so it should be stripped - user_info = await authenticator.authenticate(handler) - name = user_info['name'] - assert name == 'jtkirk' + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model['name'] == 'jtkirk' async def test_allowed_domains_no_stripping(cilogon_client): @@ -389,7 +382,7 @@ async def test_allowed_domains_no_stripping(cilogon_client): ) with raises(HTTPError): - user_info = await authenticator.authenticate(handler) + auth_model = await authenticator.get_authenticated_user(handler, None) # Test allowed domain login handler = cilogon_client.handler_for_user( @@ -398,6 +391,5 @@ async def test_allowed_domains_no_stripping(cilogon_client): ) ) - user_info = await authenticator.authenticate(handler) - name = user_info['name'] - assert name == 'jtkirk@pink.org' + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model['name'] == 'jtkirk@pink.org' diff --git a/oauthenticator/tests/test_generic.py b/oauthenticator/tests/test_generic.py index df3d4699..575a6e22 100644 --- a/oauthenticator/tests/test_generic.py +++ b/oauthenticator/tests/test_generic.py @@ -48,11 +48,10 @@ async def test_generic(get_authenticator, generic_client): authenticator = get_authenticator() handler = get_simple_handler(generic_client) - user_info = await authenticator.authenticate(handler) - assert sorted(user_info) == ['auth_state', 'name'] - name = user_info['name'] - assert name == 'wash' - auth_state = user_info['auth_state'] + auth_model = await authenticator.get_authenticated_user(handler, None) + assert sorted(auth_model) == ['admin', 'auth_state', 'name'] + assert auth_model['name'] == 'wash' + auth_state = auth_model['auth_state'] assert 'access_token' in auth_state assert 'oauth_user' in auth_state assert 'refresh_token' in auth_state @@ -64,10 +63,9 @@ async def test_generic_data(get_authenticator, generic_client): handler = get_simple_handler(generic_client) data = {'testing': 'data'} - user_info = await authenticator.authenticate(handler, data) - assert sorted(user_info) == ['auth_state', 'name'] - name = user_info['name'] - assert name == 'wash' + auth_model = await authenticator.authenticate(handler, data) + assert sorted(auth_model) == ['admin', 'auth_state', 'name'] + assert auth_model['name'] == 'wash' async def test_generic_callable_username_key(get_authenticator, generic_client): @@ -75,8 +73,8 @@ async def test_generic_callable_username_key(get_authenticator, generic_client): handler = generic_client.handler_for_user( user_model('wash', alternate_username='zoe') ) - user_info = await authenticator.authenticate(handler) - assert user_info['name'] == 'zoe' + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model['name'] == 'zoe' async def test_generic_callable_groups_claim_key_with_allowed_groups( @@ -90,8 +88,8 @@ async def test_generic_callable_groups_claim_key_with_allowed_groups( handler = generic_client.handler_for_user( user_model('wash', alternate_username='zoe', policies={'roles': ['super_user']}) ) - user_info = await authenticator.authenticate(handler) - assert user_info['name'] == 'wash' + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model['name'] == 'wash' async def test_generic_groups_claim_key_with_allowed_groups( @@ -105,8 +103,8 @@ async def test_generic_groups_claim_key_with_allowed_groups( handler = generic_client.handler_for_user( user_model('wash', alternate_username='zoe', groups=['super_user']) ) - user_info = await authenticator.authenticate(handler) - assert user_info['name'] == 'wash' + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model['name'] == 'wash' async def test_generic_groups_claim_key_nested_strings( @@ -122,8 +120,8 @@ async def test_generic_groups_claim_key_nested_strings( 'wash', alternate_username='zoe', permissions={"groups": ['super_user']} ) ) - user_info = await authenticator.authenticate(handler) - assert user_info['name'] == 'wash' + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model['name'] == 'wash' async def test_generic_groups_claim_key_nested_strings_nonexistant_key( @@ -137,8 +135,8 @@ async def test_generic_groups_claim_key_nested_strings_nonexistant_key( handler = generic_client.handler_for_user( user_model('wash', alternate_username='zoe') ) - user_info = await authenticator.authenticate(handler) - assert user_info is None + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model is None async def test_generic_groups_claim_key_with_allowed_groups_unauthorized( @@ -152,8 +150,8 @@ async def test_generic_groups_claim_key_with_allowed_groups_unauthorized( handler = generic_client.handler_for_user( user_model('wash', alternate_username='zoe', groups=['public']) ) - user_info = await authenticator.authenticate(handler) - assert user_info is None + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model is None async def test_generic_groups_claim_key_with_allowed_groups_and_admin_groups( @@ -168,9 +166,9 @@ async def test_generic_groups_claim_key_with_allowed_groups_and_admin_groups( handler = generic_client.handler_for_user( user_model('wash', alternate_username='zoe', groups=['user', 'administrator']) ) - user_info = await authenticator.authenticate(handler) - assert user_info['name'] == 'wash' - assert user_info['admin'] is True + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model['name'] == 'wash' + assert auth_model['admin'] is True async def test_generic_groups_claim_key_with_allowed_groups_and_admin_groups_not_admin( @@ -185,9 +183,30 @@ async def test_generic_groups_claim_key_with_allowed_groups_and_admin_groups_not handler = generic_client.handler_for_user( user_model('wash', alternate_username='zoe', groups=['user']) ) - user_info = await authenticator.authenticate(handler) - assert user_info['name'] == 'wash' - assert user_info['admin'] is False + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model['name'] == 'wash' + assert auth_model['admin'] is False + + +async def test_generic_groups_claim_key_with_allowed_groups_and_no_admin_groups_but_admin_users( + get_authenticator, generic_client +): + authenticator = get_authenticator( + scope=['openid', 'profile', 'roles'], + claim_groups_key='groups', + allowed_groups=['user'], + admin_groups=[], + admin_users=['wash'], + ) + handler = generic_client.handler_for_user( + user_model('wash', alternate_username='zoe', groups=['user']) + ) + + # Assert that the authenticated user is actually an admin due to being listed in `admin_users` + # Even though admin_groups is empty + auth_model = await authenticator.get_authenticated_user(handler, data=None) + assert auth_model['name'] == 'wash' + assert auth_model['admin'] is True async def test_generic_callable_groups_claim_key_with_allowed_groups_and_admin_groups( @@ -207,6 +226,6 @@ async def test_generic_callable_groups_claim_key_with_allowed_groups_and_admin_g policies={'roles': ['user', 'administrator']}, ) ) - user_info = await authenticator.authenticate(handler) - assert user_info['name'] == 'zoe' - assert user_info['admin'] is True + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model['name'] == 'zoe' + assert auth_model['admin'] is True diff --git a/oauthenticator/tests/test_github.py b/oauthenticator/tests/test_github.py index 3468b00a..8fd2bbc7 100644 --- a/oauthenticator/tests/test_github.py +++ b/oauthenticator/tests/test_github.py @@ -5,7 +5,7 @@ from io import BytesIO from urllib.parse import parse_qs, urlparse -from pytest import fixture, mark +from pytest import fixture from tornado.httpclient import HTTPResponse from tornado.httputil import HTTPHeaders from traitlets.config import Config @@ -39,16 +39,15 @@ def github_client(client): async def test_github(github_client): authenticator = GitHubOAuthenticator() handler = github_client.handler_for_user(user_model('wash')) - user_info = await authenticator.authenticate(handler) - name = user_info['name'] - assert name == 'wash' - auth_state = user_info['auth_state'] + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model['name'] == 'wash' + auth_state = auth_model['auth_state'] assert 'access_token' in auth_state assert 'github_user' in auth_state assert auth_state["github_user"] == { 'email': 'dinosaurs@space', 'id': 5, - 'login': name, + 'login': auth_model['name'], 'name': 'Hoban Washburn', } @@ -157,60 +156,43 @@ def team_membership(request): authenticator.allowed_organizations = ['blue'] handler = client.handler_for_user(user_model('caboose')) - user = await authenticator.authenticate(handler) - assert user['name'] == 'caboose' + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model['name'] == 'caboose' handler = client.handler_for_user(user_model('donut')) - user = await authenticator.authenticate(handler) - assert user is None + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model is None # reverse it, just to be safe authenticator.allowed_organizations = ['red'] handler = client.handler_for_user(user_model('caboose')) - user = await authenticator.authenticate(handler) - assert user is None + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model is None handler = client.handler_for_user(user_model('donut')) - user = await authenticator.authenticate(handler) - assert user['name'] == 'donut' + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model['name'] == 'donut' # test team membership authenticator.allowed_organizations = ['blue:alpha', 'red'] handler = client.handler_for_user(user_model('tucker')) - user = await authenticator.authenticate(handler) - assert user['name'] == 'tucker' + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model['name'] == 'tucker' handler = client.handler_for_user(user_model('grif')) - user = await authenticator.authenticate(handler) - assert user['name'] == 'grif' + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model['name'] == 'grif' handler = client.handler_for_user(user_model('texas')) - user = await authenticator.authenticate(handler) - assert user is None + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model is None client_hosts.pop() client_hosts.pop() -@mark.parametrize( - "org, username, expected", - [ - ("blue", "texas", "https://api.github.com/orgs/blue/members/texas"), - ( - "blue:alpha", - "tucker", - "https://api.github.com/orgs/blue/teams/alpha/members/tucker", - ), - ("red", "grif", "https://api.github.com/orgs/red/members/grif"), - ], -) -async def test_build_check_membership_url(org, username, expected): - output = GitHubOAuthenticator()._build_check_membership_url(org, username) - assert output == expected - - async def test_deprecated_config(caplog): cfg = Config() cfg.GitHubOAuthenticator.github_organization_whitelist = ["jupy"] diff --git a/oauthenticator/tests/test_gitlab.py b/oauthenticator/tests/test_gitlab.py index aa4845e3..044d5764 100644 --- a/oauthenticator/tests/test_gitlab.py +++ b/oauthenticator/tests/test_gitlab.py @@ -59,11 +59,10 @@ async def test_gitlab(gitlab_client): authenticator = GitLabOAuthenticator() mock_api_version(gitlab_client, '12.3.1-ee') handler = gitlab_client.handler_for_user(user_model('wash')) - user_info = await authenticator.authenticate(handler) - assert sorted(user_info) == ['auth_state', 'name'] - name = user_info['name'] - assert name == 'wash' - auth_state = user_info['auth_state'] + auth_model = await authenticator.get_authenticated_user(handler, None) + assert sorted(auth_model) == ['admin', 'auth_state', 'name'] + assert auth_model['name'] == 'wash' + auth_state = auth_model['auth_state'] assert 'access_token' in auth_state assert 'gitlab_user' in auth_state @@ -149,34 +148,31 @@ def groups_paginated(user, page, urlinfo, response): authenticator.allowed_gitlab_groups = ['blue'] handler = client.handler_for_user(group_user_model('caboose')) - user_info = await authenticator.authenticate(handler) - name = user_info['name'] - assert name == 'caboose' + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model['name'] == 'caboose' handler = client.handler_for_user(group_user_model('burns', is_admin=True)) - user_info = await authenticator.authenticate(handler) - name = user_info['name'] - assert name == 'burns' + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model['name'] == 'burns' handler = client.handler_for_user(group_user_model('grif')) - name = await authenticator.authenticate(handler) - assert name is None + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model is None handler = client.handler_for_user(group_user_model('simmons', is_admin=True)) - name = await authenticator.authenticate(handler) - assert name is None + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model is None # reverse it, just to be safe authenticator.allowed_gitlab_groups = ['red'] handler = client.handler_for_user(group_user_model('caboose')) - name = await authenticator.authenticate(handler) - assert name is None + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model is None handler = client.handler_for_user(group_user_model('grif')) - user_info = await authenticator.authenticate(handler) - name = user_info['name'] - assert name == 'grif' + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model['name'] == 'grif' client.hosts['gitlab.com'].pop() @@ -237,33 +233,33 @@ def is_member(request): # Forbidden since John has guest access handler = client.handler_for_user(john_user_model) - user_info = await authenticator.authenticate(handler) - assert user_info is None + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model is None # Authenticated since Harry has developer access to the project handler = client.handler_for_user(harry_user_model) - user_info = await authenticator.authenticate(handler) - name = user_info['name'] + auth_model = await authenticator.get_authenticated_user(handler, None) + name = auth_model['name'] assert name == 'harry' # Forbidden since Sheila doesn't have access to the project handler = client.handler_for_user(sheila_user_model) - user_info = await authenticator.authenticate(handler) - assert user_info is None + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model is None authenticator.allowed_project_ids = [123123152543] # Forbidden since the project does not exist. handler = client.handler_for_user(harry_user_model) - user_info = await authenticator.authenticate(handler) - assert user_info is None + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model is None authenticator.allowed_project_ids = [123123152543, 1231231] # Authenticated since Harry has developer access to one of the project in the list handler = client.handler_for_user(harry_user_model) - user_info = await authenticator.authenticate(handler) - name = user_info['name'] + auth_model = await authenticator.get_authenticated_user(handler, None) + name = auth_model['name'] assert name == 'harry' diff --git a/oauthenticator/tests/test_globus.py b/oauthenticator/tests/test_globus.py index 5a2bd397..8560e0b0 100644 --- a/oauthenticator/tests/test_globus.py +++ b/oauthenticator/tests/test_globus.py @@ -187,9 +187,9 @@ async def save_auth_state(self, state): async def test_globus(globus_client): authenticator = GlobusOAuthenticator() handler = globus_client.handler_for_user(user_model('wash@uflightacademy.edu')) - data = await authenticator.authenticate(handler) - assert data['name'] == 'wash' - tokens = list(data['auth_state']['tokens'].keys()) + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model['name'] == 'wash' + tokens = list(auth_model['auth_state']['tokens'].keys()) assert tokens == ['transfer.api.globus.org'] @@ -229,7 +229,7 @@ async def test_restricted_domain(globus_client): authenticator.identity_provider = 'alliance.gov' handler = globus_client.handler_for_user(user_model('wash@uflightacademy.edu')) with raises(web.HTTPError) as exc: - await authenticator.authenticate(handler) + await authenticator.get_authenticated_user(handler, None) assert exc.value.status_code == 403 @@ -239,8 +239,8 @@ async def test_namespaced_domain(globus_client): authenticator.identity_provider = '' um = user_model('wash@legitshipping.com@serenity.com') handler = globus_client.handler_for_user(um) - data = await authenticator.authenticate(handler) - assert data['name'] == 'wash' + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model['name'] == 'wash' async def test_username_from_email(globus_client): @@ -250,8 +250,8 @@ async def test_username_from_email(globus_client): authenticator.username_from_email = True um = user_model('wash@legitshipping.com@serenity.com', 'alan@tudyk.org') handler = globus_client.handler_for_user(um) - data = await authenticator.authenticate(handler) - assert data['name'] == 'alan' + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model['name'] == 'alan' async def test_username_not_from_email(globus_client): @@ -260,8 +260,8 @@ async def test_username_not_from_email(globus_client): authenticator.identity_provider = '' um = user_model('wash@legitshipping.com@serenity.com', 'alan@tudyk.org') handler = globus_client.handler_for_user(um) - data = await authenticator.authenticate(handler) - assert data['name'] == 'wash' + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model['name'] == 'wash' async def test_email_scope_added(globus_client): @@ -282,8 +282,8 @@ async def test_username_from_email_restricted_pass(globus_client): authenticator.username_from_email = True um = user_model('wash@serenity.com', 'alan@serenity.com') handler = globus_client.handler_for_user(um) - data = await authenticator.authenticate(handler) - assert data['name'] == 'alan' + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model['name'] == 'alan' async def test_username_from_email_restricted_fail(globus_client): @@ -294,7 +294,7 @@ async def test_username_from_email_restricted_fail(globus_client): um = user_model('wash@serenity.com', 'alan@tudyk.org') handler = globus_client.handler_for_user(um) with raises(web.HTTPError) as exc: - await authenticator.authenticate(handler) + await authenticator.get_authenticated_user(handler, None) assert exc.value.status_code == 403 @@ -306,9 +306,9 @@ async def test_token_exclusion(globus_client): 'groups.api.globus.org', ] handler = globus_client.handler_for_user(user_model('wash@uflightacademy.edu')) - data = await authenticator.authenticate(handler) - assert data['name'] == 'wash' - assert list(data['auth_state']['tokens'].keys()) == [] + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model['name'] == 'wash' + assert list(auth_model['auth_state']['tokens'].keys()) == [] async def test_revoke_tokens(globus_client, mock_globus_user): @@ -395,7 +395,7 @@ async def test_logout_revokes_tokens(globus_client, monkeypatch, mock_globus_use async def test_group_scope_added(globus_client): authenticator = GlobusOAuthenticator() - authenticator.allowed_globus_groups = set({'21c6bc5d-fc12-4f60-b999-76766cd596c2'}) + authenticator.allowed_globus_groups = {'21c6bc5d-fc12-4f60-b999-76766cd596c2'} assert authenticator.scope == [ 'openid', 'profile', @@ -406,34 +406,34 @@ async def test_group_scope_added(globus_client): async def test_user_in_allowed_group(globus_client): authenticator = GlobusOAuthenticator() - authenticator.allowed_globus_groups = set({'21c6bc5d-fc12-4f60-b999-76766cd596c2'}) + authenticator.allowed_globus_groups = {'21c6bc5d-fc12-4f60-b999-76766cd596c2'} handler = globus_client.handler_for_user(user_model('wash@uflightacademy.edu')) - data = await authenticator.authenticate(handler) - assert data['name'] == 'wash' + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model['name'] == 'wash' async def test_user_not_allowed(globus_client): authenticator = GlobusOAuthenticator() - authenticator.allowed_globus_groups = set({'3f1f85c4-f084-4173-9efb-7c7e0b44291a'}) + authenticator.allowed_globus_groups = {'3f1f85c4-f084-4173-9efb-7c7e0b44291a'} handler = globus_client.handler_for_user(user_model('wash@uflightacademy.edu')) - data = await authenticator.authenticate(handler) - assert data == None + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model == None async def test_user_is_admin(globus_client): authenticator = GlobusOAuthenticator() - authenticator.admin_globus_groups = set({'21c6bc5d-fc12-4f60-b999-76766cd596c2'}) + authenticator.admin_globus_groups = {'21c6bc5d-fc12-4f60-b999-76766cd596c2'} handler = globus_client.handler_for_user(user_model('wash@uflightacademy.edu')) - data = await authenticator.authenticate(handler) - assert data['name'] == 'wash' - assert data['admin'] == True + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model['name'] == 'wash' + assert auth_model['admin'] == True async def test_user_allowed_not_admin(globus_client): authenticator = GlobusOAuthenticator() - authenticator.allowed_globus_groups = set({'21c6bc5d-fc12-4f60-b999-76766cd596c2'}) - authenticator.admin_globus_groups = set({'3f1f85c4-f084-4173-9efb-7c7e0b44291a'}) + authenticator.allowed_globus_groups = {'21c6bc5d-fc12-4f60-b999-76766cd596c2'} + authenticator.admin_globus_groups = {'3f1f85c4-f084-4173-9efb-7c7e0b44291a'} handler = globus_client.handler_for_user(user_model('wash@uflightacademy.edu')) - data = await authenticator.authenticate(handler) - assert data['name'] == 'wash' - assert data['admin'] == False + auth_model = await authenticator.get_authenticated_user(handler, None) + assert auth_model['name'] == 'wash' + assert auth_model['admin'] == False diff --git a/oauthenticator/tests/test_google.py b/oauthenticator/tests/test_google.py index 7ed1c697..6aa722c7 100644 --- a/oauthenticator/tests/test_google.py +++ b/oauthenticator/tests/test_google.py @@ -1,6 +1,7 @@ import hashlib import logging import re +from unittest import mock from pytest import fixture, raises from tornado.web import HTTPError @@ -34,13 +35,18 @@ def google_client(client): async def test_google(google_client): authenticator = GoogleOAuthenticator() handler = google_client.handler_for_user(user_model('fake@email.com')) - user_info = await authenticator.authenticate(handler) - assert sorted(user_info) == ['auth_state', 'name'] - name = user_info['name'] - assert name == 'fake@email.com' - auth_state = user_info['auth_state'] - assert 'access_token' in auth_state - assert 'google_user' in auth_state + with mock.patch.object( + authenticator, + '_fetch_user_groups', + lambda *args: {'anotherone', 'fakeadmingroup'}, + ): + user_info = await authenticator.get_authenticated_user(handler, None) + assert sorted(user_info) == ['admin', 'auth_state', 'name'] + name = user_info['name'] + assert name == 'fake@email.com' + auth_state = user_info['auth_state'] + assert 'access_token' in auth_state + assert 'google_user' in auth_state async def test_google_username_claim(google_client): @@ -48,41 +54,56 @@ async def test_google_username_claim(google_client): cfg.GoogleOAuthenticator.username_claim = "sub" authenticator = GoogleOAuthenticator(config=cfg) handler = google_client.handler_for_user(user_model('fake@email.com')) - user_info = await authenticator.authenticate(handler) - assert sorted(user_info) == ['auth_state', 'name'] - name = user_info['name'] - assert name == '724f95667e2fbe903ee1b4cffcae3b25' + with mock.patch.object( + authenticator, + '_fetch_user_groups', + lambda *args: {'anotherone', 'fakeadmingroup'}, + ): + user_info = await authenticator.get_authenticated_user(handler, None) + assert sorted(user_info) == ['admin', 'auth_state', 'name'] + name = user_info['name'] + assert name == '724f95667e2fbe903ee1b4cffcae3b25' async def test_hosted_domain(google_client): authenticator = GoogleOAuthenticator(hosted_domain=['email.com']) handler = google_client.handler_for_user(user_model('fake@email.com')) - user_info = await authenticator.authenticate(handler) - name = user_info['name'] - assert name == 'fake' + with mock.patch.object( + authenticator, + '_fetch_user_groups', + lambda *args: {'anotherone', 'fakeadmingroup'}, + ): + user_info = await authenticator.get_authenticated_user(handler, None) + name = user_info['name'] + assert name == 'fake@email.com' - handler = google_client.handler_for_user(user_model('notallowed@notemail.com')) - with raises(HTTPError) as exc: - name = await authenticator.authenticate(handler) - assert exc.value.status_code == 403 + handler = google_client.handler_for_user(user_model('notallowed@notemail.com')) + with raises(HTTPError) as exc: + name = await authenticator.get_authenticated_user(handler, None) + assert exc.value.status_code == 403 async def test_multiple_hosted_domain(google_client): authenticator = GoogleOAuthenticator(hosted_domain=['email.com', 'mycollege.edu']) handler = google_client.handler_for_user(user_model('fake@email.com')) - user_info = await authenticator.authenticate(handler) - name = user_info['name'] - assert name == 'fake@email.com' - - handler = google_client.handler_for_user(user_model('fake2@mycollege.edu')) - user_info = await authenticator.authenticate(handler) - name = user_info['name'] - assert name == 'fake2@mycollege.edu' - - handler = google_client.handler_for_user(user_model('notallowed@notemail.com')) - with raises(HTTPError) as exc: - name = await authenticator.authenticate(handler) - assert exc.value.status_code == 403 + with mock.patch.object( + authenticator, + '_fetch_user_groups', + lambda *args: {'anotherone', 'fakeadmingroup'}, + ): + user_info = await authenticator.get_authenticated_user(handler, None) + name = user_info['name'] + assert name == 'fake@email.com' + + handler = google_client.handler_for_user(user_model('fake2@mycollege.edu')) + user_info = await authenticator.get_authenticated_user(handler, None) + name = user_info['name'] + assert name == 'fake2@mycollege.edu' + + handler = google_client.handler_for_user(user_model('notallowed@notemail.com')) + with raises(HTTPError) as exc: + name = await authenticator.get_authenticated_user(handler, None) + assert exc.value.status_code == 403 async def test_admin_google_groups(google_client): @@ -92,26 +113,56 @@ async def test_admin_google_groups(google_client): allowed_google_groups={'email.com': ['fakegroup']}, ) handler = google_client.handler_for_user(user_model('fakeadmin@email.com')) - admin_user_info = await authenticator.authenticate( - handler, google_groups=['anotherone', 'fakeadmingroup'] - ) - admin_user = admin_user_info['admin'] - assert admin_user == True + with mock.patch.object( + authenticator, + '_fetch_user_groups', + lambda *args: {'anotherone', 'fakeadmingroup'}, + ): + admin_user_info = await authenticator.get_authenticated_user(handler, None) + # Make sure the user authenticated successfully + assert admin_user_info + # Assert that the user is an admin + assert admin_user_info.get('admin', None) == True handler = google_client.handler_for_user(user_model('fakealloweduser@email.com')) - allowed_user_info = await authenticator.authenticate( - handler, google_groups=['anotherone', 'fakegroup'] - ) - allowed_user_groups = allowed_user_info['auth_state']['google_user'][ - 'google_groups' - ] - admin_user = allowed_user_info['admin'] - assert 'fakegroup' in allowed_user_groups - assert admin_user == False + with mock.patch.object( + authenticator, + '_fetch_user_groups', + lambda *args: {'anotherone', 'fakegroup'}, + ): + allowed_user_info = await authenticator.get_authenticated_user(handler, None) + allowed_user_groups = allowed_user_info['auth_state']['google_user'][ + 'google_groups' + ] + admin_user = allowed_user_info['admin'] + assert 'fakegroup' in allowed_user_groups + assert admin_user is False handler = google_client.handler_for_user(user_model('fakenonalloweduser@email.com')) - allowed_user_groups = await authenticator.authenticate( - handler, google_groups=['anotherone', 'fakenonallowedgroup'] + with mock.patch.object( + authenticator, + '_fetch_user_groups', + lambda *args: {'anotherone', 'fakenonallowedgroup'}, + ): + allowed_user_groups = await authenticator.get_authenticated_user(handler, None) + assert allowed_user_groups is None + + +async def test_admin_user_but_no_admin_google_groups(google_client): + authenticator = GoogleOAuthenticator( + hosted_domain=['email.com', 'mycollege.edu'], + allowed_google_groups={'email.com': ['fakegroup']}, + admin_users=['fakeadmin@email.com'], ) - assert allowed_user_groups is None + handler = google_client.handler_for_user(user_model('fakeadmin@email.com')) + with mock.patch.object( + authenticator, + '_fetch_user_groups', + lambda *args: {'anotherone', 'fakegroup'}, + ): + admin_user_info = await authenticator.get_authenticated_user(handler, data=None) + # Make sure the user authenticated successfully + assert admin_user_info + # Assert that the user is an admin + assert admin_user_info.get('admin', None) == True async def test_allowed_google_groups(google_client): @@ -120,30 +171,40 @@ async def test_allowed_google_groups(google_client): allowed_google_groups={'email.com': ['fakegroup'], ',mycollege.edu': []}, ) handler = google_client.handler_for_user(user_model('fakeadmin@email.com')) - admin_user_info = await authenticator.authenticate( - handler, google_groups=['anotherone', 'fakeadmingroup'] - ) - assert admin_user_info is None + with mock.patch.object( + authenticator, + '_fetch_user_groups', + lambda *args: {'anotherone', 'fakeadmingroup'}, + ): + admin_user_info = await authenticator.get_authenticated_user(handler, None) + assert admin_user_info is None handler = google_client.handler_for_user(user_model('fakealloweduser@email.com')) - allowed_user_info = await authenticator.authenticate( - handler, google_groups=['anotherone', 'fakegroup'] - ) - allowed_user_groups = allowed_user_info['auth_state']['google_user'][ - 'google_groups' - ] - admin_field = allowed_user_info.get('admin') - assert 'fakegroup' in allowed_user_groups - assert admin_field is None + with mock.patch.object( + authenticator, + '_fetch_user_groups', + lambda *args: {'anotherone', 'fakegroup'}, + ): + allowed_user_info = await authenticator.get_authenticated_user(handler, None) + allowed_user_groups = allowed_user_info['auth_state']['google_user'][ + 'google_groups' + ] + admin_field = allowed_user_info.get('admin') + assert 'fakegroup' in allowed_user_groups + assert admin_field is None handler = google_client.handler_for_user(user_model('fakenonalloweduser@email.com')) - allowed_user_groups = await authenticator.authenticate( - handler, google_groups=['anotherone', 'fakenonallowedgroup'] - ) - assert allowed_user_groups is None + with mock.patch.object( + authenticator, + '_fetch_user_groups', + lambda *args: {'anotherone', 'fakenonallowedgroup'}, + ): + allowed_user_groups = await authenticator.get_authenticated_user(handler, None) + assert allowed_user_groups is None handler = google_client.handler_for_user(user_model('fake@mycollege.edu')) - allowed_user_groups = await authenticator.authenticate( - handler, google_groups=['fakegroup'] - ) - assert allowed_user_groups is None + with mock.patch.object( + authenticator, '_fetch_user_groups', lambda *args: {'fakegroup'} + ): + allowed_user_groups = await authenticator.get_authenticated_user(handler, None) + assert allowed_user_groups is None async def test_admin_only_google_groups(google_client): @@ -152,11 +213,14 @@ async def test_admin_only_google_groups(google_client): admin_google_groups={'email.com': ['fakeadmingroup']}, ) handler = google_client.handler_for_user(user_model('fakeadmin@email.com')) - admin_user_info = await authenticator.authenticate( - handler, google_groups=['anotherone', 'fakeadmingroup'] - ) - admin_user = admin_user_info['admin'] - assert admin_user is True + with mock.patch.object( + authenticator, + '_fetch_user_groups', + lambda *args: {'anotherone', 'fakeadmingroup'}, + ): + admin_user_info = await authenticator.get_authenticated_user(handler, None) + admin_user = admin_user_info['admin'] + assert admin_user is True def test_deprecated_config(caplog): @@ -173,5 +237,5 @@ def test_deprecated_config(caplog): 'GoogleOAuthenticator.allowed_google_groups instead', ) in caplog.record_tuples - assert authenticator.allowed_google_groups == {'email.com': ['group']} + assert authenticator.allowed_google_groups == {'email.com': {'group'}} assert authenticator.allowed_users == {"user1"} diff --git a/oauthenticator/tests/test_mediawiki.py b/oauthenticator/tests/test_mediawiki.py index 75931ed4..2fe751a4 100644 --- a/oauthenticator/tests/test_mediawiki.py +++ b/oauthenticator/tests/test_mediawiki.py @@ -60,7 +60,7 @@ async def test_mediawiki(mediawiki): request=Mock(query='oauth_token=key&oauth_verifier=me'), find_user=Mock(return_value=None), ) - user = await authenticator.authenticate(handler) + user = await authenticator.get_authenticated_user(handler, None) assert user['name'] == 'wash' auth_state = user['auth_state'] assert auth_state['ACCESS_TOKEN_KEY'] == 'key' diff --git a/oauthenticator/tests/test_okpy.py b/oauthenticator/tests/test_okpy.py index 70235b9d..2452af08 100644 --- a/oauthenticator/tests/test_okpy.py +++ b/oauthenticator/tests/test_okpy.py @@ -26,8 +26,8 @@ def okpy_client(client): async def test_okpy(okpy_client): authenticator = OkpyOAuthenticator() handler = okpy_client.handler_for_user(user_model('testing@example.com')) - user_info = await authenticator.authenticate(handler) - assert sorted(user_info) == ['auth_state', 'name'] + user_info = await authenticator.get_authenticated_user(handler, None) + assert sorted(user_info) == ['admin', 'auth_state', 'name'] name = user_info['name'] assert name == 'testing@example.com' auth_state = user_info['auth_state'] diff --git a/oauthenticator/tests/test_openshift.py b/oauthenticator/tests/test_openshift.py index c5d8e5e9..7ccc6587 100644 --- a/oauthenticator/tests/test_openshift.py +++ b/oauthenticator/tests/test_openshift.py @@ -27,8 +27,8 @@ async def test_openshift(openshift_client): authenticator = OpenShiftOAuthenticator() authenticator.openshift_auth_api_url = "https://openshift.default.svc.cluster.local" handler = openshift_client.handler_for_user(user_model('wash')) - user_info = await authenticator.authenticate(handler) - assert sorted(user_info) == ['auth_state', 'name'] + user_info = await authenticator.get_authenticated_user(handler, None) + assert sorted(user_info) == ['admin', 'auth_state', 'name'] name = user_info['name'] assert name == 'wash' auth_state = user_info['auth_state'] @@ -41,8 +41,8 @@ async def test_openshift_allowed_groups(openshift_client): authenticator.allowed_groups = {'group1'} authenticator.openshift_auth_api_url = "https://openshift.default.svc.cluster.local" handler = openshift_client.handler_for_user(user_model('wash')) - user_info = await authenticator.authenticate(handler) - assert sorted(user_info) == ['auth_state', 'name'] + user_info = await authenticator.get_authenticated_user(handler, None) + assert sorted(user_info) == ['admin', 'auth_state', 'name'] name = user_info['name'] assert name == 'wash' auth_state = user_info['auth_state'] @@ -57,7 +57,7 @@ async def test_openshift_not_in_allowed_groups(openshift_client): authenticator.allowed_groups = {'group3'} authenticator.openshift_auth_api_url = "https://openshift.default.svc.cluster.local" handler = openshift_client.handler_for_user(user_model('wash')) - user_info = await authenticator.authenticate(handler) + user_info = await authenticator.get_authenticated_user(handler, None) assert user_info == None @@ -67,7 +67,7 @@ async def test_openshift_not_in_allowed_groups_but_is_admin(openshift_client): authenticator.admin_groups = {'group1'} authenticator.openshift_auth_api_url = "https://openshift.default.svc.cluster.local" handler = openshift_client.handler_for_user(user_model('wash')) - user_info = await authenticator.authenticate(handler) + user_info = await authenticator.get_authenticated_user(handler, None) assert sorted(user_info) == ['admin', 'auth_state', 'name'] assert user_info['admin'] == True @@ -78,7 +78,7 @@ async def test_openshift_in_allowed_groups_and_is_admin(openshift_client): authenticator.admin_groups = {'group1'} authenticator.openshift_auth_api_url = "https://openshift.default.svc.cluster.local" handler = openshift_client.handler_for_user(user_model('wash')) - user_info = await authenticator.authenticate(handler) + user_info = await authenticator.get_authenticated_user(handler, None) assert sorted(user_info) == ['admin', 'auth_state', 'name'] assert user_info['admin'] == True @@ -89,6 +89,17 @@ async def test_openshift_in_allowed_groups_and_is_not_admin(openshift_client): authenticator.admin_groups = {'group3'} authenticator.openshift_auth_api_url = "https://openshift.default.svc.cluster.local" handler = openshift_client.handler_for_user(user_model('wash')) - user_info = await authenticator.authenticate(handler) + user_info = await authenticator.get_authenticated_user(handler, None) assert sorted(user_info) == ['admin', 'auth_state', 'name'] assert user_info['admin'] == False + + +async def test_openshift_not_in_admin_users_but_not_in_admin_groups(openshift_client): + authenticator = OpenShiftOAuthenticator() + authenticator.allowed_groups = {'group1'} + authenticator.admin_users = ['wash'] + authenticator.openshift_auth_api_url = "https://openshift.default.svc.cluster.local" + handler = openshift_client.handler_for_user(user_model('wash')) + user_info = await authenticator.get_authenticated_user(handler, data=None) + assert sorted(user_info) == ['admin', 'auth_state', 'name'] + assert user_info['admin'] == True