Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[14.0] auth_oidc add call to validation endpoint #336

Open
wants to merge 2 commits into
base: 14.0
Choose a base branch
from

Conversation

stellamargonar
Copy link

add call to oauth provider validation endpoint in the auth_oidc module, as explained in #325

@sbidoul sbidoul added the no stale Use this label to prevent the automated stale action from closing this PR/Issue. label Jul 25, 2022
@ap-wtioit
Copy link
Contributor

needed this to get auth_oidc working with OpenID Connect. ID Token does not contain a user_id claim. nor is user_id listed in the standard claims.

Note: the call to the validation endpoint is not necessary from OpenID Connect Authorization Code Flow (because the ID Token can be and is verified with JWKS according to https://openid.net/specs/openid-connect-core-1_0.html#rfc.section.3.1.3.7)

however auth_oidc/auth_oauth uses the non standard claim user_id which makes this necessary. but it only works (with standard claims) because auth_oauth maps sub to user_id in _auth_oauth_validate:

         subject = next(filter(None, [
            validation.pop(key, None)
            for key in [
                'sub', # standard
                'id', # google v1 userinfo, facebook opengraph
                'user_id', # google tokeninfo, odoo (tokeninfo)
            ]
        ]), None)
        ...
        validation['user_id'] = subject

https://github.com/odoo/odoo/blob/14.0/addons/auth_oauth/models/res_users.py#L56

@stellamargonar would something like this also work for the errors you encountered in #325?

        ....
        validation = oauth_provider._parse_id_token(id_token, access_token)
        # required check
        if "sub" in validation and not "user_id" in validation:
            # set user_id for auth_oauth, user_id is not an OpenID Connect standard claim:
            # https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims
            validation["user_id"] = validation["sub"]
        elif not validation.get("user_id"):
            _logger.error("user_id claim not found in id_token (after mapping).")
            raise AccessDenied()
        ...

I tested locally together with #393 in 15.0 against our oauth/openid provider.

For reference the error i encountered without this patch:

odoo_1                                                | 2022-08-09 11:45:00,364 1 ERROR devel odoo.addons.auth_oidc.models.res_users: user_id claim not found in id_token (after mapping). 
odoo_1                                                | 2022-08-09 11:45:00,364 1 INFO devel odoo.addons.auth_oauth.controllers.main: OAuth2: access denied, redirect to main page in case a valid session exists, without setting cookies 

ap-wtioit added a commit to ap-wtioit/server-auth that referenced this pull request Nov 20, 2023
ap-wtioit added a commit to ap-wtioit/server-auth that referenced this pull request Nov 22, 2023
ap-wtioit added a commit to ap-wtioit/server-auth that referenced this pull request Dec 4, 2023
ap-wtioit added a commit to ap-wtioit/server-auth that referenced this pull request Feb 28, 2024
ap-wtioit added a commit to ap-wtioit/server-auth that referenced this pull request Feb 28, 2024
ap-wtioit added a commit to ap-wtioit/server-auth that referenced this pull request Feb 28, 2024
ap-wtioit added a commit to ap-wtioit/server-auth that referenced this pull request Mar 20, 2024
ap-wtioit added a commit to ap-wtioit/server-auth that referenced this pull request Mar 20, 2024
ap-wtioit added a commit to ap-wtioit/server-auth that referenced this pull request Oct 9, 2024
SiesslPhillip pushed a commit to grueneerde/OCA-server-auth that referenced this pull request Nov 20, 2024
Syncing from upstream OCA/server-auth (15.0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no stale Use this label to prevent the automated stale action from closing this PR/Issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants