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

RefreshTokenGrant calls finalizeScopes method #1094

Merged

Conversation

paulo-jay
Copy link

Hi,

I have an issue when using the refresh token grant when the authenticated resource owner access rights change. Scopes granted to the API client are correlated to these access rights : I don't want to grant a scope to the API Client if some conditions are not fulfilled by the resource owner profile.

In order to control if requested scopes are still valid upon application logic, the method ScopeRepositoryInterface::finalizeScopes() should be call before issuing a new access token.

Moreover, this method is called in all other Grant types.

@paulo-jay paulo-jay force-pushed the RefreshTokenGrantFinalizeScopes branch from cb8ea38 to 5657640 Compare February 20, 2020 08:56
@Sephster
Copy link
Member

Hi @paulo-jay. I want to understand a bit more about your request. In your scenario, are you trying to give different grants to the ones already provided? If you could let me know more about your specific scenario, that would be appreciated. Ideally, an example with Scopes A and B or something similar. Thank you for your efforts with this.

@paulo-jay
Copy link
Author

paulo-jay commented Feb 25, 2020

Hi @Sephster

Thank you for the reply. I'll try to better explain an example scenario below (a blog API).

I have 2 API scopes : "view_post", "edit_post".
My resource owner is a registered user with the ability to view and edit posts
My client is a native application that lets you view and edit blog posts.
I am using Authorization code grant flow + refresh token grant flow.

First app launch, no access token, the user logs in with it's login/password using Authorization code grant flow. Scopes "view_post" and "edit_post" are requested by the native app and granted by the Authorization server. The authorization server responds with an access token and a refresh token. The app can display blog posts and allow user to edit them, using the provided access token.

Now the resource owner looses it's privilege to edit posts.

Second app launch, the previous access token is expired, app uses the refresh token to get a new one. With current implementation, the new access token is valid for both "view_post" and "edit_post" scopes (same scopes than the first one).
The user tries to edit post through the app, submit it's changes, but the server returns an error "you are not allowed to do that".
What I'm trying to achieve is to tell to the app that the user is not allowed to edit post anymore, thus it should not propose this feature to the user.
The app behavior to know which actions can be made by the user it based on the granted scopes.

I would like the authorization server to generate an access token only for the "view_post" scope as the resource owner is not allowed anymore to edit posts.

This business logic is implemented in the finalizeScope method (as it should be ?).

This is the purpose of this PR, calling the business logic to check user roles before issuing a new access token.

@Sephster Sephster changed the base branch from master to 9.0.0-WIP February 25, 2020 23:04
@Sephster Sephster changed the base branch from 9.0.0-WIP to master February 25, 2020 23:08
@Sephster Sephster changed the base branch from master to 9.0.0-WIP February 25, 2020 23:08
@Sephster
Copy link
Member

Sephster commented Mar 8, 2020

Looks good to me. I'm going to merge this into version 9 though as I think this is a breaking change. Apart from that, happy to merge in. Thanks for your time with this!

@Sephster Sephster merged commit 3fc71b8 into thephpleague:9.0.0-WIP Mar 8, 2020
@paulo-jay
Copy link
Author

Thanks @Sephster, it was a pleasure to contribute !

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.

2 participants