From 36391b2a459401328014e10c9ef05c6fcbabb650 Mon Sep 17 00:00:00 2001 From: wraithfive Date: Sun, 18 Jan 2026 09:52:51 -0600 Subject: [PATCH 1/5] Allow Staff role to manage server features (except bot invite) - Treat Staff role as admin-equivalent for server management, but keep bot invites admin-only - Add actual admin check for invites and staff-aware admin checks elsewhere - Add tests covering Staff role access, admin-only invites, and no-access cases --- .../web/controller/ServerController.java | 6 +- .../discordbot/web/service/AdminService.java | 55 +++++++++++- .../discordbot/AdminServiceBranchesTest.java | 88 +++++++++++++++++++ .../com/discordbot/ServerControllerTest.java | 4 +- 4 files changed, 147 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/discordbot/web/controller/ServerController.java b/src/main/java/com/discordbot/web/controller/ServerController.java index 7d23826..d06b976 100644 --- a/src/main/java/com/discordbot/web/controller/ServerController.java +++ b/src/main/java/com/discordbot/web/controller/ServerController.java @@ -76,7 +76,7 @@ public ResponseEntity getServerInfo( /** * GET /api/servers/{guildId}/invite * Generate bot invite URL for a specific server - * SECURED: Requires OAuth2 authentication + user must be admin in that server + * SECURED: Requires OAuth2 authentication + user must be ACTUAL admin (not Staff role) * Note: Does NOT require bot to be present (that's the whole point of inviting it!) */ @GetMapping("/{guildId}/invite") @@ -89,8 +89,8 @@ public ResponseEntity> getBotInviteUrl( return ResponseEntity.status(401).build(); } - // Check if user has admin permissions (bot presence not required for invites) - if (!adminService.isUserAdminInGuild(authentication, guildId)) { + // Check if user has actual admin permissions (Staff role NOT allowed for bot invites) + if (!adminService.isUserActualAdminInGuild(authentication, guildId)) { logger.warn("Unauthorized attempt to get bot invite for guild: {}", guildId); return ResponseEntity.status(403).build(); } diff --git a/src/main/java/com/discordbot/web/service/AdminService.java b/src/main/java/com/discordbot/web/service/AdminService.java index abbd075..8a1f955 100644 --- a/src/main/java/com/discordbot/web/service/AdminService.java +++ b/src/main/java/com/discordbot/web/service/AdminService.java @@ -167,6 +167,7 @@ public List getManageableGuilds(Authentication authentication) { /** * Check if user has admin permissions in a specific guild (regardless of bot presence) + * Also allows users with "Staff" role */ public boolean isUserAdminInGuild(Authentication authentication, String guildId) { if (authentication == null || !(authentication.getPrincipal() instanceof OAuth2User)) { @@ -188,7 +189,13 @@ public boolean isUserAdminInGuild(Authentication authentication, String guildId) Long permissions = Long.parseLong(userGuild.get("permissions").toString()); boolean isAdmin = (permissions & Permission.ADMINISTRATOR.getRawValue()) != 0 || (permissions & Permission.MANAGE_SERVER.getRawValue()) != 0; - return isAdmin; + + if (isAdmin) { + return true; + } + + // Check if user has Staff role + return hasStaffRole(userId, guildId); } } @@ -196,6 +203,52 @@ public boolean isUserAdminInGuild(Authentication authentication, String guildId) return false; } + /** + * Check if user has Staff role in a guild + */ + private boolean hasStaffRole(String userId, String guildId) { + Guild guild = jda.getGuildById(guildId); + if (guild == null) { + return false; + } + + net.dv8tion.jda.api.entities.Member member = guild.getMemberById(userId); + if (member == null) { + return false; + } + + return member.getRoles().stream() + .anyMatch(role -> role.getName().equalsIgnoreCase("Staff")); + } + + /** + * Check if user has actual ADMINISTRATOR or MANAGE_SERVER permissions (no Staff role) + * Use this for restricted actions like bot invites + */ + public boolean isUserActualAdminInGuild(Authentication authentication, String guildId) { + if (authentication == null || !(authentication.getPrincipal() instanceof OAuth2User)) { + return false; + } + + String accessToken = getAccessToken(authentication); + + if (accessToken == null) { + return false; + } + + List> userGuilds = getUserGuildsFromDiscord(accessToken); + + for (Map userGuild : userGuilds) { + if (guildId.equals(userGuild.get("id"))) { + Long permissions = Long.parseLong(userGuild.get("permissions").toString()); + return (permissions & Permission.ADMINISTRATOR.getRawValue()) != 0 || + (permissions & Permission.MANAGE_SERVER.getRawValue()) != 0; + } + } + + return false; + } + /** * Check if user has admin permissions in a specific guild AND bot is present */ diff --git a/src/test/java/com/discordbot/AdminServiceBranchesTest.java b/src/test/java/com/discordbot/AdminServiceBranchesTest.java index 8bb7755..ac0daf6 100644 --- a/src/test/java/com/discordbot/AdminServiceBranchesTest.java +++ b/src/test/java/com/discordbot/AdminServiceBranchesTest.java @@ -252,6 +252,94 @@ void isUserAdminInGuild_noToken() { assertFalse(service.isUserAdminInGuild(auth, "g1")); } + @Test + @DisplayName("isUserAdminInGuild: returns true when user has Staff role") + void isUserAdminInGuild_withStaffRole() { + Authentication auth = mockAuth("user1"); + var authorizedClient = TestTokens.authorizedClient("tok"); + when(clients.loadAuthorizedClient(eq("discord"), eq("user1"))).thenReturn(authorizedClient); + + // User has no admin permissions + Map guild = new HashMap<>(); + guild.put("id", "g1"); + guild.put("name", "G"); + guild.put("permissions", "0"); + when(cache.get(eq("tok"), any())).thenReturn(List.of(guild)); + + // Mock guild and member with Staff role + Guild mockGuild = mock(Guild.class); + net.dv8tion.jda.api.entities.Member mockMember = mock(net.dv8tion.jda.api.entities.Member.class); + Role staffRole = mock(Role.class); + when(staffRole.getName()).thenReturn("Staff"); + + when(jda.getGuildById("g1")).thenReturn(mockGuild); + when(mockGuild.getMemberById("user1")).thenReturn(mockMember); + when(mockMember.getRoles()).thenReturn(List.of(staffRole)); + + assertTrue(service.isUserAdminInGuild(auth, "g1")); + } + + @Test + @DisplayName("isUserAdminInGuild: returns false when user has neither admin perms nor Staff role") + void isUserAdminInGuild_noPermissionsNoStaff() { + Authentication auth = mockAuth("user1"); + var authorizedClient = TestTokens.authorizedClient("tok"); + when(clients.loadAuthorizedClient(eq("discord"), eq("user1"))).thenReturn(authorizedClient); + + // User has no admin permissions + Map guild = new HashMap<>(); + guild.put("id", "g1"); + guild.put("name", "G"); + guild.put("permissions", "0"); + when(cache.get(eq("tok"), any())).thenReturn(List.of(guild)); + + // Mock guild and member with no Staff role + Guild mockGuild = mock(Guild.class); + net.dv8tion.jda.api.entities.Member mockMember = mock(net.dv8tion.jda.api.entities.Member.class); + Role otherRole = mock(Role.class); + when(otherRole.getName()).thenReturn("Member"); + + when(jda.getGuildById("g1")).thenReturn(mockGuild); + when(mockGuild.getMemberById("user1")).thenReturn(mockMember); + when(mockMember.getRoles()).thenReturn(List.of(otherRole)); + + assertFalse(service.isUserAdminInGuild(auth, "g1")); + } + + @Test + @DisplayName("isUserActualAdminInGuild: returns true for admin permissions only") + void isUserActualAdminInGuild_adminOnly() { + Authentication auth = mockAuth("user1"); + var authorizedClient = TestTokens.authorizedClient("tok"); + when(clients.loadAuthorizedClient(eq("discord"), eq("user1"))).thenReturn(authorizedClient); + + Map guildAdmin = new HashMap<>(); + guildAdmin.put("id", "g1"); + guildAdmin.put("name", "G"); + guildAdmin.put("permissions", String.valueOf(net.dv8tion.jda.api.Permission.ADMINISTRATOR.getRawValue())); + when(cache.get(eq("tok"), any())).thenReturn(List.of(guildAdmin)); + + assertTrue(service.isUserActualAdminInGuild(auth, "g1")); + } + + @Test + @DisplayName("isUserActualAdminInGuild: returns false even with Staff role") + void isUserActualAdminInGuild_staffNotAllowed() { + Authentication auth = mockAuth("user1"); + var authorizedClient = TestTokens.authorizedClient("tok"); + when(clients.loadAuthorizedClient(eq("discord"), eq("user1"))).thenReturn(authorizedClient); + + // User has no admin permissions + Map guild = new HashMap<>(); + guild.put("id", "g1"); + guild.put("name", "G"); + guild.put("permissions", "0"); + when(cache.get(eq("tok"), any())).thenReturn(List.of(guild)); + + // Even with Staff role, should return false + assertFalse(service.isUserActualAdminInGuild(auth, "g1")); + } + @Test @DisplayName("getGatchaRoles: returns empty when guild missing") void getGatchaRoles_guildMissing() { diff --git a/src/test/java/com/discordbot/ServerControllerTest.java b/src/test/java/com/discordbot/ServerControllerTest.java index 38dbf75..135deec 100644 --- a/src/test/java/com/discordbot/ServerControllerTest.java +++ b/src/test/java/com/discordbot/ServerControllerTest.java @@ -71,14 +71,14 @@ void getServerInfo_okOrNotFound() { @DisplayName("getBotInviteUrl enforces auth and admin perms") void getInvite_authz() { assertEquals(401, controller.getBotInviteUrl("g1", null).getStatusCode().value()); - when(adminService.isUserAdminInGuild(auth, "g1")).thenReturn(false); + when(adminService.isUserActualAdminInGuild(auth, "g1")).thenReturn(false); assertEquals(403, controller.getBotInviteUrl("g1", auth).getStatusCode().value()); } @Test @DisplayName("getBotInviteUrl returns invite when authorized") void getInvite_ok() { - when(adminService.isUserAdminInGuild(auth, "g1")).thenReturn(true); + when(adminService.isUserActualAdminInGuild(auth, "g1")).thenReturn(true); when(adminService.generateBotInviteUrl("g1")).thenReturn("https://invite"); ResponseEntity> res = controller.getBotInviteUrl("g1", auth); assertEquals(200, res.getStatusCode().value()); From 222a3e2bdd79e2d4aba67578a522ca0dddbc3fbd Mon Sep 17 00:00:00 2001 From: wraithfive Date: Sun, 18 Jan 2026 10:03:49 -0600 Subject: [PATCH 2/5] Document Staff role access boundaries --- README.md | 1 + SECURITY.md | 1 + 2 files changed, 2 insertions(+) diff --git a/README.md b/README.md index 5491adf..609bd83 100644 --- a/README.md +++ b/README.md @@ -573,6 +573,7 @@ See [DATABASE_MIGRATIONS.md](DATABASE_MIGRATIONS.md) for templates and full guid High-level security posture: - OAuth2 login with Discord; only server admins can access secured endpoints +- "Staff" role is treated as admin-equivalent for server management (except bot invites, which remain admin-only); bot must be present to detect Staff membership - CSRF protection enabled for REST via cookie token; WebSocket upgrades exempt - CORS origin is derived from `ADMIN_PANEL_URL` and restricted accordingly - Sessions and OAuth client data are stored via JDBC tables (Liquibase-managed) diff --git a/SECURITY.md b/SECURITY.md index 634ea40..60fa71e 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -52,6 +52,7 @@ We take the security of Playbot seriously. If you believe you have found a secur - Only grant bot admin access to trusted users - Regularly review who has access to the admin panel - Use strong passwords for admin accounts + - "Staff" role is treated as admin-equivalent for management actions, **except** bot invites (still admin-only); requires the bot to be present to detect Staff membership ### For Developers From c2e221835bfed2a902e1270f3df323a5b6c9ebde Mon Sep 17 00:00:00 2001 From: wraithfive Date: Sun, 18 Jan 2026 10:16:13 -0600 Subject: [PATCH 3/5] Harden admin permission parsing and tests --- .../discordbot/web/service/AdminService.java | 13 +++++++++- .../discordbot/AdminServiceBranchesTest.java | 25 +++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/discordbot/web/service/AdminService.java b/src/main/java/com/discordbot/web/service/AdminService.java index 8a1f955..8a22ca1 100644 --- a/src/main/java/com/discordbot/web/service/AdminService.java +++ b/src/main/java/com/discordbot/web/service/AdminService.java @@ -240,7 +240,18 @@ public boolean isUserActualAdminInGuild(Authentication authentication, String gu for (Map userGuild : userGuilds) { if (guildId.equals(userGuild.get("id"))) { - Long permissions = Long.parseLong(userGuild.get("permissions").toString()); + Object permissionsObj = userGuild.get("permissions"); + if (permissionsObj == null) { + return false; + } + + long permissions; + try { + permissions = Long.parseLong(permissionsObj.toString()); + } catch (NumberFormatException e) { + // Treat unparseable permissions as missing admin privileges + return false; + } return (permissions & Permission.ADMINISTRATOR.getRawValue()) != 0 || (permissions & Permission.MANAGE_SERVER.getRawValue()) != 0; } diff --git a/src/test/java/com/discordbot/AdminServiceBranchesTest.java b/src/test/java/com/discordbot/AdminServiceBranchesTest.java index ac0daf6..14ef26e 100644 --- a/src/test/java/com/discordbot/AdminServiceBranchesTest.java +++ b/src/test/java/com/discordbot/AdminServiceBranchesTest.java @@ -336,10 +336,35 @@ void isUserActualAdminInGuild_staffNotAllowed() { guild.put("permissions", "0"); when(cache.get(eq("tok"), any())).thenReturn(List.of(guild)); + // Mock guild and member with Staff role to prove Staff alone is insufficient + Guild mockGuild = mock(Guild.class); + net.dv8tion.jda.api.entities.Member mockMember = mock(net.dv8tion.jda.api.entities.Member.class); + Role staffRole = mock(Role.class); + when(staffRole.getName()).thenReturn("Staff"); + when(jda.getGuildById("g1")).thenReturn(mockGuild); + when(mockGuild.getMemberById("user1")).thenReturn(mockMember); + when(mockMember.getRoles()).thenReturn(List.of(staffRole)); + // Even with Staff role, should return false assertFalse(service.isUserActualAdminInGuild(auth, "g1")); } + @Test + @DisplayName("isUserActualAdminInGuild: returns false when permissions are not numeric") + void isUserActualAdminInGuild_permissionsParseError() { + Authentication auth = mockAuth("user1"); + var authorizedClient = TestTokens.authorizedClient("tok"); + when(clients.loadAuthorizedClient(eq("discord"), eq("user1"))).thenReturn(authorizedClient); + + Map guild = new HashMap<>(); + guild.put("id", "g1"); + guild.put("name", "G"); + guild.put("permissions", "not-a-number"); + when(cache.get(eq("tok"), any())).thenReturn(List.of(guild)); + + assertFalse(service.isUserActualAdminInGuild(auth, "g1")); + } + @Test @DisplayName("getGatchaRoles: returns empty when guild missing") void getGatchaRoles_guildMissing() { From d9770f626d34145e531e53042cef32bebd3e967e Mon Sep 17 00:00:00 2001 From: wraithfive Date: Sun, 18 Jan 2026 11:50:09 -0600 Subject: [PATCH 4/5] Extract common permission checking logic to reduce duplication --- .../discordbot/web/service/AdminService.java | 30 ++++++++----------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/src/main/java/com/discordbot/web/service/AdminService.java b/src/main/java/com/discordbot/web/service/AdminService.java index 8a22ca1..b88da72 100644 --- a/src/main/java/com/discordbot/web/service/AdminService.java +++ b/src/main/java/com/discordbot/web/service/AdminService.java @@ -182,25 +182,13 @@ public boolean isUserAdminInGuild(Authentication authentication, String guildId) return false; } - List> userGuilds = getUserGuildsFromDiscord(accessToken); - - for (Map userGuild : userGuilds) { - if (guildId.equals(userGuild.get("id"))) { - Long permissions = Long.parseLong(userGuild.get("permissions").toString()); - boolean isAdmin = (permissions & Permission.ADMINISTRATOR.getRawValue()) != 0 || - (permissions & Permission.MANAGE_SERVER.getRawValue()) != 0; - - if (isAdmin) { - return true; - } - - // Check if user has Staff role - return hasStaffRole(userId, guildId); - } + // Check if user has actual admin permissions + if (checkAdminPermissions(guildId, accessToken)) { + return true; } - logger.warn("User {} attempted to access guild {} without permissions", userId, guildId); - return false; + // Check if user has Staff role as fallback + return hasStaffRole(userId, guildId); } /** @@ -236,6 +224,14 @@ public boolean isUserActualAdminInGuild(Authentication authentication, String gu return false; } + return checkAdminPermissions(guildId, accessToken); + } + + /** + * Check if user has actual ADMINISTRATOR or MANAGE_SERVER permissions in a guild + * Extracted as a helper to avoid duplication between permission checking methods + */ + private boolean checkAdminPermissions(String guildId, String accessToken) { List> userGuilds = getUserGuildsFromDiscord(accessToken); for (Map userGuild : userGuilds) { From 9602ac09eb74bdda8b8cef728ec9ca7084fa2b33 Mon Sep 17 00:00:00 2001 From: wraithfive Date: Sun, 18 Jan 2026 12:36:39 -0600 Subject: [PATCH 5/5] Clarify isUserAdminInGuild logic: use OR instead of if-then-return fallback --- .../java/com/discordbot/web/service/AdminService.java | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/discordbot/web/service/AdminService.java b/src/main/java/com/discordbot/web/service/AdminService.java index b88da72..f85b064 100644 --- a/src/main/java/com/discordbot/web/service/AdminService.java +++ b/src/main/java/com/discordbot/web/service/AdminService.java @@ -167,7 +167,7 @@ public List getManageableGuilds(Authentication authentication) { /** * Check if user has admin permissions in a specific guild (regardless of bot presence) - * Also allows users with "Staff" role + * Returns true if user has EITHER actual admin permissions OR Staff role */ public boolean isUserAdminInGuild(Authentication authentication, String guildId) { if (authentication == null || !(authentication.getPrincipal() instanceof OAuth2User)) { @@ -182,13 +182,8 @@ public boolean isUserAdminInGuild(Authentication authentication, String guildId) return false; } - // Check if user has actual admin permissions - if (checkAdminPermissions(guildId, accessToken)) { - return true; - } - - // Check if user has Staff role as fallback - return hasStaffRole(userId, guildId); + // User is admin if they have actual permissions OR Staff role + return checkAdminPermissions(guildId, accessToken) || hasStaffRole(userId, guildId); } /**