From 682d6597e4c6d4eef8e0ed0781e91be21fa81e1c Mon Sep 17 00:00:00 2001 From: Everton Seiei Arakaki Date: Mon, 20 Nov 2023 06:03:39 +0000 Subject: [PATCH] Fix an issue where OAUTH_ADDITIONAL_CLAIMS does not recognise AzureAD with > 150 groups. #6835 --- docs/en_US/oauth2.rst | 4 ++-- web/config.py | 9 +++------ web/pgadmin/authenticate/oauth2.py | 31 ++++++++++++++++++++---------- 3 files changed, 26 insertions(+), 18 deletions(-) diff --git a/docs/en_US/oauth2.rst b/docs/en_US/oauth2.rst index cf39539b3..54895cc15 100644 --- a/docs/en_US/oauth2.rst +++ b/docs/en_US/oauth2.rst @@ -43,8 +43,8 @@ and modify the values for the following parameters: "OAUTH2_AUTO_CREATE_USER", "Set the value to *True* if you want to automatically create a pgAdmin user corresponding to a successfully authenticated Oauth2 user. Please note that password is not stored in the pgAdmin database." - "OAUTH2_ADDITIONAL_CLAIMS", "If a dictionary is provided, pgAdmin will check for a matching key and value on the user profile. - In case the profile does not have any match with the provided config, the user will receive an authorization error. + "OAUTH2_ADDITIONAL_CLAIMS", "If a dictionary is provided, pgAdmin will check for a matching key and value on the userinfo endpoint + and in the Id Token. In case there is no match with the provided config, the user will receive an authorization error. Useful for checking AzureAD_ *wids* or *groups*, GitLab_ *owner*, *maintainer* and *reporter* claims." Redirect URL diff --git a/web/config.py b/web/config.py index d20ad5cbc..2b939fba5 100644 --- a/web/config.py +++ b/web/config.py @@ -787,8 +787,9 @@ OAUTH2_CONFIG = [ 'OAUTH2_ICON': None, # UI button colour, ex: #0000ff 'OAUTH2_BUTTON_COLOR': None, - # The additional claims to check on user ID Token. This is useful to - # provide additional authorization checks before allowing access. + # The additional claims to check on user ID Token or Userinfo response. + # This is useful to provide additional authorization checks + # before allowing access. # Example for GitLab: allowing all maintainers teams, and a specific # developers group to access pgadmin: # 'OAUTH2_ADDITIONAL_CLAIMS': { @@ -806,10 +807,6 @@ OAUTH2_CONFIG = [ # 'groups': ["0760b6cf-170e-4a14-91b3-4b78e0739963"], # 'wids': ["cf1c38e5-3621-4004-a7cb-879624dced7c"], # } - # Example for any key value string check: - # 'OAUTH2_ADDITIONAL_CLAIMS': { - # 'group': "0760b6cf-170e-4a14-91b3-4b78e0739963", - # } 'OAUTH2_ADDITIONAL_CLAIMS': None, } ] diff --git a/web/pgadmin/authenticate/oauth2.py b/web/pgadmin/authenticate/oauth2.py index 8637754fb..d1b6113aa 100644 --- a/web/pgadmin/authenticate/oauth2.py +++ b/web/pgadmin/authenticate/oauth2.py @@ -122,7 +122,6 @@ class OAuth2Authentication(BaseAuthentication): def login(self, form): profile = self.get_user_profile() - current_app.logger.warning(profile) email_key = \ [value for value in self.email_keys if value in profile.keys()] email = profile[email_key[0]] if (len(email_key) > 0) else None @@ -155,20 +154,32 @@ class OAuth2Authentication(BaseAuthentication): additinal_claims = None if 'OAUTH2_ADDITIONAL_CLAIMS' in self.oauth2_config[ self.oauth2_current_client]: + additinal_claims = self.oauth2_config[ self.oauth2_current_client ]['OAUTH2_ADDITIONAL_CLAIMS'] - (valid, reason) = self.__is_additional_claims_valid(profile, - additinal_claims) + # checking oauth provider userinfo response + valid_profile, reason = self.__is_any_claim_valid(profile, + additinal_claims) + current_app.logger.debug(f"profile claims: {profile}") + current_app.logger.debug(f"reason: {reason}") - if not valid: + # checking oauth provider idtoken claims + id_token_claims = session.get('oauth2_token', {}).get('userinfo',{}) + valid_idtoken, reason = self.__is_any_claim_valid(id_token_claims, + additinal_claims) + current_app.logger.debug(f"idtoken claims: {id_token_claims}") + current_app.logger.debug(f"reason: {reason}") + + if not valid_profile and not valid_idtoken: return_msg = "The user is not authorized to login" \ - " based on the claims in the profile." \ + " based on your identity profile." \ " Please contact your administrator." audit_msg = f"The authenticated user {username} is not" \ " authorized to access pgAdmin based on OAUTH2 config. " \ - f"Reason: {reason}" + f"Reason: additional claim required {additinal_claims}, " \ + f"profile claims {profile}, idtoken cliams {id_token_claims}." current_app.logger.warning(audit_msg) return False, return_msg @@ -226,7 +237,7 @@ class OAuth2Authentication(BaseAuthentication): return True, {'username': username} - def __is_additional_claims_valid(self, profile, additional_claims): + def __is_any_claim_valid(self, identity, additional_claims): if additional_claims is None: reason = "Additional claim config is None, no check to do." return (True, reason) @@ -237,7 +248,7 @@ class OAuth2Authentication(BaseAuthentication): reason = "Additional claim check config dict is empty." return (False, reason) for key in additional_claims.keys(): - claim = profile.get(key) + claim = identity.get(key) if claim is None: continue if not isinstance(claim, list): @@ -246,7 +257,7 @@ class OAuth2Authentication(BaseAuthentication): if not isinstance(authorized_claims, list): authorized_claims = [authorized_claims] if any(item in authorized_claims for item in claim): - reason = "Claim match found. Authorizing" + reason = "Claim match found. Authorized access." return (True, reason) - reason = f"Profile does not have any of given additional claims." + reason = f"No match was found." return (False, reason)