feat(clerk-js,ui): Introduce setup MFA session task #7626
+2,728
−167
Conversation
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
🦋 Changeset detectedLatest commit: fd76d83 The changes in this PR will be included in the next version bump. This PR includes changesets to release 20 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
octoper
force-pushed
the
octoper/setup-mfa-task
branch
from
84d9b10 to
4c19ae6
Compare
octoper
force-pushed
the
octoper/setup-mfa-task
branch
from
d9e893a to
4fc95f2
Compare
Member
Author
|
!snapshot |
Collaborator
|
Hey @octoper - the snapshot version command generated the following package versions:
Tip: Use the snippet copy button below to quickly install the required packages. npm i @clerk/agent-toolkit@0.2.9-snapshot.v20260121104441 --save-exact
npm i @clerk/astro@3.0.0-snapshot.v20260121104441 --save-exact
npm i @clerk/backend@3.0.0-snapshot.v20260121104441 --save-exact
npm i @clerk/chrome-extension@3.0.0-snapshot.v20260121104441 --save-exact
npm i @clerk/clerk-js@6.0.0-snapshot.v20260121104441 --save-exact
npm i @clerk/dev-cli@1.0.0-snapshot.v20260121104441 --save-exact
npm i @clerk/expo@3.0.0-snapshot.v20260121104441 --save-exact
npm i @clerk/expo-passkeys@1.0.0-snapshot.v20260121104441 --save-exact
npm i @clerk/express@2.0.0-snapshot.v20260121104441 --save-exact
npm i @clerk/fastify@2.6.9-snapshot.v20260121104441 --save-exact
npm i @clerk/localizations@4.0.0-snapshot.v20260121104441 --save-exact
npm i @clerk/msw@0.0.1-snapshot.v20260121104441 --save-exact
npm i @clerk/nextjs@7.0.0-snapshot.v20260121104441 --save-exact
npm i @clerk/nuxt@2.0.0-snapshot.v20260121104441 --save-exact
npm i @clerk/react@6.0.0-snapshot.v20260121104441 --save-exact
npm i @clerk/react-router@3.0.0-snapshot.v20260121104441 --save-exact
npm i @clerk/shared@4.0.0-snapshot.v20260121104441 --save-exact
npm i @clerk/tanstack-react-start@1.0.0-snapshot.v20260121104441 --save-exact
npm i @clerk/testing@2.0.0-snapshot.v20260121104441 --save-exact
npm i @clerk/ui@1.0.0-snapshot.v20260121104441 --save-exact
npm i @clerk/upgrade@2.0.0-snapshot.v20260121104441 --save-exact
npm i @clerk/vue@2.0.0-snapshot.v20260121104441 --save-exact |
octoper
force-pushed
the
octoper/setup-mfa-task
branch
from
4fc95f2 to
61862a2
Compare
Member
Author
|
!snapshot |
Collaborator
|
Hey @octoper - the snapshot version command generated the following package versions:
Tip: Use the snippet copy button below to quickly install the required packages. npm i @clerk/agent-toolkit@0.2.9-snapshot.v20260122110524 --save-exact
npm i @clerk/astro@3.0.0-snapshot.v20260122110524 --save-exact
npm i @clerk/backend@3.0.0-snapshot.v20260122110524 --save-exact
npm i @clerk/chrome-extension@3.0.0-snapshot.v20260122110524 --save-exact
npm i @clerk/clerk-js@6.0.0-snapshot.v20260122110524 --save-exact
npm i @clerk/dev-cli@1.0.0-snapshot.v20260122110524 --save-exact
npm i @clerk/expo@3.0.0-snapshot.v20260122110524 --save-exact
npm i @clerk/expo-passkeys@1.0.0-snapshot.v20260122110524 --save-exact
npm i @clerk/express@2.0.0-snapshot.v20260122110524 --save-exact
npm i @clerk/fastify@2.6.9-snapshot.v20260122110524 --save-exact
npm i @clerk/localizations@4.0.0-snapshot.v20260122110524 --save-exact
npm i @clerk/msw@0.0.1-snapshot.v20260122110524 --save-exact
npm i @clerk/nextjs@7.0.0-snapshot.v20260122110524 --save-exact
npm i @clerk/nuxt@2.0.0-snapshot.v20260122110524 --save-exact
npm i @clerk/react@6.0.0-snapshot.v20260122110524 --save-exact
npm i @clerk/react-router@3.0.0-snapshot.v20260122110524 --save-exact
npm i @clerk/shared@4.0.0-snapshot.v20260122110524 --save-exact
npm i @clerk/tanstack-react-start@1.0.0-snapshot.v20260122110524 --save-exact
npm i @clerk/testing@2.0.0-snapshot.v20260122110524 --save-exact
npm i @clerk/ui@1.0.0-snapshot.v20260122110524 --save-exact
npm i @clerk/upgrade@2.0.0-snapshot.v20260122110524 --save-exact
npm i @clerk/vue@2.0.0-snapshot.v20260122110524 --save-exact |
octoper
force-pushed
the
octoper/setup-mfa-task
branch
from
61862a2 to
b41bd37
Compare
octoper
force-pushed
the
octoper/setup-mfa-task
branch
from
b41bd37 to
d3bd99e
Compare
octoper
force-pushed
the
octoper/setup-mfa-task
branch
from
d3bd99e to
364549f
Compare
octoper
force-pushed
the
octoper/setup-mfa-task
branch
from
4cf3a16 to
0a27701
Compare
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
Contributor
📝 WalkthroughWalkthroughAdds a new session task key 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ui/src/components/UserProfile/MfaBackupCodeList.tsx (1)
117-138: Inconsistent button padding across action buttons.The Download button uses
t.space.$0x25(line 119) while Print and Copy buttons uset.space.$2(lines 127, 135). This will cause visual inconsistency in the button row.🔧 Proposed fix for consistent padding
<Button variant='ghost' - sx={t => ({ width: '100%', padding: `${t.space.$0x25} 0`, borderRadius: 0 })} + sx={t => ({ width: '100%', padding: `${t.space.$2} 0`, borderRadius: 0 })} onClick={onDownloadTxtFile} >
🤖 Fix all issues with AI agents
In `@packages/localizations/src/en-US.ts`:
- Around line 911-913: The title string in the smsCode localization object uses
inconsistent casing ("Add sms code verification"); update the smsCode.title
value to use "SMS" uppercase to match other entries (e.g., change to "Add SMS
code verification") so the smsCode object’s title aligns with smsCode.subtitle
and the other keys like the "SMS code verification enabled" entry.
In `@packages/ui/src/components/SessionTasks/tasks/TaskSetupMfa/constants.ts`:
- Around line 1-3: The type for MFA_METHODS_TO_STEP is using Partial
incorrectly; replace Record<Partial<'phone_code' | 'totp'>, number> with a
direct union key type like Record<'phone_code' | 'totp', number> (or if keys
should be optional use Partial<Record<'phone_code' | 'totp', number>>), updating
the MFA_METHODS_TO_STEP constant's type accordingly.
In `@packages/ui/src/components/SessionTasks/tasks/TaskSetupMfa/index.tsx`:
- Around line 24-38: The filter mistakenly runs when user.enterpriseAccounts is
undefined (non-enterprise users), causing firstEnterpriseConnection to be
undefined and phone_code to be excluded; update the early-return in
secondFactorsAvailableToAdd so it returns availableMethods when
enterpriseAccounts is missing or empty (e.g., change the condition in the if
that currently checks user?.enterpriseAccounts && user.enterpriseAccounts.length
=== 0 to check for !user?.enterpriseAccounts || user.enterpriseAccounts.length
=== 0) so getSecondFactorsAvailableToAdd(attributes, user) is used for
non-enterprise users.
In `@packages/ui/src/components/SessionTasks/tasks/TaskSetupMfa/shared.tsx`:
- Around line 25-30: The multi-session sign-out path in handleSignOut calls
clerk.signOut(..., { sessionId: session?.id }) without ensuring session exists;
update handleSignOut to explicitly check that session is defined before passing
session.id (e.g., if otherSessions.length > 0 then if (!session) handle the edge
case by either falling back to a global signOut call or returning/logging an
error), or add a clear comment documenting that passing undefined sessionId is
intentional and expected; reference handleSignOut, otherSessions, clerk.signOut,
and session?.id when making the change.
In
`@packages/ui/src/components/SessionTasks/tasks/TaskSetupMfa/TOTPCodeFlowScreen.tsx`:
- Around line 49-62: The effect in TaskSetupMfa/TOTPCodeFlowScreen currently
runs unconditionally and calls createTOTP() before the Clerk user is guaranteed
to exist, causing the flow to stall; update the React.useEffect that calls
createTOTP() (and the related AddAuthenticatorApp render path) to bail out until
useUser() returns a truthy user (i.e., add a guard "if (!user) return;" and
include user in the effect dependency array) so it re-runs when the user becomes
available, and similarly protect any code that does
user?.verifyTOTP(...).then(...) (in VerifyTOTP) by checking for a defined user
or the verifyTOTP result before calling .then() (or by awaiting inside an async
block only when user is present) to avoid calling .then() on undefined and
ensure the loader can proceed or retry.
In `@packages/ui/src/types.ts`:
- Line 29: Remove the unused `Ref` type from the import statement (currently
"import type { MutableRefObject, Ref } from 'react';") so only
`MutableRefObject` is imported; update the import to drop `Ref` and run
typechecking/linting to ensure no other usages remain (the only referenced
symbol in this file is `MutableRefObject` around the ref usage).
In `@packages/ui/src/utils/mfa.ts`:
- Around line 8-11: The loop over Object.entries(attributes) can throw if an
attribute value is null/undefined; update the iteration in the block that
references attr, used_for_second_factor, and second_factors to guard against
nullish attr and non-array second_factors (e.g., check attr != null and
attr.used_for_second_factor truthiness, and only spread attr.second_factors if
it's an array or fallback to an empty array). This ensures
secondFactors.push(...) is only called with a safe iterable.
🧹 Nitpick comments (1)
packages/ui/src/elements/VerificationCodeCard.tsx (1)
63-67: Minor: Redundant ternary expression.The ternary on line 66 is redundant since a falsy value would already work as
undefinedfor the onClick handler.♻️ Optional simplification
<IdentityPreview identifier={props.safeIdentifier} avatarUrl={props.profileImageUrl} - onClick={props.onIdentityPreviewEditClicked ? props.onIdentityPreviewEditClicked : undefined} + onClick={props.onIdentityPreviewEditClicked} />
packages/ui/src/components/SessionTasks/tasks/TaskSetupMfa/constants.ts
Outdated
Show resolved
Hide resolved
packages/ui/src/components/SessionTasks/tasks/TaskSetupMfa/TOTPCodeFlowScreen.tsx
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@integration/tests/session-tasks-setup-mfa.test.ts`:
- Around line 29-30: The test references userIdsToBeDeleted but it is never
declared, causing a runtime ReferenceError; declare and initialize
userIdsToBeDeleted as an array (e.g., let userIdsToBeDeleted: string[] = []) in
the test file scope before tests run, push bapiUser.id into that array where
createBapiUser is called, and add an afterAll cleanup that iterates over
userIdsToBeDeleted to delete created users (using the same users service) to
ensure proper teardown.
Member
Author
|
!snapshot |
Collaborator
|
Hey @octoper - the snapshot version command generated the following package versions:
Tip: Use the snippet copy button below to quickly install the required packages. npm i @clerk/agent-toolkit@0.2.9-snapshot.v20260128194144 --save-exact
npm i @clerk/astro@3.0.0-snapshot.v20260128194144 --save-exact
npm i @clerk/backend@3.0.0-snapshot.v20260128194144 --save-exact
npm i @clerk/chrome-extension@3.0.0-snapshot.v20260128194144 --save-exact
npm i @clerk/clerk-js@6.0.0-snapshot.v20260128194144 --save-exact
npm i @clerk/dev-cli@1.0.0-snapshot.v20260128194144 --save-exact
npm i @clerk/expo@3.0.0-snapshot.v20260128194144 --save-exact
npm i @clerk/expo-passkeys@1.0.0-snapshot.v20260128194144 --save-exact
npm i @clerk/express@2.0.0-snapshot.v20260128194144 --save-exact
npm i @clerk/fastify@2.7.0-snapshot.v20260128194144 --save-exact
npm i @clerk/localizations@4.0.0-snapshot.v20260128194144 --save-exact
npm i @clerk/msw@0.0.1-snapshot.v20260128194144 --save-exact
npm i @clerk/nextjs@7.0.0-snapshot.v20260128194144 --save-exact
npm i @clerk/nuxt@2.0.0-snapshot.v20260128194144 --save-exact
npm i @clerk/react@6.0.0-snapshot.v20260128194144 --save-exact
npm i @clerk/react-router@3.0.0-snapshot.v20260128194144 --save-exact
npm i @clerk/shared@4.0.0-snapshot.v20260128194144 --save-exact
npm i @clerk/tanstack-react-start@1.0.0-snapshot.v20260128194144 --save-exact
npm i @clerk/testing@2.0.0-snapshot.v20260128194144 --save-exact
npm i @clerk/ui@1.0.0-snapshot.v20260128194144 --save-exact
npm i @clerk/upgrade@2.0.0-snapshot.v20260128194144 --save-exact
npm i @clerk/vue@2.0.0-snapshot.v20260128194144 --save-exact |
octoper
requested review from
LauraBeatris,
anagstef,
nikosdouvlis and
wobsoriano
Member
Author
|
!snapshot |
Collaborator
|
Hey @octoper - the snapshot version command generated the following package versions:
Tip: Use the snippet copy button below to quickly install the required packages. npm i @clerk/agent-toolkit@0.2.9-snapshot.v20260129231425 --save-exact
npm i @clerk/astro@3.0.0-snapshot.v20260129231425 --save-exact
npm i @clerk/backend@3.0.0-snapshot.v20260129231425 --save-exact
npm i @clerk/chrome-extension@3.0.0-snapshot.v20260129231425 --save-exact
npm i @clerk/clerk-js@6.0.0-snapshot.v20260129231425 --save-exact
npm i @clerk/dev-cli@1.0.0-snapshot.v20260129231425 --save-exact
npm i @clerk/expo@3.0.0-snapshot.v20260129231425 --save-exact
npm i @clerk/expo-passkeys@1.0.0-snapshot.v20260129231425 --save-exact
npm i @clerk/express@2.0.0-snapshot.v20260129231425 --save-exact
npm i @clerk/fastify@2.7.0-snapshot.v20260129231425 --save-exact
npm i @clerk/localizations@4.0.0-snapshot.v20260129231425 --save-exact
npm i @clerk/msw@0.0.1-snapshot.v20260129231425 --save-exact
npm i @clerk/nextjs@7.0.0-snapshot.v20260129231425 --save-exact
npm i @clerk/nuxt@2.0.0-snapshot.v20260129231425 --save-exact
npm i @clerk/react@6.0.0-snapshot.v20260129231425 --save-exact
npm i @clerk/react-router@3.0.0-snapshot.v20260129231425 --save-exact
npm i @clerk/shared@4.0.0-snapshot.v20260129231425 --save-exact
npm i @clerk/tanstack-react-start@1.0.0-snapshot.v20260129231425 --save-exact
npm i @clerk/testing@2.0.0-snapshot.v20260129231425 --save-exact
npm i @clerk/ui@1.0.0-snapshot.v20260129231425 --save-exact
npm i @clerk/upgrade@2.0.0-snapshot.v20260129231425 --save-exact
npm i @clerk/vue@2.0.0-snapshot.v20260129231425 --save-exact |
octoper
force-pushed
the
octoper/setup-mfa-task
branch
from
805b527 to
8f797de
Compare
octoper
force-pushed
the
octoper/setup-mfa-task
branch
from
8f797de to
fd76d83
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/ui/src/components/SessionTasks/index.tsx`:
- Line 58: The ref currentTaskContainer created via useRef<HTMLDivElement>(null)
in SessionTasks is never attached to a DOM node, so currentTaskContainer.current
is always null and accessing offsetHeight yields undefined; either attach this
ref to the DOM element you intend to measure (e.g., add
ref={currentTaskContainer} to the task container div that represents the
"current/previous task" so the offsetHeight read (used to compute minHeight)
becomes valid) or remove the ref and the related minHeight computation entirely
(delete currentTaskContainer, its useRef import, and any code that reads
currentTaskContainer.current.offsetHeight).
In `@packages/ui/src/contexts/components/SessionTasks.ts`:
- Around line 60-62: The constructed navigation path can create a double-slash
when VIRTUAL_ROUTER_BASE_PATH is used and startPath begins with a slash; update
the branch in SessionTasks (the block that checks VIRTUAL_ROUTER_BASE_PATH and
calls navigate) to normalize components before building the URL—either strip a
leading slash from startPath (or trailing slash from basePath) or use a small
path-join helper to concatenate basePath, startPath, and taskEndpoint so
navigate receives a single-clean path (referencing VIRTUAL_ROUTER_BASE_PATH,
startPath, taskEndpoint, and navigate).
🧹 Nitpick comments (2)
packages/ui/src/components/SessionTasks/tasks/TaskSetupMfa/SmsCodeFlowScreen.tsx (1)
72-74: Missing dependency array entries in useEffect.The effect references
preparewhich is defined inline and not memoized. While this works for the initial mount pattern, addingprepareor usinguseCallbackwould align with React best practices and avoid stale closure issues if the component re-renders.packages/ui/src/components/SessionTasks/tasks/TaskSetupMfa/TOTPCodeFlowScreen.tsx (1)
271-271: Minor: Comment says "Step 3" but index is 2.The comment at line 271 says "Step 3" but the wizard step index is 2 (0-indexed). This is a cosmetic inconsistency.
Comment on lines
+60
to
+62
| if (basePath === VIRTUAL_ROUTER_BASE_PATH) { | ||
| return navigate(`/${basePath + startPath + taskEndpoint}`); | ||
| } |
Contributor
There was a problem hiding this comment.
Potential double-slash in constructed path.
When basePath is VIRTUAL_ROUTER_BASE_PATH and startPath already starts with /, the constructed path /${basePath + startPath + taskEndpoint} could result in malformed paths with double slashes (e.g., /virtual//start/endpoint).
Consider normalizing the path or verifying that startPath handling is consistent:
🛡️ Proposed fix
if (basePath === VIRTUAL_ROUTER_BASE_PATH) {
- return navigate(`/${basePath + startPath + taskEndpoint}`);
+ const normalizedPath = `/${basePath}${startPath}${taskEndpoint}`.replace(/\/+/g, '/');
+ return navigate(normalizedPath);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (basePath === VIRTUAL_ROUTER_BASE_PATH) { | |
| return navigate(`/${basePath + startPath + taskEndpoint}`); | |
| } | |
| if (basePath === VIRTUAL_ROUTER_BASE_PATH) { | |
| const normalizedPath = `/${basePath}${startPath}${taskEndpoint}`.replace(/\/+/g, '/'); | |
| return navigate(normalizedPath); | |
| } |
🤖 Prompt for AI Agents
In `@packages/ui/src/contexts/components/SessionTasks.ts` around lines 60 - 62,
The constructed navigation path can create a double-slash when
VIRTUAL_ROUTER_BASE_PATH is used and startPath begins with a slash; update the
branch in SessionTasks (the block that checks VIRTUAL_ROUTER_BASE_PATH and calls
navigate) to normalize components before building the URL—either strip a leading
slash from startPath (or trailing slash from basePath) or use a small path-join
helper to concatenate basePath, startPath, and taskEndpoint so navigate receives
a single-clean path (referencing VIRTUAL_ROUTER_BASE_PATH, startPath,
taskEndpoint, and navigate).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
Localization
UI Improvements
Tests
Dev/Devops
✏️ Tip: You can customize this high-level summary in your review settings.