Skip to content

Localhost redirect_uri validation bug #845

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
dlehammer opened this issue Aug 3, 2022 · 1 comment
Closed

Localhost redirect_uri validation bug #845

dlehammer opened this issue Aug 3, 2022 · 1 comment
Assignees
Labels
status: invalid An issue that we don't feel is valid

Comments

@dlehammer
Copy link

Describe the bug
Hi spring-authorization-server gurus,

The localhost validation erroneously triggers in the face of the suggested host value 127.0.0.1, ex:

localhost is not allowed for the redirect_uri (http%3A%2F%2F127.0.0.1%3A4200%2Flogin). Use the IP literal (127.0.0.1) instead.

Unfortunately it took unreasonable time to pin down the root cause, as no errors was present in the logs even on TRACE-level and the symptom only manifested itself by caching the error-wrapped request instead of the expected request, ex:

2022-08-03 15:09:40.987 DEBUG 34689 --- [o-auto-1-exec-3] o.s.s.w.s.HttpSessionRequestCache        : Saved request http://localhost:44303/error?scope=openid&response_type=code&redirect_uri=https%3A%2F%2Flocalhost%3A4200%2Fdebug&state=.SNIP..

This behavior subsequently breaks the auth-flow, as the user is authenticated at this time but the cached request invalid.

Ie. the auth-flow worked as expected for redirect_uri https://oidcdebugger.com/debug , resulting in quite the head scratching and gnashing of teeth :/

To Reproduce

  1. Register a authorization_code client with a redirect_uri like http://127.0.0.1:4200/login
  2. Perform the authorization_code flow

Notice how requestedRedirectHost is null (orange arrows), triggering the first condition on line 598:
image

Expected behavior

The suggested redirect_uri host value 127.0.0.1 is accepted by the validation.

Plus another plea for logging of server-side (validation) errors (see also #159)

Sample
N/A ~ should be possible to prevent regression with unit-test ~ consider also covering a regular IP address.

@dlehammer dlehammer added the type: bug A general bug label Aug 3, 2022
@dlehammer
Copy link
Author

Bah, was a case of RestTemplate implicitly double URL-encoding the location query-parameters :/

But my plea for debug logging still stands 🙏

@jgrandja jgrandja self-assigned this Aug 15, 2022
@jgrandja jgrandja added status: invalid An issue that we don't feel is valid and removed type: bug A general bug labels Aug 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: invalid An issue that we don't feel is valid
Projects
None yet
Development

No branches or pull requests

2 participants