Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing allowed grant flows validation configuration #14

Open
yoda opened this issue Jun 15, 2023 · 3 comments · May be fixed by #15
Open

Missing allowed grant flows validation configuration #14

yoda opened this issue Jun 15, 2023 · 3 comments · May be fixed by #15
Assignees
Labels
bug Something isn't working

Comments

@yoda
Copy link

yoda commented Jun 15, 2023

After looking through a few places the available flows validation seems to be missing:

 option :allow_grant_flow_for_client,    default: ->(_grant_flow, _client) { true }

From the client_credentials flow validation:

        validate :client_supports_grant_flow, error: :unauthorized_client
...
        def validate_client_supports_grant_flow
          return if @client.blank?

          Doorkeeper.config.allow_grant_flow_for_client?(
            Doorkeeper::OAuth::CLIENT_CREDENTIALS,
            @client.application,
          )
        end

Which means if you only have client_credentials or authorization_code enabled on a grant_flow on doorkeeper this flow is not honoring the validation. Let me know if I have misunderstood.

https://github.com/doorkeeper-gem/doorkeeper/blob/f02fcb447a0b39c43cae350a600b853a0e69ee60/lib/doorkeeper/oauth/client_credentials/validator.rb#L31
https://github.com/doorkeeper-gem/doorkeeper/blob/f02fcb447a0b39c43cae350a600b853a0e69ee60/lib/doorkeeper/config.rb#L285

@irminsul irminsul self-assigned this Jun 17, 2023
@irminsul
Copy link
Member

I'll look into this next week.

@irminsul irminsul added the bug Something isn't working label Jun 17, 2023
@marco-nicola marco-nicola self-assigned this Jun 30, 2023
@marco-nicola
Copy link
Collaborator

Hi! Thanks for opening this issue, @yoda.

You are right, the current code is not validating against allow_grant_flow_for_client option. It looks like an accidental omission.

Adding a validation to the two classes handling the main requests should be enough.

A PR will follow shortly. Please feel free to have a look and see if I forgot something.

@yoda
Copy link
Author

yoda commented Nov 23, 2023

Poke

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants