Doorkeeper: Confusion about password grant_flow example in the wiki

Created on 26 Jan 2015  Â·  13Comments  Â·  Source: doorkeeper-gem/doorkeeper

When trying to implement the “password” OAuth grant flow, I noticed the “Testing” example in the wiki. I was under the impression that it was necessary to send _client_id_ and _client_secret_ with the request after reading it. This approach can also be found to be accepted on stack overflow.

Looking at the RFC at http://tools.ietf.org/html/rfc6749#section-4.3 and especially http://tools.ietf.org/html/rfc6749#section-4.3.2, I don’t see why _client_id_ and _client_secret_ would be sent for this grant flow.

Why do both of the examples above send _client_id_ and _client_secret_, then? This may not be an issue of doorkeeper/the doorkeeper wiki at all, but merely a confusion on my part regarding the OAuth specification or the wiki example. In any case, I would be happy if someone could provide further information about this.

Most helpful comment

In case anyone is curious, I still feel that it's important to authenticate the client ... and there are benefits to this like being able to know the application associated with an access token. Doorkeeper only set application_id if client credentials were passed in.

That said my configuration authenticates client credentials using Basic Authorisation only (no params for security considerations) and this is how I did it ...

resource_owner_from_credentials do |routes|
  # Since we are using Basic Authorizaion we parse the Authorization header
  # and check if the credentials are valid by fetching the application
  # matching those credentials
  authorization = request.authorization
  if authorization.present? && authorization =~ /^Basic (.*)/m
    credentials = Base64.decode64(Regexp.last_match[1]).split(/:/, 2)
    uid         = credentials.first
    secret      = credentials.second
    application = Doorkeeper::Application.by_uid_and_secret uid, secret
  end
  # Use Devise's find_for_database_authentication method in User model.
  user = User.find_for_database_authentication(email: params[:username])
  # Only accept if we have a valid application, user and valid credentials
  user if application && user && (user.valid_password? params[:password])
end

All 13 comments

Client for password flow is optional as of https://github.com/doorkeeper-gem/doorkeeper/pull/296. Answer in StackOverflow and wiki page may be updated.
Thank you for noting! :)

@j4zz @tute

I note this as closed, but my understanding is that client_id and secret_id are indeed required as part of the spec.

If you look under: http://tools.ietf.org/html/rfc6749#section-4.3.2, in particular at this statement:

_If the client type is confidential or the client was issued client
credentials (or assigned other authentication requirements), the
client MUST authenticate with the authorization server as described
in Section 3.2.1._

As the client is issued with credentials these should be authenticated as per 3.2.1 which references 2.3 which states client_id and client_secret are required. (I think the confusion is in how the spec has been written, it would make more sense having this clearer in section 4.3.2). However logically I can see that If these are not validated, it leaves us with the problem of how would you revoke access to that client for example. In fact the spec lists three reasons why the client should be validated here http://tools.ietf.org/html/rfc6749#section-3.2.1

I am not sure whether doorkeeper needs to remain un-opinionated about this? I suppose there must be cases where an implementor may be using doorkeeper without creating clients, but for those that are authentication is required as per the spec.

I guess as is, we can manually authenticate the client in the resource_owner_from_credentials block. I am not sure if this would be the best way but a quick and dirty way would be to check the client first by finding an application using Application. by_uid_and_secret(params[:client_id], params[:client_secret]) and only then authenticate by username and password if the client is valid. Though I suspect this would be better handled using the same process as for other grant_types.

What do you think?

My opinion on this is: don't use OAuth2 password flow. Clients will then be authenticated. I started encoding this opinion in https://github.com/doorkeeper-gem/doorkeeper/commit/2115a5243b4663090b49c872a1837f6dda6b0b91. You can see why it makes no sense on that flow to authenticate in this answer: https://stackoverflow.com/questions/6190381/how-to-keep-the-client-credentials-confidential-while-using-oauth2s-resource-o/6217429#6217429. Doorkeeper is the authorization server, it can't control what happens in the client.

Thank you for your input!

In case anyone is curious, I still feel that it's important to authenticate the client ... and there are benefits to this like being able to know the application associated with an access token. Doorkeeper only set application_id if client credentials were passed in.

