search

thephpleague / oauth2-server

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

v9 Typing breaks existing refresh tokens #1435

Open MartinGreen opened 3 months ago

MartinGreen commented 3 months ago

I think the typing stuff in v9 has broken compatibility with my existing refresh tokens.

Ive just updated to v9 and I was able to add all the types to my classes and everything seems to be working fine. I can create new tokens via auth code or device code and via refresh. However when I try to use an existing refresh token to get a new access token it fails with the following error:

Message: League\OAuth2\Server\Grant\AbstractGrant::issueAccessToken(): Argument #3 ($userIdentifier) must be of type ?string, int given, called in /workspace/vendor/league/oauth2-server/src/Grant/RefreshTokenGrant.php on line 81
File: /workspace/vendor/league/oauth2-server/src/Grant/AbstractGrant.php
Line: 404
Trace: /workspace/vendor/league/oauth2-server/src/Grant/RefreshTokenGrant.php(81): League\OAuth2\Server\Grant\AbstractGrant->issueAccessToken(Object(DateInterval), Object(ClientEntity), 3, Array)
       /workspace/vendor/league/oauth2-server/src/AuthorizationServer.php(173): League\OAuth2\Server\Grant\RefreshTokenGrant->respondToAccessTokenRequest(Object(Slim\Http\ServerRequest), Object(League\OAuth2\Server\ResponseTypes\BearerTokenResponse), Object(DateInterval))
       /workspace/index.php(304): League\OAuth2\Server\AuthorizationServer->respondToAccessTokenRequest(Object(Slim\Http\ServerRequest), Object(Slim\Http\Response))

I believe this is because my userid used to be an integer. Ive changed anywhere that I pass the user id into the system to convert it to a string so its working for new stuff but existing tokens contain the id as an int. Since the refresh token is handled inside the library I dont think theres anywhere I can change my code to convert it before creating the new token.

It looks like when respondToAccessTokenRequest in RefreshTokenGrant reads the old refresh token info it would need to convert it to string before calling issueAccessToken.

Thanks.

btw. The Device Code support is great I've been waiting for that! Cheers!

Maksimius commented 5 days ago

Same problem.

 TypeError: League\OAuth2\Server\Grant\AbstractGrant::issueAccessToken(): Argument #3 ($userIdentifier) must be of type ?string, int given, call
ed in /var/www/html/vendor/league/oauth2-server/src/Grant/RefreshTokenGrant.php on line 81 and defined in /var/www/html/vendor/league/oauth2-server/src/Grant/AbstractGrant.php:404
Stack trace:
#0 /var/www/html/vendor/league/oauth2-server/src/Grant/RefreshTokenGrant.php(81): League\OAuth2\Server\Grant\AbstractGrant->issueAccessToken(Object(DateInterval), Object(common\modules\oauth\models\Client),
 2459525, Array)
#1 /var/www/html/vendor/league/oauth2-server/src/AuthorizationServer.php(173): League\OAuth2\Server\Grant\RefreshTokenGrant->respondToAccessTokenRequest(Object(GuzzleHttp\Psr7\ServerRequest), Object(League\
OAuth2\Server\ResponseTypes\BearerTokenResponse), Object(DateInterval))
#2 /var/www/html/common/modules/oauth/components/AuthorizationServer.php(104): League\OAuth2\Server\AuthorizationServer->respondToAccessTokenRequest(Object(GuzzleHttp\Psr7\ServerRequest), Object(GuzzleHttp\
Psr7\Response))
ssigwart commented 2 days ago

I have a fix in https://github.com/thephpleague/oauth2-server/pull/1436, but it hasn't been reviewed or merged. I wound up manually applying it to our codebase and it's been working great.

Sephster commented 14 hours ago

thank you for the fix @ssigwart - I've got one minor comment but looks great. Will aim to get this merged in asap.