-
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 16 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 |
|---|---|---|
|
|
@@ -52,3 +52,5 @@ export default function productDetailsPage( | |
| }; | ||
| }; | ||
| } | ||
|
|
||
| export const cache = "no-cache"; | ||
| 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"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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,19 @@ 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); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await ctx.invoke.vtex.actions.session.validateSession(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // 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,56 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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.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 |
||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,12 @@ | ||
| import { getCookies, getSetCookies } from "std/http/cookie.ts"; | ||
| import { getCookies, getSetCookies, setCookie } from "std/http/cookie.ts"; | ||
| import { AppContext } from "../../mod.ts"; | ||
| import { getSegmentFromBag } from "../../utils/segment.ts"; | ||
| import { AuthResponse } from "../../utils/types.ts"; | ||
| import setLoginCookies from "../../utils/login/setLoginCookies.ts"; | ||
| import { | ||
| buildCookieJar, | ||
| proxySetCookie, | ||
| REFRESH_TOKEN_COOKIE, | ||
| } from "../../utils/cookies.ts"; | ||
|
|
||
| export interface Props { | ||
| email: string; | ||
|
|
@@ -27,6 +31,8 @@ export default async function action( | |
| } | ||
|
|
||
| const cookies = getCookies(req.headers); | ||
| const startSetCookies = getSetCookies(ctx.response.headers); | ||
| const { header: cookie } = buildCookieJar(req.headers, startSetCookies); | ||
| const VtexSessionToken = cookies?.["VtexSessionToken"] ?? null; | ||
|
|
||
| if (!VtexSessionToken) { | ||
|
|
@@ -50,6 +56,7 @@ export default async function action( | |
| headers: { | ||
| "Accept": "application/json", | ||
| "Content-Type": "application/x-www-form-urlencoded", | ||
| cookie, | ||
| }, | ||
| }, | ||
| ); | ||
|
|
@@ -61,8 +68,19 @@ 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); | ||
| await ctx.invoke.vtex.actions.session.validateSession(); | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| // TODO: REMOVE THIS AFTER TESTING AND VALIDATE IF NEEDED | ||
| 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 | ||
| }); | ||
| } | ||
| } | ||
|
Comment on lines
+71
to
+83
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. Remove duplicate Set-Cookie and avoid Path="/" for vid_rt Normalize the refresh-token cookie once via proxy transform to Path=/api/vtexid. Apply: - proxySetCookie(response.headers, ctx.response.headers, req.url);
- await ctx.invoke.vtex.actions.session.validateSession();
-
- // TODO: REMOVE THIS AFTER TESTING AND VALIDATE IF NEEDED
- 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
- });
- }
- }
+ proxySetCookie(
+ response.headers,
+ ctx.response.headers,
+ req.url,
+ (c) => c.name === REFRESH_TOKEN_COOKIE ? { ...c, path: "/api/vtexid" } : c,
+ );
+ await ctx.invoke.vtex.actions.session.validateSession();
🤖 Prompt for AI Agents |
||
|
|
||
| return data; | ||
| } | ||
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
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