-
Notifications
You must be signed in to change notification settings - Fork 0
Description
As a project maintainer, I'd like to be able to control whether Discord users can use certain slash commands based on the permissions associated with their roles.
🧠 Context
At the time of writing this issue, we have 3 slash commands included in our Discord bot. Currently, any server member can run them, but we only actually process requests from users listed in data/githubDiscordMap.json.
Today, we use a method src/infrastructure/discord/authz.ts#can that simply checks whether the current Discord user's ID is found in githubDiscordMap.
We want to change this. Instead of just checking for inclusion, the can method should verify whether the current user has all required permissions like "githubIssue:write" or "githubIssue:read".
🔄 Proposed Design
We’ll split role definitions and user-role assignments into two files for maintainability.
data/roles.json
Defines named roles and their permissions:
{
"admin": {
"permissions": ["githubIssue:write", "githubIssue:read"]
},
"readonly": {
"permissions": ["githubIssue:read"]
}
}data/githubDiscordMap.json
Maps Discord users to roles:
{
"AJaccP": {
"githubUsername": "...",
"githubId": "...",
"discordId": "...",
"roles": ["admin"]
},
"eros-mcguire": {
"githubUsername": "...",
"githubId": "...",
"discordId": "...",
"roles": ["readonly"]
}
}🛠️ Implementation Plan
-
Update
authz.ts- Load both
roles.jsonandgithubDiscordMap.json - Update the
canmethod to check if the user’s roles include all required permissions
export const can = (interaction: any, requiredPermissions: string[]): boolean => { const userId = interaction.user.id; // Find user in githubDiscordMap const userEntry = Object.values(githubDiscordMap).find(entry => entry.discordId === userId); if (!userEntry || !userEntry.roles) return false; // Aggregate permissions from all roles const userPermissions = new Set( userEntry.roles.flatMap(role => roles[role]?.permissions || []) ); // Ensure user has all required permissions return requiredPermissions.every(perm => userPermissions.has(perm)); };
- Load both
-
Write Unit Tests
-
Create
test/infrastructure/discord/authz.test.ts -
Mock
githubDiscordMapandroles.jsondata -
Write test cases to verify:
- User with correct roles can access
- User with missing permissions is denied
- Users with multiple roles get merged permissions
-
-
Refactor existing uses of
can()- Update all invocations of
can(interaction)to usecan(interaction, [...permissions])
- Update all invocations of
✅ Acceptance Criteria
-
A
roles.jsonfile exists indata/defining roles and associated permissions -
The structure of
githubDiscordMap.jsonincludes arolesarray per user -
authz.ts#canfunction has been updated to check user permissions via roles -
At least 5 unit tests cover:
- ✔️ User has one role with all permissions
- ❌ User has no roles
- ❌ User has a role missing some permissions
- ✔️ User has multiple roles that collectively satisfy permission requirement
- ❌ User is not found in
githubDiscordMap
-
No existing command functionality breaks due to the change (all existing tests should based once you update mock data)
Metadata
Metadata
Assignees
Labels
Type
Projects
Status