Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ export interface IPermissionRepository {
customRoleId: string | null;
} | null>;

getTeamById(teamId: number): Promise<{
id: number;
parentId: number | null;
} | null>;

checkRolePermission(roleId: string, permission: PermissionString): Promise<boolean>;
checkRolePermissions(roleId: string, permissions: PermissionString[]): Promise<boolean>;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,16 @@ export class PermissionRepository implements IPermissionRepository {
});
}

async getTeamById(teamId: number) {
return this.client.team.findUnique({
where: { id: teamId },
select: {
id: true,
parentId: true,
},
});
}

async checkRolePermission(roleId: string, permission: PermissionString): Promise<boolean> {
const { resource, action } = parsePermissionString(permission);
const hasPermission = await this.client.rolePermission.findFirst({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ describe("PermissionCheckService", () => {
getMembershipByMembershipId: vi.fn(),
getMembershipByUserAndTeam: vi.fn(),
getOrgMembership: vi.fn(),
getTeamById: vi.fn(),
getUserMemberships: vi.fn(),
checkRolePermission: vi.fn(),
checkRolePermissions: vi.fn(),
Expand Down Expand Up @@ -67,28 +68,14 @@ describe("PermissionCheckService", () => {

describe("checkPermission", () => {
it("should check permission with PBAC enabled", async () => {
const membership = {
mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true);
mockRepository.getMembershipByUserAndTeam.mockResolvedValueOnce({
id: 1,
teamId: 1,
userId: 1,
accepted: true,
role: "ADMIN" as MembershipRole,
customRoleId: "admin_role",
disableImpersonation: false,
createdAt: new Date(),
updatedAt: new Date(),
};

(MembershipRepository.findUniqueByUserIdAndTeamId as Mock).mockResolvedValueOnce(membership);
mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true);
mockRepository.getMembershipByMembershipId.mockResolvedValueOnce({
id: membership.id,
teamId: membership.teamId,
userId: membership.userId,
customRoleId: membership.customRoleId,
team: { parentId: null },
});
mockRepository.getOrgMembership.mockResolvedValueOnce(null);
mockRepository.checkRolePermission.mockResolvedValueOnce(true);

const result = await service.checkPermission({
Expand All @@ -99,12 +86,8 @@ describe("PermissionCheckService", () => {
});

expect(result).toBe(true);
expect(MembershipRepository.findUniqueByUserIdAndTeamId).toHaveBeenCalledWith({
userId: 1,
teamId: 1,
});
expect(mockFeaturesRepository.checkIfTeamHasFeature).toHaveBeenCalledWith(1, "pbac");
expect(mockRepository.getMembershipByMembershipId).toHaveBeenCalledWith(1);
expect(mockRepository.getMembershipByUserAndTeam).toHaveBeenCalledWith(1, 1);
expect(mockRepository.checkRolePermission).toHaveBeenCalledWith("admin_role", "eventType.create");
});

Expand Down Expand Up @@ -140,7 +123,8 @@ describe("PermissionCheckService", () => {
expect(mockRepository.checkRolePermission).not.toHaveBeenCalled();
});

it("should return false if membership not found", async () => {
it("should return false if membership not found when PBAC disabled", async () => {
mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(false);
(MembershipRepository.findUniqueByUserIdAndTeamId as Mock).mockResolvedValueOnce(null);

const result = await service.checkPermission({
Expand All @@ -153,21 +137,47 @@ describe("PermissionCheckService", () => {
expect(result).toBe(false);
});

it("should return false if PBAC enabled but no customRoleId", async () => {
const membership = {
it("should check org-level permissions when user has no team membership but PBAC is enabled", async () => {
mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true);
mockRepository.getMembershipByUserAndTeam.mockResolvedValueOnce(null);
mockRepository.getTeamById.mockResolvedValueOnce({ id: 1, parentId: 2 });
mockRepository.getOrgMembership.mockResolvedValueOnce({
id: 100,
teamId: 2,
userId: 1,
customRoleId: "org_admin_role",
});
mockRepository.checkRolePermission.mockResolvedValueOnce(true);

const result = await service.checkPermission({
userId: 1,
teamId: 1,
permission: "eventType.create",
fallbackRoles: ["ADMIN", "OWNER"],
});

expect(result).toBe(true);
expect(mockRepository.getMembershipByUserAndTeam).toHaveBeenCalledWith(1, 1);
expect(mockRepository.getTeamById).toHaveBeenCalledWith(1);
expect(mockRepository.getOrgMembership).toHaveBeenCalledWith(1, 2);
expect(mockRepository.checkRolePermission).toHaveBeenCalledWith("org_admin_role", "eventType.create");
});

it("should return false if PBAC enabled but no customRoleId on team or org", async () => {
mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true);
mockRepository.getMembershipByUserAndTeam.mockResolvedValueOnce({
id: 1,
teamId: 1,
userId: 1,
accepted: true,
role: "ADMIN" as MembershipRole,
customRoleId: null,
disableImpersonation: false,
createdAt: new Date(),
updatedAt: new Date(),
};

(MembershipRepository.findUniqueByUserIdAndTeamId as Mock).mockResolvedValueOnce(membership);
mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true);
team: { parentId: 2 },
});
mockRepository.getOrgMembership.mockResolvedValueOnce({
id: 100,
teamId: 2,
userId: 1,
customRoleId: null,
});

const result = await service.checkPermission({
userId: 1,
Expand All @@ -182,28 +192,14 @@ describe("PermissionCheckService", () => {

describe("checkPermissions", () => {
it("should check multiple permissions with PBAC enabled", async () => {
const membership = {
mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true);
mockRepository.getMembershipByUserAndTeam.mockResolvedValueOnce({
id: 1,
teamId: 1,
userId: 1,
accepted: true,
role: "ADMIN" as MembershipRole,
customRoleId: "admin_role",
disableImpersonation: false,
createdAt: new Date(),
updatedAt: new Date(),
};

(MembershipRepository.findUniqueByUserIdAndTeamId as Mock).mockResolvedValueOnce(membership);
mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true);
mockRepository.getMembershipByMembershipId.mockResolvedValueOnce({
id: membership.id,
teamId: membership.teamId,
userId: membership.userId,
customRoleId: membership.customRoleId,
team: { parentId: null },
});
mockRepository.getOrgMembership.mockResolvedValueOnce(null);
mockRepository.checkRolePermissions.mockResolvedValueOnce(true);

const result = await service.checkPermissions({
Expand All @@ -214,12 +210,8 @@ describe("PermissionCheckService", () => {
});

expect(result).toBe(true);
expect(MembershipRepository.findUniqueByUserIdAndTeamId).toHaveBeenCalledWith({
userId: 1,
teamId: 1,
});
expect(mockFeaturesRepository.checkIfTeamHasFeature).toHaveBeenCalledWith(1, "pbac");
expect(mockRepository.getMembershipByMembershipId).toHaveBeenCalledWith(1);
expect(mockRepository.getMembershipByUserAndTeam).toHaveBeenCalledWith(1, 1);
expect(mockRepository.checkRolePermissions).toHaveBeenCalledWith("admin_role", [
"eventType.create",
"team.invite",
Expand Down Expand Up @@ -259,25 +251,12 @@ describe("PermissionCheckService", () => {
});

it("should return false when permissions array is empty", async () => {
const membership = {
mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true);
mockRepository.getMembershipByUserAndTeam.mockResolvedValueOnce({
id: 1,
teamId: 1,
userId: 1,
accepted: true,
role: "MEMBER" as MembershipRole, // Change to MEMBER so fallback also fails
customRoleId: "admin_role",
disableImpersonation: false,
createdAt: new Date(),
updatedAt: new Date(),
};

(MembershipRepository.findUniqueByUserIdAndTeamId as Mock).mockResolvedValueOnce(membership);
mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true);
mockRepository.getMembershipByMembershipId.mockResolvedValueOnce({
id: membership.id,
teamId: membership.teamId,
userId: membership.userId,
customRoleId: membership.customRoleId,
team: { parentId: null },
});
mockRepository.getOrgMembership.mockResolvedValueOnce(null);
Expand All @@ -293,6 +272,35 @@ describe("PermissionCheckService", () => {
expect(result).toBe(false);
expect(mockRepository.checkRolePermissions).toHaveBeenCalledWith("admin_role", []);
});

it("should check org-level permissions when user has no team membership with checkPermissions", async () => {
mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true);
mockRepository.getMembershipByUserAndTeam.mockResolvedValueOnce(null);
mockRepository.getTeamById.mockResolvedValueOnce({ id: 1, parentId: 2 });
mockRepository.getOrgMembership.mockResolvedValueOnce({
id: 100,
teamId: 2,
userId: 1,
customRoleId: "org_admin_role",
});
mockRepository.checkRolePermissions.mockResolvedValueOnce(true);

const result = await service.checkPermissions({
userId: 1,
teamId: 1,
permissions: ["eventType.create", "team.invite"],
fallbackRoles: ["ADMIN", "OWNER"],
});

expect(result).toBe(true);
expect(mockRepository.getMembershipByUserAndTeam).toHaveBeenCalledWith(1, 1);
expect(mockRepository.getTeamById).toHaveBeenCalledWith(1);
expect(mockRepository.getOrgMembership).toHaveBeenCalledWith(1, 2);
expect(mockRepository.checkRolePermissions).toHaveBeenCalledWith("org_admin_role", [
"eventType.create",
"team.invite",
]);
});
});

describe("getUserPermissions", () => {
Expand Down Expand Up @@ -392,6 +400,33 @@ describe("PermissionCheckService", () => {
});

describe("getResourcePermissions", () => {
it("should return org permissions when user has no team membership but has org membership", async () => {
mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(true);
mockRepository.getMembershipByUserAndTeam.mockResolvedValueOnce(null);
mockRepository.getTeamById.mockResolvedValueOnce({ id: 1, parentId: 2 });
mockRepository.getOrgMembership.mockResolvedValueOnce({
id: 100,
teamId: 2,
userId: 1,
customRoleId: "org_role",
});
mockRepository.getResourcePermissionsByRoleId.mockResolvedValueOnce(["create", "read", "update"]);

const result = await service.getResourcePermissions({
userId: 1,
teamId: 1,
resource: Resource.EventType,
});

expect(result).toEqual(["eventType.create", "eventType.read", "eventType.update"]);
expect(mockRepository.getMembershipByUserAndTeam).toHaveBeenCalledWith(1, 1);
expect(mockRepository.getTeamById).toHaveBeenCalledWith(1);
expect(mockRepository.getOrgMembership).toHaveBeenCalledWith(1, 2);
expect(mockRepository.getResourcePermissionsByRoleId).toHaveBeenCalledWith(
"org_role",
Resource.EventType
);
});
it("should return empty array when PBAC is disabled", async () => {
mockFeaturesRepository.checkIfTeamHasFeature.mockResolvedValueOnce(false);

Expand Down
56 changes: 28 additions & 28 deletions packages/features/pbac/services/permission-check.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@ export class PermissionCheckService {
teamActions.forEach((action) => actions.add(action));
}

// Get org-level permissions as fallback
if (membership?.team?.parentId && orgMembership?.customRoleId) {
// Get org-level permissions (works even without team membership)
if (orgMembership?.customRoleId) {
const orgActions = await this.repository.getResourcePermissionsByRoleId(
orgMembership.customRoleId,
resource
Expand Down Expand Up @@ -108,27 +108,23 @@ export class PermissionCheckService {
return false;
}

const membership = await MembershipRepository.findUniqueByUserIdAndTeamId({
userId,
teamId,
});

if (!membership) return false;

const isPBACEnabled = await this.featuresRepository.checkIfTeamHasFeature(
teamId,
this.PBAC_FEATURE_FLAG
);

if (isPBACEnabled) {
if (!membership.customRoleId) {
this.logger.info(`PBAC is enabled for ${teamId} but no custom role is set on membership relation`);
return false;
}

return this.hasPermission({ membershipId: membership.id }, permission);
// Check if user has permission through team or org membership
return this.hasPermission({ userId, teamId }, permission);
}

// Fallback to role-based check only if user has team membership
const membership = await MembershipRepository.findUniqueByUserIdAndTeamId({
userId,
teamId,
});

if (!membership) return false;
return this.checkFallbackRoles(membership.role, fallbackRoles);
} catch (error) {
this.logger.error(error);
Expand Down Expand Up @@ -157,27 +153,23 @@ export class PermissionCheckService {
return false;
}

const membership = await MembershipRepository.findUniqueByUserIdAndTeamId({
userId,
teamId,
});

if (!membership) return false;

const isPBACEnabled = await this.featuresRepository.checkIfTeamHasFeature(
teamId,
this.PBAC_FEATURE_FLAG
);

if (isPBACEnabled) {
if (!membership.customRoleId) {
this.logger.info(`PBAC is enabled for ${teamId} but no custom role is set on membership relation`);
return false;
}

return this.hasPermissions({ membershipId: membership.id }, permissions);
// Check if user has permissions through team or org membership
return this.hasPermissions({ userId, teamId }, permissions);
}

// Fallback to role-based check only if user has team membership
const membership = await MembershipRepository.findUniqueByUserIdAndTeamId({
userId,
teamId,
});

if (!membership) return false;
return this.checkFallbackRoles(membership.role, fallbackRoles);
} catch (error) {
this.logger.error(error);
Expand Down Expand Up @@ -241,8 +233,16 @@ export class PermissionCheckService {
membership = await this.repository.getMembershipByUserAndTeam(query.userId, query.teamId);
}

// Get org membership either through the team membership or directly from teamId
if (membership?.team.parentId) {
// User has team membership, check org through that
orgMembership = await this.repository.getOrgMembership(membership.userId, membership.team.parentId);
} else if (query.userId && query.teamId) {
// No team membership, but check if team belongs to an org
const team = await this.repository.getTeamById(query.teamId);
if (team?.parentId) {
orgMembership = await this.repository.getOrgMembership(query.userId, team.parentId);
}
}

return { membership, orgMembership };
Expand Down
Loading
Loading