feat(oauth): enforce HTTPS-or-loopback redirect URIs via STRICT_REDIRECT_URIS

[appleboy]

Summary

Adds an opt-in STRICT_REDIRECT_URIS flag (default false) that enforces the OAuth 2.1 §1.5 / MCP authorization-spec requirement that every redirect URI be either loopback or HTTPS. When enabled, AuthGate rejects a redirect URI that uses plain http:// to a non-loopback host at client create/update time. Closes MCP gap #2 (redirect URIs were previously accepted over plain HTTP to any host).

AI Authorship

• No AI was used in this PR
• AI was used. Details:
  • Tool / model: Claude Code (Opus 4.7)
  • AI-authored files: all 9 files in this PR
  • Human line-by-line reviewed: none yet — please review carefully, especially the security-relevant validation logic

Change classification

• Leaf node (local impact)
• Core code (broad impact — needs line-by-line review)

validateRedirectURIs is the single choke point for all OAuth client registration paths (admin, user self-service, and Dynamic Client Registration). A bug here weakens an auth-boundary security control, so it warrants close review.

Plan reference

Goal: operators deploying AuthGate as an MCP authorization server can set STRICT_REDIRECT_URIS=true to reject plain-http redirect URIs to non-loopback hosts, at client create/update. With the flag off (default), behavior is unchanged — non-breaking for existing deployments.

Two design decisions, both deliberately chosen for backward compatibility:

• Opt-in flag rather than always-on (AuthGate is a general OAuth server, not MCP-only; existing deployments may use plain-http internal redirect URIs).
• Create/update enforcement only, not authorize-time — pre-existing clients keep working until next edited.

Verification

• Unit tests — TestIsLoopbackHost (loopback detection: localhost, 127.0.0.0/8, ::1, negatives)
• Integration tests — TestCreateClient_StrictRedirectURIs exercises the flag through ClientService.CreateClient
• At least 3 e2e-level cases (1 happy path + 2 errors):
  1. Happy: strict on + https://app.example.com/cb → accepted
  2. Error: strict on + http://app.example.com/cb → ErrInvalidRedirectURI
  3. Loopback exception + backward compat: strict on + http://127.0.0.1.../localhost → accepted; strict off + http://app.example.com/cb → still accepted
• Stress / soak test: N/A (low-frequency admin path, no concurrency change)
• Manual verification:

  STRICT_REDIRECT_URIS=true ENABLE_DYNAMIC_CLIENT_REGISTRATION=true ./bin/authgate server
  # http to a non-loopback host is rejected:
  curl -s -X POST localhost:8080/oauth/register -H 'Content-Type: application/json' \
    -d '{"client_name":"x","redirect_uris":["http://evil.example.com/cb"],"grant_types":["authorization_code"],"token_endpoint_auth_method":"none"}'
  # https://... or http://127.0.0.1:1729/cb succeed.

  Local checks run: make generate, go build ./..., make fmt, make lint (0 issues), and the services/util/bootstrap/handlers test packages all pass.

Verifiability check

• Inputs and outputs documented (config flag in CONFIGURATION.md + MCP.md)
• Reviewer can judge correctness from the table-driven tests without reading every line
• Rejections surface as the existing ErrInvalidRedirectURI → HTTP 400 at the handler

Security check

• No secrets in code
• All external inputs validated — this PR adds validation of operator-supplied redirect URIs
• Permission checks unchanged (validation runs inside existing create/update paths)
• Rate limits unchanged (DCR endpoint already rate-limited)
• Errors don't leak internals (reuses existing error, message names only the offending URI)

Risk & rollback

• Risk: an operator enables the flag and can no longer edit legacy clients that have plain-http non-loopback redirect URIs. Mitigated: default off; enforced only at create/update (not authorize), so existing clients keep working until edited; documented in the MCP checklist.
• Rollback: self-contained single commit — revert it, or set STRICT_REDIRECT_URIS=false.

Reviewer guide

• Read carefully:
  • internal/services/client.go — the new strict guard in validateRedirectURIs and the WithStrictRedirectURIs option / NewClientService opts wiring
  • internal/util/url.go — IsLoopbackHost (correctness of loopback detection is the security crux)
• Spot-check OK:
  • internal/config/config.go, internal/bootstrap/services.go — straightforward flag plumbing
  • *_test.go, docs/* — tests and documentation

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds an opt-in STRICT_REDIRECT_URIS flag to enforce OAuth 2.1 / MCP redirect-URI constraints (HTTPS-only except loopback over HTTP) during OAuth client create/update flows.

Changes:

• Introduces loopback-host detection (IsLoopbackHost) and strict redirect URI validation in validateRedirectURIs.
• Plumbs STRICT_REDIRECT_URIS from config into ClientService via a new constructor option.
• Adds unit/integration tests and documents the new configuration behavior.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
internal/util/url.go	Adds IsLoopbackHost helper used by strict redirect validation.
internal/util/url_test.go	Adds unit tests for loopback host detection.
internal/services/client.go	Extends redirect validation with strict-mode enforcement; adds service option wiring.
internal/services/client_user.go	Updates user client update path to pass strict-mode flag into validation.
internal/services/client_test.go	Expands redirect URI validation tests; adds strict-mode create-client integration test.
internal/config/config.go	Adds StrictRedirectURIs config value from STRICT_REDIRECT_URIS env var.
internal/bootstrap/services.go	Wires cfg.StrictRedirectURIs into ClientService construction.
docs/MCP.md	Documents recommended MCP deployment setting for STRICT_REDIRECT_URIS.
docs/CONFIGURATION.md	Documents the new STRICT_REDIRECT_URIS configuration key.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[codecov]

Codecov Report

❌ Patch coverage is 92.30769% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/bootstrap/services.go	0.00%	1 Missing ⚠️
internal/services/client_user.go	0.00%	0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.