feat: make orgId optional for user and team scopes in /insights and /insights/routing (#23912)

* feat: make orgId optional for user and team scopes in InsightsRoutingBaseService

- User scope authorization only checks formUserId and formTeamId IS NULL
- Team scope now supports standalone teams without organizations
- Add validation logic to return NOTHING_CONDITION if team belongs to org but no orgId provided
- Add comprehensive test coverage for null/undefined orgId scenarios in both scopes
- Aligns schema with actual usage patterns and supports teams without organizations

Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>

* feat: extend orgId optional support to InsightsBookingBaseService

- Make orgId optional for user and team scopes in InsightsBookingBaseService
- Update InsightsBookingServicePublicOptions type to allow orgId: number | null
- Add validation logic for team scope to handle missing orgId
- Add comprehensive test coverage for null/undefined orgId scenarios
- Fix type casting issues in test file
- Maintains backward compatibility while supporting teams without organizations

Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>

* fix: correct authorization logic for optional orgId in team scope

- Skip isOwnerOrAdmin check for team scope when orgId is null (standalone teams)
- Maintain security for org scope and team scope with orgId
- Fixes integration test failures for null orgId test cases

Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>

* fix: use != null instead of !== undefined for orgId checks

- Properly handle both null and undefined orgId values in authorization logic
- Fix integration test failures where null orgId was incorrectly triggering isOwnerOrAdmin check
- Ensure team scope with null orgId skips ownership validation for standalone teams

Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>

* fix: always validate team membership for team scope

- Remove orgId condition from isOwnerOrAdmin check for team scope
- Ensure both standalone teams and org-based teams require ownership validation
- Maintain orgId validation logic in buildTeamAuthorizationCondition methods

Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>

* fix: use nullish() for orgId schema validation

- Change from .optional() to .nullish() to allow both null and undefined
- Fixes schema validation when tests pass orgId: null
- Resolves authorization logic returning NOTHING_CONDITION for valid cases

Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>

* feat: replace isOwnerOrAdmin with PBAC checkPermission in insights services

- Replace isOwnerOrAdmin method in InsightsBookingBaseService with checkPermission from PermissionCheckService
- Replace isOwnerOrAdmin method in InsightsRoutingBaseService with checkPermission from PermissionCheckService
- Use permission 'insights.read' with fallback roles MembershipRole.OWNER and MembershipRole.ADMIN
- Maintain same method signature and behavior while leveraging PBAC system

Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>

* clean up types

* add comment

---------

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
Eunjae Lee
2025-09-19 14:13:21 +00:00
committed by GitHub
co-authored by Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
parent 76332a759b
commit a1922efab5
5 changed files with 407 additions and 43 deletions
@@ -324,7 +324,7 @@ function createInsightsBookingService(
options: {
scope,
userId: ctx.user.id,
orgId: ctx.user.organizationId ?? 0,
orgId: ctx.user.organizationId,
...(selectedTeamId && { teamId: selectedTeamId }),
},
filters: {
@@ -18,6 +18,7 @@ import {
replaceDateRangeColumnFilter,
} from "@calcom/features/insights/lib/bookingUtils";
import type { DateRange } from "@calcom/features/insights/server/insightsDateUtils";
import { PermissionCheckService } from "@calcom/features/pbac/services/permission-check.service";
import type { PrismaClient } from "@calcom/prisma";
import { Prisma } from "@calcom/prisma/client";
import { MembershipRole } from "@calcom/prisma/enums";
@@ -106,7 +107,7 @@ export const insightsBookingServiceOptionsSchema = z.discriminatedUnion("scope",
z.object({
scope: z.literal("user"),
userId: z.number(),
orgId: z.number(),
orgId: z.number().nullish().optional(),
}),
z.object({
scope: z.literal("org"),
@@ -116,7 +117,7 @@ export const insightsBookingServiceOptionsSchema = z.discriminatedUnion("scope",
z.object({
scope: z.literal("team"),
userId: z.number(),
orgId: z.number(),
orgId: z.number().nullish().optional(),
teamId: z.number(),
}),
]);
@@ -124,7 +125,7 @@ export const insightsBookingServiceOptionsSchema = z.discriminatedUnion("scope",
export type InsightsBookingServicePublicOptions = {
scope: "user" | "org" | "team";
userId: number;
orgId: number;
orgId: number | null | undefined;
teamId?: number;
};
@@ -435,13 +436,27 @@ export class InsightsBookingBaseService {
options: Extract<InsightsBookingServiceOptions, { scope: "team" }>
): Promise<Prisma.Sql> {
const teamRepo = new TeamRepository(this.prisma);
const childTeamOfOrg = await teamRepo.findByIdAndParentId({
id: options.teamId,
parentId: options.orgId,
select: { id: true },
});
if (options.orgId && !childTeamOfOrg) {
return NOTHING_CONDITION;
if (options.orgId) {
// team under org
const childTeamOfOrg = await teamRepo.findByIdAndParentId({
id: options.teamId,
parentId: options.orgId,
select: { id: true },
});
if (!childTeamOfOrg) {
// teamId and its orgId does not match
return NOTHING_CONDITION;
}
} else {
// standalone team
const team = await teamRepo.findById({
id: options.teamId,
});
if (team?.parentId) {
// a team without orgId is not supposed to have parentId
return NOTHING_CONDITION;
}
}
const usersFromTeam = await MembershipRepository.findAllByTeamIds({
@@ -1256,13 +1271,12 @@ export class InsightsBookingBaseService {
}
private async isOwnerOrAdmin(userId: number, targetId: number): Promise<boolean> {
// Check if the user is an owner or admin of the organization or team
const membership = await MembershipRepository.findUniqueByUserIdAndTeamId({ userId, teamId: targetId });
return Boolean(
membership &&
membership.accepted &&
membership.role &&
(membership.role === MembershipRole.OWNER || membership.role === MembershipRole.ADMIN)
);
const permissionCheckService = new PermissionCheckService();
return await permissionCheckService.checkPermission({
userId,
teamId: targetId,
permission: "insights.read",
fallbackRoles: [MembershipRole.OWNER, MembershipRole.ADMIN],
});
}
}
@@ -11,19 +11,19 @@ import {
isSingleSelectFilterValue,
} from "@calcom/features/data-table/lib/utils";
import type { DateRange } from "@calcom/features/insights/server/insightsDateUtils";
import { PermissionCheckService } from "@calcom/features/pbac/services/permission-check.service";
import type { PrismaClient } from "@calcom/prisma";
import { Prisma } from "@calcom/prisma/client";
import type { BookingStatus } from "@calcom/prisma/enums";
import { MembershipRole } from "@calcom/prisma/enums";
import { MembershipRepository } from "../repository/membership";
import { TeamRepository } from "../repository/team";
export const insightsRoutingServiceOptionsSchema = z.discriminatedUnion("scope", [
z.object({
scope: z.literal("user"),
userId: z.number(),
orgId: z.number(),
orgId: z.number().nullish().optional(),
}),
z.object({
scope: z.literal("org"),
@@ -33,7 +33,7 @@ export const insightsRoutingServiceOptionsSchema = z.discriminatedUnion("scope",
z.object({
scope: z.literal("team"),
userId: z.number(),
orgId: z.number(),
orgId: z.number().nullish().optional(),
teamId: z.number(),
}),
]);
@@ -41,8 +41,8 @@ export const insightsRoutingServiceOptionsSchema = z.discriminatedUnion("scope",
export type InsightsRoutingServicePublicOptions = {
scope: "user" | "org" | "team";
userId: number;
orgId: number | null;
teamId: number | undefined;
orgId: number | null | undefined;
teamId?: number;
};
export type InsightsRoutingServiceOptions = z.infer<typeof insightsRoutingServiceOptionsSchema>;
@@ -852,27 +852,40 @@ export class InsightsRoutingBaseService {
options: Extract<InsightsRoutingServiceOptions, { scope: "team" }>
): Promise<Prisma.Sql> {
const teamRepo = new TeamRepository(this.prisma);
const childTeamOfOrg = await teamRepo.findByIdAndParentId({
id: options.teamId,
parentId: options.orgId,
select: { id: true },
});
if (options.orgId && !childTeamOfOrg) {
return NOTHING_CONDITION;
if (options.orgId) {
// team under org
const childTeamOfOrg = await teamRepo.findByIdAndParentId({
id: options.teamId,
parentId: options.orgId,
select: { id: true },
});
if (!childTeamOfOrg) {
// teamId and its orgId does not match
return NOTHING_CONDITION;
}
} else {
// standalone team
const team = await teamRepo.findById({
id: options.teamId,
});
if (team?.parentId) {
// a team without orgId is not supposed to have parentId
return NOTHING_CONDITION;
}
}
return Prisma.sql`rfrd."formTeamId" = ${options.teamId}`;
}
private async isOwnerOrAdmin(userId: number, targetId: number): Promise<boolean> {
// Check if the user is an owner or admin of the organization or team
const membership = await MembershipRepository.findUniqueByUserIdAndTeamId({ userId, teamId: targetId });
return Boolean(
membership &&
membership.accepted &&
membership.role &&
(membership.role === MembershipRole.OWNER || membership.role === MembershipRole.ADMIN)
);
const permissionCheckService = new PermissionCheckService();
return await permissionCheckService.checkPermission({
userId,
teamId: targetId,
permission: "insights.read",
fallbackRoles: [MembershipRole.OWNER, MembershipRole.ADMIN],
});
}
private buildFormFieldSqlCondition(fieldId: string, filterValue: FilterValue): Prisma.Sql | null {
@@ -6,7 +6,10 @@ import type { Team, User, Membership } from "@calcom/prisma/client";
import { Prisma } from "@calcom/prisma/client";
import { BookingStatus, MembershipRole } from "@calcom/prisma/enums";
import { InsightsBookingBaseService as InsightsBookingService } from "../InsightsBookingBaseService";
import {
InsightsBookingBaseService as InsightsBookingService,
type InsightsBookingServicePublicOptions,
} from "../InsightsBookingBaseService";
const NOTHING_CONDITION = Prisma.sql`1=0`;
@@ -204,7 +207,7 @@ describe("InsightsBookingService Integration Tests", () => {
it("should return NOTHING for invalid options", async () => {
const service = new InsightsBookingService({
prisma,
options: null as any,
options: null as unknown as InsightsBookingServicePublicOptions,
});
const conditions = await service.getAuthorizationConditions();
@@ -345,6 +348,170 @@ describe("InsightsBookingService Integration Tests", () => {
await testData.cleanup();
});
it("should build user scope conditions with null orgId", async () => {
const testData = await createTestData({
teamRole: MembershipRole.OWNER,
orgRole: MembershipRole.OWNER,
});
const service = new InsightsBookingService({
prisma,
options: {
scope: "user",
userId: testData.user.id,
orgId: null,
},
});
const conditions = await service.getAuthorizationConditions();
expect(conditions).toEqual(Prisma.sql`("userId" = ${testData.user.id}) AND ("teamId" IS NULL)`);
await testData.cleanup();
});
it("should build user scope conditions with undefined orgId", async () => {
const testData = await createTestData({
teamRole: MembershipRole.OWNER,
orgRole: MembershipRole.OWNER,
});
const service = new InsightsBookingService({
prisma,
options: {
scope: "user",
userId: testData.user.id,
orgId: null,
},
});
const conditions = await service.getAuthorizationConditions();
expect(conditions).toEqual(Prisma.sql`("userId" = ${testData.user.id}) AND ("teamId" IS NULL)`);
await testData.cleanup();
});
it("should build team scope conditions with null orgId for standalone team", async () => {
const testData = await createTestData({
teamRole: MembershipRole.OWNER,
orgRole: MembershipRole.OWNER,
});
const standaloneTeam = await prisma.team.create({
data: {
name: "Standalone Team",
slug: `standalone-team-${Date.now()}-${Math.random().toString(36).substring(7)}`,
isOrganization: false,
parentId: null,
},
});
await prisma.membership.create({
data: {
userId: testData.user.id,
teamId: standaloneTeam.id,
role: MembershipRole.OWNER,
accepted: true,
},
});
const service = new InsightsBookingService({
prisma,
options: {
scope: "team",
userId: testData.user.id,
orgId: null,
teamId: standaloneTeam.id,
},
});
const conditions = await service.getAuthorizationConditions();
expect(conditions).toEqual(
Prisma.sql`(("teamId" = ${standaloneTeam.id}) AND ("isTeamBooking" = true)) OR (("userId" = ANY(${[
testData.user.id,
]})) AND ("isTeamBooking" = false))`
);
await prisma.membership.deleteMany({
where: { teamId: standaloneTeam.id },
});
await prisma.team.delete({
where: { id: standaloneTeam.id },
});
await testData.cleanup();
});
it("should return NOTHING_CONDITION for team scope when team belongs to org but no orgId provided", async () => {
const testData = await createTestData({
teamRole: MembershipRole.OWNER,
orgRole: MembershipRole.OWNER,
});
const service = new InsightsBookingService({
prisma,
options: {
scope: "team",
userId: testData.user.id,
orgId: null,
teamId: testData.team.id,
},
});
const conditions = await service.getAuthorizationConditions();
expect(conditions).toEqual(NOTHING_CONDITION);
await testData.cleanup();
});
it("should build team scope conditions with undefined orgId for standalone team", async () => {
const testData = await createTestData({
teamRole: MembershipRole.OWNER,
orgRole: MembershipRole.OWNER,
});
const standaloneTeam = await prisma.team.create({
data: {
name: "Standalone Team 2",
slug: `standalone-team-2-${Date.now()}-${Math.random().toString(36).substring(7)}`,
isOrganization: false,
parentId: null,
},
});
await prisma.membership.create({
data: {
userId: testData.user.id,
teamId: standaloneTeam.id,
role: MembershipRole.OWNER,
accepted: true,
},
});
const service = new InsightsBookingService({
prisma,
options: {
scope: "team",
userId: testData.user.id,
orgId: null,
teamId: standaloneTeam.id,
},
});
const conditions = await service.getAuthorizationConditions();
expect(conditions).toEqual(
Prisma.sql`(("teamId" = ${standaloneTeam.id}) AND ("isTeamBooking" = true)) OR (("userId" = ANY(${[
testData.user.id,
]})) AND ("isTeamBooking" = false))`
);
await prisma.membership.deleteMany({
where: { teamId: standaloneTeam.id },
});
await prisma.team.delete({
where: { id: standaloneTeam.id },
});
await testData.cleanup();
});
});
describe("Filter Conditions", () => {
@@ -8,7 +8,10 @@ import type { Team, User, Membership } from "@calcom/prisma/client";
import { Prisma } from "@calcom/prisma/client";
import { BookingStatus, MembershipRole } from "@calcom/prisma/enums";
import { InsightsRoutingBaseService as InsightsRoutingService } from "../../service/InsightsRoutingBaseService";
import {
InsightsRoutingBaseService as InsightsRoutingService,
type InsightsRoutingServicePublicOptions,
} from "../../service/InsightsRoutingBaseService";
// SQL condition constants for testing
const NOTHING_CONDITION = Prisma.sql`1=0`;
@@ -251,7 +254,7 @@ describe("InsightsRoutingService Integration Tests", () => {
it("should return NOTHING for invalid options", async () => {
const service = new InsightsRoutingService({
prisma,
options: null as any,
options: null as unknown as InsightsRoutingServicePublicOptions,
filters: createDefaultFilters(),
});
@@ -330,6 +333,56 @@ describe("InsightsRoutingService Integration Tests", () => {
await testData.cleanup();
});
it("should build user scope conditions with null orgId", async () => {
const testData = await createTestData({
teamRole: MembershipRole.OWNER,
orgRole: MembershipRole.OWNER,
});
const service = new InsightsRoutingService({
prisma,
options: {
scope: "user",
userId: testData.user.id,
orgId: null,
teamId: undefined,
},
filters: createDefaultFilters(),
});
const conditions = await service.getAuthorizationConditions();
expect(conditions).toEqual(
Prisma.sql`rfrd."formUserId" = ${testData.user.id} AND rfrd."formTeamId" IS NULL`
);
await testData.cleanup();
});
it("should build user scope conditions with undefined orgId", async () => {
const testData = await createTestData({
teamRole: MembershipRole.OWNER,
orgRole: MembershipRole.OWNER,
});
const service = new InsightsRoutingService({
prisma,
options: {
scope: "user",
userId: testData.user.id,
orgId: null,
teamId: undefined,
},
filters: createDefaultFilters(),
});
const conditions = await service.getAuthorizationConditions();
expect(conditions).toEqual(
Prisma.sql`rfrd."formUserId" = ${testData.user.id} AND rfrd."formTeamId" IS NULL`
);
await testData.cleanup();
});
it("should build team scope conditions", async () => {
const testData = await createTestData({
teamRole: MembershipRole.OWNER,
@@ -426,6 +479,123 @@ describe("InsightsRoutingService Integration Tests", () => {
});
await testData.cleanup();
});
it("should build team scope conditions with null orgId for standalone team", async () => {
const testData = await createTestData({
teamRole: MembershipRole.OWNER,
orgRole: MembershipRole.OWNER,
});
const standaloneTeam = await prisma.team.create({
data: {
name: "Standalone Team",
slug: `standalone-team-${randomUUID()}`,
isOrganization: false,
parentId: null,
},
});
await prisma.membership.create({
data: {
userId: testData.user.id,
teamId: standaloneTeam.id,
role: MembershipRole.OWNER,
accepted: true,
},
});
const service = new InsightsRoutingService({
prisma,
options: {
scope: "team",
userId: testData.user.id,
orgId: null,
teamId: standaloneTeam.id,
},
filters: createDefaultFilters(),
});
const conditions = await service.getAuthorizationConditions();
expect(conditions).toEqual(Prisma.sql`rfrd."formTeamId" = ${standaloneTeam.id}`);
await prisma.membership.deleteMany({
where: { teamId: standaloneTeam.id },
});
await prisma.team.delete({
where: { id: standaloneTeam.id },
});
await testData.cleanup();
});
it("should return NOTHING_CONDITION for team scope when team belongs to org but no orgId provided", async () => {
const testData = await createTestData({
teamRole: MembershipRole.OWNER,
orgRole: MembershipRole.OWNER,
});
const service = new InsightsRoutingService({
prisma,
options: {
scope: "team",
userId: testData.user.id,
orgId: null,
teamId: testData.team.id,
},
filters: createDefaultFilters(),
});
const conditions = await service.getAuthorizationConditions();
expect(conditions).toEqual(NOTHING_CONDITION);
await testData.cleanup();
});
it("should build team scope conditions with undefined orgId for standalone team", async () => {
const testData = await createTestData({
teamRole: MembershipRole.OWNER,
orgRole: MembershipRole.OWNER,
});
const standaloneTeam = await prisma.team.create({
data: {
name: "Standalone Team 2",
slug: `standalone-team-2-${randomUUID()}`,
isOrganization: false,
parentId: null,
},
});
await prisma.membership.create({
data: {
userId: testData.user.id,
teamId: standaloneTeam.id,
role: MembershipRole.OWNER,
accepted: true,
},
});
const service = new InsightsRoutingService({
prisma,
options: {
scope: "team",
userId: testData.user.id,
orgId: null,
teamId: standaloneTeam.id,
},
filters: createDefaultFilters(),
});
const conditions = await service.getAuthorizationConditions();
expect(conditions).toEqual(Prisma.sql`rfrd."formTeamId" = ${standaloneTeam.id}`);
await prisma.membership.deleteMany({
where: { teamId: standaloneTeam.id },
});
await prisma.team.delete({
where: { id: standaloneTeam.id },
});
await testData.cleanup();
});
});
describe("Filter Conditions", () => {