doorkeeper icon indicating copy to clipboard operation
doorkeeper copied to clipboard

Doorkeeper appears to be missing a way to validate client configuration before redirecting to the authentication page

Open ransombriggs opened this issue 8 months ago • 1 comments

Steps to reproduce

We are using a DIY implementation but it basically boils down to the following configuration.

Doorkeeper.configure do
  resource_owner_authenticator do
    current_user || redirect_to(sign_in_url)
  end
end

When there are no valid application clients go to the /oauth/authorize endpoint with an invalid client_id.

http://localhost:3000/oauth/authorize?client_id=invalid-client-id

Expected behavior

I was expecting to get an error that the client_id was not configured properly. I double checked the spec and it does not say if it should prevent authorization, it just says that it should not be redirected back to the invalid redirection uri. From a usability standpoint, it feels preferable to prevent this sort of behavior earlier rather than after the redirect.

If the request fails due to a missing, invalid, or mismatching redirection URI, or if the client identifier is missing or invalid, the authorization server SHOULD inform the resource owner of the error and MUST NOT automatically redirect the user-agent to the invalid redirection URI.

I did look at a workaround for this from resource_owner_authenticator itself, but it was really awkward because pre_auth calls this method as well. I think that in order to get this to behave how I would expect, the PreAuthorization would need to be split so that we could validate before we have an identity, then authorize after that the identity is valid for the client.

Actual behavior

I was redirected to sign_in_url and did not get an error until authenticated.

System Information

Gemfile.lock:

Gemfile.lock content
    doorkeeper (5.7.0)
      railties (>= 5)

ransombriggs avatar Jun 25 '24 21:06 ransombriggs