fix(auth): accept any port on loopback redirect URIs (RFC 8252 §7.3) #6

Merged

jlxq0 merged 4 commits from oauth-loopback-port into main 2026-08-25 04:49:30 +00:00

Conversation 1 Commits 4 Files changed 2 +195 -2

jlxq0 commented 2026-08-25 02:15:51 +00:00

Owner

Copy link

is_allowed_redirect_uri compared the requested redirect URI to the allowlist by exact string equality, so the allowlisted http://localhost:8787/callback could never match a native client that binds a random ephemeral port. Claude Code CLI draws a free port per session, and DCR answered unregistered redirect_uri.

RFC 8252 §7.3 requires the authorization server to allow any port for a loopback redirect URI. matches_loopback_entry implements exactly that half of the rule: candidate and allowlist entry must both be cleartext http on the same loopback host, with identical path and query. The port is the only component omitted. https and private-use entries keep byte-for-byte matching, port included, so https://claude.ai:8443/api/mcp/auth_callback still fails against https://claude.ai/api/mcp/auth_callback.

Three new tests: port-agnostic loopback matching, host/path/query still exact, and non-loopback entries still exact on port.

Each test was checked against a deliberately broken matcher before being trusted. Four mutations, each turning red on the test that covers it:

mutation	red test
relaxation removed (exact-only)	loopback_entry_matches_any_port, loopback_port_relaxation_does_not_relax_host_or_path
path check removed	loopback_port_relaxation_does_not_relax_host_or_path
host check relaxed to any loopback host	loopback_port_relaxation_does_not_relax_host_or_path
relaxation applied to every scheme	non_loopback_entries_keep_exact_port_matching

Gates, all green locally on rustc 1.96.0: cargo fmt --all --check, cargo clippy --all-targets --all-features --locked -- -D warnings, cargo test --all-features --locked (90 passed), cargo audit, cargo deny check bans licenses sources.

Cross-engine review by Codex (read-only), asked whether the relaxation admits any non-loopback URI or any loopback URI with a different host, path or query: no. Its one nuance is that url::Url normalizes dot segments before the path comparison, so /callback/../callback compares equal to /callback — the same effective path, not a different one.

No deployment change: the shipped allowlist already carries http://localhost:8787/callback, which starts matching once the rule is right.

Closes #5

`is_allowed_redirect_uri` compared the requested redirect URI to the allowlist by exact string equality, so the allowlisted `http://localhost:8787/callback` could never match a native client that binds a random ephemeral port. Claude Code CLI draws a free port per session, and DCR answered `unregistered redirect_uri`. RFC 8252 §7.3 requires the authorization server to allow any port for a loopback redirect URI. `matches_loopback_entry` implements exactly that half of the rule: candidate and allowlist entry must both be cleartext `http` on the same loopback host, with identical path and query. The port is the only component omitted. `https` and private-use entries keep byte-for-byte matching, port included, so `https://claude.ai:8443/api/mcp/auth_callback` still fails against `https://claude.ai/api/mcp/auth_callback`. Three new tests: port-agnostic loopback matching, host/path/query still exact, and non-loopback entries still exact on port. Each test was checked against a deliberately broken matcher before being trusted. Four mutations, each turning red on the test that covers it: | mutation | red test | |---|---| | relaxation removed (exact-only) | `loopback_entry_matches_any_port`, `loopback_port_relaxation_does_not_relax_host_or_path` | | path check removed | `loopback_port_relaxation_does_not_relax_host_or_path` | | host check relaxed to any loopback host | `loopback_port_relaxation_does_not_relax_host_or_path` | | relaxation applied to every scheme | `non_loopback_entries_keep_exact_port_matching` | Gates, all green locally on rustc 1.96.0: `cargo fmt --all --check`, `cargo clippy --all-targets --all-features --locked -- -D warnings`, `cargo test --all-features --locked` (90 passed), `cargo audit`, `cargo deny check bans licenses sources`. Cross-engine review by Codex (read-only), asked whether the relaxation admits any non-loopback URI or any loopback URI with a different host, path or query: no. Its one nuance is that `url::Url` normalizes dot segments before the path comparison, so `/callback/../callback` compares equal to `/callback` — the same effective path, not a different one. No deployment change: the shipped allowlist already carries `http://localhost:8787/callback`, which starts matching once the rule is right. Closes #5

jlxq0 added 1 commit 2026-08-25 02:15:52 +00:00

fix(auth): accept any port on loopback redirect URIs ... 3318ec0403

Exact string equality against the allowlist locked out every native
client that binds an ephemeral loopback port, including Claude Code CLI,
which draws a random free port per session. DCR rejected the request as
`unregistered redirect_uri` because no static entry can match.

RFC 8252 §7.3 requires the authorization server to allow any port for a
loopback redirect URI. Relax the port only there: the candidate and the
allowlist entry must both be cleartext `http` on the same loopback host,
with identical path and query. `https` and private-use entries keep
byte-for-byte matching, port included.

jlxq0 added 1 commit 2026-08-25 02:27:31 +00:00

test(auth): cover both scheme guards on loopback matching ... a70d6a8c8f