That said my configuration authenticates client credentials using Basic Authorisation only (no params for security considerations) and this is how I did it ...

resource_owner_from_credentials do |routes|
  # Since we are using Basic Authorizaion we parse the Authorization header
  # and check if the credentials are valid by fetching the application
  # matching those credentials
  authorization = request.authorization
  if authorization.present? && authorization =~ /^Basic (.*)/m
    credentials = Base64.decode64(Regexp.last_match[1]).split(/:/, 2)
    uid         = credentials.first
    secret      = credentials.second
    application = Doorkeeper::Application.by_uid_and_secret uid, secret
  end
  # Use Devise's find_for_database_authentication method in User model.
  user = User.find_for_database_authentication(email: params[:username])
  # Only accept if we have a valid application, user and valid credentials
  user if application && user && (user.valid_password? params[:password])
end

Is there any way to make required again client_id + client_secret? That's because I don't really wanna let any request use this flow (just mine). I'm trying to figure it out yet.
Thanks.

@dhyegofernando You can modify the check in the sample code I've just shared. Specifically this part that performs the actual checking from the DB.

application = Doorkeeper::Application.by_uid_and_secret uid, secret

@itsmrwave my bad I didn't see. Thank u so much.

I don't think client credentials are required for password flow because the specs clearly says this flow should only be used when there is a high degree of trust between the oAuth provider and the client in 4.3. 4.3.2 says that client authentication is required if the client type is confidential or the client was issued client credentials (or assigned other authentication requirements). What is neither if statements are true? What is the client type isn't confidential and the client wasn't issued client credentials?

At any rate, if you want to use refresh_tokens with password workflow, it looks like you will still need to provide client credentials, because the refresh_token endpoint call still requires client credentials parameters.

@itskingori I found a new way to do that

Create a file in config/initializers/doorkeeper/password_strategy_client_validator.rb

module Doorkeeper
  module PasswordStrategyClientValidator
    private

    def validate_client
      !!client
    end
  end
end

Doorkeeper::OAuth::PasswordAccessTokenRequest.prepend Doorkeeper::PasswordStrategyClientValidator

According to the Oauth2 Spec, client_id and client_secret should be set on password grant if the client is confidential, i.e. if a server is authenticating with password flow. Also the oauth2 gem includes these by default even in password auth, so this does not work with doorkeeper
https://tools.ietf.org/html/rfc6749#section-3.2.1
https://tools.ietf.org/html/rfc6749#section-4.3.2

Hey people, I'm trying to implement the password grant flow, but it only works when I send the client_id and client_secret as params for /oauth/token endpoint. is there any way to implement password grant flow without sending those params (for security reasons) from the client app?.

Hey people, I'm trying to implement the password grant flow, but it only works when I send the client_id and client_secret as params for /oauth/token endpoint. is there any way to implement password grant flow without sending those params (for security reasons) from the client app?.

same. when leaving the client_id and client_secret as params empty, error says:
{
"error": "invalid_grant",
"error_description": "The provided authorization grant is invalid, expired, revoked, does not match the redirection URI used in the authorization request, or was issued to another client."
}

image

but when I re-included it, it proceeds in OK:
image

this is confusing.

For anybody who are looking here in 2020:

Looking at the RFC at http://tools.ietf.org/html/rfc6749#section-4.3 and especially http://tools.ietf.org/html/rfc6749#section-4.3.2, I don’t see why client_id and client_secret would be sent for this grant flow.

4.3.2. Access Token Request section says:

If the client type is confidential or the client was issued client credentials (or assigned other authentication requirements), the client MUST authenticate with the authorization server as described in Section 3.2.1.

Also

The authorization server MUST:

  • require client authentication for confidential clients or for any client that was issued client credentials (or with other authentication requirements)

As all Doorkeeper applications have credentials - it's required to use client_id and client_secret with the request and conform the specs.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

reinvanimschoot picture reinvanimschoot  Â·  5Comments

ulitiy picture ulitiy  Â·  5Comments

beweinreich picture beweinreich  Â·  5Comments

rasheedhamdawi picture rasheedhamdawi  Â·  4Comments

krtschmr picture krtschmr  Â·  6Comments