fix(security): include role names as granted authorities

AuthorityService.getAuthoritiesFromRoles() only produced privilege-name
authorities (e.g. ADMIN_PRIVILEGE), never role-name authorities (e.g.
ROLE_ADMIN). Spring Security's hasRole() checks require ROLE_ADMIN to
be present in the granted authority set -- the role hierarchy bean only
expands from base role names, it does not conjure them.

This caused @PreAuthorize("hasRole('ADMIN')") on
UserEmailService.initiateAdminPasswordReset() to always deny access,
producing a 500 for any consumer of that method.

Fix: include both the role name and each privilege name when building
the authority set, matching the standard Spring Security convention.

Updated all AuthorityServiceTest assertions to reflect the new
behaviour and added three targeted tests:
- shouldIncludeRoleNamesAsAuthorities
- shouldIncludeRoleNameEvenWhenRoleHasNoPrivileges
- shouldDeduplicateWhenRoleNameMatchesPrivilegeName

Fixes #293

Author: devondragon

src/main/java/com/digitalsanctuary/spring/user/service/AuthorityService.java

@@ -1,7 +1,8 @@
 package com.digitalsanctuary.spring.user.service;
 
 import java.util.Collection;
-import java.util.stream.Collectors;
+import java.util.HashSet;
+import java.util.Set;
 import org.springframework.security.core.GrantedAuthority;
 import org.springframework.security.core.authority.SimpleGrantedAuthority;
 import org.springframework.stereotype.Service;
@@ -39,21 +40,24 @@ public Collection<? extends GrantedAuthority> getAuthoritiesFromUser(User user)
     }
 
     /**
+     * Returns a collection of Spring Security's {@link GrantedAuthority} objects that includes both the role names and
+     * the privilege names associated with the given collection of roles.
      *
-     * Returns a collection of Spring Security's GrantedAuthority objects that corresponds to the privileges associated with the given collection of
-     * roles.
+     * <p>Including role names as authorities is required for Spring Security's {@code hasRole()} checks
+     * (e.g., {@code @PreAuthorize("hasRole('ADMIN')")}) to work correctly.</p>
      *
-     * @param roles a collection of roles whose privileges should be converted into Spring Security's GrantedAuthority objects
-     * @return a collection of Spring Security's GrantedAuthority objects that corresponds to the privileges associated with the given collection of
-     *         roles
+     * @param roles a collection of roles whose names and privileges should be converted into GrantedAuthority objects
+     * @return a deduplicated set of GrantedAuthority objects containing both role names and privilege names
      */
     public Collection<? extends GrantedAuthority> getAuthoritiesFromRoles(Collection<Role> roles) {
-        // flatMap streams the roles, and maps each Role to its privileges (a Collection of Privilege objects).
-        // The stream of Collection<Privilege> objects is then flattened into a single stream of Privilege objects.
-        // Finally, each Privilege is mapped to its name as a String, wrapped in a SimpleGrantedAuthority object,
-        // and collected into a Set of GrantedAuthority objects.
-        return roles.stream().flatMap(role -> role.getPrivileges().stream()).map(Privilege::getName).map(SimpleGrantedAuthority::new)
-                .collect(Collectors.toSet());
+        Set<GrantedAuthority> authorities = new HashSet<>();
+        for (Role role : roles) {
+            authorities.add(new SimpleGrantedAuthority(role.getName()));
+            for (Privilege privilege : role.getPrivileges()) {
+                authorities.add(new SimpleGrantedAuthority(privilege.getName()));
+            }
+        }
+        return authorities;
     }
 
 }

src/test/java/com/digitalsanctuary/spring/user/service/AuthorityServiceTest.java

