Closed n0-ware closed 12 months ago
Thank you for adding this to the sprint. Until that point I had still wondered if this was a configuration on our side of if we can look forward to a resolution. Any additional information I can add to help this resolve? We appreciate your involvement.
@n0-ware Can you please replace below file in your container and restart it. I have added login to get more information about 'profile' received from oauth2 server. Kindly share logs for login after replacing file.
docker cp <path to download location>/oauth2.py <container>:/pgadmin4/pgadmin/authenticate/oauth2.py
Would it be possible that you have more then 150 group on azure?
@yogeshmahajan-1903 I have replaced the file and restarted the container. I am still receiving the message in the error I posted that my user is not allowed to login based on the claims in my profile.
Which logs are you seeking? I do not see any log files in the container but I was able to extract the attached from Portainer _pgadmin-prod_logs.zip
@EvertonSA Yes, our organization has more than 150 groups in Azure
@n0-ware
2023-10-30 11:22:53,526: WARNING pgadmin: {'sub': '<str value>', 'name': '<str value>', 'family_name': '<str value>', 'given_name': '<str value>', 'picture': 'https://graph.microsoft.com/v1.0/me/photo/$value', 'email': '<str email>'}
2023-10-30 11:22:53,526: WARNING pgadmin: The authenticated user Eddie.Hartmann@dealerbuilt.com is not authorized to access pgAdmin based on OAUTH2 config. Reason: Profile does not have any of given additional claims.
AD is not sending "groups" parameter in AD profile what you have added in OAUTH2_ADDITIONAL_CLAIMS in OAUTH2_CONFIG. Hence there is NO match for your additional oauth2 claim parameter in your profile. There is something wrong with your AD configuration.
I will have our architects review the configuration. They state we have configured it the same as other successful group claim based OAuth2 apps in the environment, though it is always possible something was missed in particular to pgAdmin.
@n0-ware Unless pgAdmin 4 receives same key as mentioned in OAUTH2_ADDITIONAL_CLAIMS, pgAdmin will throw same error. Issue is NOT in pgAdmin code, hence closing the issue.
From Microsoft docs:
"When you request all groups in your token as shown in the above example, you can't rely on the token having the groups claim in your token. There are size limits on tokens and on groups claims so that they don't become too large. When the user is a member of too many groups, your app will need to get the user's group membership from Microsoft Graph."
"The limits for groups in a groups claim are:
200 groups for JWT tokens. ..."
@yogeshmahajan-1903 Based on the reply from @EvertonSA it looks like the problem is likely the volume of groups. Is there are method to tell pgAdmin to look for the hasgroups
field in the JWT and extract from there?
When I implemented this, I used a azure ad with 2 groups. At this moment, there is no hasgroup checking
Thank you for the clarity, Everton. I assume this issue will remain open then but under the context it works, but only with < 150 groups?
The feature was implemented with GitLab as the target oauth provider and Gitlab assures groups' claims. One of the requirements of the feature was to be provider agnostic. see here https://github.com/pgadmin-org/pgadmin4/pull/6550#issuecomment-1646687916
but only with < 150 groups?
the feature works as expected. "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."
maybe this can be follow up with a feature request to implement azure specifics, but I will have to leave this to pgadmin4 maintainers
@n0-ware, @EvertonSA, We have considered this as a feature request and we will work on it when we prioritise it.
I have the same issue too. I have spent half of the day trying to figure out why Azure AD is not sending groups (or roles).
The mentioned issue with 150 group limit should not be an issue because you can have just group assigned to an OAuth App sent. See:
According to the documentation this limit also applies only for tokens, but not for GraphQL endpoints. However, nor https://graph.microsoft.com/oidc/userinfo or https://graph.microsoft.com/v1.0/me endpoints return optional claims.
Did anyone actually test Azure AD with current implementation? Or was example configuration in the implementation PR just an assumption that it will work? Also, does pgAdmin4 expects groups in an ID token or in userinfo endpoint? Or both? I suspect that Azure sends optional claims just in the ID token, but I cannot verify that.
EDIT: At least looking at this old ticket and Azure Docs it seems that userinfo endpoint never had support for optional claims indeed.
When I implemented this, I used a azure ad with 2 groups. At this moment, there is no hasgroup checking
@EvertonSA can you share an exact Azure AD configuration for app registration, and pgAdmin OAuth configuration which was used?
Sorry, my setup was destroyed. This was the documentation https://learn.microsoft.com/en-us/security/zero-trust/develop/configure-tokens-group-claims-app-roles
@khushboovashi @EvertonSA @yogeshmahajan-1903 Thank you all for your time and input. I will await this with gusto, and as always, appreciate your time and effort index in helping resolve, troubleshoot, and accurately identify this issue as a feature request and not a bug or user error. Always good to know its not a bug or PEBCAC problem.
All good, I have a feeling the solution direction taken to implement this will be the basis to also implement github organisation checks and any other provider that requires additional steps.
Sorry, my setup was destroyed. This was the documentation https://learn.microsoft.com/en-us/security/zero-trust/develop/configure-tokens-group-claims-app-roles
I have actually followed this documentation, however it only talks about ID tokens. I could be wrong (I don't know Python much), but from what I see in pgAdmin code it only expects claims in the userinfo endpoint, and doesn't use ID tokens at all. Given userinfo endpoint on Azure cannot be modified, not sure how current claim implementation ever supposed to work.
@ViliusS This is where the code reads from to compare with the input https://github.com/pgadmin-org/pgadmin4/blob/master/web/pgadmin/authenticate/oauth2.py#L186
this is called when oauth login is executed: https://github.com/pgadmin-org/pgadmin4/blob/master/web/pgadmin/authenticate/oauth2.py#L124
I understand what you are saying and I think you are right. I'm stunned by my mistake here. When I contributed to this feature, I documented it incorrectly. Apologies for the confusion will try to submit a fix for this.
disclaimer: I did test with Azure, Github, and Vault as OAuth providers, but those were tested to ensure they did not break. My true effort in covering all corner cases has happened only on GitLab.
the ID token is present and still can be checked, but indeed, it's not been checked. I will work to submit a fix to include the ID token + profile checking.
@n0-ware if you would like test this oauth2.py file https://github.com/EvertonSA/pgadmin4/blob/master/web/pgadmin/authenticate/oauth2.py
and we can confirm if the groups limit is impacting you
the ID token is present and still can be checked, but indeed, it's not been checked. I will work to submit a fix to include the ID token + profile checking.
Cool. That explains things.
I also suggest to test how this works without OIDC support, and probably document that optional claims require OpenID support. For instance, this example https://github.com/rowanruseler/helm-charts/blob/1dfe09bce7546c57bf751e91c8176bed8bf65f40/charts/pgadmin4/examples/add-oauth2-config.yaml#L83-L93 only uses standard OAuth 2.0 and GraphQL "me" endpoint, which doesn't send ID token at all, and doesn't use JWKS keys, but still works. You just cannot use claims in that case. It took me a while to figure out how it is different from OIDC.
and doesn't use JWKS key
These examples are, as far as I know, not updated. Recently I have seems lots of issues here where adding the JWKS URL is the fix, so I assume the JWKS is needed.
@ViliusS this PR https://github.com/pgadmin-org/pgadmin4/pull/6925/files adds id token check, can you have a look? if I get it right, session['oauth2_token']['userinfo']
will contain all id token claims if the id token is present on the response from oauth provider.
https://github.com/lepture/authlib/blob/master/authlib/integrations/flask_client/apps.py#L104
edit: here is the jwks check to verify claims on the id token https://github.com/lepture/authlib/blob/master/authlib/integrations/base_client/sync_openid.py#L38 and this is an user that needed the jwks endpoint https://github.com/pgadmin-org/pgadmin4/issues/6194#issuecomment-1526099378
and doesn't use JWKS key
These examples are, as far as I know, not updated. Recently I have seems lots of issues here where adding the JWKS URL is the fix, so I assume the JWKS is needed.
At least Azure AD example is fine. I've been using it a year or so now without any issues. As long one doesn't need claims support and uses User.read
scope, JWKS metadata URL is optional.
@ViliusS this PR https://github.com/pgadmin-org/pgadmin4/pull/6925/files adds id token check, can you have a look? if I get it right,
session['oauth2_token']['userinfo']
will contain all id token claims if the id token is present on the response from oauth provider.https://github.com/lepture/authlib/blob/master/authlib/integrations/flask_client/apps.py#L104
edit: here is the jwks check to verify claims on the id token https://github.com/lepture/authlib/blob/master/authlib/integrations/base_client/sync_openid.py#L38 and this is an user that needed the jwks endpoint #6194 (comment)
I have tested your patch with both, my old Azure AD configuration (the one with standard OAuth 2.0 provided in previous link) and with new OIDC configuration. Both now work without issues. Additional claims with OIDC configuration now works too.
I would improve message to the end user though. At least to me 'claims' is a technical OAuth term. Something like You are not authorized to login based on your identity profile. Please contact your IT administrator.
would be more user friendly.
Logging in case authorization fails could also be improved. Currently we only see userinfo endpoint response data. It would be great to know what pgAdmin thinks is actually required, and claims actually found in the profile/ID token. The authenticated user {username} is not authorized to access pgAdmin based on OAUTH2 configuration. Reason: additional claim required {}, claims found in the identity profile {}.
Other than that, great job!
I appreciate all your hard work on this, @EvertonSA @ViliusS. Due to my role, I could not monitor this or re-implement your changes promptly. However, I have spent the weekend refactoring and updating pgAdmin to the newest version (8.2), and I can confirm this works as expected now.
For posterity and for others that find this thread (it is now high on Google), here is the configuration for the config_local.py
.
AUTHENTICATION_SOURCES = ['oauth2', 'internal']
OAUTH2_AUTO_CREATE_USER = True
OAUTH2_CONFIG = [
{
'OAUTH2_NAME': 'Azure',
'OAUTH2_DISPLAY_NAME': 'Microsoft',
'OAUTH2_CLIENT_ID': '<CLIENT_ID>',
'OAUTH2_CLIENT_SECRET': '<CLIENT_SECRET>',
'OAUTH2_TOKEN_URL': 'https://login.microsoftonline.com/<TENANT_ID>/oauth2/v2.0/token',
'OAUTH2_AUTHORIZATION_URL': 'https://login.microsoftonline.com/<TENANT_ID>/oauth2/v2.0/authorize',
'OAUTH2_API_BASE_URL': 'https://graph.microsoft.com/v1.0/',
'OAUTH2_SERVER_METADATA_URL': 'https://login.microsoftonline.com/<TENANT_ID>/v2.0/.well-known/openid-configuration',
'OAUTH2_USERINFO_ENDPOINT': 'me',
'OAUTH2_SCOPE': 'User.Read email openid profile',
'OAUTH2_BUTTON_COLOR': '#007FFF',
'OAUTH2_ICON': 'fa-brands fa-microsoft',
'OAUTH2_ADDITIONAL_CLAIMS': {
'groups': ["<YOUR_GROUP_ID>"],
},
}
]
Regarding the App Registration in Azure, there are zero special configurations present. All is default, with the pre-loaded group claim.
Architecture is Ubuntu 22.04 running pgAdmin as a container on Portainer with custom SSL certs behind an Nginx reverse proxy. None of that stood in the way of this working.
Three cheers for secure database access, pgAdmin maintainers.
@n0-ware your configuration will probably work, but just wanted to let you know that you are mixing two different authentication types here: OAuth with and without OIDC support. If you want to use OIDC and claim group support you don't really need User.read
authentication scope. You also need to change userinfo endpoint to point to https://graph.microsoft.com/oidc/userinfo
instead of me
or you won't be able to get more advanced claim support working.
You can check both different configurations here https://github.com/rowanruseler/helm-charts/blob/87ecfd8dba3059bc07653f3df13c6b2b537e6dc1/charts/pgadmin4/examples/add-oauth2-config.yaml#L83-L112 They both are now battle tested and works as expected.
@ViliusS You are absolutely correct, and again thank you for the config education. I amended my prior post with the correct config.
'OAUTH2_USERINFO_ENDPOINT': 'me',
became 'OAUTH2_USERINFO_ENDPOINT': 'https://graph.microsoft.com/oidc/userinfo',
and the OAUTH2_SCOPE
is only email openid profile
.
Cheers again
Describe the bug
When using AzureAD as the IDP for OAuth2 with a registered application, providing
OAUTH2_ADDITIONAL_CLAIMS
with thegroups
claim results in failed authentication despite authenticating the user as a member of the assigned group. I am given the message "The user is not authorized to login based on the claims in the profile. Please contact your administrator"To Reproduce
Steps to reproduce the behavior:
config_local.py
file to create the OAuth2 connection. The following configuration, with corresponding App information, satisfies this.Redeploy the container with the following configuration. The only change here is the inclusion of
OAUTH_ADDITIONAL_CLAIMS
config_local.py
config file.Expected behavior
Use successfully logs in
Error message
Screenshots
Desktop (please complete the following information):
Additional context
I have done robust digging in attempts to extract the JWT token for analysis. Unfortunately my knowledge is limited to that of a non-dev security practitioner. My ability to troubleshoot the data-flow is limited but I am happy to expand on this and provide any additional details as required to resolve the issue.
Based on all the documentation available to me, I feel like all the configurations are done appropriately. I accept that there may be a misconfiguration in Azure, but I have been unable to identify it here and am left hoping this is a bug in pgAdmin, or this group can help me resolve the issue and provide clearer documentation for others facing the same issue.