Authorization Code grant type controller implementation

[jeremygiberson]

I'm a little confused if not concerned about how the zf-oauth2 module is suppose to be implemented in a ZF2 application.

I'm confused as to why user_id is being pulled from the query/request in the authorizeAction of AuthController.

As I understood the oauth client process some third party application of my ZF2 application would redirect the user to my application to authorize access. My ZF2 application would use its authentication state to identify the user (potentially requiring a log in) then once the user is authenticated present the user with an Authorize Third-party App prompt. My ZF2 application would then forward the user back to the third party application with a token identifying the user as an identity in my Application.

If user_id is a query parameter (rendered in the grant access form) a user could easily change the user_id value that was passed from the client to something else -- potentially a different user id.

When I was reading through the AuthController I was expecting to see the user id being pulled from an identity provided by the AuthenticationService (requiring zend-authentication module).

So I'm a little concerned right now about the security of this modules implementation. Is AuthController just a sample implementation? Or is there something I'm missing about how to use this server implementation that resolves the issue?

[TomHAnderson]

@jeremygiberson You're absolutely right. I've put in a PR for User ID Providers: https://github.com/zfcampus/zf-oauth2/pull/87

[basz]

The default behavior has not changed. Allowing any authenticated user to retrieve an access token bound to a different user_id. Not want you want.

I would say 'ZF\OAuth2\Provider\UserId\AuthenticationService' should be the default UserId Provider. Getting the user_id from the query params is dangerous.

[TomHAnderson]

I agree @basz and that's why I created the UserID Provider in the first place. However, the change you recommend is something everyone should implement anyway. Are you suggesting changing the default so the programmer doesn't need to understand how the UserID Provider is working?

[basz]

Sane defaults is all I suggest.

A programmer might still need to understand how and why the user id provider meganism is here.

Indeed I needed to write my ownProvider as the provided AuthenticationService (which does what I would expect - get the identity from Zend\Authentication\AuthenticationService) would return an 'entity' not a identifier string. (ZfcUser)

[TomHAnderson]

I had to write the same UserID Provider for ZfcUser yesterday too. @ezimuel would you mind if we put our code together and include a ZfcUser UserID Provider with zf-oauth2?

[basz]

Do you have it configurable (via factory) which entity field to use as the identifier or do you use the $user->getUsername() ?

[TomHAnderson]

https://github.com/TomHAnderson/apigility-oauth2-doctrine-skeleton/tree/master/module/Api/src/Api/Provider/UserId

I fetch the identity which is an entity and get the ID.

[basz]

the 'id', right. thanks

[ezimuel]

@basz and @TomHAnderson first of all, sorry for the delay of my reply. I think getting the user_id as query string is definitely a bad practice, we did this for demo purpose but this can be confusing and potentially dangerous for other developers. I like the idea of @basz, we can assume ZF\OAuth2\Provider\UserId\AuthenticationService as default identity manager. The OAuth2 authorization grant works without the usage of user_id, the only requirement is the client_id. We manage the user_id because we need to authorize the access from a user perspective, that means we should already have a user object, e.g. stored in session.

I'm working on a patch and I'll send it in zf-security mailing list. I close this issue and invite you to follow the mailing list for other updates, thanks!