Fix an issue where OAUTH_ADDITIONAL_CLAIMS does not recognise AzureAD with > 150 groups. #6835

pull/6985/head
Everton Seiei Arakaki 2023-11-20 06:03:39 +00:00 committed by GitHub
parent a59372cbe3
commit 682d6597e4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 26 additions and 18 deletions

View File

@ -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 "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. create a pgAdmin user corresponding to a successfully authenticated Oauth2 user.
Please note that password is not stored in the pgAdmin database." 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. "OAUTH2_ADDITIONAL_CLAIMS", "If a dictionary is provided, pgAdmin will check for a matching key and value on the userinfo endpoint
In case the profile does not have any match with the provided config, the user will receive an authorization error. 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." Useful for checking AzureAD_ *wids* or *groups*, GitLab_ *owner*, *maintainer* and *reporter* claims."
Redirect URL Redirect URL

View File

@ -787,8 +787,9 @@ OAUTH2_CONFIG = [
'OAUTH2_ICON': None, 'OAUTH2_ICON': None,
# UI button colour, ex: #0000ff # UI button colour, ex: #0000ff
'OAUTH2_BUTTON_COLOR': None, 'OAUTH2_BUTTON_COLOR': None,
# The additional claims to check on user ID Token. This is useful to # The additional claims to check on user ID Token or Userinfo response.
# provide additional authorization checks before allowing access. # This is useful to provide additional authorization checks
# before allowing access.
# Example for GitLab: allowing all maintainers teams, and a specific # Example for GitLab: allowing all maintainers teams, and a specific
# developers group to access pgadmin: # developers group to access pgadmin:
# 'OAUTH2_ADDITIONAL_CLAIMS': { # 'OAUTH2_ADDITIONAL_CLAIMS': {
@ -806,10 +807,6 @@ OAUTH2_CONFIG = [
# 'groups': ["0760b6cf-170e-4a14-91b3-4b78e0739963"], # 'groups': ["0760b6cf-170e-4a14-91b3-4b78e0739963"],
# 'wids': ["cf1c38e5-3621-4004-a7cb-879624dced7c"], # '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, 'OAUTH2_ADDITIONAL_CLAIMS': None,
} }
] ]

View File

@ -122,7 +122,6 @@ class OAuth2Authentication(BaseAuthentication):
def login(self, form): def login(self, form):
profile = self.get_user_profile() profile = self.get_user_profile()
current_app.logger.warning(profile)
email_key = \ email_key = \
[value for value in self.email_keys if value in profile.keys()] [value for value in self.email_keys if value in profile.keys()]
email = profile[email_key[0]] if (len(email_key) > 0) else None email = profile[email_key[0]] if (len(email_key) > 0) else None
@ -155,20 +154,32 @@ class OAuth2Authentication(BaseAuthentication):
additinal_claims = None additinal_claims = None
if 'OAUTH2_ADDITIONAL_CLAIMS' in self.oauth2_config[ if 'OAUTH2_ADDITIONAL_CLAIMS' in self.oauth2_config[
self.oauth2_current_client]: self.oauth2_current_client]:
additinal_claims = self.oauth2_config[ additinal_claims = self.oauth2_config[
self.oauth2_current_client self.oauth2_current_client
]['OAUTH2_ADDITIONAL_CLAIMS'] ]['OAUTH2_ADDITIONAL_CLAIMS']
(valid, reason) = self.__is_additional_claims_valid(profile, # checking oauth provider userinfo response
additinal_claims) 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" \ 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." " Please contact your administrator."
audit_msg = f"The authenticated user {username} is not" \ audit_msg = f"The authenticated user {username} is not" \
" authorized to access pgAdmin based on OAUTH2 config. " \ " 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) current_app.logger.warning(audit_msg)
return False, return_msg return False, return_msg
@ -226,7 +237,7 @@ class OAuth2Authentication(BaseAuthentication):
return True, {'username': username} 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: if additional_claims is None:
reason = "Additional claim config is None, no check to do." reason = "Additional claim config is None, no check to do."
return (True, reason) return (True, reason)
@ -237,7 +248,7 @@ class OAuth2Authentication(BaseAuthentication):
reason = "Additional claim check config dict is empty." reason = "Additional claim check config dict is empty."
return (False, reason) return (False, reason)
for key in additional_claims.keys(): for key in additional_claims.keys():
claim = profile.get(key) claim = identity.get(key)
if claim is None: if claim is None:
continue continue
if not isinstance(claim, list): if not isinstance(claim, list):
@ -246,7 +257,7 @@ class OAuth2Authentication(BaseAuthentication):
if not isinstance(authorized_claims, list): if not isinstance(authorized_claims, list):
authorized_claims = [authorized_claims] authorized_claims = [authorized_claims]
if any(item in authorized_claims for item in claim): if any(item in authorized_claims for item in claim):
reason = "Claim match found. Authorizing" reason = "Claim match found. Authorized access."
return (True, reason) return (True, reason)
reason = f"Profile does not have any of given additional claims." reason = f"No match was found."
return (False, reason) return (False, reason)