Add cart customer identity sync hooks - #3913
Conversation
Associate new carts with currently authenticated Customer Account sessions and expose lifecycle hooks for application-owned synchronization after authentication, refresh, and logout. Keep existing-cart ownership verification outside Hydrogen, document protected server-side cart bindings and fail-safe logout cleanup, and preserve session state with sanitized lifecycle-hook failures. Assisted-By: devx/4da10d8d-5225-495f-beb2-40d5a959dcde
The change adds public Customer Account lifecycle hooks and cart handler options, so consumers receive it as an additive minor release. Assisted-By: devx/4da10d8d-5225-495f-beb2-40d5a959dcde
Expose the exact authorization token and a discriminated refresh result so integrations can synchronize cart buyer identity without rereading session storage or conflating transient and definitive refresh failures. Restrict token-bearing hooks to factory-created sessions while preserving standard routes and logout hooks for custom CustomerSession implementations. Assisted-By: devx/4da10d8d-5225-495f-beb2-40d5a959dcde
| onAuthenticated: attachCartBuyerIdentity, | ||
| onTokenRefresh: refreshCartBuyerIdentity, | ||
| onLogout: clearCartBuyerIdentity, |
There was a problem hiding this comment.
note that we dont do it automatically as the use case is not for every customer.
in addition, we dont want to control how users store the cart id cookie and binding a non httpOnly cookie to a httpOnly session was flagged by security review from one of my subagents
this way we leave the responsibility to the implementer
this may be bad, because there is a lot more wiring up to do, im willing to discuss whether this is the most sane approach or an "auto sync" is a better idea
graygilmore
left a comment
There was a problem hiding this comment.
Not entirely sure how to 🎩 this one so I'm leaving the one critical thing that the bots found.
| postLogoutRedirectUri: sanitizeReturnTo(requestedReturnTo, origin, postLogoutRedirectUri), | ||
| }); | ||
| const hookError = await runSessionLifecycleHook(onLogout, context, "logout"); | ||
| if (hookError) return lifecycleHookErrorResult(await commitSession(sessionManager)); |
There was a problem hiding this comment.
🔒 Security: customerSession.logout() at line 658 clears the session and destroys tokens.idToken. It then builds the Shopify end-session URL from that token. Line 663 discards that URL when onLogout rejects and returns a 500.
The result: the app session is gone, but the Shopify IdP session stays alive. A retry cannot fix this. A second logout reads an empty session, hits if (!idToken) at line 498, and redirects home. The next login silently signs the customer back in. On a shared device, the buyer pressed "Log out" but stays signed in at Shopify.
The other hook sites (authorize, refresh) are safe. A rejected hook there only skips a redirect the user can retry. Logout is the one site where "skip the redirect" destroys state first.
Suggestion: Complete the redirect even when the hook rejects. The documented cleanup pattern (cart-sync.md:73-78) writes its marker into the session before it rethrows, and commitSession runs on both paths, so app cleanup still occurs. Update session.test.ts:779-811 to expect the 303 and note the behavior in cart-sync.md.
The alternative — run onLogout before customerSession.logout() — keeps the 500 signal but breaks the documented "runs after logout" contract. Your call. The current order is the one option with no recovery path.
frandiox
left a comment
There was a problem hiding this comment.
Thanks for working on this! I like the direction but at the same time I feel we should make things a bit simpler for the most common path? 🤔
What if we provide something like this? (name tbd)
const cartBuyerIdentity = createCartBuyerIdentitySync({
customerSession,
});
const cartHandlers = createCartServerHandlers({
cartBuyerIdentity,
// Or if we want to have top level hooks:
// onCartCreated: cartBuyerIdentity.onCartCreated,
});
const customerAccountHandlers = createCustomerAccountServerHandlers({
cartBuyerIdentity
// Or with hooks:
// ...cartBuyerIdentity.customerAccountHooks,
});Hydrogen already writes the cart cookie and receives the trusted cart ID directly from the Storefront API when it creates the cart. The helper would save that same ID in protected session storage and commit it on the response. It would not trust a cart ID sent by the browser.
createCartBuyerIdentitySync would run cartBuyerIdentityUpdate after login and refresh, clear the identity on logout, and own cookie cleanup and request ordering. The helper is opt-in and tree-shakeable.
We could still consider keeping the the lifecycle hooks if we want to be extra flexible as a low-level API and this helper can use it. If not, the helper can keep that lifecycle wiring internal and expose only the cart-specific API.
|
|
||
| ## Prerequisite: protected cart binding | ||
|
|
||
| The standard `/api/cart` handler does not create a protected server-side ownership binding for existing-cart synchronization. Before enabling these lifecycle hooks, the app must store the SFAPI-returned cart ID from its server-observed cart creation boundary in protected session storage. If the app delegates all creation directly to the standard route, wrap that server boundary or use an independently signed cart store; do not fall back to trusting the unsigned browser cart cookie. |
There was a problem hiding this comment.
If the access token expires and only a valid refresh token remains, isLoggedIn returns true, but getAccessToken does not refresh it. New carts are then created without buyer identity.
The /api/cart handler also does not save the trusted cart ID needed to attach the customer after the token is refreshed. Should the helper handle this automatically somehow? 🤔
There was a problem hiding this comment.
we have getOrRefreshAccessToken for that, but i believe this is quite complex and easy to miss
|
|
||
| If a hook rejects, Hydrogen commits the updated session and returns a sanitized server error instead of the normal redirect. Hydrogen deliberately does not log the raw hook error because it can contain tokens; log allowlisted diagnostics inside the hook before throwing. Bound downstream work to an appropriate timeout and honor `context.request.signal`. | ||
|
|
||
| Hooks can execute more than once when lifecycle requests retry or overlap. Keep synchronization idempotent by setting buyer identity to the desired state rather than applying non-repeatable side effects. |
There was a problem hiding this comment.
What if refresh + logout both overlap and refresh ends later? It's virtually re-logged-in?
Could the safe default be to always remove the protected cart binding and expire or rotate the cart cookie during logout?
| Customer Account OAuth methods require a public HTTPS origin. The writable session manager should expose the request origin; explicit `origin` options are only needed as overrides. Use a tunnel for local development and pass the framework's canonical request URL, not an untrusted forwarded host. | ||
|
|
||
| Use `createCustomerAccountServerHandlers({customerSession})` with `handleShopifyRoutes` to install the default `GET /account/login`, `GET /account/authorize`, `GET /account/refresh`, and `POST /account/logout` handlers. Pass the same request-scoped `requestContext` and `sessionManager` into `handleShopifyRoutes` once alongside the `storefrontClient`. Session managers can be read-only for `isLoggedIn()` / `getAccessToken()` and writable for `getOrRefreshAccessToken()`, `prepareLoginUrl()`, `handleOAuthCallback()`, `logout()`, and registered account handlers. `isLoggedIn()` is a read-only UI/session-presence check: it returns true for a usable access token or a refresh token that can attempt to restore one later. `getAccessToken()` still returns only a currently usable access token and never refreshes. | ||
| Pass `customerSession` to `createCartServerHandlers({customerSession})` to associate newly created carts with a currently usable customer token and mark checkout URLs in authenticated cart GET responses with `logged_in=true`. |
There was a problem hiding this comment.
I don't think we want these examples in the readme? Also it's not complete because it also needs requestContext etc.
|
|
||
| ## Prerequisite: protected cart binding | ||
|
|
||
| The standard `/api/cart` handler does not create a protected server-side ownership binding for existing-cart synchronization. Before enabling these lifecycle hooks, the app must store the SFAPI-returned cart ID from its server-observed cart creation boundary in protected session storage. If the app delegates all creation directly to the standard route, wrap that server boundary or use an independently signed cart store; do not fall back to trusting the unsigned browser cart cookie. |
There was a problem hiding this comment.
I think basically app and checkout have different meanings of "logged in"? If so maybe a returning customer could get an anonymous cart?
| "authenticated", | ||
| accessToken, | ||
| ); | ||
| if (hookError) return lifecycleHookErrorResult(await commitSession(sessionManager)); |
There was a problem hiding this comment.
So at this point the user is actually authenticated, but something in the hook went wrong? I'm not sure what's the correct response to this... 500 might be misleading since auth actually worked, and it would be stuck in the callback URL?
| ); | ||
| return false; | ||
| } catch { | ||
| log.error("customer session lifecycle hook failed", { lifecycle }); |
There was a problem hiding this comment.
Could we validate storefrontClient before updating the session, and give the app a safe way to observe hook errors? Right now every failure becomes the same generic log, so it is hard to tell what went wrong.
my original idea was on the lines of a security subagent flagged that this could mean we attach a non httpOnly cookie that can be fiddled with by the user, to an encrypted user session. to work around that i exposed the hooks that make it possible to do it yourself, but it's now down to the implementer to keep the cart cookie safe however they want i would now suggest we do it like the initial idea (binding customer account with cart handlers like my code example) and consider either encrypting the cart cookie or just live with it, it's not really a security concern, the cart id cookie is already exposed and does nothing, attaching a modified cart id to your session means nothing at all to us it felt much simpler to operate in the initial implementatioin than it does not (surely a lot less docs) |
TL;DR: Cart handlers can now associate newly created carts with the current Customer Account session, while Customer Account routes expose lifecycle hooks for application-owned synchronization of existing carts.
Before
Cart creation did not include Customer Account buyer identity, and the standard authorization, refresh, and logout routes offered no integration points for related session work.
After
getVerifiedCartIdmust resolve a cart ID from protected server-side ownership state, not an unsigned client-controlled cookie or request value.What this changes
logged_in=true.authenticated,transient, orunauthenticatedresult.Developer impact
Includes a minor changeset for
@shopify/hydrogen. The new cart option, lifecycle hooks, handler types, and documentation are additive; existing handler configuration remains unchanged. Token-bearing hooks require the session returned bycreateCustomerSession; custom session implementations can continue using standard routes andonLogout.Out of scope
Risk