-
Bug
-
Resolution: Unresolved
-
Major
-
None
-
None
-
False
-
-
False
-
---
-
-
This PR is effectively about fixing a regression introduced in #41866 here.
When quarkus-oidc initiates a redirect to the OIDC provider, the redirect URL includes an absolute redirect_uri query parameter for a provider like Keycloak to know where to return the user to once the authentication is complete.
Sometimes, the Quarkus application is hidden behind a proxy whose URL is not the same as that of Quarkus application, for example, a request goes to https://my-proxy/app and the proxy forwards it to http://localhost:8080/app - there are many variations here.
In this case, quarkus-oidc, even though it sees the current request URL as http://localhost:8080/app must set redirect_uri query parameter to https://my-proxy/app for Keycloak to redirect the user back to the proxy URL.
quarkus-oidc can set redirect_uri query parameter to https://my-proxy/app by expecting Forwarded and X-Forwarded header, but it does not always work for all the variations of how URLs must be transformed.
So in such cases, quarkus-oidc users can do quarkus.oidc.authentication.redirect-path=https://my-proxy/app to spare quarkus-oidc from trying to calculate a correct proxy url itself and it has worked very well until #41866, though it most likely is still working for some proxy url transformations.
Separately from all of the proxy related work, we added support for Strava OAuth2 in #37850 where, for the first time I added a hardening check here. The reason I did is that Strava OAuth2 only accepts domains as callbacks, not concrete URLs, for example, in Strava, you can only register http://myhost.com as a callback, but not http://myhost.com/callback as it really should have.
So this hardening code was added to prevent a situation where a Quarkus application secured with Strava OAuth2 expects a callback at its http://myhost.com/callback endpoint but gets a redirect to http://myhost.com/another-method. Now, these issues are supposed to be caught by the OAuth2 provider itself, which is exactly why the providers are expected to register concrete URLs, but with Strava, if you have http://myhost.com only, it would not a spot a problem during a code flow completion because http://myhost.com/another-method is under http://myhost.com.
So, so far so good, we did that hardening fix. Next, #41866 came in - that introduced a support for OIDC Dynamic client registration - and there when you register a client, you have to provide an absolute redirect URL, and once the registration is complete, the same absolute redirect URL is set as a quarkus.oidc.authentication.redirect-path in a dynamic OIDC tenant resolver and the Strava hardening check did not like it all as it was working with an assumption quarkus.oidc.authentication.redirect-path was a relative path, and so I update that check at to compare the absolute quarkus.oidc.authentication.redirect-path set by the dynamic registration against an absolute redirect URL that the runtime tries to reproduce by using the current absolute request URI.
And so it worked OK because those absolute URLs were equal. But it does not always work as was confirmed recently when quarkus.oidc.authentication.redirect-path=absolute_proxy_url is quite significantly different from the current request URL, where some parts of the URL path are moved into the host, etc.
So effectively #41866 broke it, and this PR excludes this check if the configured redirect URI is absolute - because it was introduced for absolute URI simply to get past this check in the OIDC dynamic registration case. Also, when it is an absolute configured URL then these URLs point to specific locations, and it is up to OAuth2 providers enforce they are matching registered callbacks.
I can't really properly test it as an integration test is running on a single port and I'm not sure it is worth trying to spawn multiple servers