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

Device Authorization Grant #1074

Merged
merged 59 commits into from
Mar 21, 2024

Conversation

lucadegasperi
Copy link
Contributor

Hello,
Since I needed the device authorization grant for a project I'm working on, I've decided to give a shot at implementing it in a presentable way.

For the most part I copied other grants code where needed and tried to keep the methods and interfaces as similar to what's already implemented as possible.

The interval checking between requests for an access token is currently not implemented, as I think this could be more easily delegated to a framework middleware than implement the logic right inside the package.

Pairing a user identifier with the device code is left outside the scope of this implementation as well.

I've created a laravel passport implementation based off this code that you can find here: https://github.com/lucadegasperi/passport/tree/device-flow

Let's make this grant happen!

@lucadegasperi
Copy link
Contributor Author

Thanks for the fixes @Sephster, do you think it's good enough to merge now?

@Sephster
Copy link
Member

Sephster commented Jan 7, 2020

Hi @lucadegasperi - sorry I still have a wee bit more to look at. Specifically around handling of pass checking and response rates. I think this probably does need including so have started to add these in. Thanks for your patience with this.

@yovchev
Copy link

yovchev commented Feb 7, 2020

@Sephster Ignore my previous comments the middleware implementation don't make sense please check this as a possible candidate for handling polling rate https://github.com/yovchev/oauth2-server/commits/device-flow-grant

@Sephster
Copy link
Member

@lucadegasperi and @yovchev sorry for the radio silence on this ticket. I have some time coming up this week so will aim to push this through asap. Cheers for your patience.

@yovchev
Copy link

yovchev commented Feb 26, 2020

No worries @Sephster I will be at hand if I can help with something.

@Sephster
Copy link
Member

@yovchev thanks for your time on this. I had a look yesterday and couldn't get the examples to work. I think this is because my system wasn't creating a cache folder as it didn't have permission to do so.

I'm thinking that it might be simpler to just do away with the cache system and instead rewrite the example with a ready made SQLite DB which would probably be easier to maintain. If we went down this route, I'd be ok with just pushing through the device code grant changes and sorting out the examples in a separate PR.

I will have a look at the changes without the examples to see if we can progress them. Thanks for your patience

@modricl
Copy link

modricl commented Nov 3, 2023

@Sephster do you have estimate date for next major?

@Sephster
Copy link
Member

Sephster commented Nov 3, 2023

Hopefully in the next few weeks. Testing underway now. If you're interested in this grant, I would welcome you giving it a spin

src/AuthorizationServer.php Outdated Show resolved Hide resolved
src/AuthorizationServer.php Outdated Show resolved Hide resolved
src/AuthorizationServer.php Outdated Show resolved Hide resolved
@sniper7kills
Copy link

@Sephster @lucadegasperi ,

Just finished taking a look and implementing this locally. Beyond the above few comments I was able to get @lucadegasperi passport implementation updated (https://github.com/RunbookSolutions/passport/tree/device-code) and got everything working using a python as the "device".

Great Work Guys!

@Sephster Sephster merged commit b4ca1d9 into thephpleague:9.0.0-WIP Mar 21, 2024
13 of 15 checks passed
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

Successfully merging this pull request may close these issues.

9 participants