fix: allow any user permission to read/update own notification preferences
@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 <noreply@anthropic.com>
This commit is contained in:
@@ -51,12 +51,14 @@ public class NotificationController {
|
|||||||
}
|
}
|
||||||
|
|
||||||
@GetMapping("/api/users/me/notification-preferences")
|
@GetMapping("/api/users/me/notification-preferences")
|
||||||
|
@RequirePermission({Permission.READ_ALL, Permission.WRITE_ALL, Permission.ANNOTATE_ALL})
|
||||||
public NotificationPreferenceDTO getPreferences(Authentication authentication) {
|
public NotificationPreferenceDTO getPreferences(Authentication authentication) {
|
||||||
AppUser user = resolveUser(authentication);
|
AppUser user = resolveUser(authentication);
|
||||||
return new NotificationPreferenceDTO(user.isNotifyOnReply(), user.isNotifyOnMention());
|
return new NotificationPreferenceDTO(user.isNotifyOnReply(), user.isNotifyOnMention());
|
||||||
}
|
}
|
||||||
|
|
||||||
@PutMapping("/api/users/me/notification-preferences")
|
@PutMapping("/api/users/me/notification-preferences")
|
||||||
|
@RequirePermission({Permission.READ_ALL, Permission.WRITE_ALL, Permission.ANNOTATE_ALL})
|
||||||
public NotificationPreferenceDTO updatePreferences(
|
public NotificationPreferenceDTO updatePreferences(
|
||||||
@RequestBody NotificationPreferenceDTO dto,
|
@RequestBody NotificationPreferenceDTO dto,
|
||||||
Authentication authentication) {
|
Authentication authentication) {
|
||||||
|
|||||||
@@ -14,7 +14,7 @@ import java.util.List;
|
|||||||
|
|
||||||
@RestController
|
@RestController
|
||||||
@RequiredArgsConstructor
|
@RequiredArgsConstructor
|
||||||
@RequirePermission(Permission.READ_ALL)
|
@RequirePermission({Permission.READ_ALL, Permission.WRITE_ALL, Permission.ANNOTATE_ALL})
|
||||||
public class UserSearchController {
|
public class UserSearchController {
|
||||||
|
|
||||||
private final UserSearchService userSearchService;
|
private final UserSearchService userSearchService;
|
||||||
|
|||||||
@@ -23,7 +23,7 @@ public class PermissionAspect {
|
|||||||
RequirePermission permission = getAnnotation(joinPoint);
|
RequirePermission permission = getAnnotation(joinPoint);
|
||||||
|
|
||||||
if (permission != null) {
|
if (permission != null) {
|
||||||
validateUserAccess(permission.value());
|
validateUserAccess(permission.value()); // value() is now Permission[]
|
||||||
}
|
}
|
||||||
|
|
||||||
return joinPoint.proceed();
|
return joinPoint.proceed();
|
||||||
@@ -43,18 +43,23 @@ public class PermissionAspect {
|
|||||||
return joinPoint.getTarget().getClass().getAnnotation(RequirePermission.class);
|
return joinPoint.getTarget().getClass().getAnnotation(RequirePermission.class);
|
||||||
}
|
}
|
||||||
|
|
||||||
private void validateUserAccess(Permission requiredPerm) {
|
private void validateUserAccess(Permission[] requiredPerms) {
|
||||||
Authentication auth = SecurityContextHolder.getContext().getAuthentication();
|
Authentication auth = SecurityContextHolder.getContext().getAuthentication();
|
||||||
|
|
||||||
if (auth == null || !auth.isAuthenticated()) {
|
if (auth == null || !auth.isAuthenticated()) {
|
||||||
throw DomainException.unauthorized("Not authenticated");
|
throw DomainException.unauthorized("Not authenticated");
|
||||||
}
|
}
|
||||||
|
|
||||||
boolean hasPermission = auth.getAuthorities().stream()
|
boolean hasAny = auth.getAuthorities().stream()
|
||||||
.anyMatch(a -> a.getAuthority().equals(requiredPerm.name()));
|
.anyMatch(a -> {
|
||||||
|
for (Permission p : requiredPerms) {
|
||||||
|
if (a.getAuthority().equals(p.name())) return true;
|
||||||
|
}
|
||||||
|
return false;
|
||||||
|
});
|
||||||
|
|
||||||
if (!hasPermission) {
|
if (!hasAny) {
|
||||||
throw DomainException.forbidden("Missing required permission: " + requiredPerm.name());
|
throw DomainException.forbidden("Missing required permission");
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -8,5 +8,5 @@ import java.lang.annotation.Target;
|
|||||||
@Target({ElementType.METHOD, ElementType.TYPE})
|
@Target({ElementType.METHOD, ElementType.TYPE})
|
||||||
@Retention(RetentionPolicy.RUNTIME)
|
@Retention(RetentionPolicy.RUNTIME)
|
||||||
public @interface RequirePermission {
|
public @interface RequirePermission {
|
||||||
Permission value(); // e.g. "ADMIN" or "WRITE_ALL"
|
Permission[] value(); // one or more — user needs any of the listed permissions
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -140,9 +140,16 @@ class NotificationControllerTest {
|
|||||||
.andExpect(status().isUnauthorized());
|
.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
|
@Test
|
||||||
@WithMockUser(username = "testuser", authorities = {"READ_ALL"})
|
@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")
|
AppUser user = AppUser.builder().id(USER_ID).username("testuser")
|
||||||
.notifyOnReply(true).notifyOnMention(false).build();
|
.notifyOnReply(true).notifyOnMention(false).build();
|
||||||
when(userService.findByUsername("testuser")).thenReturn(user);
|
when(userService.findByUsername("testuser")).thenReturn(user);
|
||||||
@@ -153,6 +160,36 @@ class NotificationControllerTest {
|
|||||||
.andExpect(jsonPath("$.notifyOnMention").value(false));
|
.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 ──────────────────────────
|
// ─── PUT /api/users/me/notification-preferences ──────────────────────────
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
@@ -173,4 +210,22 @@ class NotificationControllerTest {
|
|||||||
.andExpect(jsonPath("$.notifyOnReply").value(true))
|
.andExpect(jsonPath("$.notifyOnReply").value(true))
|
||||||
.andExpect(jsonPath("$.notifyOnMention").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));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -47,6 +47,15 @@ class UserSearchControllerTest {
|
|||||||
.andExpect(status().isForbidden());
|
.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
|
@Test
|
||||||
@WithMockUser(authorities = {"READ_ALL"})
|
@WithMockUser(authorities = {"READ_ALL"})
|
||||||
void search_returns200_whenAuthenticated() throws Exception {
|
void search_returns200_whenAuthenticated() throws Exception {
|
||||||
|
|||||||
Reference in New Issue
Block a user