@@ -110,11 +110,11 @@ void getAuthoritiesFromUser_userWithRoles_returnsCorrectAuthorities() {
         // When
         Collection<? extends GrantedAuthority> authorities = authorityService.getAuthoritiesFromUser(testUser);
 
-        // Then
+        // Then - includes both role names and privilege names
         assertThat(authorities)
-                .hasSize(2)
+                .hasSize(4)
                 .extracting(GrantedAuthority::getAuthority)
-                .containsExactlyInAnyOrder("READ_PRIVILEGE", "WRITE_PRIVILEGE");
+                .containsExactlyInAnyOrder("ROLE_USER", "ROLE_ADMIN", "READ_PRIVILEGE", "WRITE_PRIVILEGE");
     }
 
     // Tests for getAuthoritiesFromRoles
@@ -145,11 +145,11 @@ void getAuthoritiesFromRoles_singleRoleSinglePrivilege_returnsOneAuthority() {
         // When
         Collection<? extends GrantedAuthority> authorities = authorityService.getAuthoritiesFromRoles(roles);
 
-        // Then
+        // Then - includes role name and privilege name
         assertThat(authorities)
-                .hasSize(1)
+                .hasSize(2)
                 .extracting(GrantedAuthority::getAuthority)
-                .containsExactly("READ_PRIVILEGE");
+                .containsExactlyInAnyOrder("ROLE_USER", "READ_PRIVILEGE");
     }
 
     @Test
@@ -161,11 +161,11 @@ void getAuthoritiesFromRoles_singleRoleMultiplePrivileges_returnsMultipleAuthori
         // When
         Collection<? extends GrantedAuthority> authorities = authorityService.getAuthoritiesFromRoles(roles);
 
-        // Then
+        // Then - includes role name and both privilege names
         assertThat(authorities)
-                .hasSize(2)
+                .hasSize(3)
                 .extracting(GrantedAuthority::getAuthority)
-                .containsExactlyInAnyOrder("READ_PRIVILEGE", "WRITE_PRIVILEGE");
+                .containsExactlyInAnyOrder("ROLE_ADMIN", "READ_PRIVILEGE", "WRITE_PRIVILEGE");
     }
 
     @Test
@@ -177,16 +177,16 @@ void getAuthoritiesFromRoles_multipleRolesWithOverlappingPrivileges_deduplicates
         // When
         Collection<? extends GrantedAuthority> authorities = authorityService.getAuthoritiesFromRoles(roles);
 
-        // Then - READ_PRIVILEGE appears in both roles but should only appear once
+        // Then - READ_PRIVILEGE appears in both roles but should only appear once; role names included
         assertThat(authorities)
-                .hasSize(2)
+                .hasSize(4)
                 .extracting(GrantedAuthority::getAuthority)
-                .containsExactlyInAnyOrder("READ_PRIVILEGE", "WRITE_PRIVILEGE");
+                .containsExactlyInAnyOrder("ROLE_USER", "ROLE_ADMIN", "READ_PRIVILEGE", "WRITE_PRIVILEGE");
     }
 
     @Test
