-
Notifications
You must be signed in to change notification settings - Fork 21
WIP: fix Logout and session VTEX #1366
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: main
Are you sure you want to change the base?
Changes from 26 commits
0e199ed
0f5f80f
642c16c
b55ac96
da2605b
c5e23fd
ff91240
fc1c5ba
b127448
59e1533
35046f4
1d87ac0
7b16396
a04c090
fe3fcac
84fc153
9f56d67
2495519
65a9a41
db21a39
141dbd6
134fefa
8e36460
00a9ff5
ad85bab
3a5a9a2
9807fc1
e34db75
11c29d9
6b1da03
e8063df
ef77710
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,50 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { logger } from "@deco/deco/o11y"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { AppContext } from "../mod.ts"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { createClient, PaginationOptions } from "../utils/client.ts"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { VerifiedReviewsFullReview } from "../utils/types.ts"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export type Props = PaginationOptions & { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| productId: string | string[]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export default async function loader( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| props: Props, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _req: Request, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ctx: AppContext, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ): Promise<VerifiedReviewsFullReview> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const client = createClient({ ...ctx }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!client) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| aggregateRating: undefined, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| review: [], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const data = await client.fullReview(props); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return data; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const message = error instanceof Error ? error.message : String(error); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.error("Error getting full review", message); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| aggregateRating: undefined, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| review: [], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export const cache = "stale-while-revalidate"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export const cacheKey = (props: Props) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const normalized = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| productId: Array.isArray(props.productId) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ? [...props.productId].sort().join(",") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| : props.productId, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| count: props.count ?? 10, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| offset: props.offset ?? 0, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| order: props.order ?? "date_desc", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| customizeOrder: props.customizeOrder ?? false, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return `${normalized.productId}-${normalized.count}-${normalized.offset}-${normalized.order}-${normalized.customizeOrder}`; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+38
to
+50
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. Inconsistent default values between cacheKey and client implementation. The Apply this diff to align the default with the client implementation: - count: props.count ?? 10,
+ count: props.count ?? 5,📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,3 +37,5 @@ export default function productList( | |
| }); | ||
| }; | ||
| } | ||
|
|
||
| export const cache = "no-cache"; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -45,3 +45,5 @@ export default function productListingPage( | |
| }; | ||
| }; | ||
| } | ||
|
|
||
| export const cache = "no-cache"; | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,7 +1,12 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { AppContext } from "../../mod.ts"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { AuthResponse } from "../../utils/types.ts"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import setLoginCookies from "../../utils/login/setLoginCookies.ts"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { getSetCookies } from "std/http/cookie.ts"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // import setLoginCookies from "../../utils/login/setLoginCookies.ts"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { getSetCookies, setCookie } from "std/http/cookie.ts"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| buildCookieJar, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| proxySetCookie, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| REFRESH_TOKEN_COOKIE, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } from "../../utils/cookies.ts"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
1
to
+9
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. 🛠️ Refactor suggestion | 🟠 Major Unify strategy for vid_rt path across the codebase PR intent highlights path mismatch issues. Decide one policy (keep VTEX default path vs. rewrite to “/”) and apply consistently (proxy, resetPassword, accessKeySignIn, login utils). I can help extract a utility and update call sites. 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export interface Props { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| email: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -14,7 +19,7 @@ export interface Props { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export default async function action( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| props: Props, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _req: Request, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| req: Request, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ctx: AppContext, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ): Promise<AuthResponse> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { vcsDeprecated } = ctx; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -26,6 +31,8 @@ export default async function action( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const startAuthentication = await ctx.invoke.vtex.actions.authentication | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .startAuthentication({}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const startSetCookies = getSetCookies(ctx.response.headers); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { header: cookieHeader } = buildCookieJar(req.headers, startSetCookies); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const authenticationToken = startAuthentication?.authenticationToken; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!authenticationToken) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -39,17 +46,19 @@ export default async function action( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| urlencoded.append("password", props.password); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| urlencoded.append("authenticationToken", authenticationToken); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const response = await vcsDeprecated | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ["POST /api/vtexid/pub/authentication/classic/validate"]( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| {}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| body: urlencoded, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| headers: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "Content-Type": "application/x-www-form-urlencoded", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "Accept": "application/json", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const response = await vcsDeprecated[ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "POST /api/vtexid/pub/authentication/classic/validate" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ]( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| {}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| body: urlencoded, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| headers: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "Content-Type": "application/x-www-form-urlencoded", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Accept: "application/json", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cookie: cookieHeader, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!response.ok) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new Error( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -58,8 +67,26 @@ export default async function action( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const data: AuthResponse = await response.json(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const setCookies = getSetCookies(response.headers); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await setLoginCookies(data, ctx, setCookies); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| proxySetCookie(response.headers, ctx.response.headers, req.url); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // First login was made, but we need to call the refresh token at least once to get the refreshAfter for the first time | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const dataRefreshToken = await ctx.invoke.vtex.actions.authentication | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .refreshToken(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await ctx.invoke.vtex.actions.session.validateSession({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| publicProperties: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| refreshAfter: { value: dataRefreshToken.refreshAfter }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // TODO: REMOVE THIS AFTER TESTING AND VALIDATE IF NEEDED REWRITE REFRESH_TOKEN_COOKIE | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const setCookies = getSetCookies(ctx.response.headers); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for (const cookie of setCookies) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (cookie.name === REFRESH_TOKEN_COOKIE) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setCookie(ctx.response.headers, { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...cookie, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| path: "/", // default path is /api/vtexid/refreshtoken/webstore, but browser dont send to backend headers | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // TODO: REMOVE THIS AFTER TESTING AND VALIDATE IF NEEDED REWRITE REFRESH_TOKEN_COOKIE | |
| const setCookies = getSetCookies(ctx.response.headers); | |
| for (const cookie of setCookies) { | |
| if (cookie.name === REFRESH_TOKEN_COOKIE) { | |
| setCookie(ctx.response.headers, { | |
| ...cookie, | |
| path: "/", // default path is /api/vtexid/refreshtoken/webstore, but browser dont send to backend headers | |
| }); | |
| } | |
| } | |
| // If rewriting is required, expire the legacy path first to avoid duplicates | |
| const setCookies = getSetCookies(ctx.response.headers); | |
| const rt = setCookies.find((c) => c.name === REFRESH_TOKEN_COOKIE); | |
| if (rt) { | |
| if (rt.path && rt.path !== "/") { | |
| setCookie(ctx.response.headers, { | |
| name: rt.name, | |
| value: "", | |
| domain: rt.domain, | |
| path: rt.path, | |
| secure: rt.secure, | |
| httpOnly: rt.httpOnly, | |
| sameSite: rt.sameSite, | |
| maxAge: 0, | |
| expires: new Date(0), | |
| }); | |
| } | |
| setCookie(ctx.response.headers, { ...rt, path: "/" }); | |
| } |
🤖 Prompt for AI Agents
In vtex/actions/authentication/classicSignIn.ts around lines 73 to 82, the code
forces REFRESH_TOKEN_COOKIE to path "/" which can create a second vid_rt cookie
("/" plus the original "/api/.../webstore") and cause nondeterministic
logout/session behavior; remove the hardcoded path override and instead preserve
the cookie's original path when calling setCookie (or, if you must promote it to
"/", explicitly expire the original cookie at its original path before setting
the "/" cookie) and delete the temporary TODO comment.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,34 +1,57 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { getCookies, setCookie } from "std/http/mod.ts"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { AppContext } from "../../mod.ts"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { parseCookie } from "../../utils/vtexId.ts"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // import { getCookies, getSetCookies } from "std/http/cookie.ts"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import type { AppContext } from "../../mod.ts"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { proxySetCookie, REFRESH_TOKEN_COOKIE } from "../../utils/cookies.ts"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // import { setSegmentBag } from "../../utils/segment.ts"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { redirect } from "@deco/deco"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { LogoutResponse } from "../../utils/types.ts"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { deleteCookie } from "std/http/cookie.ts"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| export interface Props { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| * URL to redirect after logout | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @default "/" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| returnUrl?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @title Logout | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @description Logout the user | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @title VTEX Integration - Logout | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @description Performs user logout and cleans session data | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| export default async function action( | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| _: unknown, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| { returnUrl = "/" }: Props, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| req: Request, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ctx: AppContext, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const cookies = getCookies(req.headers); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { payload } = parseCookie(req.headers, ctx.account); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| for (const cookieName in cookies) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (cookieName.startsWith("VtexIdclientAutCookie")) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| setCookie(ctx.response.headers, { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| name: cookieName, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| value: "", | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| expires: new Date(0), | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| path: "/", | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ): Promise<LogoutResponse> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { account, vcsDeprecated } = ctx; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| const response = await vcsDeprecated["GET /api/vtexid/pub/logout"]({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| scope: account, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| headers: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| "cookie": req.headers.get("cookie") || "", | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| const sessionId = payload?.sess; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!sessionId) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!response.ok) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new Error( | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| `Logout request failed: ${response.status} ${response.statusText}`, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| await ctx.invoke.vtex.actions.session.deleteSession({ sessionId }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| const data = await response.json(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| deleteCookie(ctx.response.headers, REFRESH_TOKEN_COOKIE, { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| path: "/", | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| domain: new URL(req.url).hostname, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+44
to
+47
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. Fix: delete refresh-token cookie using VTEX path, not "/" The refresh token cookie (vid_rt) is scoped to /api/vtexid/refreshtoken/webstore. Deleting it at path "/" won’t clear the scoped cookie. Apply: - deleteCookie(ctx.response.headers, REFRESH_TOKEN_COOKIE, {
- path: "/",
- domain: new URL(req.url).hostname,
- });
+ const VTEX_REFRESH_PATH = "/api/vtexid/refreshtoken/webstore";
+ const targetHost = new URL(req.url).hostname;
+ deleteCookie(ctx.response.headers, REFRESH_TOKEN_COOKIE, {
+ path: VTEX_REFRESH_PATH,
+ domain: targetHost,
+ });📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| proxySetCookie(response.headers, ctx.response.headers, req.url); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
42
to
50
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. 🛠️ Refactor suggestion Normalize auth cookie Path on logout as well. Logout relies on Set-Cookie expirations; if Path isn’t the refresh-token path, cookies may linger for that scope. Mirror the sign-in fix here. Apply within this hunk: - const data = await response.json();
- proxySetCookie(response.headers, ctx.response.headers, req.url);
+ const contentType = response.headers.get("content-type") ?? "";
+ const data: LogoutResponse =
+ contentType.includes("application/json")
+ ? await response.json()
+ : { cookieName: null, accountCookieName: null };
+ // Proxy all cookies (domain rewrite)
+ proxySetCookie(response.headers, ctx.response.headers, req.url);
+ // Normalize Path for VTEX auth cookies
+ const targetHost = new URL(req.url).hostname;
+ const VTEX_REFRESH_PATH = "/api/vtexid/refreshtoken/webstore";
+ for (const c of getSetCookies(response.headers)) {
+ const needsAuthPath =
+ c.name === "vid_rt" || c.name.startsWith("VtexIdclientAutCookie");
+ if (needsAuthPath) {
+ setCookie(ctx.response.headers, {
+ ...c,
+ domain: targetHost,
+ path: VTEX_REFRESH_PATH,
+ });
+ }
+ }Add imports: -// import { getCookies, getSetCookies } from "std/http/cookie.ts";
+import { getSetCookies, setCookie } from "std/http/cookie.ts";📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||
| await ctx.invoke.vtex.actions.cart.updateUser(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| await ctx.invoke.vtex.actions.session | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| .validateSession(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+51
to
+53
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 chainVerify the invocation order and error handling. The code invokes 🏁 Script executed: #!/bin/bash
# Description: Check error handling patterns in other authentication flows
# Search for similar invoke patterns to understand error handling conventions
rg -nP --type=ts -C5 'invoke\.vtex\.actions\.(cart|session)\.' | rg -A3 -B3 'await.*invoke'Length of output: 3960 Length of output: 870 🏁 Script executed: #!/bin/bash
# Search for error handling patterns around invoke calls specifically
rg -nP --type=ts 'catch.*invoke|\.catch\(' vtex/actions/authentication/ -B5 -A5Length of output: 0 🏁 Script executed: #!/bin/bash
# Check if there's a higher-level error handler or middleware for actions
rg -nP --type=ts 'export.*default|handler|middleware' vtex/actions/authentication/logout.ts -B2 -A5Length of output: 262 Length of output: 1482 Add error handling for invoke calls or document the intended failure behavior. The code inconsistently handles errors: the initial logout API call (line 30) checks 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| redirect(returnUrl); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| return data; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+56
to
+57
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. Prevent open redirects in returnUrl. returnUrl appears user-controlled (derived from the request in the section). Restrict to same-origin paths to avoid phishing/open-redirect. - redirect(returnUrl);
- return data;
+ const safeLocation = (() => {
+ try {
+ if (!returnUrl) return "/";
+ if (returnUrl.startsWith("/")) return returnUrl.startsWith("//") ? "/" : returnUrl;
+ const dest = new URL(returnUrl, req.url);
+ const origin = new URL(req.url).origin;
+ return dest.origin === origin ? (dest.pathname + dest.search + dest.hash) : "/";
+ } catch {
+ return "/";
+ }
+ })();
+ redirect(safeLocation);
+ return data;📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
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.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify the context spreading pattern.
Spreading the entire
AppContextintocreateClientmay pass unnecessary properties. According to the relevant code snippet,createClientexpectsConfigVerifiedReviews | undefined. Please verify that theAppContextstructure is compatible with this expectation, or extract only the necessary properties.🏁 Script executed:
Length of output: 79
🏁 Script executed:
Length of output: 736
🏁 Script executed:
Length of output: 542
Avoid passing entire AppContext into
createClient.createClientaccepts only aConfigVerifiedReviewsobject (it only usesidWebsite), so replacewith something like
🤖 Prompt for AI Agents