-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
OAuth2 Spec interpretation: finalizeScopes() #895
Comments
Interesting use-case... It may be that this highlights a loose adherence to the spec in this area, and this is likely something we’ll address if we/you can. However, I’m keen to learn more about why scopes are being used in this way in your case because it feels like this could be handled differently/more appropriately using other methods. Personally as a client hoping for a token with the correct scopes (which themselves are presumably not changing as part of the data update), I’d expect no warning about temporary refusal of the scope. Typically I’d expect a temporary refusal as the response to the secondary request - the one to perform whatever action it is that’s being locked by the data update. What happens if someone (who has been granted the scope from earlier before the data update) tries to call an endpoint during the data update? Won’t that be problematic? |
Yes, that would be the nicest, but that would require checking permissions at the data endpoint (because data might be available for one group, but not for another at the same time, so you can't just disable the endpoint). I would like to keep the authorization and data servers seperated.
For my usecase, no. Tokens are relatively short lived, and the data update blockage can last for a week, so the first hour won't be that bad. You could imagine my usecase as a personal schedule. The schedule may not be available during holidays when a new planning is made, and thus it might be disabled for a certain group (for instance Marketing), during which period new data is inserted. After this period, they should transparently be able to get the data again. === I have currently implemented this in the following way:
An example flow is indicated below:
|
I would agree that the spec doesn't specify this usecase, but as I'm using authorization code flow (with external authentication in there through SAML) in my first-party apps, it would be bad to keep asking people to login again if the scopes are not sufficient. With this system I could request all Then, the API can dynamically insert |
I have to say this all feels like it's happening in the wrong place... maybe it works for your use-case at the moment, but I wouldn't be surprised if you face quirkier problems down the line. I would definitely recommend moving things around. For example, if you know that certain parts of the data may not be available for certain groups/teams of users for a specific amount of time, give different scopes to different groups. Still in your scenario, from the perspective of this library, your logic that's overriding approved scopes (removing the ones that are temporarily unavailable) can't be seen as anything other than a scope being approved or not by the resource owner as there is no method of persistence internally - only via the refresh token. From that perspective, I'm not sure that this specific case is something we can actually add to OAuth2 Server. To achieve this, you would need to persist the approved scopes separately (probably in a DB record) from the granted scopes (which are encrypted in the refresh token) and then apply your logic in reverse when the refresh token is used, checking to see if any of the scopes that have been approved can now be granted. |
Sadly, I don't see how this would solve anything. Do you mean giving groups different permissions ahead of time? What if it changes?
How is this last idea different from storing all approved scopes in the refresh token directly, and removing the ones that are not being granted? Do you have a solid reasoning against using the refresh token as 'internal storage', especially in a stateless environment? It seems that you are using the DB in essentially the same way, as I am the token. The refresh token will always contain the list that the user has actually approved, in the same way as the access token would if all scopes would always be granted (but this time encrypted in the refresh token). |
You can do that and I'm not saying that you shouldn't.... I'm saying that OAuth2 Server shouldn't - at least not yet. The most solid reason for this is because I believe the majority of folks use scopes in the way they were intended (this is the first mention I've seen of scopes being used in this way): If you can request them and the resource owner granted them, then it's generally assumed that the token you use has them and the refresh token can have them too. Anything outside of this is pretty case-specific and should probably be wrapped in a custom grant type. |
I think most people are indeed still using OAuth2 for third party only, and using their own more dynamic permissions system for their own apps, and thus will never need this behaviour. However, if you want to integrate your permissions system fully with these tokens, one would expect to be able to add and remove scopes/permissions dynamically, and currently this library doesn't add originally granted scopes back in after removal and re-adding. I don't think this should be implemented right now, but it would be nice if some of the verification code in the grants gets moved to it's own method, and for instance For instance for the Implicit Grant:
would be nicer if it was (or something like this):
|
I don’t feel that creating a new grant type is too onerous or insecure for your specific approach. You may find some methods could be reused and encapsulated differently. Best way is to build something out! :) Feel free to prepare a PR for review. Your input is always appreciated. And thanks for this discussion too! It’s great to see different use-cases and nice to discuss things with passionate-but-reasonable people. I’ve labelled this issue appropriately and will leave it open in case anyone wants to pick up creating an implementation of this. |
I could try to prepare a PR soon with some stuff moved around into new methods (such as in the example above), so that the core business logic (such as creating the actual tokens, or finalizing scopes) may be more easily extended, while keeping the profits of recieving the updates on those checks. I will try to make it non-breaking, so that it can be included as soon as possible. Things like #885 might even be possible then by just creating extended copies of every grant, instead of the current method of nearly copying the entire classes to change single lines in the long methods. My use-case could than also just be implemented as a custom version of every grant, by extending the originals with minimal code changes. Also, thanks to you! I do agree that although with appropriate documentation this method of granting scopes would propably work fine for most people (as if you don't change available scopes during the flow, there would be no difference), but that it may not be necessary for everyone, and making an easy extension method seems like the better option, instead of bloating the library with extra functions, that most people don't need (yet). |
I look forward to seeing what you come up with. |
Personally I don't feel this functionality should be implemented in the server. It feels very much like a use case that would only apply to a select few. I would advocate just providing some customisation that can use this library but is not included in this library. Scopes aren't generally used in this manner. It is assumed that once you have issued an access token with it's associated scopes, those scopes will remain available for the life time of the access token. If you no longer want those scopes to be active, you must revoke the access tokens and ask the end users to login again. Storing requested scopes and re-adding scopes after the access token has been issued is very much a non-standard implementation so I don't feel it should be supported here. I would advise you create a customisation for your own implementation @christiaangoossens as this will cater to your exact needs. It is an interesting use-case but one that I feel falls outside the remit/responsibilities of the core package. |
As said above, any PR that will follow out of this will NOT implement any functionality described above. It will only make creating extensions of grants easier. |
Furthermore, any advice/tips on how this should be implemented (as apparently nobody uses scopes for permissions in first party apps) is welcome from more experienced implementers of the standard. |
Thanks for the clarification @christiaangoossens. As an alternative, and probably simpler solution, why can't you just grant the scopes that are potentially going to be disabled, regardless of their current state? By doing this, you can use the API responses to actually dictate if the resource is currently accessible. You don't need to mess around with the scopes against the token, the server would simply say, does this client have permission to access this end point, and allow them to always call it. If the endpoints data isn't currently available you could then just change the endpoint to return a 403 for unauthorized or 503 for service unavailable. This does assume you'd have a fairly simplistic relationship between your scope and endpoint but if not you could always develop some middleware for the resource server to handle more nuanced relationships. |
Thanks for the advice @Sephster. The main advantages of my current method over yours seem to be the following:
Do you have another view on these points or a method to implement them with your method? I agree that it's best to use everything, including scopes, as everyone is using them, although I am not doing anything that the spec does not allow, though the way I implement this may be unexpected for developers. |
It may be important that I have a lot of very complex group to permission to endpoint relationships, so I require a lot of flexibility in permission granting. I can however deal with waiting 5 or 10 min (accesstoken lifetime) for the current access token to timeout, so further access can be prevented. I had thought of a solution to do it your way, with having a seperate server where every endpoint would be able to check if the user has the permission right now, but this would introduce a single point of failure for all endpoints, and I would, instead of 5 token refreshes per hour, have hundreds of individual endpoint requests to check if users have the permissions, which kinda negates the point of having scopes in the first place. For further insight my situation is provided below: For this API I have students and teachers. I have endpoints for the students to see their schedules, grades, homework etc. For each endpoint I have a scope (such as |
This has resulted in PR #924. I will close it, as we'll continue discussion in that PR. As said earlier, the PR does not include anything to change how scopes work, I have implemented this in a much better way which didn't require rewriting half the library 👍 |
I wrote a blog post about this, for future reference: Part 1: https://chrg.nl/2018/07/25/modern-first-party-auth/ |
The OAuth2 spec (https://tools.ietf.org/html/rfc6749#section-6) states:
This library however interprets that to say: 'the scopes that were originally in the access token', but the spec only says: 'scopes granted by the resource owner'. Therefore, if I show the resource owner a prompt to approve for instance the
test
anddemo
scopes, and they are approved, but I removedemo
in the finalizeScopes() call, because this scope is not available (right now) for the resource owner to grant, it will never be included after refreshing, even if it becomes available again lateron, while the user did grant permission.Use case
In an API I'm building OAuth2 scopes are used for permissions. People can request data from an endpoint, but that endpoint may be temporarily disabled for some users during data updates, or at the end of the year, if the data gets updated for the next year.
Therefore, applications can request a scope for that endpoint, but they may temporarily not be granted the scope. I, however, don't want to force them to login again if the scope becomes available again, because the resource owner did grant permission after all. It should transparently be added back in, as the resource owner would expect. Scopes not allowed during the initial call, whether they were available or not, were never approved, and therefore should, as the spec says, never be included, even if their availability changes.
The text was updated successfully, but these errors were encountered: