-
Notifications
You must be signed in to change notification settings - Fork 12.6k
fix: Allow TOTP 2FA without email verification #37368
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Looks like this PR is ready to merge! 🎉 |
🦋 Changeset detectedLatest commit: 816ef1d The changes in this PR will be included in the next version bump. This PR includes changesets to release 41 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 |
WalkthroughThe changes enable TOTP-based two-factor authentication without requiring verified email addresses. A pre-check blocking 2FA enablement for unverified users has been removed from the enable method, and corresponding test expectations have been updated to verify successful TOTP setup regardless of email verification status. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #37368 +/- ##
===========================================
- Coverage 68.98% 68.22% -0.76%
===========================================
Files 3358 3443 +85
Lines 114240 114759 +519
Branches 20537 20775 +238
===========================================
- Hits 78803 78290 -513
- Misses 33345 34364 +1019
- Partials 2092 2105 +13
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
.changeset/dull-deers-live.md(1 hunks)apps/meteor/app/2fa/server/methods/enable.ts(0 hunks)apps/meteor/tests/end-to-end/api/methods/2fa-enable.ts(2 hunks)
💤 Files with no reviewable changes (1)
- apps/meteor/app/2fa/server/methods/enable.ts
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use descriptive test names that clearly communicate expected behavior
Applied to files:
apps/meteor/tests/end-to-end/api/methods/2fa-enable.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use expect matchers (toEqual, toContain, toBeTruthy, toHaveLength, etc.) instead of assert statements
Applied to files:
apps/meteor/tests/end-to-end/api/methods/2fa-enable.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Prefer web-first assertions (e.g., toBeVisible, toHaveText)
Applied to files:
apps/meteor/tests/end-to-end/api/methods/2fa-enable.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use test.step() to organize complex test scenarios
Applied to files:
apps/meteor/tests/end-to-end/api/methods/2fa-enable.ts
🪛 LanguageTool
.changeset/dull-deers-live.md
[grammar] ~5-~5: Use a hyphen to join words.
Context: ...- Allows users to enable TOTP-based two factor authentication without requiring ...
(QB_NEW_EN_HYPHEN)
🔇 Additional comments (1)
apps/meteor/tests/end-to-end/api/methods/2fa-enable.ts (1)
55-78: LGTM! Test properly validates the new behavior.The test correctly verifies that users with unverified emails can now obtain TOTP secrets and QR code URLs. The assertions are consistent with other tests in the file and properly validate the response structure.
Proposed changes (including videos or screenshots)
Allows users to enable TOTP-based two factor authentication without requiring a verified email address
Issue(s)
Steps to test or reproduce
Further comments
SUP-867
Summary by CodeRabbit