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

Remove backwards compatibility hack for JWT aud #1155

Open
Sephster opened this issue Nov 24, 2020 · 3 comments
Open

Remove backwards compatibility hack for JWT aud #1155

Sephster opened this issue Nov 24, 2020 · 3 comments
Milestone

Comments

@Sephster
Copy link
Member

The latest version of lcobucci/jwt passes an array instead of a string for the aud claim. To prevent breaking changes, if this array contains a single value, we convert it to a string to retain past behaviour.

When upgrading to v9, we must remove this compatibility hack.

@Sephster Sephster added this to the 9.00 milestone Nov 24, 2020
@wurst-hans
Copy link

wurst-hans commented Mar 10, 2021

This breaks usage of my OAuth 2.0 implementation using v8.2.4.

I'm storing OAuth attributes inside my application. For this my middleware looks like:

$repository = new AccessTokenRepository();
$validator = new BearerTokenValidator($repository);
$validator->setPublicKey(new CryptKey($key, NULL, FALSE));
$request = $validator->validateAuthorization($request);
$data = $request->getAttributes();
$this->oauth2Request
    ->setAccessToken($data['oauth_access_token_id'])
    ->setClientId($data['oauth_client_id'])
    ->setUserId($data['oauth_user_id'])
    ->setScopes($data['oauth_scopes'])
;

This works fine with version 3.4.2 of lcobucci/jwt. $data['oauth_client_id'] contains a numeric ID then. But when upgrading to version 3.4.5 I get an empty array only without any value in it. So your workaround doesn't seem to work for me.

@Sephster What can I do for further investigation?

@Sephster
Copy link
Member Author

What is the aud claim returning in the later version. Can you provide a var dump of this please? Thanks

@wurst-hans
Copy link

wurst-hans commented Mar 11, 2021

I don't know exactly what you mean. Dump of $request->getAttributes() returns

array(4) {
  ["oauth_access_token_id"]=>
  string(80) "b036ab356e7cd8d5220087a1d1ba27c0d9790778fbd0338cb5905699a746f30f6e67908eb0404660"
  ["oauth_client_id"]=>
  array(0) {
  }
  ["oauth_user_id"]=>
  string(1) "1"
  ["oauth_scopes"]=>
  array(1) {
    [0]=>
    int(1)
  }
}

And I think, I have a solution for this. As you can see, I'm using numeric/integer IDs which refer to my database models (I have models for scope, clients, users etc.).

Looking into source one can see, that you pass raw value of getIdentifier() to JWT. In my case, when I cast this value to string by ->permittedFor((string)$this->getClient()->getIdentifier()) the problem seems to be solved. Sure, this should test for NULL also, but this was just a quick hack to get it working.

It seems, that it is a problem of JWT and not of OAuth. Maybe you can dig into it and decide if my approach can be integrated in OAuth2 nevertheless.

PS: Maybe I'm using OAuth2 in a completely wrong manner, but I thought it would be best to deal with entity IDs of database model. Shouldn't I?

PPS: I've seen that user identifier is casted to string already. So this may be possible for client ID too?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants