Issues with `IgnoreLocalPortUrl`

[valkum]

Bug report

We try to use oxide-auth to test some OAuth clients in tests. During that, we encountered some issues regarding the usefulness of the current implementation of IgnoreLocalPortUrl.

localhost vs. 127.0.0.1

https://www.rfc-editor.org/rfc/rfc8252.html#section-7.3 and https://datatracker.ietf.org/doc/html/draft-ietf-oauth-security-topics-23#section-4.1.3 call out to also support 127.0.0.1 for the special case of ignoring the port when validating the client-passed redirect_uri. As 127.0.0.1 (and probably the IPv6 localhost address) is considered to be safer as the hostname, I would expect that IgnoreLocalPortUrl also takes the loopback addresses.

Completing a flow

It does not seem to be possible to complete a authorization code flow. During /authorization, the redirect is generated by:

1. Using the redirect_uri of the PreGrant.
2. PreGrant (for ClientMap) gets the redirect_uri from BoundClient (primitives/registrar.rs#negotiate)
3. BoundClient (for ClientMap) gets the redirect_uri from the registered client (primitives/registrar.rs#bound_redirect) This results in a browser being redirected to the wrong location.

Reproduction

Relevant environment (http frontend library, OS, network setup?)

Steps to reproduce the behavior:

1. Using the actix example, setup the generic Endpoint using

IgnoreLocalPortUrl::new("http://localhost/endpoint")
   .unwrap()
   .into(),

2. Open http://localhost:8020/authorize?response_type=code&client_id=LocalClient&redirect_uri=http://localhost:8021/endpoint

Expected behaviour

According to rfc8252, the redirect should happen based on the requests' redirect_uri.

One simple fix would be to change bound_redirect in registrar.rs to assign the following to registered_url

match registered {
   RegisteredUrl::IgnorePortOnLocalhost(_) => {
      RegisteredUrl::Exact(url.clone().into_owned())
   }
   url @ _ => url.clone(),
}

While this fixes the underlying issue, I am not sure of the consequences of this change. Having RegisteredUrl in PreGrant conveys, semantically by the type, that the content is the URL that is registered with the Client, but the field name and usage of the field suggests otherwise.

Tracking pull request

• [ ] A pull request does not yet exist