ory/fosite

redirect_uri matching does not follow RFC3986

jgresty opened this issue · 2 comments

Preflight checklist

Describe the bug

From [RFC6749] Section 3.1.2.3

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

Fosite seems to be doing an exact string comparison instead:

if b == uri {

Reproducing the bug

An easy test is for hostnames, which are case insensitive in [RFC3986] Section 6.2.2.1:

  1. Create a client with a redirect uri which has a lower case hostname
  2. Try to authorize with that client, specifying the redirect uri hostname in upper case

eg if a client has a valid redirect uri:
https://example.com and a request is sent with redirect_uri HTTPS://EXAMPLE.com, that should be valid.

Relevant log output

No response

Relevant configuration

No response

Version

0.44.0

On which operating system are you observing this issue?

None

In which environment are you deploying?

None

Additional Context

There are far more rules than just hostname case specified in the RFC, I have just included that one as it is the easiest to describe and test.

Simple String Comparison is the current best security practice in OAuth 2.0 and is the requirement in the OpenID Connect 1.0 standard, as well as the upcoming OAuth 2.1 standard it's (likely but it's not published yet) becoming a requirement similar to in OpenID Connect 1.0. As such it's probably "most correct" to specifically look at what Simple String Comparison actually means. Syntax Based Normalization from what I can see is a different string comparison type.

Edit: For these reasons I'd argue that fosite is actually following the spec accurately, and the client implementations should normalize the redirect_uri prior to making the request or during the configuration provisioning steps. I do however wonder if there is merit in adding a method to the configurator that returns the redirect uri matching func as it may be useful in other scenarios which are not traditionally compliant.

RFC3986 Section 6.2 is the comparison ladder, but neither the OAuth 2.0 Current Best Practices, OAuth 2.1, or OpenID Connect 1.0 mention use of this comparison ladder; just the use of the singular comparison type within the ladder.

Thanks James, I agree with that. It's also mandated by the conformity test suite. Closing