refactor: use permission check service in /organizations endpoints#24099
refactor: use permission check service in /organizations endpoints#24099sean-brydon merged 10 commits intomainfrom
Conversation
WalkthroughThis change replaces legacy admin/membership checks with PermissionCheckService-based authorization across several organization/team handlers. Handlers now call checkPermission with permissions such as eventType.update, team.create, team.delete, organization.update, and team.invite, using fallbackRoles [OWNER, ADMIN]. Relevant imports for PermissionCheckService and Prisma MembershipRole were added. deleteTeam.handler now requires ctx with user, validates team existence and parentId, and uses TRPCError for NOT_FOUND, BAD_REQUEST, and UNAUTHORIZED cases. The deleteTeam DeleteOptions type and handler signature were updated to include ctx. Core business flows (member additions, team creation, deletion sequence, publish behavior) remain unchanged. Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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. Comment |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
packages/trpc/server/routers/viewer/organizations/createTeams.handler.ts
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/trpc/server/routers/viewer/teams/getUserConnectedApps.handler.ts (5)
76-85: PBAC check looks good; verify permission string and consider reusing a singleton.
- Please confirm "organization.read" matches the previous admin/owner intent for this endpoint. If stricter access is desired, pick the closest permission accordingly.
- Instantiate PermissionCheckService once at module scope to avoid per-call construction.
Apply this diff to reuse a module-scoped instance:
- if (team.parent) { - const permissionCheckService = new PermissionCheckService(); + if (team.parent) { isOrgAdminOrOwner = await permissionCheckService.checkPermission({ userId: user.id, teamId: team.parent.id, permission: "organization.read", fallbackRoles: [MembershipRole.OWNER, MembershipRole.ADMIN], }); }Add this near the top-level (e.g., after appDataMap):
const permissionCheckService = new PermissionCheckService();
67-74: Minimize Prisma payload for membership check.
Select only what you need to align with repo guidelines and reduce I/O.As per coding guidelines, apply:
const isMember = await prisma.membership.findUnique({ where: { userId_teamId: { userId: user.id, teamId: teamId, }, }, + select: { id: true }, });
119-131: Eliminate N queries: fetch credentials with a single query and prefill empty arrays.
This reduces DB round-trips and ensures users with no apps still appear with [].Apply:
- const credentialsPromises: Promise<Credential[]>[] = []; - const userConnectedAppsMap: Record<number, Apps[]> = {}; - - for (const userId of userIds) { - const cred = prisma.credential.findMany({ - where: { - userId, - }, - select: credentialSelect, - }); - credentialsPromises.push(cred); - } - - const credentialsList = await Promise.all(credentialsPromises); - - for (const credentials of credentialsList) { - const userId = credentials[0]?.userId; - - if (userId) { - userConnectedAppsMap[userId] = credentials?.map((cred) => { - const appSlug = cred.app?.slug; - let appData = appDataMap.get(appSlug); - - if (!appData) { - appData = getAppFromSlug(appSlug); - appDataMap.set(appSlug, appData); - } - - const isCalendar = cred?.app?.categories?.includes("calendar") ?? false; - const externalId = isCalendar ? cred.destinationCalendars?.[0]?.externalId : null; - return { - name: appData?.name ?? null, - logo: appData?.logo ?? null, - app: cred.app, - externalId: externalId ?? null, - }; - }); - } - } + const userConnectedAppsMap: Record<number, Apps[]> = {}; + for (const id of userIds) userConnectedAppsMap[id] = []; + + const credentials = await prisma.credential.findMany({ + where: { userId: { in: userIds } }, + select: credentialSelect, + }); + + for (const cred of credentials) { + const appSlug = cred.app?.slug; + let appData = appSlug ? appDataMap.get(appSlug) : null; + if (appSlug && !appData) { + appData = getAppFromSlug(appSlug); + appDataMap.set(appSlug, appData); + } + const isCalendar = cred?.app?.categories?.includes("calendar") ?? false; + const externalId = isCalendar ? cred.destinationCalendars?.[0]?.externalId : null; + userConnectedAppsMap[cred.userId].push({ + name: appData?.name ?? null, + logo: appData?.logo ?? null, + app: cred.app, + externalId: externalId ?? null, + }); + }Also applies to: 132-157
139-146: Avoid caching under an undefined slug key.
Guard against falsy slugs to prevent a single "undefined" cache bucket.Apply:
- let appData = appDataMap.get(appSlug); - - if (!appData) { - appData = getAppFromSlug(appSlug); - appDataMap.set(appSlug, appData); - } + let appData = appSlug ? appDataMap.get(appSlug) : null; + if (appSlug && !appData) { + appData = getAppFromSlug(appSlug); + appDataMap.set(appSlug, appData); + }Optionally also type the cache:
-const appDataMap = new Map(); +const appDataMap = new Map<string, ReturnType<typeof getAppFromSlug>>();
63-65: Use typed tRPC errors instead of generic Error.
Return proper codes (NOT_FOUND/FORBIDDEN) for better clients and telemetry.Example:
import { TRPCError } from "@trpc/server"; if (!team) { throw new TRPCError({ code: "NOT_FOUND", message: "Team not found" }); } if (!isMember && !isOrgAdminOrOwner) { throw new TRPCError({ code: "FORBIDDEN", message: "Not authorized to access this team's connected apps" }); }Also applies to: 87-89
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/trpc/server/routers/viewer/teams/getUserConnectedApps.handler.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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:
packages/trpc/server/routers/viewer/teams/getUserConnectedApps.handler.ts
**/*.{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:
packages/trpc/server/routers/viewer/teams/getUserConnectedApps.handler.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/trpc/server/routers/viewer/teams/getUserConnectedApps.handler.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/calAIPhone/providers/retellAI/services/AgentService.ts:83-88
Timestamp: 2025-08-26T20:23:28.396Z
Learning: In calcom/cal.com PR #22995, the workflow update handler in packages/trpc/server/routers/viewer/workflows/update.handler.ts includes workflow-level authorization via isAuthorized(userWorkflow, ctx.user.id, "workflow.update") which validates the user can update the workflow before calling updateToolsFromAgentId (per maintainer Udit-takkar).
🧬 Code graph analysis (1)
packages/trpc/server/routers/viewer/teams/getUserConnectedApps.handler.ts (2)
apps/api/v2/src/modules/organizations/memberships/services/organizations-membership.service.ts (1)
isOrgAdminOrOwner(38-47)packages/features/pbac/services/permission-check.service.ts (1)
PermissionCheckService(19-306)
🔇 Additional comments (6)
packages/trpc/server/routers/viewer/teams/getUserConnectedApps.handler.ts (6)
2-2: LGTM: Using PermissionCheckService is the right direction.
Switching to PBAC-backed checks aligns with the ongoing refactor.
4-4: LGTM: MembershipRole enum usage is correct.
Enum-based fallback roles avoid string drift.
18-31: Credential selection avoids leaking credential.key.
Only whitelisted fields are selected; credential.key is not exposed. Good.As per coding guidelines.
147-148: Confirm AppCategories literal.
Ensure "calendar" matches the AppCategories enum value; if an enum member exists, prefer it.If available:
const isCalendar = cred?.app?.categories?.includes(AppCategories.CALENDAR) ?? false;
91-111: Authorization model check: can any team member view others’ connections?
Current logic allows any team member (not just admins/owners) to fetch connected apps for all members of the team. Confirm this is intended; otherwise, require isOrgAdminOrOwner.
162-162: Remove default export and use named export for getUserConnectedAppsHandler. Confirm no dynamic or path-based loader relies on the default export before applying this change.
hbjORbj
changed the title
| teamId: team.parent.id, | ||
| permission: "organization.read", |
There was a problem hiding this comment.
Q:
@sean-brydon permission should be team.read here because we are already in the scope of organization here (passuing parent.id to teamId arg), am I right?
hbjORbj
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/trpc/server/routers/viewer/organizations/utils.ts (2)
68-71: Bug: users partially in teams get skipped entirely.filteredUserIds excludes a user if they’re in any one target team, preventing adding them to the remaining teams. Filter by (userId, teamId) pairs instead.
- // Filter out users who are already in teams they are being invited to - const filteredUserIds = input.userIds.filter((userId) => { - return !usersInTeams.some((membership) => membership.userId === userId); - }); + // Fetch existing (userId, teamId) pairs and skip only those pairs + const existingPairs = await prisma.membership.findMany({ + where: { + userId: { in: input.userIds }, + teamId: { in: Array.from(allowedTeamIds) }, + }, + select: { userId: true, teamId: true }, + }); + const existing = new Set(existingPairs.map((m) => `${m.userId}:${m.teamId}`)); - const membershipData = filteredUserIds.flatMap((userId) => - input.teamIds.map((teamId) => { + const membershipData = input.userIds.flatMap((userId) => + Array.from(allowedTeamIds).flatMap((teamId) => { + if (existing.has(`${userId}:${teamId}`)) return []; const userMembership = usersInOrganization.find((membership) => membership.userId === userId); const accepted = userMembership && userMembership.accepted; return { createdAt: new Date(), userId, teamId, role: MembershipRole.MEMBER, accepted: accepted || false, } as Prisma.MembershipCreateManyInput; }) );Also applies to: 75-87
36-46: Minimize Prisma payloads with select.Return only needed fields to reduce I/O, per repo guideline.
-const usersInOrganization = await prisma.membership.findMany({ +const usersInOrganization = await prisma.membership.findMany({ where: { teamId: user.organizationId, user: { id: { in: input.userIds, }, }, }, - distinct: ["userId"], + distinct: ["userId"], + select: { userId: true, accepted: true }, }); -const usersInTeams = await prisma.membership.findMany({ +const usersInTeams = await prisma.membership.findMany({ where: { userId: { in: input.userIds, }, - teamId: { - in: input.teamIds, - }, + teamId: { in: Array.from(allowedTeamIds) }, }, -}); + select: { userId: true, teamId: true }, +});Also applies to: 56-66
packages/trpc/server/routers/viewer/organizations/addMembersToEventTypes.handler.ts (1)
38-47: Validate ownership: event types must belong to provided teams within the org (security).Without this, callers can add hosts to event types outside their org/teams.
- const { eventTypeIds, teamIds, userIds } = input; + const { eventTypeIds, teamIds, userIds } = input; + + // Ensure all teams belong to the caller's organization + const allowedTeams = await prisma.team.findMany({ + where: { id: { in: teamIds }, parentId: ctx.user.organizationId }, + select: { id: true }, + }); + const allowedTeamIds = new Set(allowedTeams.map((t) => t.id)); + if (allowedTeams.length !== teamIds.length) { + throw new TRPCError({ code: "BAD_REQUEST", message: "One or more teams are not in this organization" }); + } + + // Ensure all event types belong to those teams + const allowedEventTypes = await prisma.eventType.findMany({ + where: { id: { in: eventTypeIds }, teamId: { in: Array.from(allowedTeamIds) } }, + select: { id: true }, + }); + if (allowedEventTypes.length !== eventTypeIds.length) { + throw new TRPCError({ code: "BAD_REQUEST", message: "One or more event types are not in the provided teams" }); + }packages/trpc/server/routers/viewer/organizations/publish.handler.ts (1)
40-46: Replace include with select/_count to avoid loading all members.Per repo guideline: never use include; select only what you need. You only need the count.
-const prevTeam = await prisma.team.findUnique({ - where: { - id: orgId, - }, - include: { members: true }, -}); +const prevTeam = await prisma.team.findUnique({ + where: { id: orgId }, + select: { + id: true, + metadata: true, + _count: { select: { members: true } }, + }, +}); @@ - const checkoutSession = await purchaseTeamOrOrgSubscription({ - teamId: prevTeam.id, - seatsUsed: prevTeam.members.length, + const checkoutSession = await purchaseTeamOrOrgSubscription({ + teamId: prevTeam.id, + seatsUsed: prevTeam._count.members,Also applies to: 55-59
🧹 Nitpick comments (14)
packages/trpc/server/routers/viewer/organizations/utils.ts (2)
89-95: Hardening: use skipDuplicates and await async updates.
- createMany should use skipDuplicates to handle races.
- forEach with async isn’t awaited; use Promise.all.
-await prisma.membership.createMany({ - data: membershipData, -}); +await prisma.membership.createMany({ + data: membershipData, + skipDuplicates: true, +}); - -membershipData.forEach(async ({ userId, teamId }) => { - await updateNewTeamMemberEventTypes(userId, teamId); -}); +await Promise.all( + membershipData.map(({ userId, teamId }) => + updateNewTeamMemberEventTypes(userId, teamId) + ) +);
29-34: Return 403 (FORBIDDEN) for permission failures.User is authenticated but lacks permission; prefer FORBIDDEN over UNAUTHORIZED for consistency with other handlers.
- code: "UNAUTHORIZED", + code: "FORBIDDEN",packages/trpc/server/routers/viewer/organizations/addMembersToEventTypes.handler.ts (4)
31-36: Use 403 (FORBIDDEN) when permission check fails.User is authenticated but not allowed.
- code: "UNAUTHORIZED", + code: "FORBIDDEN",
57-61: LGTM on skipDuplicates; consider transaction for atomicity.If you want add-to-teams and add-as-host to be atomic, wrap addMembersToTeams and createMany in a $transaction. Otherwise fine.
64-64: Prefer named exports over default exports.Improves tree-shaking and refactors. Change at call sites accordingly.
-export default addMembersToEventTypesHandler; +// export { addMembersToEventTypesHandler };
1-5: Import consistency for MembershipRole/prisma (minor).Elsewhere MembershipRole often comes from @calcom/prisma/enums and prisma from named import. Consider standardizing across the PR.
packages/trpc/server/routers/viewer/organizations/publish.handler.ts (3)
33-38: Use 403 (FORBIDDEN) for permission failure.Authenticated user lacking permission should yield FORBIDDEN.
- code: "UNAUTHORIZED", + code: "FORBIDDEN",
97-101: Nit: success message reads “Team” instead of “Organization”.Align the message with the flow.
- message: "Team published successfully", + message: "Organization published successfully",
103-103: Prefer named export over default (optional).-export default publishHandler; +// export { publishHandler };packages/trpc/server/routers/viewer/organizations/createTeams.handler.ts (2)
54-60: Minor: consistent import source for MembershipRole across files.Some files import from @calcom/prisma/client, others from @calcom/prisma/enums. Consider standardizing.
157-157: Prefer named exports (optional).Improves tree-shaking/refactors.
-export default createTeamsHandler; +// export { createTeamsHandler };packages/trpc/server/routers/viewer/organizations/deleteTeam.handler.ts (3)
43-48: Use 403 (FORBIDDEN) for permission failure.User is authenticated; lack of permission should not be 401.
- code: "UNAUTHORIZED", + code: "FORBIDDEN",
50-61: Wrap deletions in a transaction to avoid partial state.If team delete fails after memberships delete, you’ll strand the team. Use $transaction.
-// delete all memberships -await prisma.membership.deleteMany({ - where: { teamId: input.teamId }, -}); - -await prisma.team.delete({ - where: { id: input.teamId }, -}); +await prisma.$transaction([ + prisma.membership.deleteMany({ where: { teamId: input.teamId } }), + prisma.team.delete({ where: { id: input.teamId } }), +]);
64-64: Prefer named export over default (optional).-export default deleteTeamHandler; +// export { deleteTeamHandler };
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
packages/trpc/server/routers/viewer/organizations/addMembersToEventTypes.handler.ts(2 hunks)packages/trpc/server/routers/viewer/organizations/createTeams.handler.ts(2 hunks)packages/trpc/server/routers/viewer/organizations/deleteTeam.handler.ts(1 hunks)packages/trpc/server/routers/viewer/organizations/publish.handler.ts(2 hunks)packages/trpc/server/routers/viewer/organizations/utils.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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:
packages/trpc/server/routers/viewer/organizations/publish.handler.tspackages/trpc/server/routers/viewer/organizations/utils.tspackages/trpc/server/routers/viewer/organizations/createTeams.handler.tspackages/trpc/server/routers/viewer/organizations/addMembersToEventTypes.handler.tspackages/trpc/server/routers/viewer/organizations/deleteTeam.handler.ts
**/*.{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:
packages/trpc/server/routers/viewer/organizations/publish.handler.tspackages/trpc/server/routers/viewer/organizations/utils.tspackages/trpc/server/routers/viewer/organizations/createTeams.handler.tspackages/trpc/server/routers/viewer/organizations/addMembersToEventTypes.handler.tspackages/trpc/server/routers/viewer/organizations/deleteTeam.handler.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/trpc/server/routers/viewer/organizations/publish.handler.tspackages/trpc/server/routers/viewer/organizations/utils.tspackages/trpc/server/routers/viewer/organizations/createTeams.handler.tspackages/trpc/server/routers/viewer/organizations/addMembersToEventTypes.handler.tspackages/trpc/server/routers/viewer/organizations/deleteTeam.handler.ts
🧠 Learnings (2)
📚 Learning: 2025-08-26T20:23:28.396Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/calAIPhone/providers/retellAI/services/AgentService.ts:83-88
Timestamp: 2025-08-26T20:23:28.396Z
Learning: In calcom/cal.com PR #22995, the workflow update handler in packages/trpc/server/routers/viewer/workflows/update.handler.ts includes workflow-level authorization via isAuthorized(userWorkflow, ctx.user.id, "workflow.update") which validates the user can update the workflow before calling updateToolsFromAgentId (per maintainer Udit-takkar).
Applied to files:
packages/trpc/server/routers/viewer/organizations/publish.handler.tspackages/trpc/server/routers/viewer/organizations/utils.tspackages/trpc/server/routers/viewer/organizations/createTeams.handler.tspackages/trpc/server/routers/viewer/organizations/addMembersToEventTypes.handler.tspackages/trpc/server/routers/viewer/organizations/deleteTeam.handler.ts
📚 Learning: 2025-08-26T08:08:23.395Z
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23343
File: packages/features/insights/server/trpc-router.ts:1080-1101
Timestamp: 2025-08-26T08:08:23.395Z
Learning: In packages/features/insights/server/trpc-router.ts, when filtering personal event types (userId provided, no teamId, not isAll), the query correctly uses user.id (authenticated user) instead of the input userId parameter for security reasons. This prevents users from accessing other users' personal event types by passing arbitrary user IDs.
Applied to files:
packages/trpc/server/routers/viewer/organizations/utils.tspackages/trpc/server/routers/viewer/organizations/addMembersToEventTypes.handler.ts
🧬 Code graph analysis (5)
packages/trpc/server/routers/viewer/organizations/publish.handler.ts (2)
packages/features/pbac/services/permission-check.service.ts (2)
PermissionCheckService(19-306)hasPermission(183-201)packages/platform/libraries/index.ts (2)
MembershipRole(34-34)TRPCError(66-66)
packages/trpc/server/routers/viewer/organizations/utils.ts (1)
packages/features/pbac/services/permission-check.service.ts (2)
PermissionCheckService(19-306)hasPermission(183-201)
packages/trpc/server/routers/viewer/organizations/createTeams.handler.ts (2)
packages/features/pbac/services/permission-check.service.ts (2)
PermissionCheckService(19-306)hasPermission(183-201)packages/platform/libraries/index.ts (1)
MembershipRole(34-34)
packages/trpc/server/routers/viewer/organizations/addMembersToEventTypes.handler.ts (1)
packages/features/pbac/services/permission-check.service.ts (2)
PermissionCheckService(19-306)hasPermission(183-201)
packages/trpc/server/routers/viewer/organizations/deleteTeam.handler.ts (1)
packages/features/pbac/services/permission-check.service.ts (2)
PermissionCheckService(19-306)hasPermission(183-201)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (1)
packages/trpc/server/routers/viewer/organizations/createTeams.handler.ts (1)
45-52: Authorization check approved and permission string validated. All references toteam.createare consistent and defined in PERMISSIONS.md.
| // Check if user has permission to invite team members in the organization | ||
| const permissionCheckService = new PermissionCheckService(); | ||
| const hasPermission = await permissionCheckService.checkPermission({ | ||
| userId: user.id, | ||
| teamId: user.organizationId, | ||
| permission: "team.invite", | ||
| fallbackRoles: [MembershipRole.OWNER, MembershipRole.ADMIN], | ||
| }); | ||
|
|
||
| if (!hasPermission) { | ||
| throw new TRPCError({ | ||
| code: "UNAUTHORIZED", | ||
| message: "You are not authorized to add members to teams in this organization", | ||
| }); | ||
| } | ||
|
|
There was a problem hiding this comment.
Enforce org scoping for target teams (security).
You check permission at the org, but you never verify that every teamId in input.teamIds belongs to user.organizationId. As-is, a caller could add members to teams outside the org. Validate and constrain teamIds to the actor’s org before proceeding.
Apply a guard right after the permission check and use the validated set in subsequent queries:
// Check if user has permission to invite team members in the organization
const permissionCheckService = new PermissionCheckService();
const hasPermission = await permissionCheckService.checkPermission({
userId: user.id,
teamId: user.organizationId,
permission: "team.invite",
fallbackRoles: [MembershipRole.OWNER, MembershipRole.ADMIN],
});
if (!hasPermission) {
throw new TRPCError({
code: "UNAUTHORIZED",
message: "You are not authorized to add members to teams in this organization",
});
}
+
+ // Verify all target teams belong to this organization
+ const allowedTeams = await prisma.team.findMany({
+ where: { id: { in: input.teamIds }, parentId: user.organizationId },
+ select: { id: true },
+ });
+ const allowedTeamIds = new Set(allowedTeams.map((t) => t.id));
+ if (allowedTeams.length !== input.teamIds.length) {
+ throw new TRPCError({
+ code: "BAD_REQUEST",
+ message: "One or more teams do not belong to your organization",
+ });
+ }Based on learnings.
📝 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.
| // Check if user has permission to invite team members in the organization | |
| const permissionCheckService = new PermissionCheckService(); | |
| const hasPermission = await permissionCheckService.checkPermission({ | |
| userId: user.id, | |
| teamId: user.organizationId, | |
| permission: "team.invite", | |
| fallbackRoles: [MembershipRole.OWNER, MembershipRole.ADMIN], | |
| }); | |
| if (!hasPermission) { | |
| throw new TRPCError({ | |
| code: "UNAUTHORIZED", | |
| message: "You are not authorized to add members to teams in this organization", | |
| }); | |
| } | |
| // Check if user has permission to invite team members in the organization | |
| const permissionCheckService = new PermissionCheckService(); | |
| const hasPermission = await permissionCheckService.checkPermission({ | |
| userId: user.id, | |
| teamId: user.organizationId, | |
| permission: "team.invite", | |
| fallbackRoles: [MembershipRole.OWNER, MembershipRole.ADMIN], | |
| }); | |
| if (!hasPermission) { | |
| throw new TRPCError({ | |
| code: "UNAUTHORIZED", | |
| message: "You are not authorized to add members to teams in this organization", | |
| }); | |
| } | |
| // Verify all target teams belong to this organization | |
| const allowedTeams = await prisma.team.findMany({ | |
| where: { id: { in: input.teamIds }, parentId: user.organizationId }, | |
| select: { id: true }, | |
| }); | |
| const allowedTeamIds = new Set(allowedTeams.map((t) => t.id)); | |
| if (allowedTeams.length !== input.teamIds.length) { | |
| throw new TRPCError({ | |
| code: "BAD_REQUEST", | |
| message: "One or more teams do not belong to your organization", | |
| }); | |
| } |
🤖 Prompt for AI Agents
In packages/trpc/server/routers/viewer/organizations/utils.ts around lines 20 to
35, you check team.invite permission at the org level but never verify that each
teamId in input.teamIds actually belongs to user.organizationId; add a guard
immediately after the permission check that queries the teams table for rows
with id IN input.teamIds AND organizationId = user.organizationId, compare the
returned IDs to input.teamIds and either filter to the validated set or throw a
TRPCError if any requested teamId is outside the org, and then use the
validatedTeamIds in all subsequent operations (invites/updates) so no
external-org team can be modified.
E2E results are ready! |
3 participants
What does this PR do?
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?