-    @DisplayName("Should handle role with empty privileges")
-    void getAuthoritiesFromRoles_roleWithEmptyPrivileges_returnsEmptyAuthorities() {
+    @DisplayName("Should include role name even when role has no privileges")
+    void getAuthoritiesFromRoles_roleWithEmptyPrivileges_returnsRoleNameOnly() {
         // Given
         Role emptyRole = RoleTestDataBuilder.aRole()
                 .withName("ROLE_EMPTY")
@@ -197,8 +197,11 @@ void getAuthoritiesFromRoles_roleWithEmptyPrivileges_returnsEmptyAuthorities() {
         // When
         Collection<? extends GrantedAuthority> authorities = authorityService.getAuthoritiesFromRoles(roles);
 
-        // Then
-        assertThat(authorities).isEmpty();
+        // Then - role name is still included even with no privileges
+        assertThat(authorities)
+                .hasSize(1)
+                .extracting(GrantedAuthority::getAuthority)
+                .containsExactly("ROLE_EMPTY");
     }
 
     @Test
@@ -275,11 +278,11 @@ void getAuthoritiesFromRoles_mixedCasePrivileges_preservesCase() {
         // When
         Collection<? extends GrantedAuthority> authorities = authorityService.getAuthoritiesFromRoles(roles);
 
-        // Then
+        // Then - includes role name and all privilege names with case preserved
         assertThat(authorities)
-                .hasSize(3)
+                .hasSize(4)
                 .extracting(GrantedAuthority::getAuthority)
-                .containsExactlyInAnyOrder("read_privilege", "WRITE_PRIVILEGE", "Delete_Privilege");
+                .containsExactlyInAnyOrder("ROLE_TEST", "read_privilege", "WRITE_PRIVILEGE", "Delete_Privilege");
     }
 
     @Test
@@ -303,8 +306,8 @@ void getAuthoritiesFromRoles_largeNumberOfPrivileges_handlesEfficiently() {
         Collection<? extends GrantedAuthority> authorities = authorityService.getAuthoritiesFromRoles(roles);
         long endTime = System.currentTimeMillis();
 
-        // Then
-        assertThat(authorities).hasSize(1000);
+        // Then - 1000 privileges + 1 role name
+        assertThat(authorities).hasSize(1001);
         assertThat(endTime - startTime).isLessThan(100); // Should complete in less than 100ms
     }
 
@@ -338,10 +341,48 @@ void getAuthoritiesFromRoles_complexOverlap_correctlyDeduplicates() {
         // When
         Collection<? extends GrantedAuthority> authorities = authorityService.getAuthoritiesFromRoles(roles);
 
-        // Then
+        // Then - 3 role names + 4 unique privilege names
+        assertThat(authorities)
+                .hasSize(7)
+                .extracting(GrantedAuthority::getAuthority)
+                .containsExactlyInAnyOrder("ROLE_1", "ROLE_2", "ROLE_3", "COMMON_PRIVILEGE", "UNIQUE_1", "UNIQUE_2", "UNIQUE_3");
+    }
+
+    @Test
+    @DisplayName("Should include role names as authorities alongside privileges")
+    void shouldIncludeRoleNamesAsAuthorities() {
+        // Given
+        List<Role> roles = Arrays.asList(adminRole);
+
+        // When
+        Collection<? extends GrantedAuthority> authorities = authorityService.getAuthoritiesFromRoles(roles);
+
+        // Then - ROLE_ADMIN is present as an authority (needed for hasRole('ADMIN'))
+        assertThat(authorities)
+                .extracting(GrantedAuthority::getAuthority)
+                .contains("ROLE_ADMIN");
+    }
+
+    @Test
+    @DisplayName("Should deduplicate when role name matches a privilege name")
+    void shouldDeduplicateWhenRoleNameMatchesPrivilegeName() {
+        // Given - a role whose name also appears as a privilege name
+        Privilege roleNamePrivilege = new Privilege("ROLE_SPECIAL");
+        roleNamePrivilege.setId(10L);
+
+        Role specialRole = RoleTestDataBuilder.aRole()
+                .withName("ROLE_SPECIAL")
+                .withPrivilege(roleNamePrivilege)
+                .build();
+        List<Role> roles = Arrays.asList(specialRole);
+
+        // When
+        Collection<? extends GrantedAuthority> authorities = authorityService.getAuthoritiesFromRoles(roles);
+
+        // Then - ROLE_SPECIAL appears only once despite being both a role name and privilege name
         assertThat(authorities)
-                .hasSize(4) // COMMON_PRIVILEGE, UNIQUE_1, UNIQUE_2, UNIQUE_3
+                .hasSize(1)
                 .extracting(GrantedAuthority::getAuthority)
-                .containsExactlyInAnyOrder("COMMON_PRIVILEGE", "UNIQUE_1", "UNIQUE_2", "UNIQUE_3");
+                .containsExactly("ROLE_SPECIAL");
     }
 }