thephpleague / oauth2-server

A spec compliant, secure by default PHP OAuth 2.0 Server
https://oauth2.thephpleague.com
MIT License
6.52k stars 1.12k forks source link

Remove backwards compatibility hack for JWT aud #1155

Open Sephster opened 3 years ago

Sephster commented 3 years ago

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.

wurst-hans commented 3 years ago

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 commented 3 years ago

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

wurst-hans commented 3 years ago

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?