-
Notifications
You must be signed in to change notification settings - Fork 78
feat: proxy VTEX refresh token and tighten ValidateSession refresh logic #3261
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| import type { NextApiHandler } from 'next' | ||
|
|
||
| import discoveryConfig from 'discovery.config' | ||
| import fetch from 'isomorphic-unfetch' | ||
| import { normalizeSetCookieDomain } from 'src/utils/normalizeSetCookieForRequest' | ||
| import { sanitizeHost } from 'src/utils/utilities' | ||
|
|
||
| const VTEX_REFRESH_PATH = '/api/vtexid/refreshtoken/webstore' | ||
|
|
||
| function getSetCookieValuesFromResponse(response: Response): string[] { | ||
| const headers = response.headers as Headers & { | ||
| getSetCookie?: () => string[] | ||
| } | ||
| if (typeof headers.getSetCookie === 'function') { | ||
| return headers.getSetCookie() | ||
| } | ||
| const single = response.headers.get('set-cookie') | ||
| return single ? [single] : [] | ||
| } | ||
|
|
||
| const handler: NextApiHandler = async (request, response) => { | ||
| if (request.method !== 'POST') { | ||
| response.status(405).end() | ||
| return | ||
| } | ||
|
|
||
| if (!discoveryConfig.experimental?.refreshToken) { | ||
| response.status(404).end() | ||
| return | ||
| } | ||
|
|
||
| const cookieHeader = request.headers.cookie | ||
| if (!cookieHeader) { | ||
| console.error('[fs/refresh-token] missing Cookie header') | ||
| response.status(401).json({ status: 'Error' }) | ||
| return | ||
| } | ||
|
|
||
| const url = `${discoveryConfig.storeUrl}${VTEX_REFRESH_PATH}` | ||
|
|
||
| try { | ||
| const vtexRes = await fetch(url, { | ||
| method: 'POST', | ||
| headers: { | ||
| 'content-type': 'application/json', | ||
| cookie: cookieHeader, | ||
| Host: `${sanitizeHost(discoveryConfig.storeUrl)}`, | ||
| }, | ||
| body: JSON.stringify({}), | ||
| }) | ||
|
|
||
| const text = await vtexRes.text() | ||
| let data: Record<string, unknown> | ||
| try { | ||
| data = text ? (JSON.parse(text) as Record<string, unknown>) : {} | ||
| } catch { | ||
| console.error('[fs/refresh-token] invalid JSON body', text.slice(0, 500)) | ||
| response.status(500).json({ status: 'Error' }) | ||
| return | ||
| } | ||
|
|
||
| const setCookies = getSetCookieValuesFromResponse(vtexRes).map((c) => | ||
| normalizeSetCookieDomain({ request, setCookie: c }) | ||
| ) | ||
| if (setCookies.length > 0) { | ||
| response.setHeader('set-cookie', setCookies) | ||
| } | ||
|
|
||
| if (vtexRes.status !== 200) { | ||
| console.error( | ||
| '[fs/refresh-token] VTEX non-200', | ||
| vtexRes.status, | ||
| text.slice(0, 500) | ||
| ) | ||
| response | ||
| .status(vtexRes.status === 401 ? 401 : 500) | ||
| .json(data ?? { status: 'Error' }) | ||
| return | ||
| } | ||
|
|
||
| response.status(200).json(data) | ||
| } catch (err) { | ||
| console.error('[fs/refresh-token] proxy error', err) | ||
| response.status(500).end() | ||
| } | ||
| } | ||
|
|
||
| export default handler |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,9 @@ | ||
| import { useEffect, useState } from 'react' | ||
| import { | ||
| clearRefreshFailureCount, | ||
| incrementRefreshFailureCount, | ||
| MAX_REFRESH_RETRIES, | ||
| } from 'src/utils/refreshTokenRetry' | ||
| import { sessionStore } from '../session' | ||
| import { isRefreshTokenSuccessful, refreshTokenRequest } from './refreshToken' | ||
|
|
||
|
|
@@ -18,6 +23,7 @@ export const useRefreshToken = ( | |
| const result = await refreshTokenRequest() | ||
|
|
||
| if (isRefreshTokenSuccessful(result)) { | ||
| clearRefreshFailureCount() | ||
| // Update session with new refreshAfter timestamp | ||
| const refreshAfter = String( | ||
| Math.floor(new Date(result?.refreshAfter).getTime() / 1000) | ||
|
|
@@ -37,6 +43,13 @@ export const useRefreshToken = ( | |
| url.searchParams.set('_refresh', Date.now().toString()) | ||
| window.location.href = url.toString() | ||
| } else { | ||
| const failures = incrementRefreshFailureCount() | ||
| if (failures >= MAX_REFRESH_RETRIES) { | ||
| clearRefreshFailureCount() | ||
| sessionStore.set(sessionStore.readInitial()) | ||
| window.location.href = '/login' | ||
| return | ||
| } | ||
|
Comment on lines
+46
to
+52
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Check all call sites of validateSession and how they might overlap with useRefreshToken
echo "=== validateSession call sites ==="
rg -n "validateSession\(" --type=ts -g '!*.test.ts' -g '!*.d.ts' -C2
echo ""
echo "=== useRefreshToken usage ==="
rg -n "useRefreshToken\(" --type=ts -g '!*.test.ts' -g '!*.d.ts' -C2Repository: vtex/faststore Length of output: 5456 🏁 Script executed: cat -n packages/core/src/sdk/account/useRefreshToken.tsRepository: vtex/faststore Length of output: 3589 🏁 Script executed: cat -n packages/core/src/sdk/session/index.tsRepository: vtex/faststore Length of output: 10416 Coordinate refresh token retry counting to prevent double-increments. Both Consider centralizing the retry logic to a single call site or adding timestamp-based deduplication to prevent double-counting within a short window. 🤖 Prompt for AI Agents |
||
| // If refresh token failed, set refreshAfter to now + 1 hour | ||
| sessionStore.set({ | ||
| ...currentSession, | ||
|
|
@@ -48,6 +61,14 @@ export const useRefreshToken = ( | |
| } catch (error) { | ||
| console.error('Error during refresh token process:', error) | ||
|
|
||
| const failures = incrementRefreshFailureCount() | ||
| if (failures >= MAX_REFRESH_RETRIES) { | ||
| clearRefreshFailureCount() | ||
| sessionStore.set(sessionStore.readInitial()) | ||
| window.location.href = '/login' | ||
| return | ||
| } | ||
|
|
||
| // Set refreshAfter to postpone future requests and redirect to login | ||
| sessionStore.set({ | ||
| ...currentSession, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Retrying on 404 may cause unnecessary delays when feature is disabled server-side.
If the server has
experimental.refreshTokendisabled (returns 404), the client will retry 3 times before giving up. Consider treating 404 as a non-retryable status to fail fast when there's a client/server config mismatch.Suggested fix
if (res.status !== 200) { + // 404 means feature disabled server-side - don't retry + if (res.status === 404) { + console.warn('[refreshTokenRequest] refresh-token endpoint not enabled') + return undefined + } console.error( '[refreshTokenRequest] non-200 response', lastStatus, lastBody.slice(0, 500) ) continue }📝 Committable suggestion
🤖 Prompt for AI Agents