fix(dashboard-api): serve the strata catalog anonymously; return the caller's role - #480
Merged
Conversation
…caller's role Two independent API-side fixes for spore-host/spore-host#515 and #514, both found by tracing the portal's symptoms back through this Lambda. **GET /api/strata/catalog no longer requires credentials** (#515). handleStrataGetCatalog takes no arguments and returns a package-level slice of five formation names — there is no per-account data in it. But the route sat inside the switch below getUserFromRequest, so: $ curl -s -o /dev/null -w '%{http_code}' https://api.spore.host/api/strata/catalog 401 Browsing a static list required credentials the handler cannot use. Hoisted above the auth call, next to the Slack OAuth exemptions, and above LoadDefaultConfig too since it needs no AWS config — one fewer thing to fail on the anonymous path. Only the read-only listing moves. POST /api/strata/resolve stays gated: it reaches s3://strata-registry with this Lambda's credentials. **GET /teams/{id} now returns the caller's role** (#514). It already resolved the role to gate the request and then discarded it, so a client had to re-derive ownership by comparing OwnerARN against whatever identity it thought it had. The portal was doing exactly that, and wrongly: OwnerARN's format depends on which auth path created the team (a bare account id for portal-federated callers, a real IAM ARN for CLI ones). GET /teams already returns `role` via TeamWithRole; the detail endpoint not doing so was the gap. Also documented what OwnerARN actually is: a display field, written once at creation and never read for authorization — every owner-gated handler resolves the role from the memberships table instead. Not renamed, because the dynamodbav tag is the stored attribute name and changing it needs a data migration. That migration, and the underlying defect where the portal and CLI resolve one human to two different callerARNs, is #531. **Tests.** The existing TestHandlerStrataGetCatalog calls the handler function directly, so it passes whether the route is gated or not — it cannot see this change, which is part of why the gate went unnoticed. Added two router-level tests: an unauthenticated GET must be 200, and an unauthenticated POST to /resolve must not be. Verified load-bearing by reverting the routing change, which reproduces the live 401 and fails the new test: --- FAIL: TestHandlerStrataCatalogNeedsNoAuth status = 401, want 200 … {"success":false,"error":"authentication failed"} go build, go vet and go test all clean. Note the portal half of #515 (registry.ts requiresAuth: false, and tolerating a missing creds header) must land only AFTER this is deployed.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
scttfrdmn
added a commit
to spore-host/spore-host
that referenced
this pull request
Aug 2, 2026
…#515) (#533) The browser half of #515. The API half is spore-host/spawn#480, merged as 66cb620 and NOT YET DEPLOYED — see the note at the bottom. The catalog is the same five environment formations for everybody: the API's handler takes no arguments and reads nothing per-account. It was gated behind sign-in only because the endpoint sat inside dashboard-api's authenticated switch, so `requiresAuth: true` was a consequence of a routing accident rather than a decision about the data. The cost fell on the visitor most likely to be browsing a software list — someone deciding whether they want an account at all. - `requiresAuth: false` on both the registry entry and the surface. The shell gates on the registry entry, so a test asserts the two agree; otherwise the surface's own flag is dead and the visitor still hits a wall. - The credentials header is sent only when there are credentials. Previously `credentialsHeader(creds!)` on a null cred object would throw synchronously inside load(), so the request never left the page and the surface sat on "loading…" forever with no error. Signed-in requests still send it. - A 401 no longer reports "authentication failed". This endpoint doesn't authenticate, so a 401 means the deployed API is older than the page; blaming auth would send a signed-out reader to a sign-in screen that cannot help. - Dropped the session-expiry handler. An expired session no longer stops anyone reading a public list, so "sign in again to reload the catalog" was false. **Tests** — 7 new, all verified load-bearing by reverting the source and watching them fail (5 fail on the requiresAuth/header revert; the 401-wording one fails on its own revert, checked separately so it wasn't passing vacuously). **Driven in a real browser**, signed out, which is the state no one developing this is ever in: the surface mounts instead of the sign-in gate, the request goes out with no auth header, all five cards render with a real layout box (820x79 — happy-dom cannot see that), the slug stays visible, and the 401 variant reports the failure without mentioning authentication or sign-in. The nav also ranks the catalog above the auth-gated pages for a visitor. **Deploy ordering.** This page expects an unauthenticated endpoint. Until the dashboard-api Lambda is deployed (a manual `lambda/dashboard-api/deploy.sh` — there is no CI deploy for it), a signed-out visitor gets "the API hasn't picked up this change yet" rather than the list. Signed-in visitors are unaffected either way, since the credentials they send still validate. typecheck clean; 205/205 tests; build clean.
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.
Two independent API-side fixes, both traced back from portal symptoms. They're in one PR because they're both small, both in
lambda/dashboard-api, and neither changes behaviour for an authenticated caller.Fixes the API half of spore-host/spore-host#515 and spore-host/spore-host#514.
1.
GET /api/strata/catalogno longer requires credentials (#515)handleStrataGetCatalogtakes no arguments and returns a package-level slice of five formation names. There is no per-account data in it —catalog.tsin the portal says as much. But the route sat inside the switch belowgetUserFromRequest:So browsing a static list required credentials the handler cannot use, and the portal gated the whole surface behind sign-in as a consequence.
Hoisted above the auth call, next to the three Slack OAuth exemptions that are already there for the same structural reason. Also above
config.LoadDefaultConfig, since the handler needs no AWS config — one fewer thing to fail on the anonymous path.POST /api/strata/resolvestays gated. It reachess3://strata-registryviaresolveStrataFormationwith this Lambda's credentials, so it does real work and is not exempted. There's a test asserting that, so the two don't drift together later.2.
GET /teams/{id}returns the caller'srole(#514)handleGetTeamalready resolved the role to gate the request, then threw it away:So a client had to re-derive ownership itself. The portal did, by comparing
owner_arnto its account id — and that's wrong, becauseOwnerARN's format depends on which auth path created the team (a bare 12-digit account id for portal-federated callers, a real IAM ARN for CLI ones).GET /teamsalready returnsroleviaTeamWithRole; the detail endpoint not doing so was the gap. Now it does, and clients never need to parse an ARN to infer permission.Also documented what
OwnerARNactually is — a display field, written once at creation and never read for authorization, since every owner-gated handler resolves the role from the memberships table. Not renamed: thedynamodbavtag is the stored attribute name, so changing it is a data migration. That migration, and the larger defect underneath it (the portal and CLI resolve one human to two differentcallerARNs, making their team memberships disjoint), is spore-host/spore-host#531.Tests
The existing
TestHandlerStrataGetCatalogcallshandleStrataGetCatalog()directly, bypassing the router — so it passes whether the route is gated or not, and cannot see this change at all. That's part of why the gate went unnoticed: nothing exercised the path a browser actually takes.Two router-level tests added. Verified load-bearing by reverting the routing change, which reproduces the live 401 exactly:
go build ./...,go vet ./...,go test ./...all clean.Deploy ordering
This is safe to deploy on its own — the portal keeps working unchanged, since it still sends
X-AWS-Credentialsand they still validate.The portal-side follow-up (
registry.ts→requiresAuth: false, plus tolerating a missing creds header incatalog.ts) must land only after this is live, or a signed-out visitor gets a bare 401 where they previously got a coherent sign-in prompt.