fix(local): drop the callback allow-list II can't fetch from loopback - #172
Open
sea-snake wants to merge 1 commit into
Open
fix(local): drop the callback allow-list II can't fetch from loopback#172sea-snake wants to merge 1 commit into
sea-snake wants to merge 1 commit into
Conversation
The local login listener served `/.well-known/ii-auth-callbacks` so II's #4091 check would honour its callback. That check cannot run against a local server: browsers block II's https document from fetching a loopback URL, so the request never arrives and, since the check is fail-closed, every local sign-in would be refused. II therefore exempts a server on loopback from the allow-list and takes the callback as given (dfinity/internet-identity#4305). It trusts a local server by loopback host rather than by exact origin, stored port-less as `http://127.0.0.1`, which is what lets this listener keep binding port 0 for a fresh port per handshake. Standing in the check's place are the user's deliberate opt-in to a local connector in II Settings, II's notice before the first local sign-in for an identity on a machine, and its consent screen on every connect. So the route is dead weight, and its doc comment ("fail-closed, so serving this document is mandatory") now describes something that never happens. Remove it, leaving the handshake two routes: the pinned callback page and the slim redeem. The hosted server keeps serving its allow-list — II still enforces it for an https origin. Also record why `state` is load-bearing here in a way it isn't for a hosted server: a hosted callback sits on a domain a random page can't reach, while this listener is reachable by any page in the browser and any process on the machine, so an unguessable `state` (a v4 UUID) is what stops a forged POST to `/redeem` from injecting an identity the user never chose. II treats it as opaque and only echoes it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Removes the local callback allow-list now that Internet Identity’s paired loopback support exempts local servers.
Changes:
- Reduces the login listener to callback and redeem routes.
- Updates unit and end-to-end coverage.
- Revises deployment documentation for loopback trust.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
README.md |
Documents the hosted-only allow-list requirement. |
docs/scoping-local-deployment.md |
Updates local listener design and verification notes. |
docs/local-deployment.md |
Describes the two-route login flow. |
crates/imcp2-local/src/login/tests.rs |
Verifies retired paths return 404. |
crates/imcp2-local/src/login/routes.rs |
Removes the allow-list route and state. |
crates/imcp2-local/src/login/mod.rs |
Simplifies router setup and updates security rationale. |
crates/imcp2-local/src/e2e_local_login.rs |
Models port-less loopback trust without allow-list fetching. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+318
to
+320
| 5. Serve exactly two loopback routes: `GET /callback` (the pinned fragment-reading page) and | ||
| `POST /redeem` (slim redeem → `redeem_registration_delegation`). No `#4091` allow-list — | ||
| II exempts a local server from it, because it cannot fetch one from a loopback origin. |
Comment on lines
+727
to
+730
| The check applies to this hosted server only. II exempts a server on loopback, | ||
| because browsers block its https document from fetching a `http://127.0.0.1` | ||
| URL and the check would fail closed on every local sign-in — so `imcp2-local` | ||
| serves no such document. |
Comment on lines
+96
to
+102
| /// Unguessable on purpose (a v4 UUID), and load-bearing in a way a hosted | ||
| /// server's `state` is not: a hosted callback sits on a domain a random | ||
| /// page can't reach, while this listener is reachable by any page in the | ||
| /// browser and any process on the machine. `state` is what stops a forged | ||
| /// POST to `/redeem` from injecting an identity the user never chose. II | ||
| /// treats it as opaque and only echoes it, so keeping it unpredictable is | ||
| /// entirely this side's job. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pairs with dfinity/internet-identity#4305.
Why
The local login listener serves
/.well-known/ii-auth-callbacks(login/routes.rs) so II's #4091 check will honour its callback. That check can't run against a local server: browsers block II's https document from fetching ahttp://127.0.0.1URL, so the request never arrives — and because the check is fail-closed, every local sign-in is refused. The route's own doc comment ("fail-closed, so serving this document is mandatory") describes something that never happens.II now exempts a server on loopback from the allow-list and takes the callback as given. It trusts a local server by loopback host rather than by exact origin, stored port-less as
http://127.0.0.1— which is exactly what lets this listener keep binding port 0 and getting a fresh port per handshake. Standing in the check's place: the user's deliberate opt-in to a local connector in II Settings, II's notice before the first local sign-in for an identity on a machine, and its consent screen on every connect.What changes
login/routes.rs— theauth_callbacksroute and handler go. The handshake is two routes now: the pinned callback page and the slim redeem.RouteCtxlosescallback_url(it existed only to fill the allow-list document), sologin_routerloses that parameter.login/mod.rs— the module doc no longer cites #4091 as a reason the listener must exist; the fragment still needs a served page to read it, which is reason enough. The port-0 comment no longer claims the callback and an allow-list entry must not drift.login/tests.rs— the surface test now asserts the listener serves the pinned page and nothing else, 404-ing the retired well-known along with any other path.e2e_local_login.rs— drops the allow-list fetch step, and sets the anchor's trusted URL tohttp://127.0.0.1(the shape a local login actually runs against) rather than an arbitraryhttp://localhost:8000/mcp.The hosted server is untouched — II still enforces the allow-list for an https origin, and
src/auth.rs/src/lib.rskeep serving it.Also
Records why
stateis load-bearing here in a way it isn't for a hosted server. A hosted callback sits on a domain a random page can't reach; this listener is reachable by any page in the browser and any process on the machine, so an unguessablestate(already a v4 UUID) is what stops a forged POST to/redeemfrom injecting an identity the user never chose. II treats it as opaque and only echoes it, so keeping it unpredictable is entirely this side's job.Verification
cargo test -p imcp2-local— 21 passed.cargo check -p imcp2-local --features e2eclean.cargo fmt --checkandcargo clippy -p imcp2-local --all-targetsproduce byte-identical output before and after this change (the tree has 518 pre-existing fmt diffs onmain, 3 of them in this crate, all inserver.rsand untouched here).Merge order
This can land first — local login doesn't work against
main-era II either way, since that gate rejects a loopback callback before the allow-list is ever consulted. But it only becomes useful once #4305 ships.🤖 Generated with Claude Code