From c6f793135c851ff83dc645ca9678a7bd55c1eabd Mon Sep 17 00:00:00 2001 From: Isla <5048549+islathehut@users.noreply.github.com> Date: Tue, 7 Jul 2026 12:18:30 -0400 Subject: [PATCH 1/4] Use same validation logic for checking user permissions on role creation/modification --- packages/auth/src/team/Team.ts | 16 ++++++++++++ packages/auth/src/team/isAdminOnlyAction.ts | 6 ++++- packages/auth/src/team/validate.ts | 27 ++++++++++++--------- 3 files changed, 37 insertions(+), 12 deletions(-) diff --git a/packages/auth/src/team/Team.ts b/packages/auth/src/team/Team.ts index 2c555fd2..affa07ea 100644 --- a/packages/auth/src/team/Team.ts +++ b/packages/auth/src/team/Team.ts @@ -53,6 +53,8 @@ import type { TeamState, } from './types.js' import { isNewTeam } from './types.js' +import { canUserAddMemberToRole } from './validate.js' +import { isAdminOnlyActionType } from './isAdminOnlyAction.js' const { DEVICE, USER } = KeyType /** @@ -414,6 +416,20 @@ export class Team extends EventEmitter { }) } + /** Check if member has permissions to add members to a role */ + public memberCanAddMembersToRole(roleName: string, memberId: string): boolean { + return canUserAddMemberToRole(roleName, memberId, this.state) + } + + /** Check if member has permissions to create roles */ + public memberCanCreateRole(memberId: string): boolean { + if (!isAdminOnlyActionType('ADD_ROLE')) { + return true + } + + return this.memberIsAdmin(memberId) + } + /** ************** DEVICES */ /** Returns true if the given member has a device by the given name */ diff --git a/packages/auth/src/team/isAdminOnlyAction.ts b/packages/auth/src/team/isAdminOnlyAction.ts index 03ffa774..97813f5d 100644 --- a/packages/auth/src/team/isAdminOnlyAction.ts +++ b/packages/auth/src/team/isAdminOnlyAction.ts @@ -1,6 +1,10 @@ import { type TeamAction, type TeamLinkBody } from './types.js' export const isAdminOnlyAction = (action: TeamLinkBody) => { + return isAdminOnlyActionType(action.type) +} + +export const isAdminOnlyActionType = (actionType: TeamAction['type']): boolean => { // Any team member can do these things const nonAdminActions: Array = [ 'INVITE_DEVICE', @@ -14,5 +18,5 @@ export const isAdminOnlyAction = (action: TeamLinkBody) => { 'SET_METADATA', ] - return !nonAdminActions.includes(action.type) + return !nonAdminActions.includes(actionType) } diff --git a/packages/auth/src/team/validate.ts b/packages/auth/src/team/validate.ts index a0b3050a..2510c6e9 100644 --- a/packages/auth/src/team/validate.ts +++ b/packages/auth/src/team/validate.ts @@ -25,6 +25,21 @@ export const validate: TeamStateValidator = (previousState: TeamState, link: Tea return VALID } +export const canUserAddMemberToRole = (roleName: string, assigningUserId: string, previousState: TeamState): boolean => { + const metadata = select.getMetadata(previousState) + if (metadata.selfAssignableRoles.includes(roleName)) { + return true + } + const role = select.role(previousState, roleName) + if (role.createdBy === assigningUserId) { + return true + } + if (select.memberIsAdmin(previousState, assigningUserId)) { + return true + } + return false + } + const validators: TeamStateValidatorSet = { rootDeviceBelongsToRootUser(previousState: TeamState, link: TeamLink, extendableLogger: Logger) { const logger = extendableLogger.extend('rootDeviceBelongsToRootUser') @@ -118,17 +133,7 @@ const validators: TeamStateValidatorSet = { if (link.body.type === 'ADD_MEMBER_ROLE') { const { userId: assigningUserId } = link.body const { roleName } = link.body.payload - const metadata = select.getMetadata(previousState) - if (metadata.selfAssignableRoles.includes(roleName)) { - return VALID - } - const role = select.role(previousState, roleName) - if (role.createdBy === assigningUserId) { - return VALID - } - if (select.memberIsAdmin(previousState, assigningUserId)) { - return VALID - } + if (canUserAddMemberToRole(roleName, assigningUserId, previousState)) return VALID return fail(`User ${assigningUserId} attempted to assign role ${roleName} illegally`, previousState, link, logger) } return VALID From f4d3c1e07fde3144fd0a3975d59d2e96b8baa03c Mon Sep 17 00:00:00 2001 From: Isla <5048549+islathehut@users.noreply.github.com> Date: Tue, 7 Jul 2026 12:23:45 -0400 Subject: [PATCH 2/4] Add checks for other role actions --- packages/auth/src/team/Team.ts | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/packages/auth/src/team/Team.ts b/packages/auth/src/team/Team.ts index affa07ea..8e19853b 100644 --- a/packages/auth/src/team/Team.ts +++ b/packages/auth/src/team/Team.ts @@ -416,18 +416,33 @@ export class Team extends EventEmitter { }) } + /** Check if member has permissions to create roles */ + public memberCanPerformAction(memberId: string, actionType: TeamAction['type']): boolean { + if (!isAdminOnlyActionType(actionType)) { + return true + } + + return this.memberIsAdmin(memberId) + } + /** Check if member has permissions to add members to a role */ public memberCanAddMembersToRole(roleName: string, memberId: string): boolean { + if (!this.memberCanPerformAction(memberId, 'ADD_MEMBER_ROLE')) { + return false + } return canUserAddMemberToRole(roleName, memberId, this.state) } - /** Check if member has permissions to create roles */ + public memberCanRemoveMembersFromRole(roleName: string, memberId: string): boolean { + return this.memberCanPerformAction(memberId, 'REMOVE_MEMBER_ROLE') + } + public memberCanCreateRole(memberId: string): boolean { - if (!isAdminOnlyActionType('ADD_ROLE')) { - return true - } + return this.memberCanPerformAction(memberId, 'ADD_ROLE') + } - return this.memberIsAdmin(memberId) + public memberCanDeleteRole(roleName: string, memberId: string): boolean { + return this.memberCanPerformAction(memberId, 'REMOVE_ROLE') } /** ************** DEVICES */ From b55dfe4a8bbadac7747226d59a785034dc88eafb Mon Sep 17 00:00:00 2001 From: Isla <5048549+islathehut@users.noreply.github.com> Date: Fri, 17 Jul 2026 14:56:38 -0400 Subject: [PATCH 3/4] Refactor and add admin check to role deletion check --- packages/auth/src/team/Team.ts | 39 +++++++++++++++++++++++++++------- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/packages/auth/src/team/Team.ts b/packages/auth/src/team/Team.ts index 8e19853b..207d5bbf 100644 --- a/packages/auth/src/team/Team.ts +++ b/packages/auth/src/team/Team.ts @@ -366,7 +366,7 @@ export class Team extends EventEmitter { /** Remove a role from the team */ public removeRole = (roleName: string) => { - assert(roleName !== ADMIN, 'Cannot remove admin role') + this._isRoleRemovable(roleName, true) this.dispatch({ type: 'REMOVE_ROLE', @@ -374,6 +374,15 @@ export class Team extends EventEmitter { }) } + private _isRoleRemovable = (roleName: string, assertOnFalse: boolean): boolean => { + const anyRoleButAdmin = roleName !== ADMIN + if (assertOnFalse) { + assert(anyRoleButAdmin, 'Cannot remove admin role') + } + + return anyRoleButAdmin + } + /** Dispatch the add role action */ private _dispatchAddMemberRole(userId: string, roleName: string, lockboxes: lockbox.Lockbox[]) { this.dispatch({ @@ -416,33 +425,47 @@ export class Team extends EventEmitter { }) } - /** Check if member has permissions to create roles */ - public memberCanPerformAction(memberId: string, actionType: TeamAction['type']): boolean { + /** Check if member is priveleged enough to perform a specific action */ + private _memberHasPrivelegeToPerformAction(memberId: string, actionType: TeamAction['type']): boolean { if (!isAdminOnlyActionType(actionType)) { return true } - return this.memberIsAdmin(memberId) } /** Check if member has permissions to add members to a role */ public memberCanAddMembersToRole(roleName: string, memberId: string): boolean { - if (!this.memberCanPerformAction(memberId, 'ADD_MEMBER_ROLE')) { + if (!this._memberHasPrivelegeToPerformAction(memberId, 'ADD_MEMBER_ROLE')) { + return false + } + if (!this.memberHasRole(memberId, roleName)) { return false } return canUserAddMemberToRole(roleName, memberId, this.state) } + /** Check if member has permissions to reevoke membership from a role */ public memberCanRemoveMembersFromRole(roleName: string, memberId: string): boolean { - return this.memberCanPerformAction(memberId, 'REMOVE_MEMBER_ROLE') + if (!this._memberHasPrivelegeToPerformAction(memberId, 'REMOVE_MEMBER_ROLE')) { + return false + } + return this.memberHasRole(memberId, roleName) } + /** Check if member has permissions to create roles */ public memberCanCreateRole(memberId: string): boolean { - return this.memberCanPerformAction(memberId, 'ADD_ROLE') + return this._memberHasPrivelegeToPerformAction(memberId, 'ADD_ROLE') } + /** Check if member has permissions to delete a specific role */ public memberCanDeleteRole(roleName: string, memberId: string): boolean { - return this.memberCanPerformAction(memberId, 'REMOVE_ROLE') + if (!this._memberHasPrivelegeToPerformAction(memberId, 'REMOVE_ROLE')) { + return false + } + if (!this.memberHasRole(memberId, roleName)) { + return false + } + return this._isRoleRemovable(roleName, false) } /** ************** DEVICES */ From 74d91267a279aca2fbd104043c88a6de28e3cb2a Mon Sep 17 00:00:00 2001 From: Isla <5048549+islathehut@users.noreply.github.com> Date: Fri, 17 Jul 2026 15:34:19 -0400 Subject: [PATCH 4/4] Remove role ownership check --- packages/auth/src/team/validate.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/auth/src/team/validate.ts b/packages/auth/src/team/validate.ts index 2510c6e9..a3a6773f 100644 --- a/packages/auth/src/team/validate.ts +++ b/packages/auth/src/team/validate.ts @@ -30,10 +30,6 @@ export const canUserAddMemberToRole = (roleName: string, assigningUserId: string if (metadata.selfAssignableRoles.includes(roleName)) { return true } - const role = select.role(previousState, roleName) - if (role.createdBy === assigningUserId) { - return true - } if (select.memberIsAdmin(previousState, assigningUserId)) { return true }