Skip to content

Commit 95143bb

Browse files
anikdhabalanikdhabal
committed
address review
1 parent 1658265 commit 95143bb

4 files changed

Lines changed: 18 additions & 21 deletions

File tree

‎packages/features/pbac/services/__tests__/role-management.factory.test.ts‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -110,13 +110,15 @@ describe("RoleManagementFactory", () => {
110110
it("should allow role change when user has permission", async () => {
111111
mockPermissionCheckService.checkPermission.mockResolvedValue(true);
112112
const manager = await factory.createRoleManager(organizationId);
113-
await expect(manager.checkPermissionToChangeRole(userId, organizationId)).resolves.not.toThrow();
113+
await expect(
114+
manager.checkPermissionToChangeRole(userId, organizationId, "org")
115+
).resolves.not.toThrow();
114116
});
115117

116118
it("should throw UNAUTHORIZED when user lacks permission", async () => {
117119
mockPermissionCheckService.checkPermission.mockResolvedValue(false);
118120
const manager = await factory.createRoleManager(organizationId);
119-
await expect(manager.checkPermissionToChangeRole(userId, organizationId)).rejects.toThrow(
121+
await expect(manager.checkPermissionToChangeRole(userId, organizationId, "org")).rejects.toThrow(
120122
new RoleManagementError(
121123
"You do not have permission to change roles",
122124
RoleManagementErrorCode.UNAUTHORIZED
@@ -193,13 +195,15 @@ describe("RoleManagementFactory", () => {
193195
customRoleId: null,
194196
});
195197
const manager = await factory.createRoleManager(organizationId);
196-
await expect(manager.checkPermissionToChangeRole(userId, organizationId)).resolves.not.toThrow();
198+
await expect(
199+
manager.checkPermissionToChangeRole(userId, organizationId, "org")
200+
).resolves.not.toThrow();
197201
});
198202

199203
it("should throw UNAUTHORIZED when user is not owner", async () => {
200204
vi.mocked(isOrganisationAdmin).mockResolvedValue(false);
201205
const manager = await factory.createRoleManager(organizationId);
202-
await expect(manager.checkPermissionToChangeRole(userId, organizationId)).rejects.toThrow(
206+
await expect(manager.checkPermissionToChangeRole(userId, organizationId, "org")).rejects.toThrow(
203207
new RoleManagementError(
204208
"Only owners or admin can update roles",
205209
RoleManagementErrorCode.UNAUTHORIZED

‎packages/features/pbac/services/role-management.factory.ts‎

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { RoleService } from "./role.service";
1111

1212
interface IRoleManager {
1313
isPBACEnabled: boolean;
14-
checkPermissionToChangeRole(userId: number, targetId: number, checkAtTeamLevel?: boolean): Promise<void>;
14+
checkPermissionToChangeRole(userId: number, targetId: number, scope: "org" | "team"): Promise<void>;
1515
assignRole(
1616
userId: number,
1717
organizationId: number,
@@ -30,15 +30,11 @@ class PBACRoleManager implements IRoleManager {
3030
private readonly permissionCheckService: PermissionCheckService
3131
) {}
3232

33-
async checkPermissionToChangeRole(
34-
userId: number,
35-
targetId: number,
36-
checkAtTeamLevel?: boolean
37-
): Promise<void> {
33+
async checkPermissionToChangeRole(userId: number, targetId: number, scope: "org" | "team"): Promise<void> {
3834
const hasPermission = await this.permissionCheckService.checkPermission({
3935
userId,
4036
teamId: targetId,
41-
permission: checkAtTeamLevel ? "team.changeMemberRole" : "organization.changeMemberRole",
37+
permission: scope === "team" ? "team.changeMemberRole" : "organization.changeMemberRole",
4238
fallbackRoles: [MembershipRole.OWNER, MembershipRole.ADMIN],
4339
});
4440

@@ -100,14 +96,11 @@ class PBACRoleManager implements IRoleManager {
10096

10197
class LegacyRoleManager implements IRoleManager {
10298
public isPBACEnabled = false;
103-
async checkPermissionToChangeRole(
104-
userId: number,
105-
targetId: number,
106-
checkAtTeamLevel?: boolean
107-
): Promise<void> {
108-
const membership = checkAtTeamLevel
109-
? !!(await isTeamAdmin(userId, targetId))
110-
: !!(await isOrganisationAdmin(userId, targetId));
99+
async checkPermissionToChangeRole(userId: number, targetId: number, scope: "org" | "team"): Promise<void> {
100+
const membership =
101+
scope === "team"
102+
? !!(await isTeamAdmin(userId, targetId))
103+
: !!(await isOrganisationAdmin(userId, targetId));
111104
// Only OWNER/ADMIN can update role
112105
if (!membership) {
113106
throw new RoleManagementError(

‎packages/trpc/server/routers/viewer/organizations/updateUser.handler.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ export const updateUserHandler = async ({ ctx, input }: UpdateUserOptions) => {
4747
const roleManager = await RoleManagementFactory.getInstance().createRoleManager(organizationId);
4848

4949
try {
50-
await roleManager.checkPermissionToChangeRole(userId, organizationId);
50+
await roleManager.checkPermissionToChangeRole(userId, organizationId, "org");
5151
} catch (error) {
5252
if (error instanceof RoleManagementError) {
5353
throw new TRPCError({ code: "UNAUTHORIZED", message: error.message });

‎packages/trpc/server/routers/viewer/teams/changeMemberRole.handler.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ export const changeMemberRoleHandler = async ({ ctx, input }: ChangeMemberRoleOp
3232

3333
// Check permission to change roles
3434
try {
35-
await roleManager.checkPermissionToChangeRole(ctx.user.id, input.teamId, true);
35+
await roleManager.checkPermissionToChangeRole(ctx.user.id, input.teamId, "team");
3636
} catch (error) {
3737
throw new TRPCError({
3838
code: "UNAUTHORIZED",

0 commit comments

Comments
 (0)