Skip to content

Test auth and the OAuth loopback - #44

Merged
nrodd merged 1 commit into
tests/harness-and-cifrom
tests/auth
Sep 28, 2026
Merged

nrodd merged 1 commit into
tests/harness-and-cifrom
tests/auth

Conversation

@nrodd

@nrodd nrodd commented Sep 24, 2026 •

Copy link
Copy Markdown
Member

Chunk 2 of the test plan. src/auth.test.ts, 38 tests, plus one source change.

Based on #42 (the harness). GitHub will retarget this to main when that merges. Independent of #43 and the other chunk PRs — different files.

Source change

startCallbackServer and CallbackResult are now exported. The state filtering inside that server is a security boundary and is worth exercising directly rather than only through a full authenticate() run. Nothing else moved.

The loopback server, driven for real

Bind 127.0.0.1:0, send it actual HTTP requests. That port is reachable by any local process or a drive-by web page, so the interesting cases aren't the happy path:

  • A forged state gets a 400 and the server keeps listening — the real redirect still lands afterwards. A stray or malicious first hit must not be able to consume the one-shot and abort a login in progress. (Nothing is stealable either way thanks to PKCE + unguessable state; this just removes a needless DoS.)
  • A callback with no state at all is rejected the same way.
  • Any path other than /callback 404s.
  • A genuine error redirect is let through and folds error_description into the message.

The browser flow, end to end

authenticate() runs for real against its own loopback server: dynamic client registration → authorize URL → an actual HTTP callback → token exchange. Asserted on both requests: PKCE S256 with a 43-char challenge and a matching verifier, the state, the RFC 8707 resource indicator, and the redirect_uri. Plus registration failure, exchange failure, a token with no org, the refresh token, and openBrowser: false printing the link instead of calling open.

The rest

  • decodeTokenClaims across every shape: realm prefix, -eu1 org suffix inference, no region to infer, too few segments, non-JSON payload.
  • --api-key-oauth refuses a non-OAuth value rather than silently falling through to the /me path — that's what --api-key is for.
  • The /me path for an opaque key: Basic header, host chosen from the na1./eu1. prefix, an -eu1 org id overriding the region, and distinct messages for 401/403 vs other statuses vs a missing orgId vs an unreachable host.

One rough edge, recorded not fixed

authenticate() compares callback.state before it reads callback.error. So an error redirect that omits the state surfaces OAuth state mismatch — aborting login for safety instead of the server's actual reason. Real OAuth servers echo state on error redirects, so this is an unlikely path, and reordering it is a behavior change rather than a test. There are two tests pinning both branches; happy to flip the order in a follow-up if you'd rather.

Verification

38 tests passing (91 across the suite), typecheck clean. Mutation-checked the two load-bearing assertions — disabling the state guard and downgrading PKCE to plain — and confirmed exactly the relevant tests fail, then reverted.


Note

Low Risk
Production change is export-only with no auth logic edits; risk is limited to test maintenance and documenting existing OAuth edge-case behavior.

Overview
Adds src/auth.test.ts (~38 tests) covering authentication end-to-end: token claim parsing, the OAuth loopback callback server, supplied credentials, opaque API key resolution via /me, and the full browser OAuth flow (PKCE, dynamic registration, token exchange, failure paths).

The only production tweak is exporting startCallbackServer and CallbackResult from auth.ts so tests can hit the loopback state filtering directly—especially that forged or missing state returns 400 without consuming the one-shot callback, while valid success/error redirects still complete login.

Tests also lock in --api-key-oauth rejecting non-OAuth values (no silent fallthrough to /me) and record current behavior when an OAuth error redirect omits state (state mismatch vs server error message).

Reviewed by Cursor Bugbot for commit b3ff419. Bugbot is set up for automated code reviews on this repo. Configure here.

Chunk 2. Adds src/auth.test.ts, 38 tests. One source change: export
startCallbackServer (and its result type) so the loopback's state
filtering can be exercised directly.

The loopback server is driven for real — bind 127.0.0.1:0, send it an
actual HTTP request — because that port is reachable by any local
process or a drive-by page. A forged state gets a 400 and the server
keeps listening, so a stray first hit can't abort a login in progress;
the real redirect still lands afterwards.

The full browser flow runs end to end too: registration, the authorize
URL, a real callback, then the token exchange. PKCE S256, the state, the
RFC 8707 resource indicator and the loopback redirect_uri are all
asserted on both requests.

Also covers decodeTokenClaims across every token shape, the strict
--api-key-oauth refusal, and the /me path for an opaque key: Basic
header, host from the realm prefix, an -eu1 org overriding the region,
and distinct messages for 401/403, other statuses, a missing orgId and
an unreachable host.

Two tests record current behavior rather than endorse it: authenticate
compares state before it reads `error`, so an error redirect that omits
the state reports a mismatch instead of the server's reason.
@nrodd
nrodd merged commit 2c43a48 into tests/harness-and-ci Sep 28, 2026
5 checks passed
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