Skip to content

feat(oauth)!: default STRICT_REDIRECT_URIS to true#198

Merged
appleboy merged 1 commit into
mainfrom
worktree-oauth2
May 28, 2026
Merged

feat(oauth)!: default STRICT_REDIRECT_URIS to true#198
appleboy merged 1 commit into
mainfrom
worktree-oauth2

Conversation

@appleboy
Copy link
Copy Markdown
Member

Summary

Flips STRICT_REDIRECT_URIS to default true, so AuthGate enforces OAuth 2.1 §1.5 / MCP redirect-URI rules (loopback or HTTPS only) out of the box. Plain-http:// redirect URIs to non-loopback hosts are now rejected on client create/update unless an operator explicitly opts out.

AI Authorship

  • No AI was used in this PR
  • AI was used. Details:
    • Tool / model: Claude Code (Opus 4.7)
    • AI-authored files: internal/config/config.go, docs/CONFIGURATION.md, docs/MCP.md
    • Human line-by-line reviewed: None — spot-checked only

Change classification

  • Core code (broad impact — touches an OAuth security boundary / default behavior)
  • Leaf node

The code change itself is a one-line default flip, but it alters a security-relevant default that affects every client registration path (admin UI + Dynamic Client Registration).

Plan reference

No formal plan. Goal: close the gap noted while auditing AuthGate against the MCP authorization spec (2025-11-25) — OAuth 2.1 §1.5 requires redirect URIs to be loopback or HTTPS, but AuthGate previously defaulted this enforcement off.

Verification

  • Existing unit tests pass (go test ./internal/config/ ✓)
  • No new tests added — enforcement logic is unchanged; only the default value flipped. TestCreateClient_StrictRedirectURIs already covers the rejection path (sets the flag explicitly via builder).
  • Stress / soak test: N/A
  • Manual verification: start the server with no env override; confirm STRICT_REDIRECT_URIS is on and that creating a client with http://example.com/cb (non-loopback) is rejected, while http://127.0.0.1/cb and https://... are accepted.

Note: make generate / full build were not run in this worktree (templ _templ.go not generated here). Changes are limited to config + docs and do not touch templates.

Risk & rollback

  • Risk: BREAKING — existing clients using plain-http:// non-loopback redirect URIs will be rejected the next time they are created or updated (including via DCR). Pre-existing clients keep working until next edited.
  • Rollback: set STRICT_REDIRECT_URIS=false, or revert this commit.

Security check

  • No secrets in code
  • Tightens (not loosens) input validation on redirect URIs
  • Errors don't leak internals

Reviewer guide

  • Read carefully: internal/config/config.go (the two default flips at lines ~335 and ~594) — confirm the intent is to ship a stricter default.
  • Spot-check OK: docs/CONFIGURATION.md, docs/MCP.md (wording only).

🤖 Generated with Claude Code

- Enforce loopback-or-HTTPS redirect URIs by default per OAuth 2.1 and MCP
- Update configuration and MCP docs to reflect the new default and opt-out

BREAKING CHANGE: STRICT_REDIRECT_URIS now defaults to true. Clients with
plain-http redirect URIs to non-loopback hosts are rejected on next create or
update (including Dynamic Client Registration). Set STRICT_REDIRECT_URIS=false
to restore the previous permissive behavior.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 27, 2026 16:02
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR tightens AuthGate’s default OAuth client registration behavior by enabling strict redirect URI validation by default, aligning out-of-the-box settings with OAuth 2.1 §1.5 / MCP guidance (loopback or HTTPS redirect URIs only).

Changes:

  • Flip STRICT_REDIRECT_URIS default from false to true in internal/config/config.go.
  • Update MCP deployment guidance to reflect the new default and document the opt-out (STRICT_REDIRECT_URIS=false).
  • Update the configuration reference example to show the new default and clarify behavior.

Reviewed changes

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

File Description
internal/config/config.go Changes the default for StrictRedirectURIs to true when loading config from env.
docs/MCP.md Updates MCP-ready checklist text to reflect strict redirect enforcement now being default-on and documents opt-out.
docs/CONFIGURATION.md Updates the sample env var value/comment to reflect the new default and clarify opt-out.

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

@codecov
Copy link
Copy Markdown

codecov Bot commented May 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@appleboy appleboy merged commit ad2ae26 into main May 28, 2026
23 of 24 checks passed
@appleboy appleboy deleted the worktree-oauth2 branch May 28, 2026 13:59
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.

2 participants