Skip to content

feat(3344): Add checks for role permissions#26

Open
islathehut wants to merge 4 commits into
mainfrom
feat/3344-allow-checking-for-role-permissions
Open

feat(3344): Add checks for role permissions#26
islathehut wants to merge 4 commits into
mainfrom
feat/3344-allow-checking-for-role-permissions

Conversation

@islathehut

Copy link
Copy Markdown
Collaborator

No description provided.

@islathehut

Copy link
Copy Markdown
Collaborator Author

TryQuiet/quiet#3353

@islathehut
islathehut requested a review from adrastaea July 10, 2026 17:39
Comment thread packages/auth/src/team/validate.ts Outdated
return true
}
const role = select.role(previousState, roleName)
if (role.createdBy === assigningUserId) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This prevents us from ever revoking the permissions of a user who creates a role

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't have any effect on revocations since this is only run on ADD_MEMBER_ROLE.

Comment thread packages/auth/src/team/Team.ts Outdated
}

public memberCanDeleteRole(roleName: string, memberId: string): boolean {
return this.memberCanPerformAction(memberId, 'REMOVE_ROLE')

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is missing the assert(roleName !== ADMIN, 'Cannot remove admin role') like in removeRole that prevents the admin role from being deleted

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added

Comment thread packages/auth/src/team/Team.ts Outdated
}

/** Check if member has permissions to create roles */
public memberCanPerformAction(memberId: string, actionType: TeamAction['type']): boolean {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this accurately tests whether an action can be performed because it doesn't take into account payloads (i.e. calling REMOVE_DEVICE for someone else's device.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh yea this should be private and maybe renamed. For specific actions we'll need to add more logic to their checks, this is only meant to capture if the member is priveleged enough to run an action.

@adrastaea
adrastaea self-requested a review July 17, 2026 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants