refactor: platform managed user org admin access#22597
Conversation
WalkthroughThis change refactors organization admin and owner permissions for managed users across several modules. It introduces new service and repository methods to check for organization admin/owner roles, enables organization admins to bypass team membership checks when accessing or updating team event types, and modifies endpoints and authorization logic to allow organization admins to view all organization teams and manage team event types. Helper functions and test cases are added to support and verify these behaviors. The changes are focused on backend service, repository, controller, and API handler layers, with some adjustments in example platform code. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
keithwillcode
added
core
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (07/17/25)1 reviewer was added to this PR based on Keith Williams's automation. |
|
✅ No security or compliance issues detected. Reviewed everything up to 276b033. Security Overview
Detected Code Changes
Reply to this PR with |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
packages/platform/examples/base/src/pages/_app.tsx (1)
16-27: Consider making the names configurable for better maintainability.The implementation correctly generates predictable emails with name prefixes. However, the hardcoded names could be made more flexible.
Consider extracting the names to a constant or configuration:
+const DEMO_NAMES = ["keith", "somay", "rajiv", "morgan", "lauris"]; + function generateRandomEmail(name: string) { const localPartLength = 5; const domain = ["example.com", "example.net", "example.org"]; // ... rest of implementation }This would make it easier to modify the demo users without changing the function calls.
apps/api/v2/src/modules/atoms/services/event-types-atom.service.ts (1)
78-99: Verify security implications of context switching for organization admins.This change allows organization admins to operate in the context of the event type's owner rather than their own context. While this aligns with the PR's goal of enhancing org admin access, it's important to ensure:
- This doesn't bypass any critical security checks
- Audit logs properly reflect who made changes (the admin, not the effective user)
- The behavior is clearly documented for API consumers
The logic is sound, but the security implications should be carefully considered.
Consider adding audit logging to track when an org admin operates on behalf of another user. This would help with compliance and debugging.
packages/platform/examples/base/src/pages/api/managed-user.ts (1)
68-75: Use optional chaining for cleaner code.The static analysis correctly identifies an opportunity to use optional chaining.
- if (existingUser && existingUser.calcomUserId) { + if (existingUser?.calcomUserId) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/api/v2/src/modules/atoms/services/event-types-atom.service.ts(3 hunks)apps/api/v2/src/modules/organizations/memberships/services/organizations-membership.service.ts(1 hunks)apps/api/v2/src/modules/organizations/teams/index/organizations-teams.controller.ts(3 hunks)apps/api/v2/src/modules/organizations/teams/index/organizations-teams.repository.ts(1 hunks)apps/api/v2/src/modules/organizations/teams/index/services/organizations-teams.service.ts(1 hunks)packages/platform/examples/base/src/pages/_app.tsx(3 hunks)packages/platform/examples/base/src/pages/api/managed-user.ts(4 hunks)packages/trpc/server/routers/viewer/eventTypes/create.handler.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
apps/api/v2/src/modules/organizations/teams/index/organizations-teams.controller.ts (1)
apps/api/v2/src/modules/organizations/memberships/services/organizations-membership.service.ts (1)
isOrgAdminOrOwner(30-41)
apps/api/v2/src/modules/atoms/services/event-types-atom.service.ts (2)
apps/api/v2/src/modules/memberships/memberships.repository.ts (1)
isUserOrganizationAdmin(50-61)packages/lib/event-types/getEventTypeById.ts (1)
getEventTypeById(36-260)
packages/platform/examples/base/src/pages/api/managed-user.ts (1)
packages/platform/constants/api.ts (2)
X_CAL_SECRET_KEY(49-49)X_CAL_CLIENT_ID(50-50)
🪛 Biome (1.9.4)
packages/platform/examples/base/src/pages/api/managed-user.ts
[error] 68-68: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Check for E2E label
- GitHub Check: Security Check
🔇 Additional comments (7)
apps/api/v2/src/modules/organizations/memberships/services/organizations-membership.service.ts (1)
30-41: LGTM! Well-implemented role checking method.The
isOrgAdminOrOwnermethod correctly implements role-based access control by:
- Reusing existing repository method for consistency
- Providing clear error handling with descriptive messages
- Following the established service pattern
- Using proper return type for boolean checks
This method will enable proper authorization checks across the organization modules.
packages/platform/examples/base/src/pages/_app.tsx (1)
53-57: Email generation calls are consistent with the refactored function.The updated calls to
generateRandomEmailwith specific names align well with the function signature change and support the demo user creation workflow.apps/api/v2/src/modules/organizations/teams/index/services/organizations-teams.service.ts (1)
27-34: LGTM! Method follows established service patterns.The
getPaginatedOrgTeamsWithMembersmethod correctly:
- Maintains consistency with existing service methods
- Uses proper pagination parameters with sensible defaults
- Delegates to the repository layer appropriately
- Supports the role-based access control requirements
This method enables organization admins/owners to retrieve all teams with member information.
packages/trpc/server/routers/viewer/eventTypes/create.handler.ts (1)
92-93: LGTM! Authorization logic correctly exempts organization admins.The updated condition properly implements the organization admin privilege by:
- Adding
!isOrgAdmincheck to bypass membership role requirements- Maintaining existing system admin and membership role validations
- Following logical precedence: system admin → org admin → membership role
This change aligns with the PR objective of refactoring platform managed user org admin access.
apps/api/v2/src/modules/organizations/teams/index/organizations-teams.repository.ts (1)
111-122: LGTM! Repository method efficiently retrieves teams with member data.The
findOrgTeamsPaginatedWithMembersmethod is well-implemented:
- Correctly filters teams by organization ID
- Uses selective field inclusion for members (accepted, userId, role) to optimize performance
- Applies pagination properly with skip/take parameters
- Follows established repository patterns
- Supports the role-based access control requirements
The selective member field inclusion is particularly good for performance while providing necessary data for authorization checks.
apps/api/v2/src/modules/organizations/teams/index/organizations-teams.controller.ts (1)
60-63: LGTM! Proper dependency injection.The OrganizationsMembershipService is correctly injected following NestJS conventions.
packages/platform/examples/base/src/pages/api/managed-user.ts (1)
15-56: Excellent refactoring to reduce code duplication!The helper function properly consolidates the user creation logic and makes the code more maintainable.
apps/api/v2/src/modules/organizations/teams/index/organizations-teams.controller.ts
Show resolved
Hide resolved
apps/api/v2/src/modules/atoms/services/event-types-atom.service.ts
Outdated
Show resolved
Hide resolved
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
…er-admin-permissions
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/lib/server/repository/eventTypeRepository.ts (2)
751-764: Optimize the organization users query for better performanceThe nested query with
profiles.somecould impact performance. Consider adding database indexes and potentially restructuring the query.
Add indexes to improve query performance:
- Index on
Profile.organizationId- Index on
Team.parentId- Composite index on
EventType.userIdandEventType.teamIdConsider denormalizing the organization relationship on the EventType table for faster queries if this becomes a performance bottleneck.
Would you like me to generate the Prisma schema changes for these indexes?
749-777: Extract organization admin conditions into a separate methodConsider extracting the organization admin condition building logic into a separate private method for better readability and testability.
+ private buildOrganizationAdminConditions(currentOrganizationId: number) { + const organizationUsersEventTypesQuery = { + AND: [ + { userId: { not: null } }, + { + owner: { + profiles: { + some: { + organizationId: currentOrganizationId, + }, + }, + }, + }, + ], + }; + + const organizationTeamsEventTypesQuery = { + AND: [ + { teamId: { not: null } }, + { + team: { + parentId: currentOrganizationId, + }, + }, + ], + }; + + return [organizationUsersEventTypesQuery, organizationTeamsEventTypesQuery]; + } const orgAdminConditions = []; if (isUserOrganizationAdmin && currentOrganizationId) { - const organizationUsersEventTypesQuery = { - AND: [ - { userId: { not: null } }, - { - owner: { - profiles: { - some: { - organizationId: currentOrganizationId, - }, - }, - }, - }, - ], - }; - const organizationTeamsEventTypesQuery = { - AND: [ - { teamId: { not: null } }, - { - team: { - parentId: currentOrganizationId, - }, - }, - ], - }; - orgAdminConditions.push(organizationUsersEventTypesQuery); - orgAdminConditions.push(organizationTeamsEventTypesQuery); + orgAdminConditions.push(...this.buildOrganizationAdminConditions(currentOrganizationId)); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/lib/event-types/getEventTypeById.ts(1 hunks)packages/lib/server/repository/eventTypeRepository.ts(2 hunks)packages/trpc/server/routers/viewer/eventTypes/create.handler.ts(1 hunks)
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code. Functions like .add, .diff, .isBefore, and .isAfter are slow, especially in timezone mode. Prefer .utc() for better performance. Where possible, replace with native Date and direct .valueOf() comparisons for faster execution. Recommend using native methods or Day.js .utc() consistently in hot paths like loops.
Files:
packages/lib/server/repository/eventTypeRepository.ts
🧬 Code Graph Analysis (1)
packages/lib/server/repository/eventTypeRepository.ts (1)
apps/api/v2/src/modules/memberships/memberships.repository.ts (1)
isUserOrganizationAdmin(50-61)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/lib/event-types/getEventTypeById.ts
- packages/trpc/server/routers/viewer/eventTypes/create.handler.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code. Functions like .add, .diff, .isBefore, and .isAfter are slow, especially in timezone mode. Prefer .utc() for better performance. Where possible, replace with native Date and direct .valueOf() comparisons for faster execution. Recommend using native methods or Day.js .utc() consistently in hot paths like loops.
Files:
packages/lib/server/repository/eventTypeRepository.ts
🧬 Code Graph Analysis (1)
packages/lib/server/repository/eventTypeRepository.ts (1)
apps/api/v2/src/modules/memberships/memberships.repository.ts (1)
isUserOrganizationAdmin(50-61)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (1)
packages/lib/server/repository/eventTypeRepository.ts (1)
471-481: Ignore admin‐status verification in the repository
ThefindByIdmethod is a data-access layer API that intentionally trusts theisUserOrganizationAdminflag provided by its caller. Authorization (including any necessaryMembershipRepository.isUserOrganizationAdminchecks) is performed in the service or controller layer before invoking this repository method. You can safely ignore the previous suggestion to re-verify admin status insidefindById.Likely an incorrect or invalid review comment.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
supalarry
deleted the
lauris/cal-6118-refactor-platform-managed-user-admin-permissions
branch
3 participants
Linear CAL-6118