Removing the requested-scheme check or the allowlist-entry-scheme check
left every existing test green, so neither was proven. Both are real:
without the first, `https://localhost:3118/callback` matches a cleartext
loopback entry; without the second, an `http` candidate borrows the host
and path of a private-use entry such as `cursor://localhost/callback`.

Each new test goes red when its guard is removed and green when it is
restored.

jlxq0 commented 2026-08-25 02:27:43 +00:00

Author

Owner

Copy link

Two guards were not falsifiable by the first round of tests, and neither turns out to be redundant. Both hardenings arrived from the parallel m365-mcp work; the code already had them, the tests did not.

• Requested scheme. Without the check on the candidate, https://localhost:3118/callback matches an http://localhost:8787/callback entry.
• Entry scheme. Without the check on the allowlist entry, an http candidate borrows the host and path of a private-use entry such as cursor://localhost/callback, because the port is the only thing being ignored.

Full guard-removal table, each guard removed alone and restored:

guard removed	test that goes red
port relaxation entirely	loopback_entry_matches_any_port, loopback_port_relaxation_does_not_relax_host_or_path
requested-scheme is http	loopback_relaxation_checks_the_requested_scheme
entry-scheme is http	loopback_relaxation_checks_the_entry_scheme
host equality	loopback_port_relaxation_does_not_relax_host_or_path
path equality	loopback_port_relaxation_does_not_relax_host_or_path
query equality	loopback_port_relaxation_does_not_relax_host_or_path
requested host is loopback	nothing

The last row is honest rather than fixed. That check duplicates validate_redirect_uri, which is_allowed_redirect_uri already ran, so no input can reach matches_loopback_entry with a non-loopback http host. It stays as a deliberate second check with a comment saying so, and no test was written to manufacture coverage for it.

Gates re-run on rustc 1.96.0: fmt clean, clippy clean under -D warnings, cargo test --all-features --locked 92 passed / 0 failed, cargo audit exit 0, cargo deny check bans licenses sources ok.

Two guards were not falsifiable by the first round of tests, and neither turns out to be redundant. Both hardenings arrived from the parallel `m365-mcp` work; the code already had them, the tests did not. - **Requested scheme.** Without the check on the *candidate*, `https://localhost:3118/callback` matches an `http://localhost:8787/callback` entry. - **Entry scheme.** Without the check on the *allowlist entry*, an `http` candidate borrows the host and path of a private-use entry such as `cursor://localhost/callback`, because the port is the only thing being ignored. Full guard-removal table, each guard removed alone and restored: | guard removed | test that goes red | |---|---| | port relaxation entirely | `loopback_entry_matches_any_port`, `loopback_port_relaxation_does_not_relax_host_or_path` | | requested-scheme is `http` | `loopback_relaxation_checks_the_requested_scheme` | | entry-scheme is `http` | `loopback_relaxation_checks_the_entry_scheme` | | host equality | `loopback_port_relaxation_does_not_relax_host_or_path` | | path equality | `loopback_port_relaxation_does_not_relax_host_or_path` | | query equality | `loopback_port_relaxation_does_not_relax_host_or_path` | | requested host is loopback | **nothing** | The last row is honest rather than fixed. That check duplicates `validate_redirect_uri`, which `is_allowed_redirect_uri` already ran, so no input can reach `matches_loopback_entry` with a non-loopback `http` host. It stays as a deliberate second check with a comment saying so, and no test was written to manufacture coverage for it. Gates re-run on rustc 1.96.0: fmt clean, clippy clean under `-D warnings`, `cargo test --all-features --locked` 92 passed / 0 failed, `cargo audit` exit 0, `cargo deny check bans licenses sources` ok.

jlxq0 added 1 commit 2026-08-25 02:30:34 +00:00

test(auth): name the https-to-cleartext downgrade in the entry-scheme test ... 9b9a051507

An `https://localhost:8443/callback` entry has a loopback host, so without
the entry-scheme check it port-relaxes into cleartext `http`. That is a TLS
downgrade on an entry written expecting TLS, and worse than the lockout this
branch fixes. Assert each half of the two-sided guard separately.

jlxq0 added 1 commit 2026-08-25 04:47:06 +00:00

test(auth): assert the downgrade across canonicalised loopback spellings ... bb03561e49

`127.1` and `0177.0.0.1` canonicalise to `127.0.0.1` before the loopback
check, so all three reach the port relaxation identically. Host spelling
enforces nothing; the entry-scheme guard is the whole control.

jlxq0 merged commit 9f5a32e66d into main 2026-08-25 04:49:30 +00:00

jlxq0 deleted branch oauth-loopback-port 2026-08-25 04:49:31 +00:00

jlxq0 referenced this pull request from a commit 2026-08-25 04:49:31 +00:00

Merge pull request 'fix(auth): accept any port on loopback redirect URIs (RFC 8252 §7.3)' (#6) from oauth-loopback-port into main

jlxq0 referenced this pull request 2026-08-26 04:57:45 +00:00

docs: correct the redirect-matching and IdP-support claims #8

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

waiting-on-julian

Blocked on Julian. The ask is in the issue.

No labels

waiting-on-julian

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

jlxq0/carddav-mcp!6

No description provided.