Registered claims error after upgrading from 5.0.5 to 5.2.0.

MR !64 works for me.

For some reason, MR!66 contains a change which is already in 6.0.x, it's that // phpcs:ignore line. The attached patch had that removed and it should apply cleanly.

Still a slight issue, but now should be ok.

Cherry-picked commit 0e62f583 into the 6.0.x MR.

The patches in this PR seem to do more than resolve BC issues, they introduce new support for ID tokens and ISS claims. Valuable stuff, but a separate issue?

>The OpenID Connect token id response needed some treatment as-well.

I'd love to see those parts merged.

@jurgenhaas, @sanduhrs any chance you can tell me how to actually get the id token? /oauth/token only returns the access token.

Hi!

if using this module (v5.2.5) for login in a gitlab instance from a D10, I get the following error with the respective MR:

Could not authenticate you from OpenIDConnect because "Request uri must have schema. possibly add 'http://' to the request uri?".

Without the patch, i have the error in #1.

in gitlab I have 2 login providers: oauth2_generic (works fine) and openid_connect (error)

Config details:

gitlab_rails['omniauth_providers'] = [
  {
    name: "openid_connect",
    label: "OIC",
    args: {
      name: 'openid_connect',
      scope: ['openid', 'oauth2_access_to_profile_information'],
      response_type: 'code',
      issuer: 'https://oauth.ddev.site',
      discovery: false,
      uid_field: 'preferred_username',
      client_auth_method: 'basic',
      client_options: {
        identifier: 'gitlab',
        secret: 'gitlab',
        redirect_uri: 'https://gitlab.ddev.site/users/auth/openid_connect/callback',
        userinfo_endpoint: "https://oauth.ddev.site/oauth/userinfo",
        authorization_endpoint: "https://oauth.ddev.site/oauth/authorize",
        token_endpoint: "https://oauth.ddev.site/oauth/token"
      }
    }
  },
 {
   name: "oauth2_generic",
   label: "OAUTH",
   app_id: "git",
   app_secret: "git",
   args: {
     client_options: {
       site: "https://oauth.ddev.site",
       user_info_url: "/oauth/v1/userinfo",
       authorize_url: "/oauth/authorize",
       token_url: "/oauth/token"
     },
     user_response_structure: {
       root_path: [],
       id_path: ["sub"],
       attributes: {
         email: "email",
         name: "name"
       }
     },
     authorize_params: {
       scope: "oauth2_access_to_profile_information"
     },
     strategy_class: "OmniAuth::Strategies::OAuth2Generic"
   }
 }
]

Rebased 3257293-registered-claims-6.0.x for 6.0.0-beta6.

This issue is two years old and people are starting to maintain MRs to patch from. Isn't it about time it gets merged?

I'm happy to merge it, but the MR's are still missing test coverage, so feel free to contribute.

I think the problem is that those most interested in getting this merged lack the required insight to write proper tests.

Where does one see that "missing test coverage"?

See comments from bradjones1: #6, #8 and #23.

I'm pretty jammed with work, so anyone able to write tests will be welcome.

Adding patch file, for use in composer.json.
It is made from https://git.drupalcode.org/project/simple_oauth/-/merge_requests/66.patch

Also adding patch for merge request 64:

https://git.drupalcode.org/project/simple_oauth/-/merge_requests/64.patch

Added a test for the happy path for a request with a scope 'openid'

Tested latest MR with a drupal site site as identity provider, simple oauth 6.x and another site using openid_connect

The MR works well for me.

Remaining task (for 5.2.x): copy the Kernel tests from MR !66 to !64.

This bad commit is not compatible with lcobucci/jwt version 5 and above because Lcobucci\JWT\Token\Builder is immutable

All calls like
$builder->withClaim($claim_name, $value);

should be rewritten to
$builder = $builder->withClaim($claim_name, $value);

@chesn0k The code is tested automatically with lcobucci/jwt (5.4.0), see https://git.drupalcode.org/project/simple_oauth/-/jobs/3144336
Can you post the exact error message?

Since this issue is already available in the latest release, I suggest creating a new issue.

This won’t cause critical errors, but some of the payload might be lost, which could break client-side authorization.

Ok, I'll open a new issue for this.