Skip to content

fix(cors): AllowCredentials:true on api so dashboard /auth/exchange works (P0 follow-up)#198

Merged
mastermanas805 merged 2 commits into
masterfrom
fix/cors-allow-credentials
May 30, 2026
Merged

fix(cors): AllowCredentials:true on api so dashboard /auth/exchange works (P0 follow-up)#198
mastermanas805 merged 2 commits into
masterfrom
fix/cors-allow-credentials

Conversation

@mastermanas805

Copy link
Copy Markdown
Member

Summary

P0 follow-up to instanode-web#150 + #151 — completes the AUTH-004 fix chain.

Live verification via chrome devtools MCP after both web PRs shipped:

Access to fetch at 'https://api.instanode.dev/auth/exchange' from origin
'https://instanode.dev' has been blocked by CORS policy: The value of
the 'Access-Control-Allow-Credentials' header in the response is ''
which must be 'true' when the request's credentials mode is 'include'.

Root cause

fiberCORS.Config in internal/router/router.go:255 never set AllowCredentials. The dashboard SPA's AUTH-004 cookie-exchange POST uses credentials: 'include' to send the HttpOnly instanode_session_exchange cookie cross-origin (instanode.dev → api.instanode.dev). The CORS spec requires the response to carry Access-Control-Allow-Credentials: true for the browser to ALLOW the SPA to read the response body. Without it, fetch rejects the read with "blocked by CORS policy".

Existing localStorage-Bearer auth wasn't affected because Bearer doesn't trigger the credentials-include code path. Only AUTH-004 OAuth/magic-link logins broke.

Fix

Add AllowCredentials: true to fiberCORS.Config. Safe because AllowOrigins is an explicit allowlist ("https://instanode.dev,https://www.instanode.dev") — the CORS spec forbids credentials + wildcard origin precisely to prevent rogue sites from siphoning cookies.

Tests (new)

internal/router/cors_credentials_test.go:

  • TestCORS_AllowCredentialsHeader — actual cross-origin POST response carries Access-Control-Allow-Credentials: true
  • TestCORSPreflight_HasAllowCredentialsHeader — OPTIONS preflight response carries the same header (browsers check both)

A future router.New edit that drops AllowCredentials regresses prod login and fails these tests.

Companion PRs

  • instanodedev/instanode-web#150 (merged) — SPA reads ?signed_in=1 + POSTs /auth/exchange
  • instanodedev/instanode-web#151 (merged) — dropped Accept header to avoid preflight allowlist 403

After this api PR merges + auto-deploys, the full GitHub OAuth + magic-link flow works end-to-end.

Test plan

  • CI: existing CORS / preflight tests stay green
  • CI: new TestCORS_AllowCredentialsHeader + preflight test pass
  • After merge: live chrome MCP — click "Continue with GitHub" → land on /app (not "Sign-in failed")

🤖 Generated with Claude Code

mastermanas805 and others added 2 commits May 30, 2026 15:17
… follow-up)

Live verification via chrome devtools MCP:
  Access to fetch at 'https://api.instanode.dev/auth/exchange' from
  origin 'https://instanode.dev' has been blocked by CORS policy: The
  value of the 'Access-Control-Allow-Credentials' header in the
  response is '' which must be 'true' when the request's credentials
  mode is 'include'.

Root cause: fiberCORS.Config never set AllowCredentials. The dashboard
SPA's AUTH-004 cookie-exchange POST uses `credentials: 'include'` to
send the HttpOnly `instanode_session_exchange` cookie cross-origin
(instanode.dev → api.instanode.dev). The CORS spec requires the
response to carry `Access-Control-Allow-Credentials: true` for the
browser to ALLOW the SPA to read the response body. Without it the
fetch fails with "blocked by CORS policy" before the response body is
visible to the SPA. Existing localStorage-Bearer auth wasn't affected
because Bearer doesn't trigger the credentials-include code path.

Companion to instanode-web#150 + #151 (which added the SPA-side
cookie-exchange flow + dropped the Accept header to avoid an
unrelated preflight 403). After this api PR ships, the cookie-exchange
chain runs end-to-end: GitHub OAuth → api callback sets cookie + 302
→ SPA POSTs /auth/exchange with credentials → response includes both
the token AND Allow-Credentials: true → SPA stores token + navigates.

Safety: AllowCredentials:true requires the AllowOrigins allowlist to
be specific (no `*`). Ours is "https://instanode.dev,https://www.
instanode.dev" — explicit list — which satisfies the spec. The CORS
spec forbids credentials + wildcard origin precisely to prevent rogue
sites from siphoning cookies.

Tests added (cors_credentials_test.go): pin both the actual-CORS POST
response AND the OPTIONS preflight response carry
Access-Control-Allow-Credentials: true. A future router.New edit that
drops AllowCredentials regresses prod login and fails these tests.

`make gate` clean locally.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@mastermanas805
mastermanas805 merged commit 1057506 into master May 30, 2026
18 of 20 checks passed
@mastermanas805
mastermanas805 deleted the fix/cors-allow-credentials branch May 30, 2026 10:57
mastermanas805 added a commit that referenced this pull request May 30, 2026
Today's #198 deploy timed out at 180s even though the new ReplicaSet
came up healthy (rollout actually took ~240s on a fresh node with a
cold image pull). The job exited red, prod was correct, and the
false-failure forced a manual rerun + on-call investigation.

Bumping to 300s removes the rerun cycle. Same change ships
simultaneously in worker + provisioner deploy.yml.

Healthz verify loop extended 6×5s → 12×5s for the same reason: if
rollout takes the full timeout, the new pods are only just live when
verify starts and the public LB needs a few seconds to route.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
mastermanas805 added a commit that referenced this pull request May 30, 2026
Today's #198 deploy timed out at 180s even though the new ReplicaSet
came up healthy (rollout actually took ~240s on a fresh node with a
cold image pull). The job exited red, prod was correct, and the
false-failure forced a manual rerun + on-call investigation.

Bumping to 300s removes the rerun cycle. Same change ships
simultaneously in worker + provisioner deploy.yml.

Healthz verify loop extended 6×5s → 12×5s for the same reason: if
rollout takes the full timeout, the new pods are only just live when
verify starts and the public LB needs a few seconds to route.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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