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 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..f85b064 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) + * 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)) { @@ -181,18 +182,72 @@ public boolean isUserAdminInGuild(Authentication authentication, String guildId) return false; } + // User is admin if they have actual permissions OR Staff role + return checkAdminPermissions(guildId, accessToken) || hasStaffRole(userId, guildId); + } + + /** + * 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; + } + + 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) { 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; - return isAdmin; + 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; } } - logger.warn("User {} attempted to access guild {} without permissions", userId, guildId); return false; } diff --git a/src/test/java/com/discordbot/AdminServiceBranchesTest.java b/src/test/java/com/discordbot/AdminServiceBranchesTest.java index 8bb7755..14ef26e 100644 --- a/src/test/java/com/discordbot/AdminServiceBranchesTest.java +++ b/src/test/java/com/discordbot/AdminServiceBranchesTest.java @@ -252,6 +252,119 @@ 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)); + + // 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() { 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());