diff --git a/src/main/java/gg/modl/backend/audit/controller/AuditController.java b/src/main/java/gg/modl/backend/audit/controller/AuditController.java index 4e00287..b530a9b 100644 --- a/src/main/java/gg/modl/backend/audit/controller/AuditController.java +++ b/src/main/java/gg/modl/backend/audit/controller/AuditController.java @@ -3,7 +3,7 @@ import gg.modl.backend.audit.service.AdminDatabaseBrowserService; import gg.modl.backend.audit.service.AuditService; import gg.modl.backend.audit.service.StaffPerformanceService; -import gg.modl.backend.infrastructure.exception.ForbiddenException; +import gg.modl.backend.infrastructure.authorization.PanelAccessRule; import gg.modl.backend.infrastructure.authorization.RequiresPanelPermission; import gg.modl.backend.infrastructure.exception.ValidationException; import gg.modl.backend.infrastructure.rest.RESTMappingV1; @@ -52,7 +52,6 @@ public class AuditController { private final AuditService auditService; private final AdminDatabaseBrowserService adminDatabaseBrowserService; private final StaffPerformanceService staffPerformanceService; - private final PermissionService permissionService; private final RealtimeEventPublisher realtimeEventPublisher; @GetMapping("/staff-performance") @@ -98,6 +97,7 @@ public ResponseEntity getPunishments( } @GetMapping("/database/{table}") + @RequiresPanelPermission(rule = PanelAccessRule.SUPER_ADMIN) public ResponseEntity getDatabaseTable( @PathVariable String table, @RequestParam(defaultValue = "100") @Min(RequestValidationLimits.PAGINATION_LIMIT_MIN) @Max(RequestValidationLimits.PAGINATION_LIMIT_MAX) int limit, @@ -105,7 +105,6 @@ public ResponseEntity getDatabaseTable( HttpServletRequest request ) { Server server = RequestUtil.getRequestServer(request); - requireSuperAdmin(server, request); if (!AdminDatabaseBrowserService.ALLOWED_TABLES.contains(table)) { throw new ValidationException("Invalid table name"); @@ -116,14 +115,13 @@ public ResponseEntity getDatabaseTable( } @PostMapping("/punishments/{id}/rollback") - @RequiresPanelPermission(PermissionService.ADMIN_AUDIT_ROLLBACK) + @RequiresPanelPermission(rule = PanelAccessRule.SUPER_ADMIN, supersedesPermissions = PermissionService.ADMIN_AUDIT_ROLLBACK) public ResponseEntity rollbackPunishment( @PathVariable String id, @RequestBody(required = false) RollbackRequest rollbackRequest, HttpServletRequest request ) { Server server = RequestUtil.getRequestServer(request); - requireSuperAdmin(server, request); String performerUsername = RequestUtil.getCurrentUsername(request); String reason = rollbackRequest != null && rollbackRequest.hasReason() @@ -139,14 +137,13 @@ public ResponseEntity rollbackPunishment( } @PostMapping("/staff/{username}/rollback-all") - @RequiresPanelPermission(PermissionService.ADMIN_AUDIT_ROLLBACK) + @RequiresPanelPermission(rule = PanelAccessRule.SUPER_ADMIN, supersedesPermissions = PermissionService.ADMIN_AUDIT_ROLLBACK) public ResponseEntity rollbackAllByStaff( @PathVariable String username, @RequestBody(required = false) RollbackRequest rollbackRequest, HttpServletRequest request ) { Server server = RequestUtil.getRequestServer(request); - requireSuperAdmin(server, request); String performerUsername = RequestUtil.getCurrentUsername(request); String reason = rollbackRequest != null && rollbackRequest.hasReason() @@ -159,14 +156,13 @@ public ResponseEntity rollbackAllByStaff( } @PostMapping("/staff/{username}/rollback-date-range") - @RequiresPanelPermission(PermissionService.ADMIN_AUDIT_ROLLBACK) + @RequiresPanelPermission(rule = PanelAccessRule.SUPER_ADMIN, supersedesPermissions = PermissionService.ADMIN_AUDIT_ROLLBACK) public ResponseEntity rollbackByDateRange( @PathVariable String username, @RequestBody DateRangeRollbackRequest rollbackRequest, HttpServletRequest request ) { Server server = RequestUtil.getRequestServer(request); - requireSuperAdmin(server, request); String performerUsername = RequestUtil.getCurrentUsername(request); Date startDate = AuditProtoMapper.toDate(rollbackRequest.getStartDate()); @@ -191,14 +187,12 @@ public ResponseEntity rollbackByDateRange( } @PostMapping("/punishments/bulk-pardon") - @RequiresPanelPermission(PermissionService.ADMIN_AUDIT_ROLLBACK) + @RequiresPanelPermission(rule = PanelAccessRule.SUPER_ADMIN, supersedesPermissions = PermissionService.ADMIN_AUDIT_ROLLBACK) public ResponseEntity bulkPardon( @RequestBody BulkPunishmentActionRequest actionRequest, HttpServletRequest request ) { Server server = RequestUtil.getRequestServer(request); - requireSuperAdmin(server, request); - String performerUsername = RequestUtil.getCurrentUsername(request); int count = auditService.bulkPardonByType( server, actionRequest.getTypeOrdinalsList(), actionRequest.getReason(), performerUsername); @@ -209,13 +203,12 @@ public ResponseEntity bulkPardon( } @PostMapping("/punishments/bulk-set-expiration") - @RequiresPanelPermission(PermissionService.ADMIN_AUDIT_ROLLBACK) + @RequiresPanelPermission(rule = PanelAccessRule.SUPER_ADMIN, supersedesPermissions = PermissionService.ADMIN_AUDIT_ROLLBACK) public ResponseEntity bulkSetExpiration( @RequestBody BulkPunishmentActionRequest actionRequest, HttpServletRequest request ) { Server server = RequestUtil.getRequestServer(request); - requireSuperAdmin(server, request); if (!actionRequest.hasNewDurationMs()) { throw new ValidationException("newDurationMs is required for set-expiration"); @@ -231,13 +224,6 @@ public ResponseEntity bulkSetExpiration( true, count, "Successfully updated expiration for " + count + " punishments")); } - private void requireSuperAdmin(Server server, HttpServletRequest request) { - String email = RequestUtil.getSessionEmail(request); - if (!permissionService.isSuperAdmin(server, email)) { - throw new ForbiddenException("Only super admins can perform this action"); - } - } - private void invalidateAudit(Server server) { realtimeEventPublisher.invalidatePanel(server, PanelResource.PANEL_RESOURCE_AUDIT); } diff --git a/src/main/java/gg/modl/backend/auth/controller/PanelAuthController.java b/src/main/java/gg/modl/backend/auth/controller/PanelAuthController.java index e8c896c..76f7fe9 100644 --- a/src/main/java/gg/modl/backend/auth/controller/PanelAuthController.java +++ b/src/main/java/gg/modl/backend/auth/controller/PanelAuthController.java @@ -8,7 +8,6 @@ import gg.modl.backend.auth.session.SessionService; import gg.modl.backend.infrastructure.rest.RESTMappingV1; import gg.modl.backend.infrastructure.rest.RequestUtil; -import gg.modl.backend.role.data.StaffRole; import gg.modl.backend.role.service.PermissionService; import gg.modl.backend.role.service.RoleAuthorization; import gg.modl.backend.server.data.Server; @@ -57,6 +56,7 @@ public class PanelAuthController { private final StaffProfileService staffProfileService; private final StaffLookupCache staffLookupCache; private final PermissionService permissionService; + private final RoleAuthorization roleAuthorization; private final CookieUtil cookieUtil; private final EmailChangeService emailChangeService; @@ -158,7 +158,7 @@ public ResponseEntity updateProfile( return ResponseEntity.status(404).body(PanelAuthProtoMapper.toAuthResponse(false, "Staff member not found")); } Staff staff = result.get(); - String role = isSuperAdmin ? RoleAuthorization.SUPER_ADMIN_ROLE_NAME : permissionService.resolveRoleName(server, staff.getRoleId()); + String role = isSuperAdmin ? RoleAuthorization.SUPER_ADMIN_ROLE_NAME : permissionService.effectiveRoleName(server, staff); String minecraftUsername = minecraftUsernameOrPanel(staff); return ResponseEntity.ok(PanelAuthProtoMapper.toProfileResponse( staff.getId(), staff.getEmail(), staff.getUsername(), role, minecraftUsername, staff.getLanguage(), staff.getDateFormat())); @@ -214,7 +214,7 @@ public ResponseEntity getCurrentUser(HttpServletRequest request) { if (staffOpt.isPresent()) { Staff staff = staffOpt.get(); - String role = isSuperAdmin ? RoleAuthorization.SUPER_ADMIN_ROLE_NAME : permissionService.resolveRoleName(server, staff.getRoleId()); + String role = isSuperAdmin ? RoleAuthorization.SUPER_ADMIN_ROLE_NAME : permissionService.effectiveRoleName(server, staff); String minecraftUsername = minecraftUsernameOrPanel(staff); return ResponseEntity.ok(PanelAuthProtoMapper.toProfileResponse( staff.getId(), staff.getEmail(), staff.getUsername(), role, minecraftUsername, staff.getLanguage(), staff.getDateFormat())); @@ -321,20 +321,8 @@ public ResponseEntity getUserPermissions(HttpServletRe } Server server = RequestUtil.getRequestServer(request); + List permissions = roleAuthorization.effectivePermissionIds(server, roleAuthorization.panelPerformer(server, email)); - if (permissionService.isSuperAdmin(server, email)) { - return ResponseEntity.ok(PanelAuthProtoMapper.toPermissionsResponse(permissionService.getAllPermissionIds(server))); - } - - Optional staffOpt = staffLookupCache.findByEmail(server, email); - if (staffOpt.isEmpty()) { - return ResponseEntity.ok(PanelAuthProtoMapper.toPermissionsResponse(List.of())); - } - - String roleId = RoleAuthorization.effectiveRoleId(server, staffOpt.get()); - Optional roleOpt = permissionService.getRoleById(server, roleId); - - return roleOpt.map(staffRole -> ResponseEntity.ok(PanelAuthProtoMapper.toPermissionsResponse(staffRole.getPermissions()))) - .orElseGet(() -> ResponseEntity.ok(PanelAuthProtoMapper.toPermissionsResponse(List.of()))); + return ResponseEntity.ok(PanelAuthProtoMapper.toPermissionsResponse(permissions)); } } diff --git a/src/main/java/gg/modl/backend/billing/controller/PanelBillingController.java b/src/main/java/gg/modl/backend/billing/controller/PanelBillingController.java index 4df6f82..c9694b2 100644 --- a/src/main/java/gg/modl/backend/billing/controller/PanelBillingController.java +++ b/src/main/java/gg/modl/backend/billing/controller/PanelBillingController.java @@ -2,9 +2,11 @@ import gg.modl.backend.billing.service.BillingService; import gg.modl.backend.billing.service.UsageTrackingService; +import gg.modl.backend.infrastructure.authorization.PanelAccessRule; import gg.modl.backend.infrastructure.authorization.RequiresPanelPermission; import gg.modl.backend.infrastructure.rest.RESTMappingV1; import gg.modl.backend.infrastructure.rest.RequestUtil; +import gg.modl.backend.role.service.PermissionService; import gg.modl.backend.server.data.Server; import gg.modl.proto.modl.v1.BillingStatusResponse; import gg.modl.proto.modl.v1.CancelResponse; @@ -29,7 +31,8 @@ @RestController @RequestMapping(RESTMappingV1.PANEL_BILLING) -@RequiresPanelPermission(view = "admin.settings.view.billing", modify = "admin.settings.modify.billing") +@RequiresPanelPermission(rule = PanelAccessRule.SUPER_ADMIN, + supersedesPermissions = {PermissionService.ADMIN_SETTINGS_VIEW_BILLING, PermissionService.ADMIN_SETTINGS_MODIFY_BILLING}) @RequiredArgsConstructor public class PanelBillingController { private final BillingService billingService; @@ -39,8 +42,6 @@ public class PanelBillingController { public ResponseEntity createCheckoutSession(HttpServletRequest request) { billingService.requireStripeConfigured(); Server server = RequestUtil.getRequestServer(request); - billingService.requireSuperAdmin(server, RequestUtil.getSessionEmail(request)); - return ResponseEntity.ok(PanelBillingProtoMapper.toCheckoutSessionResponse(billingService.createCheckoutSession(server))); } @@ -48,8 +49,6 @@ public ResponseEntity createCheckoutSession(HttpServlet public ResponseEntity createPortalSession(HttpServletRequest request) { billingService.requireStripeConfigured(); Server server = RequestUtil.getRequestServer(request); - billingService.requireSuperAdmin(server, RequestUtil.getSessionEmail(request)); - return ResponseEntity.ok(PanelBillingProtoMapper.toPortalSessionResponse(billingService.createPortalSession(server))); } @@ -57,8 +56,6 @@ public ResponseEntity createPortalSession(HttpServletRequ public ResponseEntity cancelSubscription(HttpServletRequest request) { billingService.requireStripeConfigured(); Server server = RequestUtil.getRequestServer(request); - billingService.requireSuperAdmin(server, RequestUtil.getSessionEmail(request)); - return ResponseEntity.ok(PanelBillingProtoMapper.toCancelResponse(billingService.cancelSubscription(server))); } @@ -66,15 +63,12 @@ public ResponseEntity cancelSubscription(HttpServletRequest requ public ResponseEntity resubscribe(HttpServletRequest request) { billingService.requireStripeConfigured(); Server server = RequestUtil.getRequestServer(request); - billingService.requireSuperAdmin(server, RequestUtil.getSessionEmail(request)); - return ResponseEntity.ok(PanelBillingProtoMapper.toResubscribeResponse(billingService.resubscribe(server))); } @GetMapping("/status") public ResponseEntity getBillingStatus(HttpServletRequest request) { Server server = RequestUtil.getRequestServer(request); - billingService.requireSuperAdmin(server, RequestUtil.getSessionEmail(request)); billingService.reconcileBillingStatus(server); return ResponseEntity.ok(PanelBillingProtoMapper.toBillingStatusResponse(billingService.getBillingStatus(server))); } @@ -82,7 +76,6 @@ public ResponseEntity getBillingStatus(HttpServletRequest @GetMapping("/usage") public ResponseEntity getUsage(HttpServletRequest request) { Server server = RequestUtil.getRequestServer(request); - billingService.requireSuperAdmin(server, RequestUtil.getSessionEmail(request)); return ResponseEntity.ok(PanelBillingProtoMapper.toUsageResponse(usageTrackingService.getUsage(server))); } @@ -93,8 +86,6 @@ public ResponseEntity updateUsageBillingSettings( ) { billingService.requireStripeConfigured(); Server server = RequestUtil.getRequestServer(request); - billingService.requireSuperAdmin(server, RequestUtil.getSessionEmail(request)); - return ResponseEntity.ok(PanelBillingProtoMapper.toUsageBillingSettingsResponse( usageTrackingService.updateUsageBillingSettings(server, settingsRequest.getEnabled()))); } @@ -105,7 +96,6 @@ public ResponseEntity updateStorageLimit( HttpServletRequest request ) { Server server = RequestUtil.getRequestServer(request); - billingService.requireSuperAdmin(server, RequestUtil.getSessionEmail(request)); long maxStorageLimitBytes = body.getMaxStorageLimitBytes(); usageTrackingService.updateStorageLimit(server, maxStorageLimitBytes); @@ -119,7 +109,6 @@ public ResponseEntity updateOverageLimits( HttpServletRequest request ) { Server server = RequestUtil.getRequestServer(request); - billingService.requireSuperAdmin(server, RequestUtil.getSessionEmail(request)); int maxStorageOverageGB = body.hasMaxStorageOverageGbValue() ? body.getMaxStorageOverageGbValue() : body.getMaxStorageOverageGb(); int maxAiOverageRequests = body.hasMaxAiOverageRequestsValue() ? body.getMaxAiOverageRequestsValue() : body.getMaxAiOverageRequests(); diff --git a/src/main/java/gg/modl/backend/billing/service/BillingService.java b/src/main/java/gg/modl/backend/billing/service/BillingService.java index 8a88833..b915b94 100644 --- a/src/main/java/gg/modl/backend/billing/service/BillingService.java +++ b/src/main/java/gg/modl/backend/billing/service/BillingService.java @@ -5,14 +5,12 @@ import com.stripe.model.checkout.Session; import gg.modl.backend.infrastructure.exception.ConflictException; import gg.modl.backend.infrastructure.exception.ExternalServiceException; -import gg.modl.backend.infrastructure.exception.ForbiddenException; import gg.modl.backend.infrastructure.exception.ResourceNotFoundException; import gg.modl.backend.billing.dto.response.BillingStatusResponse; import gg.modl.backend.billing.dto.response.CancelResponse; import gg.modl.backend.billing.dto.response.CheckoutSessionResponse; import gg.modl.backend.billing.dto.response.PortalSessionResponse; import gg.modl.backend.billing.dto.response.ResubscribeResponse; -import gg.modl.backend.role.service.PermissionService; import gg.modl.backend.server.data.Server; import gg.modl.backend.server.data.ServerPlan; import gg.modl.backend.server.data.SubscriptionStatus; @@ -28,7 +26,6 @@ public class BillingService { private final StripeService stripeService; private final ServerMutationHelper serverMutationHelper; - private final PermissionService permissionService; public void requireStripeConfigured() { if (!stripeService.isConfigured()) { @@ -36,12 +33,6 @@ public void requireStripeConfigured() { } } - public void requireSuperAdmin(Server server, String email) { - if (email == null || !permissionService.isSuperAdmin(server, email)) { - throw new ForbiddenException("Only the super admin can manage billing"); - } - } - public void syncCustomerEmail(Server server, String newEmail) { String customerId = server.getStripeCustomerId(); if (!stripeService.isConfigured() || customerId == null || customerId.isBlank()) { diff --git a/src/main/java/gg/modl/backend/infrastructure/authorization/PanelAccessPolicyResolver.java b/src/main/java/gg/modl/backend/infrastructure/authorization/PanelAccessPolicyResolver.java index f7533e4..3ebd036 100644 --- a/src/main/java/gg/modl/backend/infrastructure/authorization/PanelAccessPolicyResolver.java +++ b/src/main/java/gg/modl/backend/infrastructure/authorization/PanelAccessPolicyResolver.java @@ -45,7 +45,17 @@ public List resolvePolicies(HttpServletRequest request) { } public Optional resolve(HandlerMethod handlerMethod) { - return Optional.ofNullable(findAnnotation(handlerMethod)).map(PanelAccessPolicyResolver::toPolicy); + return resolveAnnotation(handlerMethod).map(PanelAccessPolicyResolver::toPolicy); + } + + public Optional resolveAnnotation(HandlerMethod handlerMethod) { + RequiresPanelPermission methodAnnotation = handlerMethod.getMethodAnnotation(RequiresPanelPermission.class); + return methodAnnotation != null ? Optional.of(methodAnnotation) : resolveTypeAnnotation(handlerMethod); + } + + public Optional resolveTypeAnnotation(HandlerMethod handlerMethod) { + return Optional.ofNullable( + AnnotatedElementUtils.findMergedAnnotation(handlerMethod.getBeanType(), RequiresPanelPermission.class)); } private List siblingPolicies(RequestMappingHandlerMapping handlerMapping, HttpServletRequest request) { @@ -71,20 +81,13 @@ private boolean ensureParsedRequestPath(RequestMappingHandlerMapping handlerMapp return true; } - private static RequiresPanelPermission findAnnotation(HandlerMethod handlerMethod) { - RequiresPanelPermission methodAnnotation = handlerMethod.getMethodAnnotation(RequiresPanelPermission.class); - if (methodAnnotation != null) { - return methodAnnotation; - } - return AnnotatedElementUtils.findMergedAnnotation(handlerMethod.getBeanType(), RequiresPanelPermission.class); - } - private static PanelAccessPolicy toPolicy(RequiresPanelPermission annotation) { return switch (annotation.rule()) { case PERMIT_ALL -> PermitAllPolicy.INSTANCE; case PLAYER_ACCESS -> PlayerAccessPolicy.INSTANCE; case PUNISHMENT_TYPE_ACCESS -> PunishmentTypeAccessPolicy.INSTANCE; case APPEAL_REPLY -> AppealReplyPolicy.INSTANCE; + case SUPER_ADMIN -> SuperAdminOnlyPolicy.INSTANCE; case REQUIRE_PERMISSION -> new ReadWritePermissionPolicy(viewPermission(annotation), modifyPermission(annotation)); }; } diff --git a/src/main/java/gg/modl/backend/infrastructure/authorization/PanelAccessRule.java b/src/main/java/gg/modl/backend/infrastructure/authorization/PanelAccessRule.java index e4ed7d1..30ca487 100644 --- a/src/main/java/gg/modl/backend/infrastructure/authorization/PanelAccessRule.java +++ b/src/main/java/gg/modl/backend/infrastructure/authorization/PanelAccessRule.java @@ -5,5 +5,6 @@ public enum PanelAccessRule { PERMIT_ALL, PLAYER_ACCESS, PUNISHMENT_TYPE_ACCESS, - APPEAL_REPLY + APPEAL_REPLY, + SUPER_ADMIN } diff --git a/src/main/java/gg/modl/backend/infrastructure/authorization/PanelAuthorizationBootstrapValidator.java b/src/main/java/gg/modl/backend/infrastructure/authorization/PanelAuthorizationBootstrapValidator.java index 499cd56..117220f 100644 --- a/src/main/java/gg/modl/backend/infrastructure/authorization/PanelAuthorizationBootstrapValidator.java +++ b/src/main/java/gg/modl/backend/infrastructure/authorization/PanelAuthorizationBootstrapValidator.java @@ -1,9 +1,12 @@ package gg.modl.backend.infrastructure.authorization; import gg.modl.backend.infrastructure.rest.RouteGroups; +import gg.modl.backend.role.service.PermissionService; import java.util.ArrayList; +import java.util.LinkedHashSet; import java.util.List; import java.util.Set; +import java.util.stream.Stream; import lombok.RequiredArgsConstructor; import org.springframework.beans.factory.ObjectProvider; import org.springframework.beans.factory.SmartInitializingSingleton; @@ -19,20 +22,113 @@ public class PanelAuthorizationBootstrapValidator implements SmartInitializingSi private final ObjectProvider handlerMappingProvider; private final PanelAccessPolicyResolver policyResolver; + private static final class ViolationGroup { + private final String description; + private final List offenders = new ArrayList<>(); + + private ViolationGroup(String description) { + this.description = description; + } + + private void record(String offender) { + offenders.add(offender); + } + + private boolean isViolated() { + return !offenders.isEmpty(); + } + + private String render() { + return description + ": " + offenders; + } + } + @Override public void afterSingletonsInstantiated() { - List unguarded = new ArrayList<>(); - handlerMappingProvider.getObject().getHandlerMethods().forEach((mappingInfo, handlerMethod) -> { - if (requiresPanelAuthorization(mappingInfo) && policyResolver.resolve(handlerMethod).isEmpty()) { - unguarded.add(describe(mappingInfo, handlerMethod)); + RequestMappingHandlerMapping handlerMapping = handlerMappingProvider.getObject(); + ViolationGroup unguarded = new ViolationGroup( + "Panel endpoints without a @RequiresPanelPermission policy (fail-closed default would deny them)"); + ViolationGroup weakenedSuperAdminControllers = new ViolationGroup( + "Panel endpoints overriding a rule = SUPER_ADMIN controller with a weaker method-level policy"); + ViolationGroup ignoredEnforcedPermissions = new ViolationGroup( + "Panel endpoints combining rule = SUPER_ADMIN with an enforced permission that rule = SUPER_ADMIN ignores " + + "(declare catalog linkage with supersedesPermissions instead)"); + ViolationGroup danglingSupersededPermissions = new ViolationGroup( + "Panel endpoints declaring supersedesPermissions without rule = SUPER_ADMIN"); + ViolationGroup unflaggedSuperseded = new ViolationGroup( + "Super-admin-ruled panel endpoints superseding permissions that are not flagged superAdminOnly in the catalog"); + ViolationGroup leakedSuperAdminPermissions = new ViolationGroup( + "Panel endpoints enforcing a superAdminOnly permission without rule = SUPER_ADMIN"); + ViolationGroup unruledSuperAdminPermissions = new ViolationGroup( + "Catalog permissions flagged superAdminOnly but superseded by no super-admin-ruled panel endpoint"); + Set supersededPermissions = new LinkedHashSet<>(); + + handlerMapping.getHandlerMethods().forEach((mappingInfo, handlerMethod) -> { + if (!requiresPanelAuthorization(mappingInfo)) { + return; + } + RequiresPanelPermission annotation = policyResolver.resolveAnnotation(handlerMethod).orElse(null); + String route = describe(mappingInfo, handlerMethod); + if (annotation == null) { + unguarded.record(route); + return; } + if (annotation.rule() != PanelAccessRule.SUPER_ADMIN) { + if (declaresSuperAdminControllerRule(handlerMethod)) { + weakenedSuperAdminControllers.record(route); + } + enforcedPermissions(annotation) + .filter(PermissionService::isSuperAdminOnly) + .forEach(permission -> leakedSuperAdminPermissions.record(route + " enforces '" + permission + "'")); + supersedes(annotation) + .forEach(permission -> danglingSupersededPermissions.record(route + " supersedes '" + permission + "'")); + return; + } + enforcedPermissions(annotation) + .forEach(permission -> ignoredEnforcedPermissions.record(route + " enforces '" + permission + "'")); + supersedes(annotation).forEach(permission -> { + supersededPermissions.add(permission); + if (!PermissionService.isSuperAdminOnly(permission)) { + unflaggedSuperseded.record(route + " supersedes '" + permission + "'"); + } + }); }); - if (!unguarded.isEmpty()) { - throw new IllegalStateException( - "Panel endpoints without a @RequiresPanelPermission policy (fail-closed default would deny them): " + unguarded); + + PermissionService.superAdminOnlyPermissionIds().stream() + .filter(permission -> !supersededPermissions.contains(permission)) + .sorted() + .forEach(unruledSuperAdminPermissions::record); + + failIfViolated(unguarded, weakenedSuperAdminControllers, ignoredEnforcedPermissions, danglingSupersededPermissions, + unflaggedSuperseded, leakedSuperAdminPermissions, unruledSuperAdminPermissions); + } + + private boolean declaresSuperAdminControllerRule(HandlerMethod handlerMethod) { + return policyResolver.resolveTypeAnnotation(handlerMethod) + .filter(typeAnnotation -> typeAnnotation.rule() == PanelAccessRule.SUPER_ADMIN) + .isPresent(); + } + + private static void failIfViolated(ViolationGroup... groups) { + List violations = Stream.of(groups) + .filter(ViolationGroup::isViolated) + .map(ViolationGroup::render) + .toList(); + if (!violations.isEmpty()) { + throw new IllegalStateException(String.join("; ", violations)); } } + private static Stream enforcedPermissions(RequiresPanelPermission annotation) { + return Stream.of(annotation.value(), annotation.view(), annotation.modify()) + .filter(permission -> !permission.isBlank()); + } + + private static Stream supersedes(RequiresPanelPermission annotation) { + return Stream.of(annotation.supersedesPermissions()) + .filter(permission -> !permission.isBlank()); + } + private boolean requiresPanelAuthorization(RequestMappingInfo mappingInfo) { return patternsOf(mappingInfo).stream().anyMatch(this::isGuardedPanelPattern); } diff --git a/src/main/java/gg/modl/backend/infrastructure/authorization/PanelPrincipalPermissions.java b/src/main/java/gg/modl/backend/infrastructure/authorization/PanelPrincipalPermissions.java index 75e7bba..988da43 100644 --- a/src/main/java/gg/modl/backend/infrastructure/authorization/PanelPrincipalPermissions.java +++ b/src/main/java/gg/modl/backend/infrastructure/authorization/PanelPrincipalPermissions.java @@ -4,7 +4,7 @@ import gg.modl.backend.server.data.Server; import org.jetbrains.annotations.Nullable; -public record PanelPrincipalPermissions(Server server, @Nullable String roleId, PermissionService permissionService) { +public record PanelPrincipalPermissions(Server server, @Nullable String roleId, boolean superAdmin, PermissionService permissionService) { public boolean has(String permission) { return permissionService.hasPermission(server, roleId, permission); } diff --git a/src/main/java/gg/modl/backend/infrastructure/authorization/RequiresPanelPermission.java b/src/main/java/gg/modl/backend/infrastructure/authorization/RequiresPanelPermission.java index 377394a..9879e05 100644 --- a/src/main/java/gg/modl/backend/infrastructure/authorization/RequiresPanelPermission.java +++ b/src/main/java/gg/modl/backend/infrastructure/authorization/RequiresPanelPermission.java @@ -15,4 +15,6 @@ String modify() default ""; PanelAccessRule rule() default PanelAccessRule.REQUIRE_PERMISSION; + + String[] supersedesPermissions() default {}; } diff --git a/src/main/java/gg/modl/backend/infrastructure/authorization/SuperAdminOnlyPolicy.java b/src/main/java/gg/modl/backend/infrastructure/authorization/SuperAdminOnlyPolicy.java new file mode 100644 index 0000000..94545d7 --- /dev/null +++ b/src/main/java/gg/modl/backend/infrastructure/authorization/SuperAdminOnlyPolicy.java @@ -0,0 +1,15 @@ +package gg.modl.backend.infrastructure.authorization; + +public enum SuperAdminOnlyPolicy implements PanelAccessPolicy { + INSTANCE; + + @Override + public boolean permitsWithoutRole(PanelAccessRequest request) { + return false; + } + + @Override + public boolean permitsWithRole(PanelAccessRequest request, PanelPrincipalPermissions permissions) { + return permissions.superAdmin(); + } +} diff --git a/src/main/java/gg/modl/backend/infrastructure/filter/PanelPermissionFilter.java b/src/main/java/gg/modl/backend/infrastructure/filter/PanelPermissionFilter.java index 0bd8b65..67d58a9 100644 --- a/src/main/java/gg/modl/backend/infrastructure/filter/PanelPermissionFilter.java +++ b/src/main/java/gg/modl/backend/infrastructure/filter/PanelPermissionFilter.java @@ -10,15 +10,12 @@ import gg.modl.backend.role.service.PermissionService; import gg.modl.backend.role.service.RoleAuthorization; import gg.modl.backend.server.data.Server; -import gg.modl.backend.staff.data.Staff; -import gg.modl.backend.staff.service.StaffLookupCache; import jakarta.servlet.FilterChain; import jakarta.servlet.ServletException; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; import java.io.IOException; import java.util.List; -import java.util.Optional; import lombok.RequiredArgsConstructor; import org.jetbrains.annotations.NotNull; import org.springframework.stereotype.Component; @@ -28,7 +25,7 @@ @RequiredArgsConstructor public class PanelPermissionFilter extends OncePerRequestFilter { private final PermissionService permissionService; - private final StaffLookupCache staffLookupCache; + private final RoleAuthorization roleAuthorization; private final PanelAccessPolicyResolver policyResolver; @Override @@ -53,7 +50,8 @@ protected void doFilterInternal( return; } - if (permissionService.isSuperAdmin(server, email)) { + boolean superAdmin = permissionService.isSuperAdmin(server, email); + if (superAdmin) { filterChain.doFilter(request, response); return; } @@ -70,9 +68,8 @@ protected void doFilterInternal( return; } - Optional staffOpt = staffLookupCache.findByEmail(server, email); - String roleId = staffOpt.map(staff -> RoleAuthorization.effectiveRoleId(server, staff)).orElse(null); - PanelPrincipalPermissions permissions = new PanelPrincipalPermissions(server, roleId, permissionService); + String roleId = roleAuthorization.panelRoleId(server, email); + PanelPrincipalPermissions permissions = new PanelPrincipalPermissions(server, roleId, superAdmin, permissionService); if (policies.stream().anyMatch(policy -> policy.permitsWithRole(accessRequest, permissions))) { filterChain.doFilter(request, response); diff --git a/src/main/java/gg/modl/backend/player/service/PunishmentLifecycleService.java b/src/main/java/gg/modl/backend/player/service/PunishmentLifecycleService.java index 78298bf..d110d76 100644 --- a/src/main/java/gg/modl/backend/player/service/PunishmentLifecycleService.java +++ b/src/main/java/gg/modl/backend/player/service/PunishmentLifecycleService.java @@ -2,7 +2,6 @@ import gg.modl.backend.database.mongo.repository.PlayerMongoRepository; import gg.modl.backend.database.mongo.repository.PunishmentMongoRepository; -import gg.modl.backend.database.mongo.repository.StaffMongoRepository; import gg.modl.backend.ticket.service.TicketService; import gg.modl.backend.infrastructure.exception.ResourceNotFoundException; import gg.modl.backend.player.dto.request.MinecraftCreatePunishmentRequest; @@ -45,7 +44,7 @@ import gg.modl.backend.infrastructure.validation.SafeUrls; import gg.modl.backend.log.service.LogService; import gg.modl.backend.role.service.PermissionService; -import gg.modl.backend.staff.data.Staff; +import gg.modl.backend.role.service.RoleAuthorization; import gg.modl.backend.infrastructure.util.IdGenerator; import gg.modl.backend.settings.service.WebhookSettingsService; import org.springframework.stereotype.Service; @@ -62,9 +61,9 @@ public class PunishmentLifecycleService { private final OffenderThresholdSettingsService thresholdSettingsService; private final PunishmentDurationCalculator durationCalculator; private final IssuerNameResolver issuerNameResolver; - private final StaffMongoRepository staffRepository; private final PunishmentQueryService punishmentQueryService; private final PermissionService permissionService; + private final RoleAuthorization roleAuthorization; private final WebhookSettingsService webhookSettingsService; private final PunishmentRealtimePublisher realtimePublisher; private final LogService logService; @@ -115,10 +114,11 @@ private void requireReasonWithinLimit(Object reason) { } public void validatePunishmentPermission(Server server, String email, int typeOrdinal) { - if (email == null) { + RoleAuthorization.PerformerAuthority performer = roleAuthorization.panelPerformer(server, email); + if (!performer.identified()) { throw new ForbiddenException("No authenticated user found for permission check"); } - if (permissionService.isSuperAdmin(server, email)) { + if (performer.superAdmin()) { return; } @@ -126,11 +126,7 @@ public void validatePunishmentPermission(Server server, String email, int typeOr .orElseThrow(() -> new ValidationException("Invalid punishment type")); String applyPermission = PermissionService.punishmentApplyPermissionId(type.getName()); - String roleId = staffRepository.findByEmailIgnoreCase(server, email) - .map(Staff::getRoleId) - .orElse(null); - - if (!permissionService.hasPermission(server, roleId, applyPermission)) { + if (!permissionService.hasPermission(server, performer.roleId(), applyPermission)) { throw new ForbiddenException("You do not have permission to apply this punishment type"); } } diff --git a/src/main/java/gg/modl/backend/player/service/PunishmentMutationService.java b/src/main/java/gg/modl/backend/player/service/PunishmentMutationService.java index 63dcd63..d534349 100644 --- a/src/main/java/gg/modl/backend/player/service/PunishmentMutationService.java +++ b/src/main/java/gg/modl/backend/player/service/PunishmentMutationService.java @@ -3,7 +3,6 @@ import gg.modl.backend.database.mongo.repository.PlayerMongoRepository; import gg.modl.backend.database.mongo.repository.PunishmentMongoRepository; import gg.modl.backend.infrastructure.exception.ResourceNotFoundException; -import gg.modl.backend.database.mongo.repository.StaffMongoRepository; import gg.modl.backend.player.data.Player; import gg.modl.backend.player.data.punishment.Punishment; import gg.modl.backend.player.data.punishment.PunishmentModification; @@ -39,7 +38,6 @@ public class PunishmentMutationService { private final TicketService ticketService; private final AppealWorkflowTransitionService appealWorkflowTransitionService; private final IssuerNameResolver issuerNameResolver; - private final StaffMongoRepository staffRepository; private final PunishmentQueryService punishmentQueryService; private final PunishmentLifecycleService punishmentLifecycleService; private final PunishmentRealtimePublisher realtimePublisher; diff --git a/src/main/java/gg/modl/backend/realtime/auth/RealtimeTopicAuthorizer.java b/src/main/java/gg/modl/backend/realtime/auth/RealtimeTopicAuthorizer.java index 6099123..7ee7a11 100644 --- a/src/main/java/gg/modl/backend/realtime/auth/RealtimeTopicAuthorizer.java +++ b/src/main/java/gg/modl/backend/realtime/auth/RealtimeTopicAuthorizer.java @@ -1,11 +1,9 @@ package gg.modl.backend.realtime.auth; import gg.modl.backend.role.service.PermissionService; +import gg.modl.backend.role.service.RoleAuthorization; import gg.modl.backend.server.data.Server; -import gg.modl.backend.staff.data.Staff; -import gg.modl.backend.staff.service.StaffLookupCache; import gg.modl.proto.modl.v1.Topic; -import java.util.Optional; import lombok.RequiredArgsConstructor; import org.springframework.stereotype.Component; @@ -13,7 +11,7 @@ @RequiredArgsConstructor public class RealtimeTopicAuthorizer { private final PermissionService permissionService; - private final StaffLookupCache staffLookupCache; + private final RoleAuthorization roleAuthorization; public boolean canSubscribe(RealtimePrincipal principal, Topic topic) { if (topic == null || topic == Topic.TOPIC_UNSPECIFIED) { @@ -57,8 +55,7 @@ private boolean hasPanelPermission(RealtimePrincipal principal, String permissio return true; } - Optional staffOpt = staffLookupCache.findByEmail(server, email); - String roleId = staffOpt.map(Staff::getRoleId).orElse(null); + String roleId = roleAuthorization.panelRoleId(server, email); return roleId != null && permissionService.hasPermission(server, roleId, permission); } diff --git a/src/main/java/gg/modl/backend/role/controller/PanelRoleController.java b/src/main/java/gg/modl/backend/role/controller/PanelRoleController.java index c376191..48dc100 100644 --- a/src/main/java/gg/modl/backend/role/controller/PanelRoleController.java +++ b/src/main/java/gg/modl/backend/role/controller/PanelRoleController.java @@ -56,7 +56,7 @@ public ResponseEntity getAllRoles(HttpServletRequest requ @GetMapping("/permissions") public ResponseEntity getPermissions(HttpServletRequest request) { Server server = RequestUtil.getRequestServer(request); - List permissions = permissionService.getAllPermissions(server); + List permissions = permissionService.getGrantablePermissions(server); Map categories = permissionService.getPermissionCategories(); return ResponseEntity.ok(PanelRoleProtoMapper.toPermissionsResponse(permissions, categories)); } diff --git a/src/main/java/gg/modl/backend/role/data/Permission.java b/src/main/java/gg/modl/backend/role/data/Permission.java index 3e9ee0f..2c88c0f 100644 --- a/src/main/java/gg/modl/backend/role/data/Permission.java +++ b/src/main/java/gg/modl/backend/role/data/Permission.java @@ -5,9 +5,14 @@ public record Permission( String name, String description, String category, - String parentId + String parentId, + boolean superAdminOnly ) { public Permission(String id, String name, String description, String category) { - this(id, name, description, category, null); + this(id, name, description, category, null, false); + } + + public Permission(String id, String name, String description, String category, String parentId) { + this(id, name, description, category, parentId, false); } } diff --git a/src/main/java/gg/modl/backend/role/service/PermissionService.java b/src/main/java/gg/modl/backend/role/service/PermissionService.java index 2885df6..8d00c08 100644 --- a/src/main/java/gg/modl/backend/role/service/PermissionService.java +++ b/src/main/java/gg/modl/backend/role/service/PermissionService.java @@ -6,6 +6,7 @@ import gg.modl.backend.database.mongo.repository.StaffRoleMongoRepository; import gg.modl.backend.role.data.Permission; import gg.modl.backend.role.data.StaffRole; +import gg.modl.backend.staff.data.Staff; import gg.modl.backend.server.data.Server; import gg.modl.backend.settings.data.PunishmentType; import gg.modl.backend.settings.service.PunishmentTypeService; @@ -36,6 +37,8 @@ public class PermissionService { .build(); public static final String ADMIN_SETTINGS_VIEW = "admin.settings.view"; + public static final String ADMIN_SETTINGS_VIEW_BILLING = "admin.settings.view.billing"; + public static final String ADMIN_SETTINGS_MODIFY_BILLING = "admin.settings.modify.billing"; public static final String ADMIN_SETTINGS_VIEW_PUNISHMENTS = "admin.settings.view.punishments"; public static final String ADMIN_SETTINGS_MODIFY_PUNISHMENTS = "admin.settings.modify.punishments"; public static final String PUNISHMENT_APPLY_PREFIX = "punishment.apply."; @@ -62,7 +65,7 @@ public class PermissionService { new Permission(ADMIN_SETTINGS_VIEW_PUNISHMENTS, "View Punishments Config", "View punishment type configuration", "admin", ADMIN_SETTINGS_VIEW), new Permission("admin.settings.view.content", "View Content", "View homepage cards, knowledgebase, media", "admin", ADMIN_SETTINGS_VIEW), new Permission("admin.settings.view.domain", "View Domain", "View custom domain configuration", "admin", ADMIN_SETTINGS_VIEW), - new Permission("admin.settings.view.billing", "View Billing", "View billing, subscription, and payment info", "admin", ADMIN_SETTINGS_VIEW), + new Permission(ADMIN_SETTINGS_VIEW_BILLING, "View Billing", "View billing, subscription, and payment info", "admin", ADMIN_SETTINGS_VIEW, true), new Permission("admin.settings.view.migration", "View Migration", "View import/export data configuration", "admin", ADMIN_SETTINGS_VIEW), new Permission("admin.settings.view.storage", "View Storage", "View storage configuration", "admin", ADMIN_SETTINGS_VIEW), new Permission("admin.settings.modify", "Modify Settings", "Full control over system settings (includes all sub-permissions)", "admin"), @@ -70,7 +73,7 @@ public class PermissionService { "admin.settings.modify"), new Permission("admin.settings.modify.content", "Modify Content", "Edit homepage cards, knowledgebase, media", "admin", "admin.settings.modify"), new Permission("admin.settings.modify.domain", "Modify Domain", "Change custom domain configuration", "admin", "admin.settings.modify"), - new Permission("admin.settings.modify.billing", "Modify Billing", "Update subscription and payment methods", "admin", "admin.settings.modify"), + new Permission(ADMIN_SETTINGS_MODIFY_BILLING, "Modify Billing", "Update subscription and payment methods", "admin", "admin.settings.modify", true), new Permission("admin.settings.modify.migration", "Modify Migration", "Import/export data between platforms", "admin", "admin.settings.modify"), new Permission("admin.settings.modify.storage", "Modify Storage", "Configure storage backends and limits", "admin", "admin.settings.modify"), new Permission(ADMIN_STAFF_MANAGE, "Manage Staff", "Full staff management (includes all sub-permissions)", "admin"), @@ -80,7 +83,7 @@ public class PermissionService { new Permission("admin.audit.view.dashboard", "View Dashboard", "View dashboard statistics", "admin", ADMIN_AUDIT_VIEW), new Permission("admin.audit.view.analytics", "View Analytics", "View player and ticket analytics", "admin", ADMIN_AUDIT_VIEW), new Permission("admin.audit.view.logs", "View Logs", "View audit trail of staff actions", "admin", ADMIN_AUDIT_VIEW), - new Permission(ADMIN_AUDIT_ROLLBACK, "Rollback Audit Actions", "Roll back punishments and perform destructive bulk audit operations", "admin"), + new Permission(ADMIN_AUDIT_ROLLBACK, "Rollback Audit Actions", "Roll back punishments and perform destructive bulk audit operations", "admin", null, true), new Permission(PUNISHMENT_VIEW, "View Punishments", "View player profiles, punishments, and linked accounts", "punishment"), new Permission(PUNISHMENT_MODIFY, "Modify Punishments", "Full control over existing punishments (includes all sub-permissions)", "punishment"), new Permission("punishment.modify.pardon", "Pardon Punishments", "Pardon punishments and clear associated points", "punishment", PUNISHMENT_MODIFY), @@ -110,6 +113,11 @@ public class PermissionService { new Permission("ticket.delete.all", "Delete Tickets", "Delete tickets from the system", "ticket") ); + private static final Set SUPER_ADMIN_ONLY_PERMISSION_IDS = BASE_PERMISSIONS.stream() + .filter(Permission::superAdminOnly) + .map(Permission::id) + .collect(Collectors.toUnmodifiableSet()); + private static final Map PERMISSION_CATEGORIES = Map.of( "punishment", "Punishment Permissions", "ticket", "Ticket Permissions", @@ -135,6 +143,32 @@ public List getAllPermissions(Server server) { return all; } + public List getGrantablePermissions(Server server) { + return getAllPermissions(server).stream() + .filter(permission -> !permission.superAdminOnly()) + .toList(); + } + + public List getGrantablePermissionIds(Server server) { + return getGrantablePermissions(server).stream().map(Permission::id).toList(); + } + + public static boolean isSuperAdminOnly(String permissionId) { + return SUPER_ADMIN_ONLY_PERMISSION_IDS.contains(permissionId); + } + + public static Set superAdminOnlyPermissionIds() { + return SUPER_ADMIN_ONLY_PERMISSION_IDS; + } + + public static List grantedPermissionIds(StaffRole role) { + List permissions = role.getPermissions(); + if (permissions == null) { + return List.of(); + } + return permissions.stream().filter(permissionId -> !isSuperAdminOnly(permissionId)).toList(); + } + public List getPunishmentPermissions(Server server) { List punishmentTypes = punishmentTypeService.getPunishmentTypes(server); List permissions = new ArrayList<>(); @@ -204,6 +238,9 @@ private boolean computeHasPermission(Server server, String roleId, String permis if (role == null) { return false; } + if (isSuperAdminOnly(permission) && !RoleAuthorization.isSuperAdminRole(role)) { + return false; + } return RoleAuthorization.roleGrants(role, permission); } @@ -268,6 +305,14 @@ public Map resolveRoleNames(Server server, Collection ro return names; } + public String assignedRoleName(Server server, Staff staff) { + return resolveRoleName(server, staff.getRoleId()); + } + + public String effectiveRoleName(Server server, Staff staff) { + return resolveRoleName(server, RoleAuthorization.effectiveRoleId(server, staff)); + } + public String resolveRoleName(Server server, String roleId) { if (roleId == null || roleId.isBlank()) { return ""; diff --git a/src/main/java/gg/modl/backend/role/service/RoleAuthorization.java b/src/main/java/gg/modl/backend/role/service/RoleAuthorization.java index adbb792..0b25097 100644 --- a/src/main/java/gg/modl/backend/role/service/RoleAuthorization.java +++ b/src/main/java/gg/modl/backend/role/service/RoleAuthorization.java @@ -6,6 +6,7 @@ import gg.modl.backend.role.data.StaffRole; import gg.modl.backend.server.data.Server; import gg.modl.backend.staff.data.Staff; +import gg.modl.backend.staff.service.StaffLookupCache; import java.util.List; import lombok.RequiredArgsConstructor; import org.jetbrains.annotations.Nullable; @@ -26,6 +27,7 @@ public class RoleAuthorization { private final PermissionService permissionService; private final StaffMongoRepository staffRepository; + private final StaffLookupCache staffLookupCache; public record PerformerAuthority(String email, String roleId, boolean superAdmin, boolean identified) { public static PerformerAuthority unidentified() { @@ -40,10 +42,16 @@ public PerformerAuthority panelPerformer(Server server, @Nullable String session if (isSuperAdminEmail(server, sessionEmail)) { return new PerformerAuthority(sessionEmail, null, true, true); } - String roleId = staffRepository.findByEmailIgnoreCase(server, sessionEmail) + return new PerformerAuthority(sessionEmail, panelRoleId(server, sessionEmail), false, true); + } + + public String panelRoleId(Server server, @Nullable String email) { + if (email == null || email.isBlank()) { + return null; + } + return staffLookupCache.findByEmail(server, email) .map(staff -> effectiveRoleId(server, staff)) .orElse(null); - return new PerformerAuthority(sessionEmail, roleId, false, true); } public PerformerAuthority minecraftPerformer(Server server, @Nullable String actingStaffId) { @@ -56,6 +64,17 @@ public PerformerAuthority minecraftPerformer(Server server, @Nullable String act .orElseGet(PerformerAuthority::unidentified); } + public List effectivePermissionIds(Server server, PerformerAuthority performer) { + if (performer.superAdmin()) { + return permissionService.getAllPermissionIds(server); + } + return permissionService.getRoleById(server, performer.roleId()) + .map(role -> permissionService.getGrantablePermissionIds(server).stream() + .filter(permissionId -> roleGrants(role, permissionId)) + .toList()) + .orElseGet(List::of); + } + public void requireStaffManage(Server server, PerformerAuthority performer, String requiredManagePermission) { if (!performer.identified()) { throw new ForbiddenException(NO_AUTHORITY_MESSAGE); diff --git a/src/main/java/gg/modl/backend/role/service/RoleService.java b/src/main/java/gg/modl/backend/role/service/RoleService.java index 24baa5c..f19e6b0 100644 --- a/src/main/java/gg/modl/backend/role/service/RoleService.java +++ b/src/main/java/gg/modl/backend/role/service/RoleService.java @@ -125,7 +125,7 @@ public boolean updateRolePermissions(Server server, String id, List perm return false; } - Set validPermissions = new HashSet<>(permissionService.getAllPermissionIds(server)); + Set validPermissions = new HashSet<>(permissionService.getGrantablePermissionIds(server)); List requested = permissions != null ? permissions : List.of(); List filtered = requested.stream() .filter(validPermissions::contains) @@ -154,7 +154,7 @@ public RoleResponse createRole(Server server, RoleRequest request, RoleAuthoriza String roleName = request.name() != null ? request.name().trim() : ""; ensureRoleNameAvailable(server, roleName, null); - Set validPermissions = new HashSet<>(permissionService.getAllPermissionIds(server)); + Set validPermissions = new HashSet<>(permissionService.getGrantablePermissionIds(server)); List filteredPermissions = request.permissions() .stream() .filter(validPermissions::contains) @@ -237,7 +237,7 @@ public Optional updateRole(Server server, String id, RoleRequest r String roleName = request.name() != null ? request.name().trim() : ""; ensureRoleNameAvailable(server, roleName, id); - Set validPermissions = new HashSet<>(permissionService.getAllPermissionIds(server)); + Set validPermissions = new HashSet<>(permissionService.getGrantablePermissionIds(server)); List filteredPermissions = request.permissions() .stream() .filter(validPermissions::contains) @@ -328,11 +328,10 @@ public void createDefaultRoles(Server server) { .filter(p -> !p.contains("blacklist")) .toList(); - List superAdminPerms = new ArrayList<>(permissionService.getAllPermissionIds(server)); + List superAdminPerms = new ArrayList<>(permissionService.getGrantablePermissionIds(server)); List adminPerms = new ArrayList<>(List.of( PermissionService.ADMIN_SETTINGS_VIEW, PermissionService.ADMIN_STAFF_MANAGE, PermissionService.ADMIN_AUDIT_VIEW, - PermissionService.ADMIN_AUDIT_ROLLBACK, PermissionService.PUNISHMENT_VIEW, PermissionService.PUNISHMENT_MODIFY, PermissionService.TICKET_VIEW_ALL, PermissionService.TICKET_REPLY_ALL, PermissionService.APPEAL_MODIFY, PermissionService.TICKET_CLOSE_ALL, PermissionService.STAFF_CHAT_TOGGLE, PermissionService.STAFF_CHAT_CLEAR, PermissionService.STAFF_CHAT_SLOW, diff --git a/src/main/java/gg/modl/backend/settings/controller/PanelAiSuggestionController.java b/src/main/java/gg/modl/backend/settings/controller/PanelAiSuggestionController.java index 7d844b8..57064f0 100644 --- a/src/main/java/gg/modl/backend/settings/controller/PanelAiSuggestionController.java +++ b/src/main/java/gg/modl/backend/settings/controller/PanelAiSuggestionController.java @@ -5,6 +5,7 @@ import gg.modl.backend.infrastructure.authorization.RequiresPanelPermission; import gg.modl.backend.infrastructure.rest.RESTMappingV1; import gg.modl.backend.infrastructure.rest.RequestUtil; +import gg.modl.backend.role.service.PermissionService; import gg.modl.backend.server.data.Server; import gg.modl.proto.modl.v1.AISuggestionActionResponse; import gg.modl.proto.modl.v1.ApplyAIPunishmentRequest; @@ -18,7 +19,7 @@ @RestController @RequestMapping(RESTMappingV1.PANEL_SETTINGS) -@RequiresPanelPermission(view = "admin.settings.view.punishments", modify = "admin.settings.modify.punishments") +@RequiresPanelPermission(view = PermissionService.ADMIN_SETTINGS_VIEW_PUNISHMENTS, modify = PermissionService.ADMIN_SETTINGS_MODIFY_PUNISHMENTS) @RequiredArgsConstructor public class PanelAiSuggestionController { private final AITicketAnalysisService aiTicketAnalysisService; diff --git a/src/main/java/gg/modl/backend/settings/controller/PanelApiKeyController.java b/src/main/java/gg/modl/backend/settings/controller/PanelApiKeyController.java index ca2e04c..5cde20f 100644 --- a/src/main/java/gg/modl/backend/settings/controller/PanelApiKeyController.java +++ b/src/main/java/gg/modl/backend/settings/controller/PanelApiKeyController.java @@ -1,10 +1,9 @@ package gg.modl.backend.settings.controller; +import gg.modl.backend.infrastructure.authorization.PanelAccessRule; import gg.modl.backend.infrastructure.authorization.RequiresPanelPermission; -import gg.modl.backend.infrastructure.exception.ForbiddenException; import gg.modl.backend.infrastructure.rest.RESTMappingV1; import gg.modl.backend.infrastructure.rest.RequestUtil; -import gg.modl.backend.role.service.PermissionService; import gg.modl.backend.server.data.Server; import gg.modl.backend.settings.service.ApiKeySettingsService; import gg.modl.proto.modl.v1.ApiKeyDeleteResponse; @@ -23,11 +22,10 @@ @RestController @RequestMapping(RESTMappingV1.PANEL_SETTINGS) -@RequiresPanelPermission(view = "admin.settings.view", modify = "admin.settings.modify") +@RequiresPanelPermission(rule = PanelAccessRule.SUPER_ADMIN) @RequiredArgsConstructor public class PanelApiKeyController { private final ApiKeySettingsService apiKeySettingsService; - private final PermissionService permissionService; private final SettingsInvalidationPublisher settingsInvalidationPublisher; @PostMapping("/api-keys/{type}/generate") @@ -36,7 +34,6 @@ public ApiKeyGenerateResponse generateApiKey( HttpServletRequest request ) { Server server = RequestUtil.getRequestServer(request); - requireSuperAdmin(server, request); String apiKey = apiKeySettingsService.generateApiKey(server, type); settingsInvalidationPublisher.invalidateSettings(server); return PanelSettingsProtoMapper.toApiKeyGenerateResponse("API key generated successfully", apiKey); @@ -48,7 +45,6 @@ public ResponseEntity revealApiKey( HttpServletRequest request ) { Server server = RequestUtil.getRequestServer(request); - requireSuperAdmin(server, request); String apiKey = apiKeySettingsService.revealApiKey(server, type); if (apiKey == null) { @@ -64,7 +60,6 @@ public ResponseEntity deleteApiKey( HttpServletRequest request ) { Server server = RequestUtil.getRequestServer(request); - requireSuperAdmin(server, request); boolean deleted = apiKeySettingsService.deleteApiKey(server, type); if (!deleted) { @@ -81,15 +76,7 @@ public ApiKeyExistsResponse checkApiKeyExists( HttpServletRequest request ) { Server server = RequestUtil.getRequestServer(request); - requireSuperAdmin(server, request); boolean exists = apiKeySettingsService.hasApiKey(server, type); return PanelSettingsProtoMapper.toApiKeyExistsResponse(exists); } - - private void requireSuperAdmin(Server server, HttpServletRequest request) { - String email = RequestUtil.getSessionEmail(request); - if (!permissionService.isSuperAdmin(server, email)) { - throw new ForbiddenException("Only super admins can manage API keys"); - } - } } diff --git a/src/main/java/gg/modl/backend/settings/controller/PanelSettingsController.java b/src/main/java/gg/modl/backend/settings/controller/PanelSettingsController.java index c0947f3..e22ee6f 100644 --- a/src/main/java/gg/modl/backend/settings/controller/PanelSettingsController.java +++ b/src/main/java/gg/modl/backend/settings/controller/PanelSettingsController.java @@ -5,6 +5,7 @@ import gg.modl.backend.infrastructure.validation.BeanValidationRunner; import gg.modl.backend.infrastructure.rest.RESTMappingV1; import gg.modl.backend.infrastructure.rest.RequestUtil; +import gg.modl.backend.role.service.PermissionService; import gg.modl.backend.server.data.Server; import gg.modl.backend.settings.data.AIModerationSettings; import gg.modl.backend.settings.data.GeneralSettings; @@ -57,7 +58,7 @@ @RestController @RequestMapping(RESTMappingV1.PANEL_SETTINGS) -@RequiresPanelPermission(view = "admin.settings.view", modify = "admin.settings.modify") +@RequiresPanelPermission(view = PermissionService.ADMIN_SETTINGS_VIEW, modify = "admin.settings.modify") @RequiredArgsConstructor public class PanelSettingsController { private final GeneralSettingsService generalSettingsService; @@ -116,7 +117,7 @@ public TicketLabelSettingsEnvelope patchTicketLabelSettings( } @GetMapping("/status-thresholds") - @RequiresPanelPermission(view = "admin.settings.view.punishments", modify = "admin.settings.modify.punishments") + @RequiresPanelPermission(view = PermissionService.ADMIN_SETTINGS_VIEW_PUNISHMENTS, modify = PermissionService.ADMIN_SETTINGS_MODIFY_PUNISHMENTS) public OffenderThresholdSettingsEnvelope getStatusThresholds(HttpServletRequest request) { Server server = RequestUtil.getRequestServer(request); return PanelSettingsProtoMapper.toOffenderThresholdSettingsEnvelope( @@ -124,7 +125,7 @@ public OffenderThresholdSettingsEnvelope getStatusThresholds(HttpServletRequest } @PatchMapping("/status-thresholds") - @RequiresPanelPermission(view = "admin.settings.view.punishments", modify = "admin.settings.modify.punishments") + @RequiresPanelPermission(view = PermissionService.ADMIN_SETTINGS_VIEW_PUNISHMENTS, modify = PermissionService.ADMIN_SETTINGS_MODIFY_PUNISHMENTS) public OffenderThresholdSettingsEnvelope patchStatusThresholds( @RequestBody PatchStatusThresholdSettingsRequest body, HttpServletRequest request @@ -163,7 +164,7 @@ public ReplayRetentionSettingsEnvelope patchReplayRetentionSettings( } @GetMapping("/ai-moderation") - @RequiresPanelPermission(view = "admin.settings.view.punishments", modify = "admin.settings.modify.punishments") + @RequiresPanelPermission(view = PermissionService.ADMIN_SETTINGS_VIEW_PUNISHMENTS, modify = PermissionService.ADMIN_SETTINGS_MODIFY_PUNISHMENTS) public gg.modl.proto.modl.v1.AIModerationSettings getAIModerationSettings(HttpServletRequest request) { Server server = RequestUtil.getRequestServer(request); AIModerationSettings settings = aiModerationSettingsService.getAIModerationSettings(server); @@ -171,7 +172,7 @@ public gg.modl.proto.modl.v1.AIModerationSettings getAIModerationSettings(HttpSe } @PatchMapping("/ai-moderation") - @RequiresPanelPermission(view = "admin.settings.view.punishments", modify = "admin.settings.modify.punishments") + @RequiresPanelPermission(view = PermissionService.ADMIN_SETTINGS_VIEW_PUNISHMENTS, modify = PermissionService.ADMIN_SETTINGS_MODIFY_PUNISHMENTS) public gg.modl.proto.modl.v1.AIModerationSettings updateAIModerationSettings( @RequestBody UpdateAIModerationSettingsRequest requestBody, HttpServletRequest request diff --git a/src/main/java/gg/modl/backend/staff/service/MinecraftStaffService.java b/src/main/java/gg/modl/backend/staff/service/MinecraftStaffService.java index 8141aa5..7266d0a 100644 --- a/src/main/java/gg/modl/backend/staff/service/MinecraftStaffService.java +++ b/src/main/java/gg/modl/backend/staff/service/MinecraftStaffService.java @@ -154,7 +154,7 @@ private static String roleNameOrFallback(Map rolesById, Strin private static List rolePermissions(Map rolesById, String roleId) { StaffRole role = roleId != null ? rolesById.get(roleId) : null; - return role != null && role.getPermissions() != null ? role.getPermissions() : List.of(); + return role != null ? PermissionService.grantedPermissionIds(role) : List.of(); } public List getMinecraftStaffPermissions(Server server) { @@ -273,6 +273,6 @@ public List getAvailablePlayers(Server server) { } private StaffResponse toStaffResponse(Server server, Staff staff, String status) { - return StaffResponseFactory.of(staff, status, permissionService.resolveRoleName(server, staff.getRoleId())); + return StaffResponseFactory.of(staff, status, permissionService.assignedRoleName(server, staff)); } } diff --git a/src/main/java/gg/modl/backend/staff/service/StaffService.java b/src/main/java/gg/modl/backend/staff/service/StaffService.java index c107954..1ff01f6 100644 --- a/src/main/java/gg/modl/backend/staff/service/StaffService.java +++ b/src/main/java/gg/modl/backend/staff/service/StaffService.java @@ -95,7 +95,7 @@ public long countStaffIncludingSuperAdmin(Server server) { } private StaffResponse toStaffResponse(Server server, Staff staff, String status) { - return StaffResponseFactory.of(staff, status, permissionService.resolveRoleName(server, staff.getRoleId())); + return StaffResponseFactory.of(staff, status, permissionService.assignedRoleName(server, staff)); } private static String fallbackRoleName(Map roleNamesById, String roleId) { diff --git a/src/main/java/gg/modl/backend/storage/controller/PanelStorageController.java b/src/main/java/gg/modl/backend/storage/controller/PanelStorageController.java index 693ce9d..5c7ce57 100644 --- a/src/main/java/gg/modl/backend/storage/controller/PanelStorageController.java +++ b/src/main/java/gg/modl/backend/storage/controller/PanelStorageController.java @@ -1,13 +1,12 @@ package gg.modl.backend.storage.controller; -import gg.modl.backend.infrastructure.exception.ForbiddenException; +import gg.modl.backend.infrastructure.authorization.PanelAccessRule; import gg.modl.backend.infrastructure.authorization.RequiresPanelPermission; import gg.modl.backend.infrastructure.exception.ValidationException; import gg.modl.backend.infrastructure.rest.RESTMappingV1; import gg.modl.backend.infrastructure.rest.RequestUtil; import gg.modl.backend.infrastructure.validation.RequestValidationLimits; import gg.modl.backend.replay.service.ReplayDeletionService; -import gg.modl.backend.role.service.PermissionService; import gg.modl.backend.server.data.Server; import gg.modl.backend.storage.dto.response.StorageFileResponse; import gg.modl.backend.storage.service.MediaValidationService; @@ -42,7 +41,6 @@ public class PanelStorageController { private final StorageQuotaService quotaService; private final StorageMetadataService storageMetadataService; private final StorageSyncService storageSyncService; - private final PermissionService permissionService; private final MediaValidationService validationService; private final ReplayDeletionService replayDeletionService; @@ -84,11 +82,9 @@ public ResponseEntity bulkDelete( } @PostMapping("/sync") + @RequiresPanelPermission(rule = PanelAccessRule.SUPER_ADMIN) public ResponseEntity syncFiles(HttpServletRequest request) { Server server = RequestUtil.getRequestServer(request); - if (!permissionService.isSuperAdmin(server, RequestUtil.getSessionEmail(request))) { - throw new ForbiddenException("Only super admins can trigger a storage sync"); - } int synced = storageSyncService.syncServerFiles(server, true); return ResponseEntity.ok(StorageProtoMapper.toStorageSyncResponse(synced)); } diff --git a/src/test/java/gg/modl/backend/auth/controller/PanelAuthControllerTest.java b/src/test/java/gg/modl/backend/auth/controller/PanelAuthControllerTest.java index f75a9f2..793607b 100644 --- a/src/test/java/gg/modl/backend/auth/controller/PanelAuthControllerTest.java +++ b/src/test/java/gg/modl/backend/auth/controller/PanelAuthControllerTest.java @@ -21,6 +21,7 @@ import gg.modl.backend.infrastructure.rest.RequestAttribute; import gg.modl.backend.infrastructure.util.CookieUtil; import gg.modl.backend.role.service.PermissionService; +import gg.modl.backend.role.service.RoleAuthorization; import gg.modl.backend.server.ServerService; import gg.modl.backend.server.data.Server; import gg.modl.backend.server.data.ServerPlan; @@ -86,6 +87,7 @@ void emailChangeStillInvalidatesOldEmailSessions() { staffProfileService, mock(StaffLookupCache.class), permissionService, + mock(RoleAuthorization.class), new CookieUtil(authConfiguration), emailChangeService ); @@ -203,6 +205,7 @@ private PanelAuthController createController(AuthConfiguration authConfiguration mock(StaffProfileService.class), mock(StaffLookupCache.class), mock(PermissionService.class), + mock(RoleAuthorization.class), new CookieUtil(authConfiguration), mock(EmailChangeService.class) ); diff --git a/src/test/java/gg/modl/backend/billing/service/BillingServiceTest.java b/src/test/java/gg/modl/backend/billing/service/BillingServiceTest.java index e9b540a..ac8ebca 100644 --- a/src/test/java/gg/modl/backend/billing/service/BillingServiceTest.java +++ b/src/test/java/gg/modl/backend/billing/service/BillingServiceTest.java @@ -8,7 +8,6 @@ import static org.mockito.Mockito.when; import com.stripe.model.checkout.Session; -import gg.modl.backend.role.service.PermissionService; import gg.modl.backend.server.data.Server; import gg.modl.backend.server.data.ServerPlan; import gg.modl.backend.server.service.ServerMutationHelper; @@ -27,14 +26,11 @@ class BillingServiceTest { @Mock private ServerMutationHelper serverMutationHelper; - @Mock - private PermissionService permissionService; - private BillingService billingService; @BeforeEach void setUp() { - billingService = new BillingService(stripeService, serverMutationHelper, permissionService); + billingService = new BillingService(stripeService, serverMutationHelper); } @Test diff --git a/src/test/java/gg/modl/backend/infrastructure/authorization/PanelAuthorizationBootstrapValidatorTest.java b/src/test/java/gg/modl/backend/infrastructure/authorization/PanelAuthorizationBootstrapValidatorTest.java new file mode 100644 index 0000000..c155099 --- /dev/null +++ b/src/test/java/gg/modl/backend/infrastructure/authorization/PanelAuthorizationBootstrapValidatorTest.java @@ -0,0 +1,173 @@ +package gg.modl.backend.infrastructure.authorization; + +import gg.modl.backend.infrastructure.rest.RESTMappingV1; +import gg.modl.backend.role.service.PermissionService; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.ObjectProvider; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RestController; +import org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping; + +class PanelAuthorizationBootstrapValidatorTest { + + @RestController + @RequestMapping(RESTMappingV1.PREFIX_PANEL + "/bootstrap-probe") + static class UnflaggedPermissionOnSuperAdminRuleController { + @GetMapping("/unflagged") + @RequiresPanelPermission(rule = PanelAccessRule.SUPER_ADMIN, supersedesPermissions = PermissionService.ADMIN_SETTINGS_VIEW) + String unflagged() { + return ""; + } + } + + @RestController + @RequestMapping(RESTMappingV1.PREFIX_PANEL + "/bootstrap-probe") + static class FlaggedPermissionsFullyRuledController { + @GetMapping("/billing") + @RequiresPanelPermission(rule = PanelAccessRule.SUPER_ADMIN, + supersedesPermissions = {PermissionService.ADMIN_SETTINGS_VIEW_BILLING, PermissionService.ADMIN_SETTINGS_MODIFY_BILLING}) + String billing() { + return ""; + } + + @GetMapping("/rollback") + @RequiresPanelPermission(rule = PanelAccessRule.SUPER_ADMIN, supersedesPermissions = PermissionService.ADMIN_AUDIT_ROLLBACK) + String rollback() { + return ""; + } + } + + @RestController + @RequestMapping(RESTMappingV1.PREFIX_PANEL + "/bootstrap-probe") + static class FlaggedPermissionEnforcedWithoutSuperAdminRuleController { + @GetMapping("/leaked") + @RequiresPanelPermission(PermissionService.ADMIN_AUDIT_ROLLBACK) + String leaked() { + return ""; + } + } + + @RestController + @RequestMapping(RESTMappingV1.PREFIX_PANEL + "/bootstrap-probe") + static class SuperAdminRuleWithEnforcedPermissionController { + @GetMapping("/ignored-enforcement") + @RequiresPanelPermission(value = PermissionService.ADMIN_SETTINGS_VIEW, rule = PanelAccessRule.SUPER_ADMIN) + String ignoredEnforcement() { + return ""; + } + } + + @RestController + @RequestMapping(RESTMappingV1.PREFIX_PANEL + "/bootstrap-probe") + static class SupersedesWithoutSuperAdminRuleController { + @GetMapping("/dangling-supersedes") + @RequiresPanelPermission(value = PermissionService.ADMIN_SETTINGS_VIEW, + supersedesPermissions = PermissionService.ADMIN_AUDIT_ROLLBACK) + String danglingSupersedes() { + return ""; + } + } + + @RestController + @RequestMapping(RESTMappingV1.PREFIX_PANEL + "/bootstrap-probe") + @RequiresPanelPermission(rule = PanelAccessRule.SUPER_ADMIN) + static class SuperAdminControllerWeakenedByMethodOverrideController { + @GetMapping("/weakened") + @RequiresPanelPermission(PermissionService.ADMIN_SETTINGS_VIEW) + String weakened() { + return ""; + } + } + + @RestController + @RequestMapping(RESTMappingV1.PREFIX_PANEL + "/bootstrap-probe") + static class UnguardedController { + @GetMapping("/unguarded") + String unguarded() { + return ""; + } + } + + @Test + void superAdminRuledHandlerNamingAnUnflaggedPermissionIsRejected() { + IllegalStateException failure = Assertions.assertThrows(IllegalStateException.class, + () -> validate(new UnflaggedPermissionOnSuperAdminRuleController(), new FlaggedPermissionsFullyRuledController())); + + Assertions.assertTrue(failure.getMessage().contains("not flagged superAdminOnly"), failure.getMessage()); + Assertions.assertTrue(failure.getMessage().contains(PermissionService.ADMIN_SETTINGS_VIEW), failure.getMessage()); + } + + @Test + void flaggedPermissionNamedByNoSuperAdminRuledHandlerIsRejected() { + IllegalStateException failure = Assertions.assertThrows(IllegalStateException.class, + () -> validate(new UnflaggedPermissionOnSuperAdminRuleController())); + + Assertions.assertTrue(failure.getMessage().contains("superseded by no super-admin-ruled panel endpoint"), failure.getMessage()); + Assertions.assertTrue(failure.getMessage().contains(PermissionService.ADMIN_AUDIT_ROLLBACK), failure.getMessage()); + } + + @Test + void flaggedPermissionEnforcedWithoutSuperAdminRuleIsRejected() { + IllegalStateException failure = Assertions.assertThrows(IllegalStateException.class, + () -> validate(new FlaggedPermissionEnforcedWithoutSuperAdminRuleController(), new FlaggedPermissionsFullyRuledController())); + + Assertions.assertTrue(failure.getMessage().contains("without rule = SUPER_ADMIN"), failure.getMessage()); + Assertions.assertTrue(failure.getMessage().contains(PermissionService.ADMIN_AUDIT_ROLLBACK), failure.getMessage()); + } + + @Test + void superAdminRuledHandlerDeclaringAnEnforcedPermissionIsRejected() { + IllegalStateException failure = Assertions.assertThrows(IllegalStateException.class, + () -> validate(new SuperAdminRuleWithEnforcedPermissionController(), new FlaggedPermissionsFullyRuledController())); + + Assertions.assertTrue(failure.getMessage().contains("combining rule = SUPER_ADMIN with an enforced permission"), + failure.getMessage()); + Assertions.assertTrue(failure.getMessage().contains(PermissionService.ADMIN_SETTINGS_VIEW), failure.getMessage()); + } + + @Test + void supersedesPermissionsWithoutSuperAdminRuleIsRejected() { + IllegalStateException failure = Assertions.assertThrows(IllegalStateException.class, + () -> validate(new SupersedesWithoutSuperAdminRuleController(), new FlaggedPermissionsFullyRuledController())); + + Assertions.assertTrue(failure.getMessage().contains("declaring supersedesPermissions without rule = SUPER_ADMIN"), + failure.getMessage()); + Assertions.assertTrue(failure.getMessage().contains(PermissionService.ADMIN_AUDIT_ROLLBACK), failure.getMessage()); + } + + @Test + void methodLevelOverrideWeakeningASuperAdminControllerIsRejected() { + IllegalStateException failure = Assertions.assertThrows(IllegalStateException.class, + () -> validate(new SuperAdminControllerWeakenedByMethodOverrideController(), + new FlaggedPermissionsFullyRuledController())); + + Assertions.assertTrue( + failure.getMessage().contains("overriding a rule = SUPER_ADMIN controller with a weaker method-level policy"), + failure.getMessage()); + Assertions.assertTrue(failure.getMessage().contains("/weakened"), failure.getMessage()); + } + + @Test + void unguardedPanelHandlerIsRejected() { + IllegalStateException failure = Assertions.assertThrows(IllegalStateException.class, + () -> validate(new UnguardedController(), new FlaggedPermissionsFullyRuledController())); + + Assertions.assertTrue(failure.getMessage().contains("without a @RequiresPanelPermission policy"), failure.getMessage()); + } + + @Test + void productionHandlerMappingSatisfiesEveryInvariant() { + Assertions.assertDoesNotThrow(() -> validatorFor(PanelHandlerMappingTestSupport.buildHandlerMapping()).afterSingletonsInstantiated()); + } + + private void validate(Object... controllers) { + validatorFor(PanelHandlerMappingTestSupport.handlerMappingOf(controllers)).afterSingletonsInstantiated(); + } + + private PanelAuthorizationBootstrapValidator validatorFor(RequestMappingHandlerMapping handlerMapping) { + ObjectProvider provider = PanelHandlerMappingTestSupport.providerOf(handlerMapping); + return new PanelAuthorizationBootstrapValidator(provider, new PanelAccessPolicyResolver(provider)); + } +} diff --git a/src/test/java/gg/modl/backend/infrastructure/filter/PanelAuthorizationMatrixTest.java b/src/test/java/gg/modl/backend/infrastructure/filter/PanelAuthorizationMatrixTest.java index 55d5857..f17e0f3 100644 --- a/src/test/java/gg/modl/backend/infrastructure/filter/PanelAuthorizationMatrixTest.java +++ b/src/test/java/gg/modl/backend/infrastructure/filter/PanelAuthorizationMatrixTest.java @@ -1,5 +1,7 @@ package gg.modl.backend.infrastructure.filter; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -13,6 +15,8 @@ import gg.modl.backend.server.data.ServerPlan; import gg.modl.backend.staff.data.Staff; import gg.modl.backend.staff.service.StaffLookupCache; +import gg.modl.backend.role.service.RoleAuthorization; +import gg.modl.backend.database.mongo.repository.StaffMongoRepository; import jakarta.servlet.FilterChain; import java.util.ArrayList; import java.util.List; @@ -33,7 +37,7 @@ class PanelAuthorizationMatrixTest { private static final String DENY_BODY = "{\"success\":false,\"status\":403,\"error\":\"Insufficient permissions\",\"message\":\"Insufficient permissions\"}"; - private enum Kind { PERMISSION, PERMIT, PLAYER_READ, APPEAL_REPLY } + private enum Kind { PERMISSION, PERMIT, PLAYER_READ, APPEAL_REPLY, SUPER_ADMIN } private record Row(String method, String path, Kind kind, String permission) { static Row permission(String method, String path, String permission) { @@ -51,6 +55,10 @@ static Row playerRead(String path) { static Row appealReply(String path) { return new Row("POST", path, Kind.APPEAL_REPLY, null); } + + static Row superAdminOnly(String method, String path) { + return new Row(method, path, Kind.SUPER_ADMIN, null); + } } private static final List MATRIX = List.of( @@ -61,11 +69,11 @@ static Row appealReply(String path) { Row.permission("GET", "/v1/panel/dashboard/metrics", "admin.audit.view.dashboard"), Row.permission("GET", "/v1/panel/analytics/overview", "admin.audit.view.analytics"), Row.permission("GET", "/v1/panel/audit/staff-performance", "admin.audit.view.logs"), - Row.permission("POST", "/v1/panel/audit/punishments/p1/rollback", "admin.audit.rollback"), - Row.permission("POST", "/v1/panel/audit/staff/staff1/rollback-all", "admin.audit.rollback"), - Row.permission("POST", "/v1/panel/audit/staff/staff1/rollback-date-range", "admin.audit.rollback"), - Row.permission("POST", "/v1/panel/audit/punishments/bulk-pardon", "admin.audit.rollback"), - Row.permission("POST", "/v1/panel/audit/punishments/bulk-set-expiration", "admin.audit.rollback"), + Row.superAdminOnly("POST", "/v1/panel/audit/punishments/p1/rollback"), + Row.superAdminOnly("POST", "/v1/panel/audit/staff/staff1/rollback-all"), + Row.superAdminOnly("POST", "/v1/panel/audit/staff/staff1/rollback-date-range"), + Row.superAdminOnly("POST", "/v1/panel/audit/punishments/bulk-pardon"), + Row.superAdminOnly("POST", "/v1/panel/audit/punishments/bulk-set-expiration"), Row.permission("GET", "/v1/panel/logs", "admin.audit.view.logs"), Row.permission("POST", "/v1/panel/replays/r1/label", "punishment.modify"), Row.permission("POST", "/v1/panel/players/uuid1/notes", "punishment.modify"), @@ -77,8 +85,8 @@ static Row appealReply(String path) { Row.permission("POST", "/v1/panel/ticket-subscriptions/updates/u1/read", "ticket.reply.all"), Row.permission("GET", "/v1/panel/appeals/a1", "ticket.view.all"), Row.permission("PATCH", "/v1/panel/appeals/a1/status", "appeal.modify"), - Row.permission("GET", "/v1/panel/billing/status", "admin.settings.view.billing"), - Row.permission("POST", "/v1/panel/billing/checkout-session", "admin.settings.modify.billing"), + Row.superAdminOnly("GET", "/v1/panel/billing/status"), + Row.superAdminOnly("POST", "/v1/panel/billing/checkout-session"), Row.permission("GET", "/v1/panel/homepage-cards", "admin.settings.view.content"), Row.permission("POST", "/v1/panel/homepage-cards", "admin.settings.modify.content"), Row.permission("GET", "/v1/panel/knowledgebase/categories", "admin.settings.view.content"), @@ -101,8 +109,8 @@ static Row appealReply(String path) { Row.playerRead("/v1/panel/settings/punishment-types"), Row.permission("GET", "/v1/panel/settings/domain", "admin.settings.view.domain"), Row.permission("POST", "/v1/panel/settings/domain", "admin.settings.modify.domain"), - Row.permission("POST", "/v1/panel/settings/api-keys/minecraft/generate", "admin.settings.modify"), - Row.permission("GET", "/v1/panel/settings/api-keys/minecraft/exists", "admin.settings.view"), + Row.superAdminOnly("POST", "/v1/panel/settings/api-keys/minecraft/generate"), + Row.superAdminOnly("GET", "/v1/panel/settings/api-keys/minecraft/exists"), Row.permit("GET", "/v1/panel/dashboard/alerts"), Row.permit("POST", "/v1/panel/players/uuid1/punishments"), Row.permit("POST", "/v1/panel/settings/ai-apply-punishment/t1"), @@ -110,6 +118,17 @@ static Row appealReply(String path) { Row.playerRead("/v1/panel/players/uuid1"), Row.playerRead("/v1/panel/players/uuid1/punishments/active"), Row.playerRead("/v1/panel/players/punishments/search"), + Row.superAdminOnly("GET", "/v1/panel/audit/database/players"), + Row.superAdminOnly("POST", "/v1/panel/billing/portal-session"), + Row.superAdminOnly("POST", "/v1/panel/billing/cancel"), + Row.superAdminOnly("POST", "/v1/panel/billing/resubscribe"), + Row.superAdminOnly("POST", "/v1/panel/billing/usage-settings"), + Row.superAdminOnly("POST", "/v1/panel/billing/storage-limit"), + Row.superAdminOnly("POST", "/v1/panel/billing/overage-limits"), + Row.superAdminOnly("GET", "/v1/panel/billing/usage"), + Row.superAdminOnly("GET", "/v1/panel/settings/api-keys/minecraft/reveal"), + Row.superAdminOnly("DELETE", "/v1/panel/settings/api-keys/minecraft"), + Row.superAdminOnly("POST", "/v1/panel/storage/sync"), Row.appealReply("/v1/panel/appeals/a1/replies") ); @@ -118,7 +137,11 @@ void authorizationMatrixIsPreservedForEveryMappedRoute() { List failures = new ArrayList<>(); for (Row row : MATRIX) { - assertGranted(row, "with-permission", staffWith(row), failures); + if (row.kind() == Kind.SUPER_ADMIN) { + assertDenied(row, "holding-every-permission", staffWith(row), failures); + } else { + assertGranted(row, "with-permission", staffWith(row), failures); + } if (row.kind() != Kind.PERMIT) { assertDenied(row, "without-permission", staffWithout(), failures); } @@ -237,6 +260,8 @@ private void grantPermissions(Mocks mocks, Row row) { case PERMIT -> { } case PLAYER_READ -> lenient().when(permissionService.hasPermission(mocks.server(), ROLE, "punishment.view")).thenReturn(true); case APPEAL_REPLY -> lenient().when(permissionService.hasPermission(mocks.server(), ROLE, "appeal.modify")).thenReturn(true); + case SUPER_ADMIN -> lenient() + .when(permissionService.hasPermission(eq(mocks.server()), eq(ROLE), anyString())).thenReturn(true); } } @@ -270,7 +295,7 @@ private Outcome invoke(String method, String path, Consumer setup) { private static final PanelAccessPolicyResolver POLICY_RESOLVER = PanelHandlerMappingTestSupport.buildResolver(); private PanelPermissionFilter newFilter(PermissionService permissionService, StaffLookupCache staffLookupCache) { - return new PanelPermissionFilter(permissionService, staffLookupCache, POLICY_RESOLVER); + return new PanelPermissionFilter(permissionService, roleAuthorization(permissionService, staffLookupCache), POLICY_RESOLVER); } private record Mocks(PermissionService permissionService, StaffLookupCache staffLookupCache, @@ -279,4 +304,9 @@ private record Mocks(PermissionService permissionService, StaffLookupCache staff private record Outcome(boolean granted, int status, String body, String contentType) { } + + private static RoleAuthorization roleAuthorization(PermissionService permissionService, + StaffLookupCache staffLookupCache) { + return new RoleAuthorization(permissionService, mock(StaffMongoRepository.class), staffLookupCache); + } } diff --git a/src/test/java/gg/modl/backend/infrastructure/filter/PanelPermissionFilterTest.java b/src/test/java/gg/modl/backend/infrastructure/filter/PanelPermissionFilterTest.java index 35e81db..2ff2218 100644 --- a/src/test/java/gg/modl/backend/infrastructure/filter/PanelPermissionFilterTest.java +++ b/src/test/java/gg/modl/backend/infrastructure/filter/PanelPermissionFilterTest.java @@ -1,5 +1,8 @@ package gg.modl.backend.infrastructure.filter; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; @@ -16,6 +19,8 @@ import gg.modl.backend.server.data.ServerPlan; import gg.modl.backend.staff.data.Staff; import gg.modl.backend.staff.service.StaffLookupCache; +import gg.modl.backend.role.service.RoleAuthorization; +import gg.modl.backend.database.mongo.repository.StaffMongoRepository; import jakarta.servlet.FilterChain; import java.util.Optional; import org.junit.jupiter.api.Assertions; @@ -36,7 +41,7 @@ class PanelPermissionFilterTest { void permitPolicySkipsStaffLookup() throws Exception { PermissionService permissionService = mock(PermissionService.class); StaffLookupCache staffLookupCache = mock(StaffLookupCache.class); - PanelPermissionFilter filter = new PanelPermissionFilter(permissionService, staffLookupCache, POLICY_RESOLVER); + PanelPermissionFilter filter = new PanelPermissionFilter(permissionService, roleAuthorization(permissionService, staffLookupCache), POLICY_RESOLVER); when(permissionService.isSuperAdmin(SERVER, STAFF_EMAIL)).thenReturn(false); MockHttpServletRequest request = authenticated("GET", RESTMappingV1.PANEL_DASHBOARD + "/alerts", STAFF_EMAIL); MockHttpServletResponse response = new MockHttpServletResponse(); @@ -52,7 +57,7 @@ void permitPolicySkipsStaffLookup() throws Exception { void superAdminBypassesPolicyResolutionAndStaffLookup() throws Exception { PermissionService permissionService = mock(PermissionService.class); StaffLookupCache staffLookupCache = mock(StaffLookupCache.class); - PanelPermissionFilter filter = new PanelPermissionFilter(permissionService, staffLookupCache, POLICY_RESOLVER); + PanelPermissionFilter filter = new PanelPermissionFilter(permissionService, roleAuthorization(permissionService, staffLookupCache), POLICY_RESOLVER); when(permissionService.isSuperAdmin(SERVER, SUPER_ADMIN_EMAIL)).thenReturn(true); MockHttpServletRequest request = authenticated("POST", RESTMappingV1.PANEL_STAFF, SUPER_ADMIN_EMAIL); MockHttpServletResponse response = new MockHttpServletResponse(); @@ -68,7 +73,7 @@ void superAdminBypassesPolicyResolutionAndStaffLookup() throws Exception { void unauthenticatedRequestIsDeniedWithFrozenBody() throws Exception { PermissionService permissionService = mock(PermissionService.class); StaffLookupCache staffLookupCache = mock(StaffLookupCache.class); - PanelPermissionFilter filter = new PanelPermissionFilter(permissionService, staffLookupCache, POLICY_RESOLVER); + PanelPermissionFilter filter = new PanelPermissionFilter(permissionService, roleAuthorization(permissionService, staffLookupCache), POLICY_RESOLVER); MockHttpServletRequest request = new MockHttpServletRequest("GET", RESTMappingV1.PANEL_TICKETS); request.setAttribute(RequestAttribute.SERVER, SERVER); MockHttpServletResponse response = new MockHttpServletResponse(); @@ -86,7 +91,7 @@ void unauthenticatedRequestIsDeniedWithFrozenBody() throws Exception { void unmappedPanelRouteFailsClosed() throws Exception { PermissionService permissionService = mock(PermissionService.class); StaffLookupCache staffLookupCache = mock(StaffLookupCache.class); - PanelPermissionFilter filter = new PanelPermissionFilter(permissionService, staffLookupCache, POLICY_RESOLVER); + PanelPermissionFilter filter = new PanelPermissionFilter(permissionService, roleAuthorization(permissionService, staffLookupCache), POLICY_RESOLVER); when(permissionService.isSuperAdmin(SERVER, STAFF_EMAIL)).thenReturn(false); MockHttpServletRequest request = authenticated("GET", RESTMappingV1.PREFIX_PANEL + "/does-not-exist", STAFF_EMAIL); MockHttpServletResponse response = new MockHttpServletResponse(); @@ -102,7 +107,7 @@ void unmappedPanelRouteFailsClosed() throws Exception { void appealReplyWriteIsGrantedByTicketReplyAll() throws Exception { PermissionService permissionService = mock(PermissionService.class); StaffLookupCache staffLookupCache = mock(StaffLookupCache.class); - PanelPermissionFilter filter = new PanelPermissionFilter(permissionService, staffLookupCache, POLICY_RESOLVER); + PanelPermissionFilter filter = new PanelPermissionFilter(permissionService, roleAuthorization(permissionService, staffLookupCache), POLICY_RESOLVER); when(permissionService.isSuperAdmin(SERVER, STAFF_EMAIL)).thenReturn(false); Staff staff = Staff.builder().email(STAFF_EMAIL).roleId("helper").build(); when(staffLookupCache.findByEmail(SERVER, STAFF_EMAIL)).thenReturn(Optional.of(staff)); @@ -121,7 +126,7 @@ void appealReplyWriteIsGrantedByTicketReplyAll() throws Exception { void appealStatusWriteIsNotGrantedByTicketReplyAll() throws Exception { PermissionService permissionService = mock(PermissionService.class); StaffLookupCache staffLookupCache = mock(StaffLookupCache.class); - PanelPermissionFilter filter = new PanelPermissionFilter(permissionService, staffLookupCache, POLICY_RESOLVER); + PanelPermissionFilter filter = new PanelPermissionFilter(permissionService, roleAuthorization(permissionService, staffLookupCache), POLICY_RESOLVER); when(permissionService.isSuperAdmin(SERVER, STAFF_EMAIL)).thenReturn(false); Staff staff = Staff.builder().email(STAFF_EMAIL).roleId("helper").build(); when(staffLookupCache.findByEmail(SERVER, STAFF_EMAIL)).thenReturn(Optional.of(staff)); @@ -136,6 +141,43 @@ void appealStatusWriteIsNotGrantedByTicketReplyAll() throws Exception { Assertions.assertEquals(403, response.getStatus()); } + @Test + void billingStatusIsDeniedForStaffHoldingSettingsViewAndModify() throws Exception { + PermissionService permissionService = mock(PermissionService.class); + StaffLookupCache staffLookupCache = mock(StaffLookupCache.class); + PanelPermissionFilter filter = new PanelPermissionFilter(permissionService, roleAuthorization(permissionService, staffLookupCache), POLICY_RESOLVER); + when(permissionService.isSuperAdmin(SERVER, STAFF_EMAIL)).thenReturn(false); + Staff staff = Staff.builder().email(STAFF_EMAIL).roleId("admin").build(); + lenient().when(staffLookupCache.findByEmail(SERVER, STAFF_EMAIL)).thenReturn(Optional.of(staff)); + lenient().when(permissionService.hasPermission(eq(SERVER), eq("admin"), anyString())).thenReturn(true); + MockHttpServletRequest request = authenticated("GET", RESTMappingV1.PANEL_BILLING + "/status", STAFF_EMAIL); + MockHttpServletResponse response = new MockHttpServletResponse(); + FilterChain chain = mock(FilterChain.class); + + filter.doFilter(request, response, chain); + + verify(chain, never()).doFilter(request, response); + Assertions.assertEquals(403, response.getStatus()); + Assertions.assertEquals(DENY_BODY, response.getContentAsString()); + Assertions.assertEquals("application/json", response.getContentType()); + } + + @Test + void superAdminIsGrantedOnSuperAdminOnlyRouteWithoutStaffLookup() throws Exception { + PermissionService permissionService = mock(PermissionService.class); + StaffLookupCache staffLookupCache = mock(StaffLookupCache.class); + PanelPermissionFilter filter = new PanelPermissionFilter(permissionService, roleAuthorization(permissionService, staffLookupCache), POLICY_RESOLVER); + when(permissionService.isSuperAdmin(SERVER, SUPER_ADMIN_EMAIL)).thenReturn(true); + MockHttpServletRequest request = authenticated("GET", RESTMappingV1.PANEL_BILLING + "/status", SUPER_ADMIN_EMAIL); + MockHttpServletResponse response = new MockHttpServletResponse(); + FilterChain chain = mock(FilterChain.class); + + filter.doFilter(request, response, chain); + + verify(chain).doFilter(request, response); + verifyNoInteractions(staffLookupCache); + } + private MockHttpServletRequest authenticated(String method, String path, String email) { MockHttpServletRequest request = new MockHttpServletRequest(method, path); request.setAttribute(RequestAttribute.SERVER, SERVER); @@ -144,4 +186,9 @@ private MockHttpServletRequest authenticated(String method, String path, String request.setAttribute(RequestAttribute.SESSION, session); return request; } + + private static RoleAuthorization roleAuthorization(PermissionService permissionService, + StaffLookupCache staffLookupCache) { + return new RoleAuthorization(permissionService, mock(StaffMongoRepository.class), staffLookupCache); + } } diff --git a/src/test/java/gg/modl/backend/player/service/PunishmentLifecycleServiceReasonValidationTest.java b/src/test/java/gg/modl/backend/player/service/PunishmentLifecycleServiceReasonValidationTest.java index 1d87e58..c17f4e6 100644 --- a/src/test/java/gg/modl/backend/player/service/PunishmentLifecycleServiceReasonValidationTest.java +++ b/src/test/java/gg/modl/backend/player/service/PunishmentLifecycleServiceReasonValidationTest.java @@ -10,13 +10,13 @@ import gg.modl.backend.database.mongo.repository.PlayerMongoRepository; import gg.modl.backend.database.mongo.repository.PunishmentMongoRepository; -import gg.modl.backend.database.mongo.repository.StaffMongoRepository; import gg.modl.backend.infrastructure.exception.ResourceNotFoundException; import gg.modl.backend.infrastructure.exception.ValidationException; import gg.modl.backend.infrastructure.validation.RequestValidationLimits; import gg.modl.backend.log.service.LogService; import gg.modl.backend.player.dto.request.CreatePunishmentRequest; import gg.modl.backend.role.service.PermissionService; +import gg.modl.backend.role.service.RoleAuthorization; import gg.modl.backend.server.data.Server; import gg.modl.backend.settings.service.OffenderThresholdSettingsService; import gg.modl.backend.settings.service.PunishmentTypeService; @@ -49,9 +49,9 @@ void setUp() { mock(OffenderThresholdSettingsService.class), mock(PunishmentDurationCalculator.class), mock(IssuerNameResolver.class), - mock(StaffMongoRepository.class), mock(PunishmentQueryService.class), mock(PermissionService.class), + mock(RoleAuthorization.class), mock(WebhookSettingsService.class), mock(PunishmentRealtimePublisher.class), mock(LogService.class) diff --git a/src/test/java/gg/modl/backend/player/service/PunishmentPermissionAuthorityTest.java b/src/test/java/gg/modl/backend/player/service/PunishmentPermissionAuthorityTest.java new file mode 100644 index 0000000..8986076 --- /dev/null +++ b/src/test/java/gg/modl/backend/player/service/PunishmentPermissionAuthorityTest.java @@ -0,0 +1,127 @@ +package gg.modl.backend.player.service; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import gg.modl.backend.database.mongo.repository.PlayerMongoRepository; +import gg.modl.backend.database.mongo.repository.PunishmentMongoRepository; +import gg.modl.backend.database.mongo.repository.StaffMongoRepository; +import gg.modl.backend.infrastructure.exception.ForbiddenException; +import gg.modl.backend.log.service.LogService; +import gg.modl.backend.role.service.PermissionService; +import gg.modl.backend.role.service.RoleAuthorization; +import gg.modl.backend.server.data.Server; +import gg.modl.backend.server.data.ServerPlan; +import gg.modl.backend.settings.data.PunishmentType; +import gg.modl.backend.settings.service.OffenderThresholdSettingsService; +import gg.modl.backend.settings.service.PunishmentTypeService; +import gg.modl.backend.settings.service.WebhookSettingsService; +import gg.modl.backend.staff.data.Staff; +import gg.modl.backend.staff.service.StaffLookupCache; +import gg.modl.backend.ticket.service.TicketService; +import java.util.Optional; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +class PunishmentPermissionAuthorityTest { + + private static final String ADMIN_EMAIL = "admin@example.com"; + private static final String STALE_SUPER_ADMIN_EMAIL = "old-owner@example.com"; + private static final String MODERATOR_EMAIL = "mod@example.com"; + private static final String MODERATOR_ROLE_ID = "moderator"; + private static final int BAN_ORDINAL = 2; + private static final String BAN_APPLY_PERMISSION = "punishment.apply.manual-ban"; + + private Server server; + private PermissionService permissionService; + private StaffMongoRepository staffRepository; + private StaffLookupCache staffLookupCache; + private PunishmentLifecycleService lifecycleService; + + @BeforeEach + void setUp() { + server = new Server("server", "domain", "db", ADMIN_EMAIL, true, ServerPlan.FREE); + server.setId("server-id"); + + permissionService = mock(PermissionService.class); + staffRepository = mock(StaffMongoRepository.class); + staffLookupCache = mock(StaffLookupCache.class); + + PunishmentTypeService punishmentTypeService = mock(PunishmentTypeService.class); + PunishmentType banType = new PunishmentType(); + banType.setName("Manual Ban"); + when(punishmentTypeService.getPunishmentTypeByOrdinal(server, BAN_ORDINAL)).thenReturn(Optional.of(banType)); + + lifecycleService = new PunishmentLifecycleService( + mock(PlayerMongoRepository.class), + mock(PunishmentMongoRepository.class), + mock(TicketService.class), + mock(PlayerStatusCalculator.class), + punishmentTypeService, + mock(OffenderThresholdSettingsService.class), + mock(PunishmentDurationCalculator.class), + mock(IssuerNameResolver.class), + mock(PunishmentQueryService.class), + permissionService, + new RoleAuthorization(permissionService, staffRepository, staffLookupCache), + mock(WebhookSettingsService.class), + mock(PunishmentRealtimePublisher.class), + mock(LogService.class) + ); + } + + private void givenStaff(String email, String roleId) { + Staff staff = Staff.builder().email(email).roleId(roleId).build(); + when(staffLookupCache.findByEmail(server, email)).thenReturn(Optional.of(staff)); + } + + @Test + void staleSuperAdminRoleOnANonAdminEmailGrantsNothing() { + givenStaff(STALE_SUPER_ADMIN_EMAIL, RoleAuthorization.SUPER_ADMIN_ROLE_ID); + when(permissionService.hasPermission(server, RoleAuthorization.SUPER_ADMIN_ROLE_ID, BAN_APPLY_PERMISSION)) + .thenReturn(true); + + assertThrows(ForbiddenException.class, + () -> lifecycleService.validatePunishmentPermission(server, STALE_SUPER_ADMIN_EMAIL, BAN_ORDINAL)); + } + + @Test + void serverAdministratorBypassesTypePermissions() { + assertDoesNotThrow(() -> lifecycleService.validatePunishmentPermission(server, ADMIN_EMAIL, BAN_ORDINAL)); + } + + @Test + void staffWithTheApplyPermissionIsAllowed() { + givenStaff(MODERATOR_EMAIL, MODERATOR_ROLE_ID); + when(permissionService.hasPermission(server, MODERATOR_ROLE_ID, BAN_APPLY_PERMISSION)).thenReturn(true); + + assertDoesNotThrow(() -> lifecycleService.validatePunishmentPermission(server, MODERATOR_EMAIL, BAN_ORDINAL)); + } + + @Test + void staffWithoutTheApplyPermissionIsRejected() { + givenStaff(MODERATOR_EMAIL, MODERATOR_ROLE_ID); + when(permissionService.hasPermission(server, MODERATOR_ROLE_ID, BAN_APPLY_PERMISSION)).thenReturn(false); + + assertThrows(ForbiddenException.class, + () -> lifecycleService.validatePunishmentPermission(server, MODERATOR_EMAIL, BAN_ORDINAL)); + } + + @Test + void anEmailWithNoStaffRecordIsRejected() { + when(staffLookupCache.findByEmail(server, "ghost@example.com")).thenReturn(Optional.empty()); + + assertThrows(ForbiddenException.class, + () -> lifecycleService.validatePunishmentPermission(server, "ghost@example.com", BAN_ORDINAL)); + } + + @Test + void anUnauthenticatedCallerIsRejected() { + assertThrows(ForbiddenException.class, + () -> lifecycleService.validatePunishmentPermission(server, null, BAN_ORDINAL)); + assertThrows(ForbiddenException.class, + () -> lifecycleService.validatePunishmentPermission(server, " ", BAN_ORDINAL)); + } +} diff --git a/src/test/java/gg/modl/backend/player/service/PunishmentServiceTest.java b/src/test/java/gg/modl/backend/player/service/PunishmentServiceTest.java index bb0281d..76f7ba3 100644 --- a/src/test/java/gg/modl/backend/player/service/PunishmentServiceTest.java +++ b/src/test/java/gg/modl/backend/player/service/PunishmentServiceTest.java @@ -12,7 +12,6 @@ import gg.modl.backend.database.mongo.repository.PlayerMongoRepository; import gg.modl.backend.database.mongo.repository.PunishmentMongoRepository; -import gg.modl.backend.database.mongo.repository.StaffMongoRepository; import gg.modl.backend.ticket.service.AppealWorkflowTransitionService; import gg.modl.backend.ticket.service.TicketService; import gg.modl.backend.player.data.Player; @@ -27,6 +26,7 @@ import gg.modl.backend.server.data.ServerPlan; import gg.modl.backend.settings.service.OffenderThresholdSettingsService; import gg.modl.backend.role.service.PermissionService; +import gg.modl.backend.role.service.RoleAuthorization; import gg.modl.backend.settings.service.PunishmentTypeService; import gg.modl.backend.settings.service.WebhookSettingsService; import gg.modl.backend.log.service.LogService; @@ -73,15 +73,15 @@ class PunishmentServiceTest { @Mock private IssuerNameResolver issuerNameResolver; - @Mock - private StaffMongoRepository staffRepository; - @Mock private PunishmentQueryService punishmentQueryService; @Mock private PermissionService permissionService; + @Mock + private RoleAuthorization roleAuthorization; + @Mock private WebhookSettingsService webhookSettingsService; @@ -106,9 +106,9 @@ void setUp() { thresholdSettingsService, durationCalculator, issuerNameResolver, - staffRepository, punishmentQueryService, permissionService, + roleAuthorization, webhookSettingsService, realtimePublisher, logService @@ -119,7 +119,6 @@ void setUp() { ticketService, appealWorkflowTransitionService, issuerNameResolver, - staffRepository, punishmentQueryService, punishmentLifecycleService, realtimePublisher diff --git a/src/test/java/gg/modl/backend/realtime/auth/RealtimeTopicAuthorizerTest.java b/src/test/java/gg/modl/backend/realtime/auth/RealtimeTopicAuthorizerTest.java index feab914..7d0d40c 100644 --- a/src/test/java/gg/modl/backend/realtime/auth/RealtimeTopicAuthorizerTest.java +++ b/src/test/java/gg/modl/backend/realtime/auth/RealtimeTopicAuthorizerTest.java @@ -11,6 +11,8 @@ import gg.modl.backend.staff.data.Staff; import gg.modl.backend.staff.service.StaffLookupCache; import gg.modl.proto.modl.v1.Topic; +import gg.modl.backend.role.service.RoleAuthorization; +import gg.modl.backend.database.mongo.repository.StaffMongoRepository; import java.util.Optional; import org.junit.jupiter.api.Test; @@ -20,7 +22,7 @@ class RealtimeTopicAuthorizerTest { void panelTicketTopicsRequireReadPermission() { PermissionService permissionService = mock(PermissionService.class); StaffLookupCache staffLookupCache = mock(StaffLookupCache.class); - RealtimeTopicAuthorizer authorizer = new RealtimeTopicAuthorizer(permissionService, staffLookupCache); + RealtimeTopicAuthorizer authorizer = new RealtimeTopicAuthorizer(permissionService, roleAuthorization(permissionService, staffLookupCache)); Server server = server(); Staff staff = Staff.builder() @@ -40,7 +42,7 @@ void panelTicketTopicsRequireReadPermission() { void panelPermissionScopesAreEnforcedPerTopic() { PermissionService permissionService = mock(PermissionService.class); StaffLookupCache staffLookupCache = mock(StaffLookupCache.class); - RealtimeTopicAuthorizer authorizer = new RealtimeTopicAuthorizer(permissionService, staffLookupCache); + RealtimeTopicAuthorizer authorizer = new RealtimeTopicAuthorizer(permissionService, roleAuthorization(permissionService, staffLookupCache)); Server server = server(); Staff staff = Staff.builder() @@ -57,7 +59,7 @@ void panelPermissionScopesAreEnforcedPerTopic() { @Test void panelCannotSubscribeToMinecraftTopics() { - RealtimeTopicAuthorizer authorizer = new RealtimeTopicAuthorizer(mock(PermissionService.class), mock(StaffLookupCache.class)); + RealtimeTopicAuthorizer authorizer = new RealtimeTopicAuthorizer(mock(PermissionService.class), mock(RoleAuthorization.class)); assertFalse(authorizer.canSubscribe( RealtimePrincipal.panel(server(), "staff@example.com"), @@ -67,7 +69,7 @@ void panelCannotSubscribeToMinecraftTopics() { @Test void minecraftCanSubscribeToAllMinecraftTopics() { - RealtimeTopicAuthorizer authorizer = new RealtimeTopicAuthorizer(mock(PermissionService.class), mock(StaffLookupCache.class)); + RealtimeTopicAuthorizer authorizer = new RealtimeTopicAuthorizer(mock(PermissionService.class), mock(RoleAuthorization.class)); RealtimePrincipal principal = RealtimePrincipal.minecraft(server(), "instance-1"); assertTrue(authorizer.canSubscribe(principal, Topic.TOPIC_MINECRAFT_PERMISSIONS)); @@ -87,4 +89,9 @@ private Server server() { server.setId("server-id"); return server; } + + private static RoleAuthorization roleAuthorization(PermissionService permissionService, + StaffLookupCache staffLookupCache) { + return new RoleAuthorization(permissionService, mock(StaffMongoRepository.class), staffLookupCache); + } } diff --git a/src/test/java/gg/modl/backend/role/service/PermissionServiceTest.java b/src/test/java/gg/modl/backend/role/service/PermissionServiceTest.java index 1cdd7a1..50896e3 100644 --- a/src/test/java/gg/modl/backend/role/service/PermissionServiceTest.java +++ b/src/test/java/gg/modl/backend/role/service/PermissionServiceTest.java @@ -83,6 +83,22 @@ void resolveRoleNamesBuildsIdToNameMap() { assertEquals("Helper", names.get("helper")); } + @Test + void superAdminOnlyPermissionsAreExcludedFromTheGrantableCatalog() { + PermissionService service = newService(mock(StaffRoleMongoRepository.class)); + Server server = server(); + + List all = service.getAllPermissionIds(server); + List grantable = service.getGrantablePermissionIds(server); + + assertTrue(all.containsAll(PermissionService.superAdminOnlyPermissionIds())); + assertTrue(PermissionService.superAdminOnlyPermissionIds().stream().noneMatch(grantable::contains)); + assertTrue(PermissionService.isSuperAdminOnly(PermissionService.ADMIN_SETTINGS_VIEW_BILLING)); + assertTrue(PermissionService.isSuperAdminOnly(PermissionService.ADMIN_SETTINGS_MODIFY_BILLING)); + assertTrue(PermissionService.isSuperAdminOnly(PermissionService.ADMIN_AUDIT_ROLLBACK)); + assertFalse(PermissionService.isSuperAdminOnly(PermissionService.ADMIN_SETTINGS_VIEW)); + } + private PermissionService newService(StaffRoleMongoRepository roleRepository) { return new PermissionService(roleRepository, mock(PunishmentTypeService.class), mock(StaffMongoRepository.class)); } diff --git a/src/test/java/gg/modl/backend/role/service/RoleAuthorizationTest.java b/src/test/java/gg/modl/backend/role/service/RoleAuthorizationTest.java index c0b6b05..5824f2b 100644 --- a/src/test/java/gg/modl/backend/role/service/RoleAuthorizationTest.java +++ b/src/test/java/gg/modl/backend/role/service/RoleAuthorizationTest.java @@ -20,6 +20,7 @@ import gg.modl.backend.server.service.ServerTimestampService; import gg.modl.backend.settings.service.PunishmentTypeService; import gg.modl.backend.staff.data.Staff; +import gg.modl.backend.staff.service.StaffLookupCache; import java.util.ArrayList; import java.util.List; import java.util.Optional; @@ -48,7 +49,7 @@ void setUp() { punishmentTypeService = mock(PunishmentTypeService.class); serverTimestampService = mock(ServerTimestampService.class); permissionService = new PermissionService(roleRepository, punishmentTypeService, staffRepository); - roleAuthorization = new RoleAuthorization(permissionService, staffRepository); + roleAuthorization = new RoleAuthorization(permissionService, staffRepository, mock(StaffLookupCache.class)); server = new Server("server", "domain", "db", ADMIN_EMAIL, true, ServerPlan.FREE); server.setId("server-id"); when(punishmentTypeService.getPunishmentTypes(server)).thenReturn(List.of()); @@ -223,6 +224,52 @@ void permissionedNonSuperAdminCannotManageEqualOrHigherRole() { assertEquals(HIGHER_AUTHORITY_MESSAGE, higherError.getMessage()); } + @Test + void effectivePermissionIdsExpandSettingsViewWithoutBilling() { + givenRole("admin", 1, PermissionService.ADMIN_SETTINGS_VIEW); + + List effective = roleAuthorization.effectivePermissionIds(server, staffPerformer("admin")); + + assertTrue(effective.contains(PermissionService.ADMIN_SETTINGS_VIEW)); + assertTrue(effective.contains("admin.settings.view.content")); + assertTrue(effective.contains("admin.settings.view.domain")); + assertTrue(effective.contains("admin.settings.view.storage")); + assertTrue(effective.contains("admin.settings.view.migration")); + assertTrue(effective.contains(PermissionService.ADMIN_SETTINGS_VIEW_PUNISHMENTS)); + assertFalse(effective.contains(PermissionService.ADMIN_SETTINGS_VIEW_BILLING)); + } + + @Test + void effectivePermissionIdsForSuperAdminIncludeSuperAdminOnlyPermissions() { + List effective = roleAuthorization.effectivePermissionIds(server, + new RoleAuthorization.PerformerAuthority(ADMIN_EMAIL, null, true, true)); + + assertTrue(effective.containsAll(PermissionService.superAdminOnlyPermissionIds())); + } + + @Test + void effectivePermissionIdsDropStoredSuperAdminOnlyPermissionWithoutMigration() { + givenRole("admin", 1, PermissionService.ADMIN_SETTINGS_VIEW_BILLING, + PermissionService.ADMIN_AUDIT_ROLLBACK, "ticket.view.all"); + + List effective = roleAuthorization.effectivePermissionIds(server, staffPerformer("admin")); + + assertFalse(effective.contains(PermissionService.ADMIN_SETTINGS_VIEW_BILLING)); + assertFalse(effective.contains(PermissionService.ADMIN_AUDIT_ROLLBACK)); + assertTrue(effective.contains("ticket.view.all")); + assertFalse(permissionService.hasPermission(server, "admin", PermissionService.ADMIN_SETTINGS_VIEW_BILLING)); + assertFalse(permissionService.hasPermission(server, "admin", PermissionService.ADMIN_AUDIT_ROLLBACK)); + } + + @Test + void effectivePermissionIdsAreEmptyWithoutARole() { + assertTrue(roleAuthorization.effectivePermissionIds(server, RoleAuthorization.PerformerAuthority.unidentified()).isEmpty()); + } + + private RoleAuthorization.PerformerAuthority staffPerformer(String roleId) { + return new RoleAuthorization.PerformerAuthority("staff@example.com", roleId, false, true); + } + private RoleService roleService() { return new RoleService( roleRepository, diff --git a/src/test/java/gg/modl/backend/role/service/RoleServiceTest.java b/src/test/java/gg/modl/backend/role/service/RoleServiceTest.java index ddea415..e4a5c87 100644 --- a/src/test/java/gg/modl/backend/role/service/RoleServiceTest.java +++ b/src/test/java/gg/modl/backend/role/service/RoleServiceTest.java @@ -18,6 +18,7 @@ import gg.modl.backend.server.data.Server; import gg.modl.backend.server.data.ServerPlan; import gg.modl.backend.server.service.ServerTimestampService; +import gg.modl.backend.staff.service.StaffLookupCache; import java.util.ArrayList; import java.util.List; import java.util.Optional; @@ -38,12 +39,12 @@ void defaultTicketRolesIncludeAppealModifyPermission() { roleRepository, staffRepository, permissionService, - new RoleAuthorization(permissionService, staffRepository), + new RoleAuthorization(permissionService, staffRepository, mock(StaffLookupCache.class)), mock(ServerTimestampService.class) ); Server server = new Server("server", "domain", "db", "admin@example.com", true, ServerPlan.FREE); when(permissionService.getPunishmentPermissions(server)).thenReturn(List.of()); - when(permissionService.getAllPermissionIds(server)).thenReturn(List.of( + when(permissionService.getGrantablePermissionIds(server)).thenReturn(List.of( "ticket.view.all", "ticket.reply.all", "appeal.modify" @@ -54,6 +55,7 @@ void defaultTicketRolesIncludeAppealModifyPermission() { ArgumentCaptor captor = ArgumentCaptor.forClass(StaffRole.class); verify(roleRepository, org.mockito.Mockito.times(4)).insertRoleIfAbsent(org.mockito.Mockito.eq(server), captor.capture()); for (StaffRole role : captor.getAllValues()) { + assertFalse(role.getPermissions().stream().anyMatch(PermissionService::isSuperAdminOnly), role.getId()); if (!"super-admin".equals(role.getId())) { assertTrue(role.getPermissions().contains("appeal.modify"), role.getId()); } @@ -96,7 +98,7 @@ void updateRolePermissionsFiltersInvalidPermissionIds() { .permissions(new ArrayList<>(List.of("ticket.reply.all", "ticket.close.all"))) .build(); when(roleRepository.findById(server, "custom-1")).thenReturn(Optional.of(role)); - when(permissionService.getAllPermissionIds(server)).thenReturn(List.of("ticket.reply.all", "ticket.close.all")); + when(permissionService.getGrantablePermissionIds(server)).thenReturn(List.of("ticket.reply.all", "ticket.close.all")); when(roleRepository.saveEntity(eq(server), any())).thenAnswer(inv -> inv.getArgument(1)); boolean result = roleService.updateRolePermissions( @@ -123,7 +125,7 @@ void updateRolePermissionsRejectsExpansionWithPerformerIdentity() { .permissions(new ArrayList<>(List.of("ticket.reply.all"))) .build(); when(roleRepository.findById(server, "custom-target")).thenReturn(Optional.of(targetRole)); - when(permissionService.getAllPermissionIds(server)).thenReturn(List.of("ticket.reply.all", "punishment.modify")); + when(permissionService.getGrantablePermissionIds(server)).thenReturn(List.of("ticket.reply.all", "punishment.modify")); when(permissionService.getRoleById(server, "custom-performer")).thenReturn(Optional.of(performerRole)); when(permissionService.hasPermission(server, "custom-performer", RoleAuthorization.MANAGE_ROLES_PERMISSION)) .thenReturn(true); @@ -146,7 +148,7 @@ private RoleService roleService(StaffRoleMongoRepository roleRepository, Permiss roleRepository, staffRepository, permissionService, - new RoleAuthorization(permissionService, staffRepository), + new RoleAuthorization(permissionService, staffRepository, mock(StaffLookupCache.class)), mock(ServerTimestampService.class)); } diff --git a/src/test/java/gg/modl/backend/staff/service/StaffServiceRoleDisplayTest.java b/src/test/java/gg/modl/backend/staff/service/StaffServiceRoleDisplayTest.java new file mode 100644 index 0000000..0d410a4 --- /dev/null +++ b/src/test/java/gg/modl/backend/staff/service/StaffServiceRoleDisplayTest.java @@ -0,0 +1,100 @@ +package gg.modl.backend.staff.service; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import gg.modl.backend.auth.WebAuthnService; +import gg.modl.backend.auth.session.SessionService; +import gg.modl.backend.database.mongo.repository.InvitationMongoRepository; +import gg.modl.backend.database.mongo.repository.StaffMongoRepository; +import gg.modl.backend.database.mongo.repository.StaffRoleMongoRepository; +import gg.modl.backend.role.data.StaffRole; +import gg.modl.backend.role.service.PermissionService; +import gg.modl.backend.role.service.RoleAuthorization; +import gg.modl.backend.server.data.Server; +import gg.modl.backend.server.data.ServerPlan; +import gg.modl.backend.server.service.ServerTimestampService; +import gg.modl.backend.settings.service.GeneralSettingsService; +import gg.modl.backend.settings.service.PunishmentTypeService; +import gg.modl.backend.staff.data.Staff; +import gg.modl.backend.staff.dto.response.StaffResponse; +import java.util.Date; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import org.junit.jupiter.api.Test; + +class StaffServiceRoleDisplayTest { + + private static final String ADMIN_EMAIL = "owner@example.com"; + private static final String STALE_EMAIL = "stale@example.com"; + + @Test + void staleSuperAdminRoleStillRendersItsStoredRoleName() { + StaffMongoRepository staffRepository = mock(StaffMongoRepository.class); + PermissionService permissionService = mock(PermissionService.class); + InvitationMongoRepository invitationRepository = mock(InvitationMongoRepository.class); + Server server = server(); + + when(staffRepository.findAll(server)).thenReturn(List.of(staleSuperAdminStaff())); + when(invitationRepository.findActiveInvitations(eq(server), any(Date.class))).thenReturn(List.of()); + when(permissionService.resolveRoleNames(eq(server), any())) + .thenReturn(Map.of(RoleAuthorization.SUPER_ADMIN_ROLE_ID, RoleAuthorization.SUPER_ADMIN_ROLE_NAME)); + + List staff = staffService(invitationRepository, staffRepository, permissionService).getAllStaff(server); + + assertEquals(RoleAuthorization.SUPER_ADMIN_ROLE_NAME, + staff.stream().filter(entry -> STALE_EMAIL.equals(entry.email())).findFirst().orElseThrow().role()); + } + + @Test + void staleSuperAdminRoleConfersNoEffectiveRoleName() { + StaffRoleMongoRepository roleRepository = mock(StaffRoleMongoRepository.class); + PermissionService permissionService = + new PermissionService(roleRepository, mock(PunishmentTypeService.class), mock(StaffMongoRepository.class)); + Server server = server(); + when(roleRepository.findById(server, RoleAuthorization.SUPER_ADMIN_ROLE_ID)).thenReturn(Optional.of(StaffRole.builder() + .id(RoleAuthorization.SUPER_ADMIN_ROLE_ID) + .name(RoleAuthorization.SUPER_ADMIN_ROLE_NAME) + .build())); + + Staff stale = staleSuperAdminStaff(); + + assertEquals(RoleAuthorization.SUPER_ADMIN_ROLE_NAME, permissionService.assignedRoleName(server, stale)); + assertEquals("", permissionService.effectiveRoleName(server, stale)); + } + + private StaffService staffService(InvitationMongoRepository invitationRepository, + StaffMongoRepository staffRepository, + PermissionService permissionService) { + return new StaffService( + invitationRepository, + staffRepository, + permissionService, + mock(RoleAuthorization.class), + mock(ServerTimestampService.class), + mock(WebAuthnService.class), + mock(SessionService.class), + mock(GeneralSettingsService.class), + mock(StaffLookupCache.class) + ); + } + + private Staff staleSuperAdminStaff() { + return Staff.builder() + .id("stale-staff-id") + .email(STALE_EMAIL) + .username("stale") + .roleId(RoleAuthorization.SUPER_ADMIN_ROLE_ID) + .build(); + } + + private Server server() { + Server server = new Server("Server", "server", "server_db", ADMIN_EMAIL, true, ServerPlan.FREE); + server.setId("server-id"); + return server; + } +} diff --git a/src/test/java/gg/modl/backend/staff/service/StaffServiceRoleSecurityTest.java b/src/test/java/gg/modl/backend/staff/service/StaffServiceRoleSecurityTest.java index 51bf6c9..dfceb5c 100644 --- a/src/test/java/gg/modl/backend/staff/service/StaffServiceRoleSecurityTest.java +++ b/src/test/java/gg/modl/backend/staff/service/StaffServiceRoleSecurityTest.java @@ -20,6 +20,7 @@ import gg.modl.backend.server.data.ServerPlan; import gg.modl.backend.server.service.ServerTimestampService; import gg.modl.backend.staff.data.Staff; +import gg.modl.backend.staff.service.StaffLookupCache; import java.util.List; import java.util.Optional; import org.junit.jupiter.api.Test; @@ -28,7 +29,7 @@ class StaffServiceRoleSecurityTest { private final StaffMongoRepository staffRepository = mock(StaffMongoRepository.class); private final PermissionService permissionService = mock(PermissionService.class); - private final RoleAuthorization roleAuthorization = new RoleAuthorization(permissionService, staffRepository); + private final RoleAuthorization roleAuthorization = new RoleAuthorization(permissionService, staffRepository, mock(StaffLookupCache.class)); private final StaffService service = new StaffService( mock(InvitationMongoRepository.class), staffRepository,