client yielded to allow_grant_flow_for_client config can either be a client or an application object

[aclemons]

Steps to reproduce

When setting allow_grant_flow_for_client in the doorkeeper config, the client that is yielded can be of two different types.

• https://github.com/doorkeeper-gem/doorkeeper/blob/v.5.3.1/lib/doorkeeper/oauth/pre_authorization.rb#L36 yields the application.
• https://github.com/doorkeeper-gem/doorkeeper/blob/v.5.3.1/lib/doorkeeper/oauth/client_credentials/validator.rb#L29 yields the client.
• https://github.com/doorkeeper-gem/doorkeeper/blob/v.5.3.1/lib/doorkeeper/oauth/password_access_token_request.rb#L53 yields the client.

Is this intentional? It means when configuring the lamba, we must deal with the differing types.

This is using the latest released version 5.3.1. (links above to the code in 5.3.1)

Thanks

[nbulaj]

Hi @aclemons . It's a bug, in all the cases client must be application instance. Would you like to prepare a PR?

[aclemons]

Yes, I'll do this. Thanks.

[aclemons]

I've open a pull request to address the client credentials case. This is the one I found leading me to open this issue. Looking at the password_access_token_request.rb case, I'm not sure if it is correct or not. The spec passes an application as "client" here. I think this is incorrect after looking at the prod code here.

module Doorkeeper
  module Request
    class Password < Strategy
      delegate :credentials, :resource_owner, :parameters, :client, to: :server

      def request
        @request ||= OAuth::PasswordAccessTokenRequest.new(
          Doorkeeper.config,
          client,
          resource_owner,
          parameters,
        )
      end
    end
  end
end

Client is what server.client returns, which is a client instance, not an application. Client delegates scopes to the application, so I guess that is why no one has noticed.

If we agree, I'll update the spec and change the code in password_access_token_request.rb to use client.application.