fix: guard authenticated account routes - #204
Merged
Merged
Conversation
Contributor
|
@laaichiu is attempting to deploy a commit to the Spree Commerce Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
WalkthroughThe PR adds protected account routing, shared authenticated shells, validated and market-aware login redirects, refresh-token recovery, localized account pages, and editable profile management with supporting tests. ChangesAccount authentication and storefront
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant SpreeMiddleware
participant AccountLayout
participant AuthShell
participant AccountPage
Browser->>SpreeMiddleware: Request protected account route
SpreeMiddleware->>Browser: Redirect to login when credentials are absent
SpreeMiddleware->>AccountLayout: Continue request with session cookie
AccountLayout->>AuthShell: Provide login href and page children
AuthShell->>AccountPage: Render authenticated page
AccountPage->>Browser: Return localized account content
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
laaichiu
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/app/[country]/[locale]/(storefront)/account/(authenticated)/addresses/page.tsx (1)
14-14: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftAdd localized route metadata for the new account pages.
src/app/[country]/[locale]/(storefront)/account/(authenticated)/addresses/page.tsx#L14-L14: add typed route metadata.src/app/[country]/[locale]/(storefront)/account/(authenticated)/orders/page.tsx#L13-L13: add typed route metadata.src/app/[country]/[locale]/(storefront)/account/(authenticated)/orders/[id]/page.tsx#L15-L17: add typed route metadata.src/app/[country]/[locale]/(storefront)/account/(authenticated)/credit-cards/page.tsx#L11-L13: add typed route metadata.src/app/[country]/[locale]/(storefront)/account/(authenticated)/gift-cards/page.tsx#L11-L11: add typed route metadata.src/app/[country]/[locale]/(storefront)/account/(authenticated)/profile/page.tsx#L215-L229: split the client editor from a server route wrapper, then export typed metadata from the wrapper.As per coding guidelines, “Use Next.js Metadata APIs, including typed
MetadataandgenerateMetadata, for route metadata.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/`[country]/[locale]/(storefront)/account/(authenticated)/addresses/page.tsx at line 14, Add typed Next.js route metadata using the Metadata API for addresses/page.tsx (lines 14-14), orders/page.tsx (lines 13-13), orders/[id]/page.tsx (lines 15-17), credit-cards/page.tsx (lines 11-13), and gift-cards/page.tsx (line 11). In profile/page.tsx (lines 215-229), split the client editor from the server route wrapper, then export typed metadata from the wrapper; use existing localization conventions for the metadata values.Source: Coding guidelines
src/lib/spree/__tests__/auth-helpers.test.ts (1)
102-115: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd stale/expired coverage for the new no-access-token branch.
Only the success path is tested here. The new branch in
ensureFreshSessionalso needs to classify refresh failures as"stale"(refresh token survives) vs"expired"(refresh token cleared) — mirroring the already-tested expired-token branch — but neither failure case is covered for this new code path.✅ Suggested additional test cases
it("reports stale when refresh fails but the refresh token survives", async () => { mockCookieState.access = undefined; mockCookieState.refresh = "rt-1"; mockClient.auth.refresh.mockRejectedValueOnce(new Error("network error")); const state = await ensureFreshSession(); expect(state).toBe("stale"); }); it("reports expired when refresh fails and the refresh token is cleared", async () => { mockCookieState.access = undefined; mockCookieState.refresh = "rt-1"; mockClient.auth.refresh.mockImplementationOnce(async () => { mockCookieState.refresh = undefined; throw Object.assign(new Error("invalid_grant"), { status: 401 }); }); const state = await ensureFreshSession(); expect(state).toBe("expired"); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/spree/__tests__/auth-helpers.test.ts` around lines 102 - 115, Add tests in auth-helpers.test.ts for the no-access-token path in ensureFreshSession: verify a refresh failure that leaves the refresh token intact returns "stale", and a failure that clears it returns "expired". Preserve the existing successful refresh assertions and model the two failure behaviors using rejected refresh calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/app/`[country]/[locale]/(storefront)/account/(authenticated)/orders/page.tsx:
- Around line 22-23: Update the orders loading flow around getOrders and the
completed_at filter so it retrieves and combines all required pages before
filtering and rendering, rather than limiting results to the first 50 records.
Preserve the existing exclusion of incomplete orders and ensure older completed
orders remain visible.
---
Nitpick comments:
In
`@src/app/`[country]/[locale]/(storefront)/account/(authenticated)/addresses/page.tsx:
- Line 14: Add typed Next.js route metadata using the Metadata API for
addresses/page.tsx (lines 14-14), orders/page.tsx (lines 13-13),
orders/[id]/page.tsx (lines 15-17), credit-cards/page.tsx (lines 11-13), and
gift-cards/page.tsx (line 11). In profile/page.tsx (lines 215-229), split the
client editor from the server route wrapper, then export typed metadata from the
wrapper; use existing localization conventions for the metadata values.
In `@src/lib/spree/__tests__/auth-helpers.test.ts`:
- Around line 102-115: Add tests in auth-helpers.test.ts for the no-access-token
path in ensureFreshSession: verify a refresh failure that leaves the refresh
token intact returns "stale", and a failure that clears it returns "expired".
Preserve the existing successful refresh assertions and model the two failure
behaviors using rejected refresh calls.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 29fb60ad-4167-480e-84f7-d98900f06cac
📒 Files selected for processing (19)
src/app/[country]/[locale]/(storefront)/account/(authenticated)/addresses/page.tsxsrc/app/[country]/[locale]/(storefront)/account/(authenticated)/credit-cards/page.tsxsrc/app/[country]/[locale]/(storefront)/account/(authenticated)/gift-cards/page.tsxsrc/app/[country]/[locale]/(storefront)/account/(authenticated)/layout.test.tsxsrc/app/[country]/[locale]/(storefront)/account/(authenticated)/layout.tsxsrc/app/[country]/[locale]/(storefront)/account/(authenticated)/orders/[id]/page.tsxsrc/app/[country]/[locale]/(storefront)/account/(authenticated)/orders/page.tsxsrc/app/[country]/[locale]/(storefront)/account/(authenticated)/profile/page.tsxsrc/app/[country]/[locale]/(storefront)/account/layout.tsxsrc/app/[country]/[locale]/(storefront)/account/page.tsxsrc/components/account/AccountShell.tsxsrc/components/account/AuthenticatedAccountShell.tsxsrc/components/account/__tests__/AuthenticatedAccountShell.test.tsxsrc/lib/spree/__tests__/auth-helpers.test.tssrc/lib/spree/auth-helpers.tssrc/lib/spree/middleware.test.tssrc/lib/spree/middleware.tssrc/lib/utils/__tests__/account-redirect.test.tssrc/lib/utils/account-redirect.ts
💤 Files with no reviewable changes (1)
- src/app/[country]/[locale]/(storefront)/account/layout.tsx
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/app/[country]/[locale]/(storefront)/account/(authenticated)/addresses/page.tsx (1)
14-14: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftAdd localized route metadata for the new account pages.
src/app/[country]/[locale]/(storefront)/account/(authenticated)/addresses/page.tsx#L14-L14: add typed route metadata.src/app/[country]/[locale]/(storefront)/account/(authenticated)/orders/page.tsx#L13-L13: add typed route metadata.src/app/[country]/[locale]/(storefront)/account/(authenticated)/orders/[id]/page.tsx#L15-L17: add typed route metadata.src/app/[country]/[locale]/(storefront)/account/(authenticated)/credit-cards/page.tsx#L11-L13: add typed route metadata.src/app/[country]/[locale]/(storefront)/account/(authenticated)/gift-cards/page.tsx#L11-L11: add typed route metadata.src/app/[country]/[locale]/(storefront)/account/(authenticated)/profile/page.tsx#L215-L229: split the client editor from a server route wrapper, then export typed metadata from the wrapper.As per coding guidelines, “Use Next.js Metadata APIs, including typed
MetadataandgenerateMetadata, for route metadata.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/`[country]/[locale]/(storefront)/account/(authenticated)/addresses/page.tsx at line 14, Add typed Next.js route metadata using the Metadata API for addresses/page.tsx (lines 14-14), orders/page.tsx (lines 13-13), orders/[id]/page.tsx (lines 15-17), credit-cards/page.tsx (lines 11-13), and gift-cards/page.tsx (line 11). In profile/page.tsx (lines 215-229), split the client editor from the server route wrapper, then export typed metadata from the wrapper; use existing localization conventions for the metadata values.Source: Coding guidelines
src/lib/spree/__tests__/auth-helpers.test.ts (1)
102-115: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd stale/expired coverage for the new no-access-token branch.
Only the success path is tested here. The new branch in
ensureFreshSessionalso needs to classify refresh failures as"stale"(refresh token survives) vs"expired"(refresh token cleared) — mirroring the already-tested expired-token branch — but neither failure case is covered for this new code path.✅ Suggested additional test cases
it("reports stale when refresh fails but the refresh token survives", async () => { mockCookieState.access = undefined; mockCookieState.refresh = "rt-1"; mockClient.auth.refresh.mockRejectedValueOnce(new Error("network error")); const state = await ensureFreshSession(); expect(state).toBe("stale"); }); it("reports expired when refresh fails and the refresh token is cleared", async () => { mockCookieState.access = undefined; mockCookieState.refresh = "rt-1"; mockClient.auth.refresh.mockImplementationOnce(async () => { mockCookieState.refresh = undefined; throw Object.assign(new Error("invalid_grant"), { status: 401 }); }); const state = await ensureFreshSession(); expect(state).toBe("expired"); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/spree/__tests__/auth-helpers.test.ts` around lines 102 - 115, Add tests in auth-helpers.test.ts for the no-access-token path in ensureFreshSession: verify a refresh failure that leaves the refresh token intact returns "stale", and a failure that clears it returns "expired". Preserve the existing successful refresh assertions and model the two failure behaviors using rejected refresh calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/app/`[country]/[locale]/(storefront)/account/(authenticated)/orders/page.tsx:
- Around line 22-23: Update the orders loading flow around getOrders and the
completed_at filter so it retrieves and combines all required pages before
filtering and rendering, rather than limiting results to the first 50 records.
Preserve the existing exclusion of incomplete orders and ensure older completed
orders remain visible.
---
Nitpick comments:
In
`@src/app/`[country]/[locale]/(storefront)/account/(authenticated)/addresses/page.tsx:
- Line 14: Add typed Next.js route metadata using the Metadata API for
addresses/page.tsx (lines 14-14), orders/page.tsx (lines 13-13),
orders/[id]/page.tsx (lines 15-17), credit-cards/page.tsx (lines 11-13), and
gift-cards/page.tsx (line 11). In profile/page.tsx (lines 215-229), split the
client editor from the server route wrapper, then export typed metadata from the
wrapper; use existing localization conventions for the metadata values.
In `@src/lib/spree/__tests__/auth-helpers.test.ts`:
- Around line 102-115: Add tests in auth-helpers.test.ts for the no-access-token
path in ensureFreshSession: verify a refresh failure that leaves the refresh
token intact returns "stale", and a failure that clears it returns "expired".
Preserve the existing successful refresh assertions and model the two failure
behaviors using rejected refresh calls.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 29fb60ad-4167-480e-84f7-d98900f06cac
📒 Files selected for processing (19)
src/app/[country]/[locale]/(storefront)/account/(authenticated)/addresses/page.tsxsrc/app/[country]/[locale]/(storefront)/account/(authenticated)/credit-cards/page.tsxsrc/app/[country]/[locale]/(storefront)/account/(authenticated)/gift-cards/page.tsxsrc/app/[country]/[locale]/(storefront)/account/(authenticated)/layout.test.tsxsrc/app/[country]/[locale]/(storefront)/account/(authenticated)/layout.tsxsrc/app/[country]/[locale]/(storefront)/account/(authenticated)/orders/[id]/page.tsxsrc/app/[country]/[locale]/(storefront)/account/(authenticated)/orders/page.tsxsrc/app/[country]/[locale]/(storefront)/account/(authenticated)/profile/page.tsxsrc/app/[country]/[locale]/(storefront)/account/layout.tsxsrc/app/[country]/[locale]/(storefront)/account/page.tsxsrc/components/account/AccountShell.tsxsrc/components/account/AuthenticatedAccountShell.tsxsrc/components/account/__tests__/AuthenticatedAccountShell.test.tsxsrc/lib/spree/__tests__/auth-helpers.test.tssrc/lib/spree/auth-helpers.tssrc/lib/spree/middleware.test.tssrc/lib/spree/middleware.tssrc/lib/utils/__tests__/account-redirect.test.tssrc/lib/utils/account-redirect.ts
💤 Files with no reviewable changes (1)
- src/app/[country]/[locale]/(storefront)/account/layout.tsx
🛑 Comments failed to post (1)
src/app/[country]/[locale]/(storefront)/account/(authenticated)/orders/page.tsx (1)
22-23: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Do not silently truncate order history.
Lines 22-23 fetch only the first 50 records, then discard incomplete ones. Customers with older orders beyond that page will never see them. Add pagination/load-more behavior or retrieve all required pages before rendering.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/`[country]/[locale]/(storefront)/account/(authenticated)/orders/page.tsx around lines 22 - 23, Update the orders loading flow around getOrders and the completed_at filter so it retrieves and combines all required pages before filtering and rendering, rather than limiting results to the first 50 records. Preserve the existing exclusion of incomplete orders and ensure older completed orders remain visible.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lib/utils/__tests__/account-redirect.test.ts (1)
50-82: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd explicit return types to the new callbacks.
src/lib/utils/__tests__/account-redirect.test.ts#L50-L82: annotate new suite and test callbacks with: void.src/hooks/__tests__/useCountrySwitch.test.ts#L25-L27: annotate the mock implementation with: string.src/hooks/__tests__/useCountrySwitch.test.ts#L124-L159: annotate the async test callback with: Promise<void>.As per coding guidelines, use strict TypeScript: define explicit function return types.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/utils/__tests__/account-redirect.test.ts` around lines 50 - 82, Annotate the callbacks in the affected tests with explicit return types: add : void to the rebaseAccountRedirect describe and test callbacks in src/lib/utils/__tests__/account-redirect.test.ts (lines 50-82), add : string to the mock implementation in src/hooks/__tests__/useCountrySwitch.test.ts (lines 25-27), and add : Promise<void> to the async test callback in src/hooks/__tests__/useCountrySwitch.test.ts (lines 124-159).Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/lib/utils/__tests__/account-redirect.test.ts`:
- Around line 50-82: Annotate the callbacks in the affected tests with explicit
return types: add : void to the rebaseAccountRedirect describe and test
callbacks in src/lib/utils/__tests__/account-redirect.test.ts (lines 50-82), add
: string to the mock implementation in
src/hooks/__tests__/useCountrySwitch.test.ts (lines 25-27), and add :
Promise<void> to the async test callback in
src/hooks/__tests__/useCountrySwitch.test.ts (lines 124-159).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: cfd896d6-9572-458b-ac34-950461c66f2a
📒 Files selected for processing (4)
src/hooks/__tests__/useCountrySwitch.test.tssrc/hooks/useCountrySwitch.tssrc/lib/utils/__tests__/account-redirect.test.tssrc/lib/utils/account-redirect.ts
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
damianlegawiec
approved these changes
Jul 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #202
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes