laravel / passport

Laravel Passport provides OAuth2 server support to Laravel.
https://laravel.com/docs/passport
MIT License
3.28k stars 776 forks source link

Token scopes no longer stored the same. #1697

Closed joelharkes closed 10 months ago

joelharkes commented 11 months ago

Passport Version

11.9.2

Laravel Version

latest

PHP Version

8.2

Database Driver & Version

SQL Server

Description

we recently upgraded: https://github.com/laravel/passport/compare/v11.8.8...v11.9.2

We have some overwrites on the passport library but found a problem in the code:

Before we used to store the scope as: "scope1 scope2" space delimited string. But now the code expects a Json_encoded field, otherwise it will always return true as the result for scope checking:

image image

Steps To Reproduce

Use old passport version 11.8.8 store a client scopes as space delimited string. but not it stores it as json_encoded array.

we had to do quite a migration to not break our codebase.

driesvints commented 11 months ago

@axlon thoughts here? So far only one report though. The PR is already three months old almost so not sure if reverting is an option?

driesvints commented 11 months ago

@joelharkes how are you storing scopes as a space delimiting string? Can you share some code?

axlon commented 11 months ago

@driesvints In my PR I assumed the changes would always be fully backwards compatible, because any previously existing scopes column would already have a cast in place to have the scopes come out of the model as an array, overwriting the casts provided by Passport's client.

It appears that @joelharkes is manually casting the scopes (encoding them before passing them to the model, and decoding them outside of the model). The easiest way to fix this would probably be to either make eloquent not use a cast for the scopes (by extending the client model and removing that specific cast) or creating a custom cast that deals with space delimited strings (which would require some code changes to classes that access the scopes on the client).

driesvints commented 11 months ago

@axlon thanks! I'll await a code example from @joelharkes before continuing.

driesvints commented 10 months ago

Closing this issue because it's inactive, already solved, old or not relevant anymore. Feel to open up a new issue if you're still experiencing this.