fix(ui): Prompt reverification for SMS set-default and hide it when TOTP is enrolled - #10042
alexcarpenter wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 673a890 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 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 |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (3)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
Included review availability: This review used your included allowance. 5 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 8 reviews per hour. 📝 WalkthroughWalkthroughSetting an SMS second factor as default now uses reverification when required. The SMS menu does not offer that action when TOTP is enabled or the phone is already the default. Tests cover the reverification retry and the menu state when an authenticator app is enrolled. Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The SMS default action is hidden when TOTP is enabled, and no merge-blocking issue remains after normal checks. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/electron
@clerk/electron-passkeys
@clerk/eslint-plugin
@clerk/expo
@clerk/expo-biometrics
@clerk/expo-google-signin
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/mosaic
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
API Changes Report
Summary
🔴 Breaking changes index (1)Every breaking change, up front. Full diffs are in the package sections below.
@clerk/uiCurrent version: 1.38.1 Subpath
|
wobsoriano
left a comment
There was a problem hiding this comment.
thanks for the screenshots!
Description
Fixes two problems with the SMS "Set as default" action in
<UserProfile />'s MFA section, reported in #9984:phone.makeDefaultSecondFactor()directly, so asession_reverification_requiredresponse surfaced as a raw error. It's now wrapped inuseReverification, matching the other MFA and phone actions. This was missed when reverification was added in feat(clerk-react,nextjs,shared): Introduce experimentaluseReverification#4362; fix(clerk-js): Trigger re-verification when changing primary email #5162 fixed the same gap for set-primary.!showTOTPguard was dropped in feat(clerk-js): Retheme pages to cards #2349.The third item in the issue (sign-in always starting with TOTP instead of honoring
defaultSecondFactor) is intentional perdetermineStartingSignInSecondFactorand isn't changed here.Screenshots
Set as default offered while an authenticator app is enrolled
Reverification required
The "before" reverification shot uses a simulated
session_reverification_required403, since the session was still within the reverification window. After completing reverification, the action retries and the number becomes the default.Refs #9984
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change