Embed auth fix #3: validate token in /auth/{authenticated,user} directly - #2290
Merged
Merged
Conversation
PR #2289's previous fix tried to read getRequestUser(r) at the top of the handler, but /auth/authenticated and /auth/user live on the insecureRouter — a separate subRouter that doesn't have extractMiddleware. So getRequestUser always returned nil. Resolve the token ourselves via s.authMiddleware.getUserFromToken(). That makes ?access_token=... and Authorization: Bearer work for these two endpoints, which is what the React app needs to load the user when embedded with no session cookie. Cookie / session paths unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Spec-Ref: helix-specs@4628938:001890_do-gateways-start-up
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.
Summary
PR #2289's
/auth/authenticatedfix didn't actually work because I missed that the endpoint is oninsecureRouter— a separate subRouter that doesn't haveextractMiddleware. SogetRequestUser(r)always returned nil at the top of the handler.E2E test confirmed: with the deployed code,
curl -H "Authorization: Bearer hl-..." https://meta.helix.ml/api/v1/auth/authenticatedstill returns{"authenticated":false}even though the same Bearer token works fine on/api/v1/spec-tasks/....Fix
In both
/auth/authenticatedand/auth/userhandlers, resolve the token ourselves vias.authMiddleware.getUserFromToken(). That coversAuthorization: Bearer,?access_token=, andx-api-keycallers.Existing cookie / BFF session paths kept exactly as they were — this is purely an additional branch at the top of each handler.
Why two endpoints
The React app's account context loads the user in two steps:
/auth/authenticated→{authenticated: true}to skip the login redirect/auth/user→ user object to populate stateBoth need to recognize the embed token, otherwise the app either redirects to
/login(step 1 fails) or never finishes initialization (step 2 fails).Test plan
go build ./api/pkg/server/cleancurl -H "Authorization: Bearer ${HELIX_API_KEY}" https://meta.helix.ml/api/v1/auth/authenticatedreturns{"authenticated":true}meta.helix.ml/embed/task/{id}?access_token={key}renders the task content (not the login redirect)🤖 Generated with Claude Code