Skip to content

Validate redirect URIs at OAuth registration and authorize#57

Merged
dgellow merged 1 commit into
mainfrom
sam/oauth-redirect-uri-validation
Apr 27, 2026
Merged

Validate redirect URIs at OAuth registration and authorize#57
dgellow merged 1 commit into
mainfrom
sam/oauth-redirect-uri-validation

Conversation

@dgellow
Copy link
Copy Markdown
Contributor

@dgellow dgellow commented Apr 27, 2026

Open dynamic client registration accepted any redirect_uri value without validation. An attacker could register a client whose redirect URI pointed at their own host, then use the auth flow to collect a victim's authorization code at that host. Adds an allowedRedirectUriHosts allowlist enforced at /register and re-checked at /authorize. Required in non-dev. Set allowAnyRedirectUriHost to opt out for development.

Also moves redirect_uri validation up front in /authorize so failures there return a direct 400 instead of 302'ing to the raw query value, per RFC 6749 §3.1.2.4. The standard OAuth error channel (302 to a registered URI with ?error=...) is preserved for other validation failures.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request implements a redirect-URI host allowlist policy for OAuth client registration and authorization to prevent open redirect vulnerabilities. It introduces new configuration options, allowedRedirectUriHosts and allowAnyRedirectUriHost, along with structural validation checks and comprehensive tests. Feedback focuses on ensuring the file: scheme is explicitly blocked across configuration loading, static validation, and runtime checks to align with the documentation. Additionally, there is a recommendation to refine the port matching logic to handle default ports more flexibly, ensuring that explicit ports in the allowlist correctly match URIs where the port is omitted but implied.

Comment thread internal/oauth/client_val.go
Comment thread internal/oauth/client_val.go
Comment thread internal/config/load.go
Comment thread internal/config/validation.go
Open dynamic client registration accepted any redirect_uri value
without validation. An attacker could register a client whose redirect
URI pointed at their own host and then, via phishing, walk a victim
through the OAuth flow to receive that victim's authorization code at
the attacker's host. Adds an allowedRedirectUriHosts allowlist enforced
at /register and re-checked at /authorize. Required in non-dev; set
allowAnyRedirectUriHost to opt out for development.

Also moves redirect_uri validation up front in /authorize so failures
there return a direct 400 instead of 302'ing to the raw query value,
per RFC 6749 §3.1.2.4. The standard OAuth error channel (302 to a
registered URI with ?error=...) is preserved for other validation
failures.
@dgellow dgellow force-pushed the sam/oauth-redirect-uri-validation branch from 4b2419a to 4910248 Compare April 27, 2026 16:35
@dgellow dgellow merged commit e9283c1 into main Apr 27, 2026
2 checks passed
@dgellow dgellow deleted the sam/oauth-redirect-uri-validation branch April 27, 2026 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant