fix(core): use authenticator route for setpassword#3380
Conversation
Migrate the My Account "Reset password" flow off the deprecated VTEX ID endpoint (/api/vtexid/pub/authentication/classic/setpassword) onto the new authenticator route (/api/authenticator/pub/authentication/classic/setpassword), passing the account name explicitly via the `an` query string and falling back to config.api.storeId when it is missing. The hook's public surface (useSetPassword / setPassword) and the request body, method, credentials, and authStatus error mapping are unchanged. Adds unit coverage for useSetPassword (URL composition, body, success and error paths, accountName fallback). Ref: SFS-3056 Co-authored-by: Cursor <cursoragent@cursor.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughuseSetPassword now encodes an account identifier as ChangesSet-Password Endpoint Migration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/sdk/account/useSetPassword.ts (1)
42-121:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winAdd
accountNameto the useCallback dependency array.The
setPasswordcallback usesaccountName(lines 46, 56) but omits it from the dependency array. IfaccountNamechanges between renders, the callback will continue using the stale value, sending the wrong account identifier to the API.🔧 Proposed fix
- }, []) + }, [accountName])🤖 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 `@packages/core/src/sdk/account/useSetPassword.ts` around lines 42 - 121, The setPassword callback (defined with useCallback) closes over accountName but the dependency array is empty, which can cause stale accountName to be used; update the useCallback dependencies to include accountName (and any other external values like config or authMessage if they are not stable) so setPassword recalculates when accountName changes, e.g., add accountName to the dependency array of the useCallback that defines setPassword.
🧹 Nitpick comments (1)
packages/core/test/sdk/account/useSetPassword.test.ts (1)
44-147: 💤 Low valueConsider adding a test for special characters in accountName.
While URL encoding is handled correctly in the implementation, an explicit test verifying that special characters (spaces,
&,=, etc.) inaccountNameare properly encoded in the query string would strengthen confidence in edge cases.💡 Example test case
it('properly URL-encodes special characters in accountName', async () => { mockFetch .mockResolvedValueOnce(okResponse({})) .mockResolvedValueOnce(okResponse({ authStatus: 'success' })) const { result } = renderHook(() => useSetPassword('my store & co')) await act(async () => { await result.current.setPassword(validInput) }) const url = String(getSetPasswordCall()?.[0]) expect(url).toContain('an=my%20store%20%26%20co') })🤖 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 `@packages/core/test/sdk/account/useSetPassword.test.ts` around lines 44 - 147, Add a test in the useSetPassword suite to verify accountName URL-encoding: mock responses with mockFetch (using okResponse), render the hook with a name containing special characters (e.g., "my store & co") via renderHook(() => useSetPassword('my store & co')), call result.current.setPassword(validInput) inside act, then inspect getSetPasswordCall() and assert the request URL contains the properly encoded query parameter (an=my%20store%20%26%20co). Reference useSetPassword, getSetPasswordCall, mockFetch, okResponse, and validInput when adding the test.
🤖 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.
Outside diff comments:
In `@packages/core/src/sdk/account/useSetPassword.ts`:
- Around line 42-121: The setPassword callback (defined with useCallback) closes
over accountName but the dependency array is empty, which can cause stale
accountName to be used; update the useCallback dependencies to include
accountName (and any other external values like config or authMessage if they
are not stable) so setPassword recalculates when accountName changes, e.g., add
accountName to the dependency array of the useCallback that defines setPassword.
---
Nitpick comments:
In `@packages/core/test/sdk/account/useSetPassword.test.ts`:
- Around line 44-147: Add a test in the useSetPassword suite to verify
accountName URL-encoding: mock responses with mockFetch (using okResponse),
render the hook with a name containing special characters (e.g., "my store &
co") via renderHook(() => useSetPassword('my store & co')), call
result.current.setPassword(validInput) inside act, then inspect
getSetPasswordCall() and assert the request URL contains the properly encoded
query parameter (an=my%20store%20%26%20co). Reference useSetPassword,
getSetPasswordCall, mockFetch, okResponse, and validInput when adding the test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4ea60d6f-5461-46bf-a02c-285ae75edf83
📒 Files selected for processing (2)
packages/core/src/sdk/account/useSetPassword.tspackages/core/test/sdk/account/useSetPassword.test.ts
|
Testei com as mudanças na loja de b2b e o fluxo não funcionou, está dando mensagem de InvalidToken. |
Updated the setPassword function to explicitly pass the account name as a query parameter in the startLogin request. This change improves the URL composition for the authentication process. Additionally, added unit tests to verify the new behavior, ensuring that the correct API endpoints and request bodies are used. Ref: SFS-3056
@faststore/api
@faststore/cli
@faststore/components
@faststore/core
@faststore/diagnostics
@faststore/lighthouse
@faststore/sdk
@faststore/ui
commit: |
|
eduardoformiga
left a comment
There was a problem hiding this comment.
Blocker resolvido internamente, podemos seguir com o merge.
## Summary Graduate the `4.3.0-dev.8` prerelease cycle to the stable `4.3.0` release on `latest`. Merging this PR triggers CD on `main`, which runs `lerna version --conventional-graduate` and publishes all `@faststore/*` packages to the `latest` dist-tag. ### Features - Password Protection (v4) (#3276) - File upload via Order Entry Service (OES) (#3334) - Add sitemap to CMS Landing Page content-type (#3386) ### Bug Fixes - **core:** use authenticator route for setpassword (#3380) - migrate partytown `@builder.io` → `@qwik.dev@0.14.0` (#3394) - **api:** use account-scoped cookie in orderEntry headers (#3395) - forward auth token only via account-scoped cookie (#3381) - **core:** propagate upstream error status instead of always 500 (#3379) ## Pre-flight (faststore-release skill) - Working tree clean; `release.yml` has `fetch-depth: 0` - All 8 packages have `repository.url` - No breaking changes since `v4.2.0` - `dev → main` merges cleanly (no CHANGELOG/codegen conflicts) - No unmerged hotfixes on `main` (accidental `4.2.1` was fully reverted in #3375) ## Test plan - [ ] CI green on this PR - [ ] After merge, confirm CD publishes all 8 packages at `4.3.0` under `latest` Made with [Cursor](https://cursor.com) --------- Co-authored-by: Matheus P. Silva <cout.matheusps@gmail.com> Co-authored-by: vtexgithubbot <vtexgithubbot@github.com> Co-authored-by: Lucas Feijó <lucas.portela@vtex.com> Co-authored-by: Luiz Falcão <39093175+llfalcao@users.noreply.github.com> Co-authored-by: Artur Santiago <artur.santiago@cubos.io> Co-authored-by: Larícia Mota <laricia.mota@vtex.com.br> Co-authored-by: Sahan Jayawardana <sahan@clouda.io> Co-authored-by: Mateus Pontes <mateuspo10@gmail.com> Co-authored-by: Matheus Martins <mathews_2010@outlook.com> Co-authored-by: renatomaurovtex <167437775+renatomaurovtex@users.noreply.github.com> Co-authored-by: Leandro Rodrigues <leandro.rodrigues@vtex.com> Co-authored-by: Fanny Chien <fanny.chien@vtex.com> Co-authored-by: Arthur Andrade <arthurfelandrade@gmail.com> Co-authored-by: Leandro Rodrigues <leandro.swf@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Thiago Pereira <thiago.pereira@vtex.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Giuliana Rigaud <62848434+giurigaud@users.noreply.github.com> Co-authored-by: renato <renato.neto@cubos.io> Co-authored-by: Bruna Santos <brunassdev@gmail.com> Co-authored-by: BrunaCubos <104789782+BrunaCubos@users.noreply.github.com> Co-authored-by: Ícaro Oliveira <icarovinici@gmail.com> Co-authored-by: Bruna Santos <bruna.santos@cubos.io> Co-authored-by: Everton Ataide <everton.ataide@vtex.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Otavio Moreira Meirelles <otavio.meirelles@vtex.com> Co-authored-by: Marco Cardoso <marcopaulo@outlook.com> Co-authored-by: Gabriel Paladino <gabpaladino@users.noreply.github.com> Co-authored-by: CodeRabbit <noreply@coderabbit.ai> Co-authored-by: dk-portal[bot] <134092483+dk-portal[bot]@users.noreply.github.com> Co-authored-by: Rodrigo Tavares <rodrigo.tavares@vtex.com> Co-authored-by: vitorflg <vitor.gomes@vtex.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Válber Laux <valber.laux@vtex.com>

0 New Issues
0 Fixed Issues
0 Accepted Issues
Summary
Migrates the My Account → Security → Reset password flow off the deprecated VTEX ID endpoint onto the new authenticator route, per SFS-3056.
useSetPasswordnow posts to/api/authenticator/pub/authentication/classic/setpassword?expireSessions=true&an={accountName}instead of/api/vtexid/pub/authentication/classic/setpassword. Method (POST),credentials: 'include', themultipart/form-databody (login,currentPassword,newPassword,accesskey,recaptcha) and theauthStatus → authMessagemapping are all unchanged, so the success/error toasts and drawer behavior stay identical.authStatusvalues (wrongcredentials,invalidemail, etc.) still resolve to the same user-facing messages; the non-2xx and thrown-error paths are untouched.accountNamepropagation: the account name is now passed explicitly via theanquery string (encodeURIComponent-ed), sourced from theuseSetPassword(accountName)argument already plumbed throughSecurityDrawer, with a defensive fallback toconfig.api.storeIdsoanis never empty.The change is isolated to
packages/core/src/sdk/account/useSetPassword.ts. No public API, GraphQL schema, codegen, or UI changes.Tests
Adds
packages/core/test/sdk/account/useSetPassword.test.tscovering:expireSessions=true&an=…(and absence of/api/vtexid/);POST+credentials: 'include'+ form-data body shape;authStatus: success);wrongcredentials→Wrong credentials);accountNamefallback toconfig.api.storeId.All 5 new tests pass. Pre-existing failures in
test/pages/api/graphql.test.tsandtest/utils/cookieCacheBusting.test.tsare unrelated (verified they fail without this change; the former belongs to the separate, not-yet-implemented BFF 4xx forwarding spec).Out of scope
/api/vtexid/pub/...callers (startlogininside the same hook,logoutinOrganizationDrawer) — tracked as follow-ups.password.graphqltypes (this flow is REST-only).Spec
specs/replace-vtexid-setpassword-route.md(status: Done). Thespecs/directory is git-ignored in this repo, so the spec file is not part of this PR.Made with Cursor
Summary by CodeRabbit
Refactor
Tests