@ W-21066921 @W-21201637: Add HttpOnly session cookies for SLAS private client proxy#3680
Conversation
When MRT_DISABLE_HTTPONLY_SESSION_COOKIES is 'false', token responses from SLAS are intercepted: access_token, refresh_token, and idp_access_token are set as HttpOnly cookies and stripped from the response body. The client receives access_token_expires_at for expiry checks without needing the JWT. Server-side (pwa-kit-runtime): - applyHttpOnlySessionCookies() intercepts token responses, sets HttpOnly cookies with siteId suffix, and strips tokens from body - applyProxyRequestAuthHeader() reads access token from HttpOnly cookie and sets Authorization header for SCAPI proxy requests - isScapiDomain() utility for identifying Commerce API domains - Configurable tokenResponseEndpoints and slasEndpointsRequiringAccessToken regexes for controlling which endpoints are processed Client-side (commerce-sdk-react): - useHttpOnlySessionCookies flag on Auth and CommerceApiProvider - isAccessTokenExpired() uses access_token_expires_at when HttpOnly enabled - handleTokenResponse() skips storing tokens in localStorage when HttpOnly - Provider ensures fetch credentials allow cookies to be sent Note: TAOB (Trusted Agent on Behalf) and refresh token flows with HttpOnly cookies will be handled in follow-up work. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
| if (expiresAt == null || expiresAt === '') return true | ||
| const expiresAtSec = Number(expiresAt) | ||
| if (Number.isNaN(expiresAtSec)) return true | ||
| const bufferSeconds = 60 |
There was a problem hiding this comment.
The purpose of bufferSeconds is to avoid race conditions where a token is technically still valid when the check happens, but expires by the time the SCAPI request reaches the server. By refreshing 60 seconds early, you ensure the token is still valid when the downstream API processes the request.
The original code has the same pattern
| @@ -1,3 +1,6 @@ | |||
| ## [Unreleased] | |||
There was a problem hiding this comment.
This is just to avoid merge conflict when we merge in to develop branch
| ssrFunctionNodeVersion: '24.x', | ||
| // Store the session cookies as HttpOnly for enhanced security. | ||
| disableHttpOnlySessionCookies: false, | ||
| disableHttpOnlySessionCookies: true, |
There was a problem hiding this comment.
We will enable httponly cookies once we add the implementation for slas public client
| isTokenEndpoint | ||
| ) { | ||
| try { | ||
| workingBuffer = applyHttpOnlySessionCookies( |
There was a problem hiding this comment.
running applyHttpOnlySessionCookies before onSLASPrivateProxyRes
- ensures tokens are secured in HttpOnly cookies before any custom code runs.
- applyHttpOnlySessionCookies is guaranteed to have unmodified slas response
|
|
||
| // Refresh token cookie TTL defaults (seconds). Must stay in sync with commerce-sdk-react auth constants. | ||
| const DEFAULT_SLAS_REFRESH_TOKEN_GUEST_TTL = 30 * 24 * 60 * 60 | ||
| const DEFAULT_SLAS_REFRESH_TOKEN_REGISTERED_TTL = 90 * 24 * 60 * 60 |
There was a problem hiding this comment.
Let's separate these auth-specific functions into a separate file since build-remote-server.js is quite a large file that does too many things already.
| } | ||
|
|
||
| // IDP access token | ||
| const idpAccessToken = parsed.idp_access_token |
There was a problem hiding this comment.
I wonder if we should break this function down into a couple of helpers to aid with readability like applyHttpOnlyAccessToken, applyHttpOnlyIdpToken, etc.
| // This will be used to read the correct access token cookie | ||
| try { | ||
| const config = getConfig({buildDirectory: options.buildDir}) | ||
| options.siteId = config?.app?.commerceAPI?.parameters?.siteId || null |
There was a problem hiding this comment.
What happens in a multi-site scenario where a shopper changes sites?
In the app, we determine site id using the logic in AppConfig.restore user has the ability to change it. But here, I think this is set once from the config on server startup so I don't know how this works with multi-site?
There was a problem hiding this comment.
good point. I will create a followup ticket to look into multi-site
| * @param targetHost {String} the target hostname (host+port) | ||
| * @param slasEndpointsRequiringAccessToken {RegExp} regex for SLAS auth endpoints that need Bearer token | ||
| */ | ||
| export const applyProxyRequestAuthHeader = ({ |
There was a problem hiding this comment.
This is a function specifically for SCAPI / SLAS calls. We might be able to get away with a more explicit name like applySCAPIAuthHeaders and only go into this function if isSCAPIDomain is true
| caching | ||
| caching, | ||
| siteId = null, | ||
| slasEndpointsRequiringAccessToken = /\/oauth2\/logout/ |
There was a problem hiding this comment.
We're hard coding /\/oauth2\/logout/ so we might want this in a constants file in case we want to update this default so we only have to change this in 1 place.
There was a problem hiding this comment.
With the new SLAS_ENDPOINTS_REQUIRING_ACCESS_TOKEN constant, could we set this as the following?
| slasEndpointsRequiringAccessToken = /\/oauth2\/logout/ | |
| slasEndpointsRequiringAccessToken = SLAS_ENDPOINTS_REQUIRING_ACCESS_TOKEN |
| storageType: 'local', | ||
| key: 'uido' | ||
| }, | ||
| 'cc-at-expires': { |
There was a problem hiding this comment.
We must validate if cc-at-expires cookies is also set by Hybrid Auth in ECOM whenever a token is generated by hybrid auth.
There was a problem hiding this comment.
Thanks for reminding. We had a discussion with ECOM and we are aligned on the cookies
| const configLogger = logger || console | ||
|
|
||
| // When HttpOnly cookies are enabled, ensure fetch credentials allow cookies to be sent. | ||
| const effectiveFetchOptions = |
There was a problem hiding this comment.
nit: Consider wrapping this in a useMemo to avoid creating a new object reference on every render.
shethj
left a comment
There was a problem hiding this comment.
Left some non-blocking comments.
Thanks for the great work! :D
vcua-mobify
left a comment
There was a problem hiding this comment.
Just a non-blocking comment. Yes, let's follow up on multi-site in a follow up PR.
Thanks @unandyala !
| caching | ||
| caching, | ||
| siteId = null, | ||
| slasEndpointsRequiringAccessToken = /\/oauth2\/logout/ |
There was a problem hiding this comment.
With the new SLAS_ENDPOINTS_REQUIRING_ACCESS_TOKEN constant, could we set this as the following?
| slasEndpointsRequiringAccessToken = /\/oauth2\/logout/ | |
| slasEndpointsRequiringAccessToken = SLAS_ENDPOINTS_REQUIRING_ACCESS_TOKEN |
| appHostname, | ||
| appProtocol, | ||
| siteId = null, | ||
| slasEndpointsRequiringAccessToken = /\/oauth2\/logout/ |
Summary
MRT_DISABLE_HTTPONLY_SESSION_COOKIESis'false', SLAS token responses are intercepted by the proxy:access_token,refresh_token, andidp_access_tokenare set as HttpOnly cookies (with siteId suffix) and stripped from the response bodyaccess_token_expires_atin the response body for expiry checks without needing the JWTAuthorization: Bearerheader for SCAPI requests automatically viaapplyProxyRequestAuthHeader()useHttpOnlySessionCookiesflag onAuthandCommerceApiProvidercontrols client-side behavior: skips storing tokens in localStorage, usesaccess_token_expires_atfor expiry checks, and ensures fetch credentials allow cookiestokenResponseEndpointsandslasEndpointsRequiringAccessTokenregexes allow customization inssr.jsFollow-up work
Test plan
MRT_DISABLE_HTTPONLY_SESSION_COOKIES=falseaccess_token_expires_atis presentAuthorization: Bearerheader from cookiehttponly-cookies.mov
How To Test
useSLASPrivateClient: truein ssr.jsenablePWAKitPrivateClient={true}in _app-config/index.jsx on the CommerceApiProviderdisableHttpOnlySessionCookies: falseunder ssrParameters in config/default.jspackages/template-retail-react-app🤖 Generated with Claude Code