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

Extension of the SessionInterface #20

Closed
cziegenberg opened this issue Mar 21, 2013 · 5 comments
Closed

Extension of the SessionInterface #20

cziegenberg opened this issue Mar 21, 2013 · 5 comments

Comments

@cziegenberg
Copy link
Contributor

I'm happy to see, that the dev version now contains the handling of the scope parameter, it basically works, but an important feature is missing.

The SessionInterface contains only one method "getScope", and this return the scope info or false is the scope is invalid. That's okay in simple cases, but it's not possible to differ between the owner type and the owner id.

Think about resource types that should only accessible for special clients. Theoretically every client can request every scope it wants, as there is no limitation on who has the right to use which scope.

I know that this behavior depends on the individual implementation, but at the moment there is no chance to implement such a limitation, as the SessionInterface has no information about the owner type and the owner id when the getScope method is called.

I modified the code and extended the getScope method as follows:
...
public function getScope($scope, $type, $typeId);
...

Also I extended the calls of this method in the ClientCredentials and Password Grant classes.

This solves the problem. Now I can create my own session storage class and return the scope information or false if the owner type and/or owner id is not allowed to use it (this information is saved in custom tables).

(Perhaps the parameters $type and $typeId are misleading, but this is how they are names in the SessionInterface.)

Would be great if you could implement this. Thanks.

@alexbilbie
Copy link
Contributor

Interesting - we've also implemented the same functionality but it can be done without editing the library's code.

We've done it by having an extra column in the scopes database that limits a scope to "autonomous only" clients (i.e. where the user doesn't have to be involved) and then in class that implements the SessionInterface I also return this flag along with the other required parameters. Likewise the class that implements ClientInterface returns an additional flag if a client is autonomous only. Then I just do some additional checks in my own code using existing functions.

I don't think at the moment it is worth implementing in the library, but I will write a tutorial to show how to do it.

@cziegenberg
Copy link
Contributor Author

I generally know what you mean, but how do you get the required information "using existing functions"?

If you use the Client Credentials Grant you can get the client id from the parameters, okay, but if you use the Password Grant, the user login (where you get the user id) is done directly before the scope check and there is no chance to get it - so I would not only have to check all parameters again in my scope class, but also do the login and other checks a second time.

This could be avoided by the extension - and the existing functionality would not be influenced as the additional parameters don't have to be used.

@alexbilbie
Copy link
Contributor

Am just going to mention #21 in here as both requests are asking for similar requirements (i.e. limiting clients to certain scopes/grants)

@lapause
Copy link
Contributor

lapause commented Mar 21, 2013

I concur with @ziege. Those usecases are quite common with OAuth, and updates described in #20 & #21 seem to me more good practice than hacking.

It's not about cluttering the library with a bunch of methods rarely used, just to think about possible advanced checks and extend parameters/return values in interfaces accordingly.

It does not complexify their implementation, and API requiring advanced permission controls could be developped without forcing the programmers to extend half the library classes.

As this library is one of the cleanest and most up-to-date implementation of OAuth 2 in PHP, it really should not miss an opportunity to support (not implement) out-of-the-box advanced usecases.

@alexbilbie
Copy link
Contributor

Now implemented in the dev branch. See #24 comments.

Going to close this issue.

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

No branches or pull requests

3 participants