Made client optional for password grant type

[ianunruh]

This pull request is an attempt to bring Doorkeeper more in line with the OAuth 2.0 spec, specifically section 4.3. It should address issue #288.

The client_id and client_secret parameters are no longer required when creating an access token using the Resource Owner Password Credentials flow.

   grant_type
         REQUIRED.  Value MUST be set to "password".
   username
         REQUIRED.  The resource owner username, encoded as UTF-8.
   password
         REQUIRED.  The resource owner password, encoded as UTF-8.
   scope
         OPTIONAL.  The scope of the access request as described by Section 3.3.

Consequently, the relationship between the access token and application models was changed, by removing the hard dependency between the two.

[tute]

As stated in http://stackoverflow.com/a/6217429/356060 it seems to make sense to not even try to identify the client.

[ianunruh]

It seems to me like it doesn't need to happen, which makes sense to me.

• The user will never see an authorization screen with details about the application, the client app has the user's credentials already.
• If I were malicious, I would just use someone else's client_id .
• The client secret can't be protected since it's on a mobile/native app (which is the use case for this, as far as I can tell).
• The spec never shows usage of the client_id/client_secret in the request for this flow, whereas it will explicitly show it in other flows.

I see the value in trying to tie the access token to an application, however. Maybe it could optionally look for a client_id? What is lost by separating the token from the application?

[szechyjs]

I agree with Ian, this password grant is primarily used for native/mobile apps. Think about the twitter or facebook app. You start the app, it asks for your username/password, it makes a token request and it stores the token locally and you never authenticate again (unless you sign out, or it expires). As per the spec, the client_id/client_secret should not be used in this token request. This is because they can not be securely stored in the application.

[kevintom]

I was banging my head on this, I should have checked the pull requests first.

thanks for this

although this PR totally does what I need, and I agree that it seems to be pretty useless to try to identify the client,
my reading of this part of the final spec

http://tools.ietf.org/html/rfc6749#section-4.3.2

seems to imply that if you include a client_id then you have to validate it (i.e.: there should be a client_secret which seems like a bad idea if this request is coming from a smartphone app)

however if you don't include the client_id in the request, then this PR performs correctly according to my understanding of the spec.

Ultimately I guess it depends how important following the spec exactly is

thanks again

[kevintom]

just to add to my reading of the spec

in this example for the password grant type

POST /token HTTP/1.1
Host: server.example.com
Authorization: Basic czZCaGRSa3F0MzpnWDFmQmF0M2JW
Content-Type: application/x-www-form-urlencoded

grant_type=password&username=johndoe&password=A3ddj3w

it looks like they are using Basic Authorization for specifying the client
and not passing it in the body, which is why it might have been over looked

This line in particular: Authorization: Basic czZCaGRSa3F0MzpnWDFmQmF0M2JW

[ianunruh]

Hmmm that is true, but it appears that client credentials are optional. If you keep reading, here are the requirements of the authorization server:

o  require client authentication for confidential clients or for any
   client that was issued client credentials (or with other
   authentication requirements),
o  authenticate the client if client authentication is included, and
o  validate the resource owner password credentials.

So, it appears to be as I suggested earlier. That is, if the client credentials are included, they should be authenticated. That way, you could link access tokens to the applications. However, if no client authentication is provided, it will still work.

It's not quite clear, however, how you would enforce this for confidential clients or clients that have been issued client credentials. Maybe in your API documentation, you require people to use their client credentials on confidential clients. I don't think it's possible to do it in a programmatic way though.

[kevintom]

agreed, not clear at all

i guess in certain cases if the behavior was dependent on the value of the client_id, then you would be "required" to use your client credentials, or at least incentivized to use the client_id and client_secret.

you could also have a flag on the client that marked it as confidential, then in your controller you could only return responses to requests with client_ids that had been authenticated.(accordingly you would ignore requests who access_token didn't have an application_id defined)

that's a pretty contrived scenario, but it's the only one that pops into mind when i try and fit the "openness" of the spec into reality

[tute]

@ianunruh it seems Mongo models need ot be upgraded, could you please rebase and fix the build https://travis-ci.org/applicake/doorkeeper/jobs/12556090?
Thank you! :)

[ianunruh]

I rewrote my patch. After re-reading the OAuth specification and comments from those here, I've determined that it is more accurate to simply make client authentication optional for the password grant type.

[tute]

Thank you!

[sparksp]

Do you have any plans to finish the work you've started here? Simply making the client optional is, in my opinion, not enough. Now this meets only one of the three requirements, "validate the resource owner password credentials."

There is now no way of requiring client authentication (within doorkeeper at least), and if client authentication is included it is not validated.

You can test the later by including invalid client credentials in a password grant - the grant will succeed, and no application will be linked to the access token.

As a data provider, I want to make sure that only clients who I have issued with credentials are able to authorise anyone with my service. The client is not just so that the User can confirm who's asking for access to their data. Moreover, if we're concerned that a client's secret has been compromised then it can be revoked and a new one issued.

[ianunruh]

I definitely overlooked validation of the client if it was included. That shouldn't be a problem, I'll submit a PR for that soon.

As for the 1st requirement, it could be a Doorkeeper option to only allow valid, authenticated clients for this grant type. If you have application approval turned on, than this should satisfy that requirement.

[swistaczek]

@ianunruh thanks for pull request, I have question about refresh token and missing client auth, did you created any pull request related with that feature?

edit: #326 :)