Always requiring `redirect_uri` is not compliant to RFC 6749

[westnordost]

It looks like doorkeeper always requires the redirect_uri parameter to be set in authorization requests.

https://github.com/doorkeeper-gem/doorkeeper/blob/9fc81d5009aef533ca8116285148cb6e37549ff2/lib/doorkeeper/oauth/pre_authorization.rb#L100-L107

This does not seem to be compliant to RFC 6749 - The OAuth 2.0 Authorization Framework:

• 4.1.1. Authorization Request lists redirect_uri as OPTIONAL

• 4.1.3. Access Token Request lists redirect_uri only as REQUIRED if it was supplied in the authorization request

• 3.1.2.3. Dynamic Configuration clarifies that redirect_uri is REQUIRED only in case no redirection URI has been specified during client registration:

  If multiple redirection URIs have been registered, if only part of the redirection URI has been registered, or if no redirection URI has been registered, the client MUST include a redirection URI with the authorization request using the "redirect_uri" request parameter.

  When a redirection URI is included in an authorization request, the authorization server MUST compare and match the value received against at least one of the registered redirection URIs (or URI components) as defined in [RFC3986] Section 6, if any redirection URIs were registered. If the client registration included the full redirection URI, the authorization server MUST compare the two URIs using simple string comparison as defined in [RFC3986] Section 6.2.1.

I understand that by default, doorkeeper also does not allow leaving the redirect url blank during client registration (but it can be changed in the configuration).

Expected behavior

• if client registered a redirection URI, the redirect_uri parameter is optional in the authorization request
• the redirect_uri parameter is only required in the access token request if it was specified in the authorization request. In this case, it must match

Actual behavior

• redirect_uri parameter is always required in the authorization request and access token request

Related

• #1518 - there was something done in response to the ticket, but later reverted 🤷
• https://github.com/openstreetmap/openstreetmap-website/issues/4363

[westnordost]

Note that in the draft OAuth 2.1 specification, redirect_uri has been removed from the access token request, or, for backward compatibility, made optional. The reason is

In OAuth 2.1, authorization code injection is prevented by the code_challenge and code_verifier parameters, making the inclusion of the redirect_uri parameter serve no purpose in the token request. As such, it has been removed.

(see https://datatracker.ietf.org/doc/html/draft-ietf-oauth-v2-1-09#section-10.2-2)

[nbulaj]

4.1.1. Authorization Request lists redirect_uri as OPTIONAL

4.1.3. Access Token Request lists redirect_uri only as REQUIRED if it was supplied in the authorization request

3.1.2.3. Dynamic Configuration clarifies that redirect_uri is REQUIRED only in case no redirection URI has been specified during client registration:

What I was thinking about is:

Access Token Request always have to check redirect_uri because grant always have a redirect_uri from authorization request (which in turn allow it to be blank, but in such case grant copies redirect URI from the client - it's from 3.1.2.3 - "i_f no redirection URI has been registered, the client MUST include a redirection URI"_). So at the end redirect uri is always present for Access token request :thinking:

[ThisIsMissEm]

Also keep in mind that removing the redirect_uri in the Access Token Request can only happen when PKCE is enabled and in use — which it is now a Security Best Current Practice to do so for all clients.

Most likely we should enable PKCE by default now, and warn if it is not enabled.

(OAuth 2.1 also deprecates a lot of things, like Implicit and Resource Owner Password Grant flows)