Repository navigation
feat(vtex): forward the full request cookie for session-scoped account reads and writes - #10
Merged
Merged
Conversation
…ad/writes profile/address/payment/user store-graphql calls forwarded only the VtexId cookies (parseCookie), dropping vtex_session — so @withCurrentProfile could not resolve a session-scoped (e.g. telesales-impersonated) profile and writes misdirected. Forward the full request cookie, matching the cart actions. updateProfile also injected payload.sub as the profile email; under an operator admin token that is the OPERATOR's email, which would re-key the impersonated customer's account. Derive the target email from the impersonated-customer session cookie when the token audience is admin. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The block read `vtex-impersonated-customer-email` as a COOKIE and threw
"Could not resolve the target profile email" when it was absent. Nothing ever
writes that cookie — impersonation sets it as a session public property via
editSession — so the throw fired on every profile update under a telesales
session.
It was also unnecessary: store-graphql discards the field
(node/resolvers/profile/services.ts, `const { email, ...profileWithoutEmail }`,
since 2.173.2) and writes to the session-resolved currentProfile. The forwarded
cookie is what targets the write; this field is only present because
ProfileInput.email is String! in the schema.
So `email: payload.sub` is restored with a one-line why, the client-writable
cookie read is gone, and the block's security argument goes with it — there is
nothing left to defend.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Why
parseCookieforwards only the VtexId cookies and dropsvtex_session. store-graphql's@withCurrentProfileresolves the profile from that session — so without it, every accountloader/action resolves the cookie holder instead of the session-scoped shopper.
That makes telesales impersonation impossible: an operator browsing on a customer's behalf
carries an admin token plus a session that names the customer, and these nine endpoints only
ever saw the admin.
What changed
New
forwardCookie(headers)invtex/utils/vtexId.tsreturns the full request cookie. Thenine account loaders/actions below now use it in place of
parseCookie().cookie, matching thecookie forwarding the cart actions already do inline:
user.ts,profile/getCurrentProfile.ts,address/getUserAddresses.ts,payment/userPayments.tsprofile/updateProfile.ts,address/{create,update,delete}.ts,payment/deletePaymentToken.tsparseCookieis still used for the JWT payload (sub,userId) — only the forwarded cookiestring changed. The forwarded cookie is what targets the write; no request field does.
10 files, +46/−20.
Known risks, deliberately not addressed here
Both were found by review and left alone on purpose — this app should change as little as
possible, and neither is reachable-and-harmful today.
A stale session can now resolve to
nullwhere it previously fell back to the cookie.In
withCurrentProfile, session takes precedence, andgetCurrentProfileFromSessiononlythrows when
!session || !session.id— a valid session without a profile returnscurrentProfile: nullsuccessfully, so the cookie fallback never runs. Before this changegetSessionalways threw (novtex_sessionforwarded) and the fallback always ran.Reachability of that window is unproven; an ordinary logged-in shopper was verified live
through these endpoints with the change active. If it ever bites, the targeted fix is to
gate
forwardCookieonpayload.audience === "admin", which comes from the signed JWT.newsletter/updateNewsletterOptInstill subscribes with the cookie holder's email.It is not in the nine, and under a telesales session it would name the operator. Left as-is
because it is inert:
checkUserAuthorizationgates the write onstoreUserAuthToken, whichan operator's admin token is not, so the mutation is a silent no-op with either email —
fixing it changes no observable behaviour and would add a round-trip plus a throw path to an
endpoint with no consumer.
Downstream
oficina-reservaonfeat/callcenter-operatorpins this branch atdeno.json:48. The pinpoints at a commit that exists only on this branch, so the site build depends on this landing
on
main-altand being repinned to the merged SHA.🤖 Generated with Claude Code