fix: resolve account locking and booking modal 404 issues #22891
Conversation
- Add unlockUser function for admin account unlocking - Improve rate limiting logic with better error handling - Fix booking page 404 errors for locked users - Add admin UI component for unlocking users - Add comprehensive documentation and test script Closes calcom#20322
|
@saurabhhh777 is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis pull request introduces a comprehensive fix for issues related to user account locking due to rate limiting. It adds a new function to unlock user accounts programmatically, exposes this functionality via a new admin TRPC mutation and UI component, and improves error handling throughout the booking page and rate limiting logic. The booking page now explicitly checks for locked user accounts and returns a 404 if the profile is missing or locked. Documentation has been added to guide developers through the new workflow and explain the root cause and solution for the locking issue. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes were found. All modifications align with the objectives described in the linked issue. Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
|
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (08/04/25)1 reviewer was added to this PR based on Keith Williams's automation. "Add community label" took an action on this PR • (08/04/25)1 label was added to this PR based on Keith Williams's automation. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/lib/autoLock.ts (1)
179-257: LGTM! Well-structured unlock functionality with proper error handling.The
unlockUserfunction properly mirrors thelockUserfunction and follows good practices with Prismaselectqueries and comprehensive logging.However, address the static analysis warning about switch declarations:
case "apiKey": + { const hashedApiKey = hashAPIKey(identifier); const apiKey = await prisma.apiKey.findUnique({ where: { hashedKey: hashedApiKey }, include: { user: { select: { id: true, email: true, username: true, }, }, }, }); if (!apiKey?.user) { throw new Error("No user found for this API key."); } user = await prisma.user.update({ where: { id: apiKey.user.id }, data: { locked: false }, select: { id: true, email: true, username: true, }, }); break; + }packages/lib/checkRateLimitAndThrowError.ts (1)
15-28: Good implementation with room for improvement on email detection.The lock check logic is well-implemented and follows coding guidelines correctly by using
selectinstead ofinclude. The error message is clear and actionable.Consider improving email detection robustness:
- if (identifier.includes("@")) { + if (identifier.includes("@") && identifier.match(/^[^\s@]+@[^\s@]+\.[^\s@]+$/)) {This ensures the identifier is actually a valid email format rather than just containing an "@" symbol.
docs/developing/guides/fix-account-locking-issue.md (1)
73-118: Excellent usage instructions and guidance.The documentation provides clear, actionable instructions for both developers and users, along with comprehensive prevention strategies and testing guidance.
Consider the minor style improvement suggested by static analysis:
- 3. **Check rate limiting settings:** + 3. **Check rate-limiting settings:**This makes the compound adjective more formally correct.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/web/app/(booking-page-wrapper)/[user]/page.tsx(2 hunks)apps/web/modules/settings/admin/components/UnlockUserButton.tsx(1 hunks)apps/web/server/lib/[user]/getServerSideProps.ts(2 hunks)docs/developing/guides/fix-account-locking-issue.md(1 hunks)packages/lib/autoLock.ts(1 hunks)packages/lib/checkRateLimitAndThrowError.ts(1 hunks)packages/trpc/server/routers/viewer/admin/_router.ts(2 hunks)packages/trpc/server/routers/viewer/admin/unlockUserAccount.handler.ts(1 hunks)packages/trpc/server/routers/viewer/admin/unlockUserAccount.schema.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.tsx
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
apps/web/modules/settings/admin/components/UnlockUserButton.tsxapps/web/app/(booking-page-wrapper)/[user]/page.tsx
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
apps/web/modules/settings/admin/components/UnlockUserButton.tsxapps/web/server/lib/[user]/getServerSideProps.tspackages/trpc/server/routers/viewer/admin/_router.tsapps/web/app/(booking-page-wrapper)/[user]/page.tsxpackages/lib/autoLock.tspackages/trpc/server/routers/viewer/admin/unlockUserAccount.schema.tspackages/lib/checkRateLimitAndThrowError.tspackages/trpc/server/routers/viewer/admin/unlockUserAccount.handler.ts
**/*.ts
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
apps/web/server/lib/[user]/getServerSideProps.tspackages/trpc/server/routers/viewer/admin/_router.tspackages/lib/autoLock.tspackages/trpc/server/routers/viewer/admin/unlockUserAccount.schema.tspackages/lib/checkRateLimitAndThrowError.tspackages/trpc/server/routers/viewer/admin/unlockUserAccount.handler.ts
🧠 Learnings (5)
📚 Learning: in signup-view.tsx, when checking if redirecturl contains certain strings, using explicit && checks ...
Learnt from: Anshumancanrock
PR: calcom/cal.com#22570
File: apps/web/modules/signup-view.tsx:253-253
Timestamp: 2025-07-21T21:33:23.371Z
Learning: In signup-view.tsx, when checking if redirectUrl contains certain strings, using explicit && checks (redirectUrl && redirectUrl.includes()) is preferred over optional chaining (redirectUrl?.includes()) to ensure the result is always a boolean rather than potentially undefined. This approach provides cleaner boolean contracts for downstream conditional logic.
Applied to files:
apps/web/modules/settings/admin/components/UnlockUserButton.tsx
📚 Learning: in the insightsbookingservice (packages/lib/server/service/insightsbooking.ts), the constructor stor...
Learnt from: eunjae-lee
PR: calcom/cal.com#22702
File: packages/lib/server/service/insightsBooking.ts:120-124
Timestamp: 2025-07-24T08:39:06.185Z
Learning: In the InsightsBookingService (packages/lib/server/service/insightsBooking.ts), the constructor stores null for invalid options or filters but this is handled safely through null checks in buildFilterConditions() and buildAuthorizationConditions() methods. The service uses defensive programming to return safe fallback conditions (null or NOTHING_CONDITION) rather than throwing errors on invalid inputs.
Applied to files:
apps/web/app/(booking-page-wrapper)/[user]/page.tsx
📚 Learning: in the failedbookingsbyfield component (packages/features/insights/components/failedbookingsbyfield....
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.389Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
Applied to files:
apps/web/app/(booking-page-wrapper)/[user]/page.tsx
📚 Learning: applies to **/*.ts : ensure the `credential.key` field is never returned from trpc endpoints or apis...
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*.ts : Ensure the `credential.key` field is never returned from tRPC endpoints or APIs
Applied to files:
packages/trpc/server/routers/viewer/admin/unlockUserAccount.schema.tspackages/trpc/server/routers/viewer/admin/unlockUserAccount.handler.ts
📚 Learning: the filterhostsbysameroundrobinhost function in packages/lib/bookings/filterhostsbysameroundrobinhos...
Learnt from: CarinaWolli
PR: calcom/cal.com#22296
File: packages/lib/bookings/filterHostsBySameRoundRobinHost.ts:41-42
Timestamp: 2025-07-22T11:42:47.623Z
Learning: The filterHostsBySameRoundRobinHost function in packages/lib/bookings/filterHostsBySameRoundRobinHost.ts has a known limitation where it doesn't work correctly with fixed hosts or round robin groups. This is pre-existing technical debt that was already broken before the round robin groups feature. CarinaWolli has documented this in Linear issue CAL-6134 for future fix.
Applied to files:
docs/developing/guides/fix-account-locking-issue.md
🪛 Biome (2.1.2)
packages/lib/autoLock.ts
[error] 216-216: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 217-228: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🪛 LanguageTool
docs/developing/guides/fix-account-locking-issue.md
[style] ~73-~73: Consider using a different verb for a more formal wording.
Context: ...ocked) { notFound(); } ``` ## How to Fix the Issue ### For Developers 1. **Run...
(FIX_RESOLVE)
[uncategorized] ~87-~87: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... Click "Unlock User" button 3. Check rate limiting settings: - Review `packages/lib/r...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
🔇 Additional comments (16)
apps/web/server/lib/[user]/getServerSideProps.ts (2)
38-38: LGTM! Proper type extension for locked status.The optional
lockedproperty correctly extends the profile type to support the account locking functionality.
148-148: LGTM! Correct propagation of locked status.The
lockedproperty is properly assigned from the user data, maintaining consistency with the type definition.packages/trpc/server/routers/viewer/admin/_router.ts (2)
6-6: LGTM! Proper schema import.The import follows the established pattern for schema imports in this router.
37-40: LGTM! Consistent admin procedure implementation.The
unlockUserAccountmutation follows the established pattern with proper authorization, input validation, and dynamic handler import.packages/trpc/server/routers/viewer/admin/unlockUserAccount.schema.ts (1)
1-8: LGTM! Well-defined schema for unlock functionality.The schema properly validates the required
userIdand optionalapps/web/app/(booking-page-wrapper)/[user]/page.tsx (2)
17-54: LGTM! Robust error handling for metadata generation.The try-catch wrapper with fallback metadata ensures the page remains functional even if metadata generation fails. The fallback values are appropriate.
59-73: LGTM! Proper handling of locked users and errors.The implementation correctly:
- Wraps data fetching in try-catch for error resilience
- Explicitly checks for locked users and returns 404 (appropriate UX)
- Uses Next.js
notFound()for proper 404 handling- Provides consistent error response for various failure scenarios
packages/trpc/server/routers/viewer/admin/unlockUserAccount.handler.ts (3)
7-10: Well-defined input validation schema.The schema properly validates the required
userIdand optional
19-51: Solid handler implementation with proper data selection.The handler logic correctly unlocks by both identifiers when provided and verifies the operation. The Prisma query properly uses
selectand only returns safe fields, adhering to the coding guideline about not exposing sensitive data likecredential.key.
52-60: Proper error handling with security considerations.The error handling correctly preserves specific TRPC errors while wrapping unexpected errors with generic messages to avoid leaking internal implementation details.
apps/web/modules/settings/admin/components/UnlockUserButton.tsx (4)
9-18: Well-structured component setup.The props interface is clearly defined and the component properly uses React hooks for state management and localization.
20-29: Excellent TRPC integration with proper user feedback.The mutation setup correctly handles success and error cases with appropriate toast notifications and callback execution.
31-41: Robust async handler with proper state management.The function correctly manages loading state with try/finally to ensure cleanup even on errors, and passes the required parameters to the mutation.
43-70: Excellent UI implementation following localization guidelines.The component properly uses
t()for all text strings, implements appropriate loading states, and provides a clear confirmation dialog for the unlock action.docs/developing/guides/fix-account-locking-issue.md (2)
1-17: Clear and comprehensive problem description.The documentation accurately describes the user-facing symptoms and correctly identifies the root cause as rate limiting triggering automatic account locking.
18-72: Comprehensive technical solution documentation.The solution components are well-documented with accurate code examples and correct file references that match the actual implementations.
What does this PR do?
This PR fixes Issue #20322 where users were experiencing account locking due to rate limiting, causing 404 errors on booking pages and login failures.
Problem
Solution
unlockUserfunction for admin account unlockingFiles Changed
packages/lib/autoLock.ts- Added unlockUser functionpackages/lib/checkRateLimitAndThrowError.ts- Improved rate limitingapps/web/app/(booking-page-wrapper)/[user]/page.tsx- Fixed 404 handlingpackages/trpc/server/routers/viewer/admin/- Added unlock APIapps/web/modules/settings/admin/components/UnlockUserButton.tsx- Admin UIdocs/developing/guides/fix-account-locking-issue.md- DocumentationVisual Demo (For contributors especially)
This is a backend bug fix that resolves account locking issues. The changes are primarily in error handling and admin functionality, so screenshots aren't necessary. The functionality can be tested using the provided test script.
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Test Steps:
Run the test script:
Test unlock functionality:
Test booking page:
Environment Variables:
Expected Behavior:
Checklist
Closes #20322