Wrong password return 400 instead of 401

[pioz]

I'm running this test on my rails 7 app with doorkeeper 5.6.0

class AuthControllerTest < ActionDispatch::IntegrationTest
  setup do
    @app_client = create(:doorkeeper_application)
    @user = create(:user, username: 'pioz')
  end

  def test_login_fail
    post oauth_token_path, params: {
      username: 'pioz',
      password: 'qwertyz', # the correct password is 'qwerty'
      client_id: @app_client.uid,
      client_secret: @app_client.secret,
      grant_type: 'password'
    }
    assert_response :unauthorized
  end
end

But I get the Expected response to be a <401: unauthorized>, but was a <400: Bad Request>
(if I set the correct 'qwerty' password I get 200 and the right response).

Is it intentional for Doorkeeper to return error 400? If not, is possible to return 401?

[nbulaj]

Hi @pioz . What is the Doorkeeper configuration you have? 400 is about bad request, could be that password flow isn't enabled (for example).

We have specs which checks for 401 when invalid client credentials provided: https://github.com/doorkeeper-gem/doorkeeper/blob/main/spec/requests/flows/password_spec.rb#L226-L257

[pioz]

Here my doorkeeper.rb:

Doorkeeper.configure do
  # Change the ORM that doorkeeper will use (requires ORM extensions installed).
  # Check the list of supported ORMs here: https://github.com/doorkeeper-gem/doorkeeper#orms
  orm :active_record

  # This block will be called to check whether the resource owner is authenticated or not.
  resource_owner_authenticator do
    User.find_by(id: doorkeeper_token&.resource_owner_id) || redirect_to('http://localhost:3000/authorize.html', allow_other_host: true)
  end

  resource_owner_from_credentials do |_routes|
    User.login(params[:username], params[:password])
  end

  # If you are planning to use Doorkeeper in Rails 5 API-only application, then you might
  # want to use API mode that will skip all the views management and change the way how
  # Doorkeeper responds to a requests.
  #
  api_only

  # Issue access tokens with refresh token (disabled by default), you may also
  # pass a block which accepts `context` to customize when to give a refresh
  # token or not. Similar to +custom_access_token_expires_in+, `context` has
  # the following properties:
  #
  # `client` - the OAuth client application (see Doorkeeper::OAuth::Client)
  # `grant_type` - the grant type of the request (see Doorkeeper::OAuth)
  # `scopes` - the requested scopes (see Doorkeeper::OAuth::Scopes)
  #
  use_refresh_token

  # Define access token scopes for your provider
  # For more information go to
  # https://doorkeeper.gitbook.io/guides/ruby-on-rails/scopes
  #
  default_scopes  :app
  optional_scopes :admin, :ifttt

  # Allows to restrict only certain scopes for grant_type.
  # By default, all the scopes will be available for all the grant types.
  #
  # Keys to this hash should be the name of grant_type and
  # values should be the array of scopes for that grant type.
  # Note: scopes should be from configured_scopes (i.e. default or optional)
  #
  scopes_by_grant_type password: [:app]

  # Specify what grant flows are enabled in array of Strings. The valid
  # strings and the flows they enable are:
  #
  # "authorization_code" => Authorization Code Grant Flow
  # "implicit"           => Implicit Grant Flow
  # "password"           => Resource Owner Password Credentials Grant Flow
  # "client_credentials" => Client Credentials Grant Flow
  #
  # If not specified, Doorkeeper enables authorization_code and
  # client_credentials.
  #
  # implicit and password grant flows have risks that you should understand
  # before enabling:
  #   https://datatracker.ietf.org/doc/html/rfc6819#section-4.4.2
  #   https://datatracker.ietf.org/doc/html/rfc6819#section-4.4.3
  #
  grant_flows %w[authorization_code client_credentials implicit password]

  # Allows to customize OAuth grant flows that +each+ application support.
  # You can configure a custom block (or use a class respond to `#call`) that must
  # return `true` in case Application instance supports requested OAuth grant flow
  # during the authorization request to the server. This configuration +doesn't+
  # set flows per application, it only allows to check if application supports
  # specific grant flow.
  #
  # For example you can add an additional database column to `oauth_applications` table,
  # say `t.array :grant_flows, default: []`, and store allowed grant flows that can
  # be used with this application there. Then when authorization requested Doorkeeper
  # will call this block to check if specific Application (passed with client_id and/or
  # client_secret) is allowed to perform the request for the specific grant type
  # (authorization, password, client_credentials, etc).
  #
  # Example of the block:
  #
  #   ->(flow, client) { client.grant_flows.include?(flow) }
  #
  # In case this option invocation result is `false`, Doorkeeper server returns
  # :unauthorized_client error and stops the request.
  #
  # @param allow_grant_flow_for_client [Proc] Block or any object respond to #call
  # @return [Boolean] `true` if allow or `false` if forbid the request
  #
  allow_grant_flow_for_client do |grant_flow, client|
    # `grant_flows` is an Array column with grant
    # flows that application supports
    client.grant_flows.include?(grant_flow)
  end

  # Under some circumstances you might want to have applications auto-approved,
  # so that the user skips the authorization step.
  # For example if dealing with a trusted application.
  #
  skip_authorization do |_resource_owner, _client|
    true
  end
end

[nbulaj]

Does this returns true ? client.grant_flows.include?(grant_flow) Could you please debug?
It's hard for me to debug your application 😃 So you have to try to find where exactly it catches this 400 status

[pioz]

Yes, it returns true. No problem, I'm here at your complete disposal!

[pioz]

Any update on this?

[pioz]

Additional info:
I can login successfully if I pass the correct username and password:

class OauthControllerTest < ActionDispatch::IntegrationTest
  include AuthHelper

  setup do
    @app_client = create(:doorkeeper_application, confidential: false, grant_flows: ['password'])
    @user = create(:user, username: 'pioz', email: 'email@example.org')
  end

  def test_login_success
    post oauth_token_path, params: {
      username: 'pioz',
      password: 'qwerty',
      client_id: @app_client.uid,
      grant_type: 'password'
    }
    assert_response :ok
    assert_access_token(response.parsed_body)
  end

  def test_login_fail_wrong_password
    post oauth_token_path, params: {
      username: 'pioz',
      password: 'wrong',
      client_id: @app_client.uid,
      grant_type: 'password'
    }
    assert_response :unauthorized # fail with Expected response to be a <401: unauthorized>, but was a <400: Bad Request>
  end
end

[nbulaj]

Hi @pioz . I checked OAuth RFC once again

4.3. Resource Owner Password Credentials Grant
4.3.3. Access Token Response
If the access token request is valid and authorized, the authorization server issues an access token and optional refresh token as described in Section 5.1. If the request failed client authentication or is invalid, the authorization server returns an error response as described in Section 5.2.

Checking 5.2 Error Response.

The authorization server responds with an HTTP 400 (Bad Request) status code (unless specified otherwise) and includes the following parameters with the response:
 
[...]
 
invalid_grant
    The provided authorization grant (e.g., authorization code, resource owner credentials) or refresh token is invalid, expired, revoked, does not match the redirection URI used in the authorization request, or was issued to another client.

So Doorkeeper just follows the RFC. And that's not a bug.

401 is used for client (not resource owner) authentication (see the same section):

The authorization server MAY return an HTTP 401 (Unauthorized) status code to indicate which HTTP authentication schemes are supported.

[pioz]

Perfect!