From 23d00055142c8fb6173db21b06e6febb2e5482d1 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sat, 28 Mar 2026 08:05:15 +0100 Subject: [PATCH] fix: allow any user permission to read/update own notification preferences MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @RequirePermission now accepts Permission[] so a single annotation can express "any of these" rather than a single required permission. PermissionAspect updated accordingly — all existing single-value usages compile unchanged (Java auto-wraps scalars in arrays for annotation attrs). NotificationController: preference endpoints (GET/PUT /api/users/me/ notification-preferences) override the class-level READ_ALL gate with {READ_ALL, WRITE_ALL, ANNOTATE_ALL} so users without READ_ALL can still manage their own settings. Notification list endpoints retain READ_ALL. UserSearchController: same broadened set so ANNOTATE_ALL users can search for users to @mention when writing comments. Tests: added WRITE_ALL and ANNOTATE_ALL passing cases for preferences and user search; added 403 case for preferences with no permission; confirmed WRITE_ALL cannot reach notification list endpoints. Co-Authored-By: Claude Sonnet 4.6 --- .../controller/NotificationController.java | 2 + .../controller/UserSearchController.java | 2 +- .../security/PermissionAspect.java | 17 ++++-- .../security/RequirePermission.java | 2 +- .../NotificationControllerTest.java | 57 ++++++++++++++++++- .../controller/UserSearchControllerTest.java | 9 +++ 6 files changed, 80 insertions(+), 9 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/controller/NotificationController.java b/backend/src/main/java/org/raddatz/familienarchiv/controller/NotificationController.java index 75eee19e..1e2ddd63 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/controller/NotificationController.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/controller/NotificationController.java @@ -51,12 +51,14 @@ public class NotificationController { } @GetMapping("/api/users/me/notification-preferences") + @RequirePermission({Permission.READ_ALL, Permission.WRITE_ALL, Permission.ANNOTATE_ALL}) public NotificationPreferenceDTO getPreferences(Authentication authentication) { AppUser user = resolveUser(authentication); return new NotificationPreferenceDTO(user.isNotifyOnReply(), user.isNotifyOnMention()); } @PutMapping("/api/users/me/notification-preferences") + @RequirePermission({Permission.READ_ALL, Permission.WRITE_ALL, Permission.ANNOTATE_ALL}) public NotificationPreferenceDTO updatePreferences( @RequestBody NotificationPreferenceDTO dto, Authentication authentication) { diff --git a/backend/src/main/java/org/raddatz/familienarchiv/controller/UserSearchController.java b/backend/src/main/java/org/raddatz/familienarchiv/controller/UserSearchController.java index f2333d5e..5dbb51c8 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/controller/UserSearchController.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/controller/UserSearchController.java @@ -14,7 +14,7 @@ import java.util.List; @RestController @RequiredArgsConstructor -@RequirePermission(Permission.READ_ALL) +@RequirePermission({Permission.READ_ALL, Permission.WRITE_ALL, Permission.ANNOTATE_ALL}) public class UserSearchController { private final UserSearchService userSearchService; diff --git a/backend/src/main/java/org/raddatz/familienarchiv/security/PermissionAspect.java b/backend/src/main/java/org/raddatz/familienarchiv/security/PermissionAspect.java index 4a8c17f5..0b11ba31 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/security/PermissionAspect.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/security/PermissionAspect.java @@ -23,7 +23,7 @@ public class PermissionAspect { RequirePermission permission = getAnnotation(joinPoint); if (permission != null) { - validateUserAccess(permission.value()); + validateUserAccess(permission.value()); // value() is now Permission[] } return joinPoint.proceed(); @@ -43,18 +43,23 @@ public class PermissionAspect { return joinPoint.getTarget().getClass().getAnnotation(RequirePermission.class); } - private void validateUserAccess(Permission requiredPerm) { + private void validateUserAccess(Permission[] requiredPerms) { Authentication auth = SecurityContextHolder.getContext().getAuthentication(); if (auth == null || !auth.isAuthenticated()) { throw DomainException.unauthorized("Not authenticated"); } - boolean hasPermission = auth.getAuthorities().stream() - .anyMatch(a -> a.getAuthority().equals(requiredPerm.name())); + boolean hasAny = auth.getAuthorities().stream() + .anyMatch(a -> { + for (Permission p : requiredPerms) { + if (a.getAuthority().equals(p.name())) return true; + } + return false; + }); - if (!hasPermission) { - throw DomainException.forbidden("Missing required permission: " + requiredPerm.name()); + if (!hasAny) { + throw DomainException.forbidden("Missing required permission"); } } } diff --git a/backend/src/main/java/org/raddatz/familienarchiv/security/RequirePermission.java b/backend/src/main/java/org/raddatz/familienarchiv/security/RequirePermission.java index e71c5ba7..f2c89f12 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/security/RequirePermission.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/security/RequirePermission.java @@ -8,5 +8,5 @@ import java.lang.annotation.Target; @Target({ElementType.METHOD, ElementType.TYPE}) @Retention(RetentionPolicy.RUNTIME) public @interface RequirePermission { - Permission value(); // e.g. "ADMIN" or "WRITE_ALL" + Permission[] value(); // one or more — user needs any of the listed permissions } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/controller/NotificationControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/controller/NotificationControllerTest.java index 1014aac5..6c988d74 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/controller/NotificationControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/controller/NotificationControllerTest.java @@ -140,9 +140,16 @@ class NotificationControllerTest { .andExpect(status().isUnauthorized()); } + @Test + @WithMockUser(username = "testuser") + void getPreferences_returns403_whenUserHasNoPermission() throws Exception { + mockMvc.perform(get("/api/users/me/notification-preferences")) + .andExpect(status().isForbidden()); + } + @Test @WithMockUser(username = "testuser", authorities = {"READ_ALL"}) - void getPreferences_returnsCurrentPreferences() throws Exception { + void getPreferences_returns200_whenUserHasReadAll() throws Exception { AppUser user = AppUser.builder().id(USER_ID).username("testuser") .notifyOnReply(true).notifyOnMention(false).build(); when(userService.findByUsername("testuser")).thenReturn(user); @@ -153,6 +160,36 @@ class NotificationControllerTest { .andExpect(jsonPath("$.notifyOnMention").value(false)); } + @Test + @WithMockUser(username = "testuser", authorities = {"WRITE_ALL"}) + void getPreferences_returns200_whenUserHasWriteAll() throws Exception { + AppUser user = AppUser.builder().id(USER_ID).username("testuser") + .notifyOnReply(false).notifyOnMention(true).build(); + when(userService.findByUsername("testuser")).thenReturn(user); + + mockMvc.perform(get("/api/users/me/notification-preferences")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.notifyOnMention").value(true)); + } + + @Test + @WithMockUser(username = "testuser", authorities = {"ANNOTATE_ALL"}) + void getPreferences_returns200_whenUserHasAnnotateAll() throws Exception { + AppUser user = AppUser.builder().id(USER_ID).username("testuser") + .notifyOnReply(false).notifyOnMention(false).build(); + when(userService.findByUsername("testuser")).thenReturn(user); + + mockMvc.perform(get("/api/users/me/notification-preferences")) + .andExpect(status().isOk()); + } + + @Test + @WithMockUser(username = "testuser", authorities = {"WRITE_ALL"}) + void getNotifications_returns403_whenUserHasOnlyWriteAll() throws Exception { + mockMvc.perform(get("/api/notifications")) + .andExpect(status().isForbidden()); + } + // ─── PUT /api/users/me/notification-preferences ────────────────────────── @Test @@ -173,4 +210,22 @@ class NotificationControllerTest { .andExpect(jsonPath("$.notifyOnReply").value(true)) .andExpect(jsonPath("$.notifyOnMention").value(true)); } + + @Test + @WithMockUser(username = "testuser", authorities = {"WRITE_ALL"}) + void updatePreferences_returns200_whenUserHasWriteAll() throws Exception { + AppUser user = AppUser.builder().id(USER_ID).username("testuser") + .notifyOnReply(false).notifyOnMention(false).build(); + when(userService.findByUsername("testuser")).thenReturn(user); + + AppUser updated = AppUser.builder().id(USER_ID).username("testuser") + .notifyOnReply(true).notifyOnMention(false).build(); + when(notificationService.updatePreferences(USER_ID, true, false)).thenReturn(updated); + + mockMvc.perform(put("/api/users/me/notification-preferences") + .contentType(MediaType.APPLICATION_JSON) + .content("{\"notifyOnReply\":true,\"notifyOnMention\":false}")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.notifyOnReply").value(true)); + } } diff --git a/backend/src/test/java/org/raddatz/familienarchiv/controller/UserSearchControllerTest.java b/backend/src/test/java/org/raddatz/familienarchiv/controller/UserSearchControllerTest.java index 896ae452..0020a76c 100644 --- a/backend/src/test/java/org/raddatz/familienarchiv/controller/UserSearchControllerTest.java +++ b/backend/src/test/java/org/raddatz/familienarchiv/controller/UserSearchControllerTest.java @@ -47,6 +47,15 @@ class UserSearchControllerTest { .andExpect(status().isForbidden()); } + @Test + @WithMockUser(authorities = {"ANNOTATE_ALL"}) + void search_returns200_whenUserHasAnnotateAll() throws Exception { + when(userSearchService.search("Hans")).thenReturn(List.of()); + + mockMvc.perform(get("/api/users/search").param("q", "Hans")) + .andExpect(status().isOk()); + } + @Test @WithMockUser(authorities = {"READ_ALL"}) void search_returns200_whenAuthenticated() throws Exception {