Skip to content

Allow punishment-type reads for roles with punishment permissions #33

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
byteful merged 1 commit into from
Aug 10, 2026
Merged

Allow punishment-type reads for roles with punishment permissions #33

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/main/java/gg/modl/backend/infrastructure/authorization/PanelAccessPolicyResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ 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 REQUIRE_PERMISSION -> new ReadWritePermissionPolicy(viewPermission(annotation), modifyPermission(annotation));
};
Expand Down
1 change: 1 addition & 0 deletions src/main/java/gg/modl/backend/infrastructure/authorization/PanelAccessRule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,6 @@ public enum PanelAccessRule {
REQUIRE_PERMISSION,
PERMIT_ALL,
PLAYER_ACCESS,
PUNISHMENT_TYPE_ACCESS,
APPEAL_REPLY
}
4 changes: 1 addition & 3 deletions src/main/java/gg/modl/backend/infrastructure/authorization/PlayerAccessPolicy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@
public enum PlayerAccessPolicy implements PanelAccessPolicy {
INSTANCE;

private static final String PUNISHMENT_APPLY_PREFIX = "punishment.apply.";

@Override
public boolean permitsWithoutRole(PanelAccessRequest request) {
return false;
Expand All @@ -17,7 +15,7 @@ public boolean permitsWithRole(PanelAccessRequest request, PanelPrincipalPermiss
if (request.isReadOnly()) {
return permissions.has(PermissionService.PUNISHMENT_VIEW)
|| permissions.has(PermissionService.PUNISHMENT_MODIFY)
|| permissions.hasAnyWithPrefix(PUNISHMENT_APPLY_PREFIX);
|| permissions.hasAnyWithPrefix(PermissionService.PUNISHMENT_APPLY_PREFIX);
}
return permissions.has(PermissionService.PUNISHMENT_MODIFY);
}
Expand Down
23 changes: 23 additions & 0 deletions src/main/java/gg/modl/backend/infrastructure/authorization/PunishmentTypeAccessPolicy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package gg.modl.backend.infrastructure.authorization;

import gg.modl.backend.role.service.PermissionService;

public enum PunishmentTypeAccessPolicy implements PanelAccessPolicy {
INSTANCE;

@Override
public boolean permitsWithoutRole(PanelAccessRequest request) {
return false;
}

@Override
public boolean permitsWithRole(PanelAccessRequest request, PanelPrincipalPermissions permissions) {
if (request.isReadOnly()) {
return permissions.has(PermissionService.ADMIN_SETTINGS_VIEW_PUNISHMENTS)
|| permissions.has(PermissionService.PUNISHMENT_VIEW)
|| permissions.has(PermissionService.PUNISHMENT_MODIFY)
|| permissions.hasAnyWithPrefix(PermissionService.PUNISHMENT_APPLY_PREFIX);
}
return permissions.has(PermissionService.ADMIN_SETTINGS_MODIFY_PUNISHMENTS);
}
}
9 changes: 6 additions & 3 deletions src/main/java/gg/modl/backend/role/service/PermissionService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ public class PermissionService {
.build();

public static final String ADMIN_SETTINGS_VIEW = "admin.settings.view";
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.";
public static final String ADMIN_STAFF_MANAGE = "admin.staff.manage";
public static final String ADMIN_AUDIT_VIEW = "admin.audit.view";
public static final String ADMIN_AUDIT_ROLLBACK = "admin.audit.rollback";
Expand All @@ -56,14 +59,14 @@ public class PermissionService {

private static final List<Permission> BASE_PERMISSIONS = List.of(
new Permission(ADMIN_SETTINGS_VIEW, "View Settings", "View all system settings (includes all sub-permissions)", "admin"),
new Permission("admin.settings.view.punishments", "View Punishments Config", "View punishment type configuration", "admin", ADMIN_SETTINGS_VIEW),
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.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"),
new Permission("admin.settings.modify.punishments", "Modify Punishments Config", "Create/edit/delete punishment types", "admin",
new Permission(ADMIN_SETTINGS_MODIFY_PUNISHMENTS, "Modify Punishments Config", "Create/edit/delete punishment types", "admin",
"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"),
Expand Down Expand Up @@ -150,7 +153,7 @@ public List<Permission> getPunishmentPermissions(Server server) {
}

public static String punishmentApplyPermissionId(String typeName) {
return "punishment.apply." + typeName.toLowerCase().replace(" ", "-");
return PUNISHMENT_APPLY_PREFIX + typeName.toLowerCase().replace(" ", "-");
}

public void renamePunishmentApplyPermission(Server server, String oldName, String newName) {
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/gg/modl/backend/settings/controller/PanelPunishmentTypeController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
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.ValidationException;
import gg.modl.backend.infrastructure.validation.BeanValidationRunner;
Expand Down Expand Up @@ -30,7 +31,7 @@

@RestController
@RequestMapping(RESTMappingV1.PANEL_SETTINGS + "/punishment-types")
@RequiresPanelPermission(view = "admin.settings.view.punishments", modify = "admin.settings.modify.punishments")
@RequiresPanelPermission(rule = PanelAccessRule.PUNISHMENT_TYPE_ACCESS)
@RequiredArgsConstructor
public class PanelPunishmentTypeController {
private final PunishmentTypeService punishmentTypeService;
Expand Down
1 change: 1 addition & 0 deletions src/test/java/gg/modl/backend/infrastructure/filter/PanelAuthorizationMatrixTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ static Row appealReply(String path) {
Row.permission("POST", "/v1/panel/settings/ai-dismiss-suggestion/t1", "admin.settings.modify.punishments"),
Row.permission("GET", "/v1/panel/settings/punishment-types", "admin.settings.view.punishments"),
Row.permission("POST", "/v1/panel/settings/punishment-types", "admin.settings.modify.punishments"),
Row.playerRead("/v1/panel/settings/punishment-types"),

@greptile-apps greptile-apps Bot Aug 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Apply-only access remains untested

The new Row.playerRead case grants punishment.view, so it never exercises the hasAnyWithPrefix branch intended for roles holding only punishment.apply.<type>. A regression in that branch would restore the moderator-facing 403 while this authorization matrix continues to pass.

Context Used: Ensure all code meets SOLID, DRY, and KISS softwar... (source)

Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/test/java/gg/modl/backend/infrastructure/filter/PanelAuthorizationMatrixTest.java
Line: 101

Comment:
**Apply-only access remains untested**

The new `Row.playerRead` case grants `punishment.view`, so it never exercises the `hasAnyWithPrefix` branch intended for roles holding only `punishment.apply.<type>`. A regression in that branch would restore the moderator-facing 403 while this authorization matrix continues to pass.

**Context Used:** Ensure all code meets SOLID, DRY, and KISS softwar... ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Claude Code

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"),
Expand Down
Loading