Closed (fixed)
Project:
Simple OAuth (OAuth2) & OpenID Connect
Version:
5.x-dev
Component:
Code
Priority:
Major
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
30 Nov 2020 at 19:59 UTC
Updated:
21 May 2021 at 00:39 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
paul121 commentedComment #3
paul121 commentedMy first patch fixed the core of the JWT related changes, but looks like other errors have been introduced... and seems to be related to another change that came with v8.2. Described in this issue: https://github.com/thephpleague/oauth2-server/issues/1161
Comment #4
paul121 commentedOkay, I was able to make tests pass by setting a default
redirectUrivalue on consumers. The problem is that v8.2 made a change that requires clients to have a redirect URI set, else it throws aninvalid_clienterror. In my patch I chose to usebase_url()because that seemed sensible, but it really doesn't seem like a client should be required to have a redirect URI set. So perhaps this should still be considered an issue with v8.2 ofleague/oauth2-server.I believe this is the issue affecting #3185037: Always invalid_client on after core update
Another change to consider before adopting v8.2 is that it drops support for PHP 7.2. It seems that most recent versions of Drupal require 7.3, but perhaps simple_oauth v5 still supports PHP 7.2?
If we can't update to v8.2 now it would still be great to pin
lcobucci/jwtat 3.3.3 as a short-term solution.Comment #5
yvmarques commentedAccording to the specs, redirect_uri is a required information which must be validated against the preregistered redirect_uri from the client.
https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest
So it looks like oauth2-server is now enforcing this. The problem is that we often have multiple redirect_uri possible; therefore it would be great to change the config form to allow multiple redirect_uri (I guess a textearea with a redirect_uri per line) and send this parameter as array to the OAuth Server.
If you use composer with Drupal, you can pin lcobucci/jwt at 3.3.3 directly in your composer.json at the root.
Comment #6
bradjones1This is actually way more insidious and broad; our requirement on
league/oauth2-serveris ^8.0, which in turn can give you either 3.x or 4.x oflcobucci/jwt; this can silently break your install if for instance you use--with-dependencies.So yes, pinning is the answer here. I have a number of commits to make on 5.x this weekend; the issues relating to redirect URIs are tangentially related but let's make this issue re: the version drift (and you already opened #3185909: [OAuth 2.1] Require clients to be configured with a redirect URI for that.
#3185037: Always invalid_client on after core update is related vis-a-vis version drift but also appears to now enforce a requirement on redirect URI, perhaps related to the recent inclusion of OIDC support.
Comment #9
bradjones1Comment #10
bradjones1Handling the test failure on 5.x on a separate issue.
Comment #11
bradjones1Actually looks like that failure was not related, anyway, the downgrades are such, from the previous constraint:
Comment #13
cosmicdreams commentedMay we please have this fix on the 4.x branch as well. simple_oauth_fallback_header and lighting_api modules still use the 4.x branch of this module. While those modules are upgrading to the 5.x branch it would be nice to be able to avoid this issue.
Comment #14
bradjones1I welcome a port on a new issue. Thanks.