fix(vtex): remove isLoggedIn check from middleware#1565
Conversation
CDN already handles cache bypass for logged-in users via VtexIdclientAutCookie cookie check, making this check redundant.
Tagging OptionsShould a new tag be published when this PR is merged?
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
📝 WalkthroughWalkthroughThe middleware now determines page cacheability solely based on segment-level rules, removing cookie-based login state detection. Previously, pages were marked uncacheable if a VTEX ID client cookie was present; this check is eliminated, allowing authenticated requests to participate in CDN caching when segments permit. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@vtex/middleware.ts`:
- Line 30: The current cache decision uses isCacheableSegment(ctx) but omits
authentication, allowing authenticated (personalized) responses to be cached;
update the middleware where const cacheable = isCacheableSegment(ctx) to also
check app-side auth and force non-cacheable for authenticated requests (e.g.,
const cacheable = isCacheableSegment(ctx) && !isAuthenticated(ctx)); implement
or call a small helper isAuthenticated(ctx) that inspects the app's auth
indicator present on the request context (for example ctx.state.user,
ctx.vtex?.authToken, or whatever your app sets when a user is signed in) and
ensure any authenticated request sets cacheable = false so personalized
responses are never marked cacheable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| ); | ||
|
|
||
| const cacheable = isCacheableSegment(ctx) && !isLoggedIn; | ||
| const cacheable = isCacheableSegment(ctx); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Check whether any in-app auth cache guard still exists:"
rg -n -C3 --type=ts 'isCacheableSegment\(ctx\)|PAGE_CACHE_ALLOWED_KEY|VtexIdclientAutCookie|VTEX_ID_CLIENT_COOKIE'
echo
echo "Check if Cloudflare/CDN bypass rules are versioned in this repo:"
rg -n -i -C2 'cloudflare|cdn|cache bypass|bypass cache|VtexIdclientAutCookie'Repository: deco-cx/apps
Length of output: 50369
🏁 Script executed:
# Read the isCacheableSegment function to see if it checks authentication
sed -n '60,90p' vtex/utils/segment.tsRepository: deco-cx/apps
Length of output: 1027
🏁 Script executed:
# Check for any Cloudflare worker files or wrangler configs
find . -type f \( -name "wrangler.toml" -o -name "*.wrangler.json" -o -name "*cloudflare*" -o -name "*.cf.ts" -o -name "*.cf.js" \) 2>/dev/null | head -20Repository: deco-cx/apps
Length of output: 38
🏁 Script executed:
# Check for environment or config files that might define Cloudflare bypass rules
find . -type f \( -name ".env*" -o -name "*.env" -o -name "*config*" \) | xargs grep -l "cloudflare\|bypass\|cache" 2>/dev/null | head -10Repository: deco-cx/apps
Length of output: 38
🏁 Script executed:
# Search for isAnonymous function definition
rg -n "isAnonymous" vtex/utils/segment.ts -A 10 -B 2Repository: deco-cx/apps
Length of output: 1275
🏁 Script executed:
# Check if there's any recent mention of Cloudflare worker rules or CDN bypass in comments/docs
rg -i "cloudflare.*worker|worker.*cloudflare|cdn.*bypass|cache.*bypass" --type=ts --type=js --type=toml --type=json -C 3Repository: deco-cx/apps
Length of output: 565
🏁 Script executed:
# Look for VtexIdclientAutCookie checks in the middleware or related auth functions
rg -n "hasAuthCookie|VtexIdclientAutCookie" vtex/ -B 2 -A 2Repository: deco-cx/apps
Length of output: 26316
🏁 Script executed:
# Check the isAnonymous function more thoroughly, especially what segment fields it checks
sed -n '39,63p' vtex/utils/segment.tsRepository: deco-cx/apps
Length of output: 517
🏁 Script executed:
# Verify if Cloudflare rules are documented elsewhere or in deployment configs
find . -type f \( -name "*.md" -o -name "*.txt" -o -name "deco.json" \) | xargs grep -l -i "cloudflare\|cache\|bypass" 2>/dev/null | head -10Repository: deco-cx/apps
Length of output: 73
Add app-side authentication check to prevent caching of personalized responses.
isCacheableSegment(ctx) at line 30 does not check authentication status—it only evaluates segment fields (campaigns, utm_campaign, utm_source, utmi_campaign, channel, priceTables, regionId). This allows authenticated requests to be marked cacheable, creating a cache leak risk if the Cloudflare bypass rule is missing or misconfigured in any environment.
The suggested fix provides essential defense-in-depth:
🔒 Suggested fix
+ const hasAuthCookie = Boolean(
+ cookies.VtexIdclientAutCookie ||
+ cookies[`${ctx.account}VtexIdclientAutCookie`],
+ );
- const cacheable = isCacheableSegment(ctx);
+ const cacheable = isCacheableSegment(ctx) && !hasAuthCookie;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@vtex/middleware.ts` at line 30, The current cache decision uses
isCacheableSegment(ctx) but omits authentication, allowing authenticated
(personalized) responses to be cached; update the middleware where const
cacheable = isCacheableSegment(ctx) to also check app-side auth and force
non-cacheable for authenticated requests (e.g., const cacheable =
isCacheableSegment(ctx) && !isAuthenticated(ctx)); implement or call a small
helper isAuthenticated(ctx) that inspects the app's auth indicator present on
the request context (for example ctx.state.user, ctx.vtex?.authToken, or
whatever your app sets when a user is signed in) and ensure any authenticated
request sets cacheable = false so personalized responses are never marked
cacheable.
1 participant
Summary
isLoggedIncheck from VTEX middlewareVtexIdclientAutCookiecookie check, making this check redundantTest plan
Summary by cubic
Removed the login check from the VTEX middleware so cacheability depends only on segment rules. Logged-in users still bypass cache at the CDN via the
VtexIdclientAutCookierule, avoiding redundant logic.VTEX_ID_CLIENT_COOKIEimport andisLoggedIncheck;cacheablenow uses onlyisCacheableSegment(ctx).Written for commit aaa06a7. Summary will update on new commits.
Summary by CodeRabbit