Migrating from lucadegasperi/oauth2-server-laravel

[livingos]

I have been discovering a few gaps in Passport features compared to lucadegasperi/oauth2-server-laravel. The package has been deprecated in favour of passport, but with several projects using it I'm finding Passport is not really able to replace it yet.

Not sure whether this is best place for this but thought I'd add an issue for highlighting the differences at present.

So some of the pain points:

• client ID's should be UUIDs see Client_ID  #14 for others who share this opinion. This is a show-stopper for me at present.

• in oauth2-server-laravel it was possible to restrict clients to specific scopes. Generally it had a more complex data model. From seeing how scopes have been implemented in Passport, there may be a slightly different interpretation of them here. Any client can request a token for any scope.

• error handling was better with exceptions for oauth type errors being bubbled up from underlying package. PR Adds invalid scopes exception #202 heads in this direction but more meaningful exceptions would be good.

There will be more, and am hoping to try and find time to work on some of this, so I can move over to Passport.

[heisian]

Unfortunately in terms of backwards compatibility it seems we're out of luck.

For anyone who was using Laravel 5.1.x w/ lucadegasperi's package, the upgrade to laravel 5.3.x won't allow using it in parallel, which is what I wanted to do to slowly introduce passport while keeping the old oauth as a future deprecation.

Alas, the way things have worked out it seems like we'll just have to make a hard switch to laravel/passport. So much for the whole breaking changes thing. But this was a special case and 'native' oauth support wasn't in the framework at the time.

We can only hope for additional features as the package matures, or try to make some pull requests ourselves. I did address your first bullet point in a PR: #168

However, Taylor doesn't think it's a useful feature, so no merge.

[livingos]

I think we should remember the previous package was not official, so it is a little unfair to suggest these are breaking changes in Laravel 53. This is just how life is when relying on package from the community.

The main problem is that the author of the original package is probably quite busy. The underlying League package had stepped up a new version which would have required some major changes to the Laravel package and so probably saw an opportunity to consolidate and throw effort in to an official package.

The issue is that for any current projects Passport is not ready and does things differently. When I first looked at it a month or so ago things like client credentials grants didn't even work. And each time I try to use it I find something else.

So for now it just isn't mature enough to rely on. But it would be great to have an official oauth package so we should try and contribute nudging it in the right direction.

Or simply fork the old package and carry on with that! In the end the important package is the package it is wrapping.

[heisian]

Yeah totally, not an 'official' package - but it was pretty darn good and probably the go-to for OAuth 2 for many of us.

I do think Passport is much better in its simplicity, and from looking at the source of both, also cleaner. Lucadegasperi's package was a bit harder to modify but in my implementation I had done quite a bit of customization, which I am having to re-do again with Passport.

My main fear is that Passport's spec might change in 1 or 2 versions enough to prompt me to have to re-write certain things but as you said, that's the nature of working with packages.

For your second bullet point, my workaround is to only allow the server to determine a preset range of scopes available for the client requesting the token, depending on the route. Not sure how this compares to other people's implementations.

For client UUIDs I think there's enough desire for this that it'll get into the package eventually, I think Taylor has just deemed it unnecessary for a first version. It sort of is, sort of isn't? But at the end of the day I've never used or seen any API that uses plain integers as a client ID.

here's my fork in which I use version 4 UUIDs as client IDs: https://github.com/heisian/passport (located in the master branch)

[mstephens]

Just thought I'd echo the same comments above as to @livingos

I know Passport is in its infancy (and it's a great package), but having plain integers as the client ID seems like it may only be a short-lived decision? Perhaps it could be configured with configuration settings and a string data type (as per lucadegasperi/oauth2-server-laravel) set to allow both? Not as current, with the assumption of the primary key as the client ID.

It's a shame the lucadegasperi/oauth2-server-laravel is now depreciated, but migrating to Passport from it is extremely difficult with the lack of UUID. So much so we're going to hold in the hope 5.4 sees this addition. Would also echo the more granular exceptions too.

[josh9060]

I agree that client id's should implement UUID. There was a PR that implemented this functionality, without breaking existing projects I believe. However, it wasn't merged... not sure why?

[heisian]

yes, i made that PR. it added a new uuid column while preserving the original id auto_increment for indexing. setting a flag tells passport whether to use id or uuid in lookups. you can check in my fork on the master branch for this.

…

On Dec 9, 2016 10:51 AM, "Josh Gallagher" ***@***.***> wrote: I agree that client id's should implement UUID. There was a PR that implemented this functionality, without breaking existing projects I believe. However, it wasn't merged... not sure why? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#207 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AD8zBGHx0l3eqdqQJIbWxUkhBGtoAHxQks5rGaNKgaJpZM4K-_-s> .

[mstephens]

@heisian @taylorotwell Do you know if UUID is planned for the next release of Passport?

With the depreciation of the oauth2-server-laravel package (depreciated from 5.2 and not supported beyond 5.3), I feel this is a welcome and needed addition.

Happy to contribute but wondered if this was already being tasked by someone.

[themsaid]

Closing for lack of activity, hope you got the help you needed :)

[mstephens]

Hi @themsaid, it would be great to know if this is planned in a future Laravel release?

If not, perhaps I can start working on a fork. I believe alpha-numeric IDs is something Passport should provide (after all the OAuth spec does allow it).

[themsaid]

There's no plan for it at the moment I'm afraid :)

[heisian]

Seems the passport contributors have made a pretty hardline "no" on using random client IDs with the idea being that no security challenges are solved by doing so.

The cleanest solution may be, instead of forking the package to add UUID support (like I did), to create your own pivot table to match UUIDs (or whatever you want to use) with client IDs and write middleware to handle the translation for you before the data gets to Passport.

[mstephens]

@heisian Sounds like that might be useful and something I'll have to do too? Do you have it up on a repo somewhere?

[heisian]

no, just a thought.. you'd pretty much make a migration for the table, then write some custom middleware.. the Laravel docs will the best resource on how to do these things.

[heisian]

you'd also need to write some sort of handler to update your pivot table when new Clients are created by passport.