Merge pull request #72 from devondragon/feature/upgrade-spring-user-framework-4.4.0

Upgrade to SpringUserFramework 4.4.0

Author: devondragon

build.gradle

@@ -39,7 +39,7 @@ repositories {
 
 dependencies {
     // DigitalSanctuary Spring User Framework
-    implementation 'com.digitalsanctuary:ds-spring-user-framework:4.3.1'
+    implementation 'com.digitalsanctuary:ds-spring-user-framework:4.4.0'
 
     // WebAuthn support (Passkey authentication)
     implementation 'org.springframework.security:spring-security-webauthn'
@@ -80,7 +80,9 @@ dependencies {
     developmentOnly 'org.springframework.boot:spring-boot-docker-compose'
 
     // Utility libraries
-    implementation 'org.passay:passay:1.6.6'
+    // (passay is provided transitively by ds-spring-user-framework at the version it requires; the demo
+    //  app does not use passay directly, so declaring/pinning it here previously forced a conflicting
+    //  downgrade of the library's required passay version.)
     implementation 'com.google.guava:guava:33.5.0-jre'
     implementation 'org.hibernate.validator:hibernate-validator'
 

src/main/java/com/digitalsanctuary/spring/demo/config/MfaSecurityConfig.java

@@ -12,6 +12,7 @@
 import com.digitalsanctuary.spring.user.persistence.model.User;
 import com.digitalsanctuary.spring.user.mail.MailService;
 import com.digitalsanctuary.spring.user.service.SessionInvalidationService;
+import com.digitalsanctuary.spring.user.service.TokenHasher;
 import com.digitalsanctuary.spring.user.service.UserEmailService;
 import com.digitalsanctuary.spring.user.service.UserVerificationService;
 
@@ -36,8 +37,9 @@ public CustomUserEmailService(
             UserVerificationService userVerificationService,
             PasswordResetTokenRepository passwordTokenRepository,
             ApplicationEventPublisher eventPublisher,
-            SessionInvalidationService sessionInvalidationService) {
-        super(mailService, userVerificationService, passwordTokenRepository, eventPublisher, sessionInvalidationService);
+            SessionInvalidationService sessionInvalidationService,
+            TokenHasher tokenHasher) {
+        super(mailService, userVerificationService, passwordTokenRepository, eventPublisher, sessionInvalidationService, tokenHasher);
     }
 
     @Override

src/main/java/com/digitalsanctuary/spring/demo/service/CustomUserEmailService.java

@@ -7,9 +7,9 @@
 #   ./gradlew bootRun --args='--spring.profiles.active=local,mfa'
 #
 # Notes:
-# - The challenge page (user.mfa.webauthnEntryPointUri) must be in unprotectedURIs. The framework
-#   redirects partially-authenticated users to it; if the page itself required full authentication,
-#   the redirect would loop forever.
+# - The challenge page (user.mfa.webauthnEntryPointUri) is auto-unprotected by the framework: it
+#   unprotects the configured factor entry-point URIs so the partial-auth redirect cannot loop. (No
+#   need to list it in unprotectedURIs by hand.)
 # - The passkey registration endpoints (/webauthn/register/options, /webauthn/register) are also
 #   unprotected here so that a partially-authenticated user can enroll their first passkey. Spring
 #   Security still requires an authenticated principal to register a credential; this only relaxes
@@ -19,4 +19,4 @@ user:
   mfa:
     enabled: true
   security:
-    unprotectedURIs: /,/index.html,/favicon.ico,/apple-touch-icon-precomposed.png,/css/*,/js/*,/js/user/*,/js/event/*,/js/utils/*,/img/**,/user/registration,/user/registration/passwordless,/user/resendRegistrationToken,/user/resetPassword,/user/registrationConfirm,/user/changePassword,/user/savePassword,/oauth2/authorization/*,/login,/user/login,/user/login.html,/swagger-ui.html,/swagger-ui/**,/v3/api-docs/**,/event/,/event/list.html,/event/**,/about.html,/error,/error.html,/webauthn/authenticate/**,/login/webauthn,/user/mfa/webauthn-challenge.html,/webauthn/register/options,/webauthn/register
+    unprotectedURIs: /,/index.html,/favicon.ico,/apple-touch-icon-precomposed.png,/css/*,/js/*,/js/user/*,/js/event/*,/js/utils/*,/img/**,/user/registration,/user/registration/passwordless,/user/resendRegistrationToken,/user/resetPassword,/user/registrationConfirm,/user/changePassword,/user/savePassword,/oauth2/authorization/*,/login,/user/login,/user/login.html,/swagger-ui.html,/swagger-ui/**,/v3/api-docs/**,/event/,/event/list.html,/event/**,/about.html,/error,/error.html,/webauthn/authenticate/**,/login/webauthn,/webauthn/register/options,/webauthn/register

src/main/resources/application-mfa.yml

@@ -136,7 +136,7 @@ user:
     bcryptStrength: 12 # The bcrypt strength to use for password hashing.  The higher the number, the longer it takes to hash the password.  The default is 12.  The minimum is 4.  The maximum is 31.
     testHashTime: true # If true, the test hash time will be logged to the console on startup.  This is useful for determining the optimal bcryptStrength value.
     defaultAction: deny # The default action for all requests.  This can be either deny or allow.
-    unprotectedURIs: /,/index.html,/favicon.ico,/apple-touch-icon-precomposed.png,/css/*,/js/*,/js/user/*,/js/event/*,/js/utils/*,/img/**,/user/registration,/user/registration/passwordless,/user/resendRegistrationToken,/user/resetPassword,/user/registrationConfirm,/user/changePassword,/user/savePassword,/oauth2/authorization/*,/login,/user/login,/user/login.html,/swagger-ui.html,/swagger-ui/**,/v3/api-docs/**,/event/,/event/list.html,/event/**,/about.html,/error,/error.html,/webauthn/authenticate/**,/login/webauthn,/user/mfa/webauthn-challenge.html # A comma delimited list of URIs that should not be protected by Spring Security if the defaultAction is deny.
+    unprotectedURIs: /,/index.html,/favicon.ico,/apple-touch-icon-precomposed.png,/css/*,/js/*,/js/user/*,/js/event/*,/js/utils/*,/img/**,/user/registration,/user/registration/passwordless,/user/resendRegistrationToken,/user/resetPassword,/user/registrationConfirm,/user/changePassword,/user/savePassword,/oauth2/authorization/*,/login,/user/login,/user/login.html,/swagger-ui.html,/swagger-ui/**,/v3/api-docs/**,/event/,/event/list.html,/event/**,/about.html,/error,/error.html,/webauthn/authenticate/**,/login/webauthn # A comma delimited list of URIs that should not be protected by Spring Security if the defaultAction is deny.
     protectedURIs: /protected.html # A comma delimited list of URIs that should be protected by Spring Security if the defaultAction is allow.
     disableCSRFdURIs: /no-csrf-test # A comma delimited list of URIs that should not be protected by CSRF protection. This may include API endpoints that need to be called without a CSRF token.
 

src/main/resources/application.yml

@@ -12,10 +12,11 @@
 /**
  * Guards the consistency of the MFA configuration files themselves.
  *
- * The MFA entry point pages must be listed in {@code user.security.unprotectedURIs}: the framework's
- * access-denied handler redirects partially-authenticated users to the entry point URI, and if that page is itself
- * protected the redirect loops forever. The framework only auto-unprotects {@code /user/mfa/status}, not the entry
- * point pages, so the demo config has to keep these two settings in sync by hand.
+ * As of the framework release containing the #313 fix, the framework auto-unprotects the configured MFA factor
+ * entry-point URIs ({@code user.mfa.passwordEntryPointUri} / {@code user.mfa.webauthnEntryPointUri}), so the demo no
+ * longer has to list the WebAuthn challenge page in {@code user.security.unprotectedURIs} by hand. These tests now only
+ * assert what is still the demo's own responsibility: MFA must be opt-in (disabled by default), and the passkey
+ * <em>enrollment</em> endpoints (which are not factor entry points) must be reachable by partially-authenticated users.
  */
 @DisplayName("MFA Config Consistency Tests")
 class MfaConfigConsistencyTest {
@@ -44,19 +45,6 @@ private static List<String> unprotectedUris(Map<String, Object> yaml) {
         return Arrays.stream(uris.toString().split(",")).map(String::trim).toList();
     }
 
-    @Test
-    @DisplayName("application.yml unprotects the configured WebAuthn challenge page")
-    void baseConfigUnprotectsWebauthnEntryPoint() {
-        Map<String, Object> yaml = loadYaml("/application.yml");
-        Map<String, Object> mfa = section(yaml, "user", "mfa");
-
-        String webauthnEntryPoint = String.valueOf(mfa.get("webauthnEntryPointUri"));
-        assertThat(webauthnEntryPoint).as("user.mfa.webauthnEntryPointUri should be configured").isNotEqualTo("null");
-        assertThat(unprotectedUris(yaml))
-                .as("the WebAuthn challenge page must be unprotected or MFA redirects loop forever")
-                .contains(webauthnEntryPoint);
-    }
-
     @Test
     @DisplayName("application.yml leaves MFA disabled by default (opt-in via the mfa profile)")
     void baseConfigLeavesMfaDisabled() {
@@ -67,16 +55,16 @@ void baseConfigLeavesMfaDisabled() {
     }
 
     @Test
-    @DisplayName("mfa profile enables MFA and unprotects the challenge page and passkey enrollment endpoints")
-    void mfaProfileEnablesMfaAndUnprotectsEntryPoint() {
+    @DisplayName("mfa profile enables MFA and unprotects the passkey enrollment endpoints")
+    void mfaProfileEnablesMfaAndUnprotectsEnrollmentEndpoints() {
         Map<String, Object> yaml = loadYaml("/application-mfa.yml");
         Map<String, Object> mfa = section(yaml, "user", "mfa");
         assertThat(mfa.get("enabled")).isEqualTo(Boolean.TRUE);
 
+        // The WebAuthn challenge page (the configured factor entry point) is now auto-unprotected by the
+        // framework, so it no longer needs to be listed here. The passkey ENROLLMENT endpoints are not factor
+        // entry points, so partially-authenticated users still need them unprotected to enroll a first passkey.
         List<String> uris = unprotectedUris(yaml);
-        assertThat(uris).contains("/user/mfa/webauthn-challenge.html");
-        // Partially-authenticated users need to be able to enroll their first passkey, otherwise new
-        // accounts can never satisfy the WEBAUTHN factor.
         assertThat(uris).contains("/webauthn/register/options", "/webauthn/register");
     }
 }

src/test/java/com/digitalsanctuary/spring/demo/mfa/MfaConfigConsistencyTest.java

@@ -5,6 +5,7 @@
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
+import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Test;
@@ -16,12 +17,16 @@
 import org.springframework.test.context.bean.override.mockito.MockitoBean;
 import org.springframework.test.web.servlet.MockMvc;
 import org.springframework.test.web.servlet.MvcResult;
+import org.springframework.transaction.PlatformTransactionManager;
+import org.springframework.transaction.TransactionDefinition;
 import org.springframework.transaction.annotation.Transactional;
+import org.springframework.transaction.support.TransactionTemplate;
 import com.digitalsanctuary.spring.demo.UserDemoApplication;
 import com.digitalsanctuary.spring.user.dto.UserDto;
 import com.digitalsanctuary.spring.user.mail.MailService;
 import com.digitalsanctuary.spring.user.persistence.model.User;
 import com.digitalsanctuary.spring.user.persistence.repository.UserRepository;
+import com.digitalsanctuary.spring.user.persistence.repository.VerificationTokenRepository;
 import com.digitalsanctuary.spring.user.service.UserService;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import jakarta.persistence.EntityManager;
@@ -50,24 +55,61 @@ class UserApiIntegrationTestFixed {
     @Autowired
     private UserService userService;
 
+    @Autowired
+    private VerificationTokenRepository verificationTokenRepository;
+
+    @Autowired
+    private PlatformTransactionManager transactionManager;
+
     @MockitoBean
     private MailService mailService;
 
     @PersistenceContext
     private EntityManager entityManager;
 
+    private static final String TEST_EMAIL = "test@example.com";
+
     private UserDto testUserDto;
 
     @BeforeEach
     void setUp() {
+        // Start from a clean slate. Registration (via the API and via UserService.registerNewUserAccount)
+        // commits the new user in its own transaction as of 4.4.0 — it does NOT roll back with the test's
+        // @Transactional. Without a committed cleanup, the user persisted by one test method collides with
+        // the next (UserAlreadyExistException / 409 Conflict).
+        deleteTestUserCommitted();
+
         testUserDto = new UserDto();
         testUserDto.setFirstName("Test");
         testUserDto.setLastName("User");
-        testUserDto.setEmail("test@example.com");
+        testUserDto.setEmail(TEST_EMAIL);
         testUserDto.setPassword("SecurePass123!");
         testUserDto.setMatchingPassword("SecurePass123!");
     }
 
+    @AfterEach
+    void tearDown() {
+        // Remove the user committed by this test so it cannot leak into other tests.
+        deleteTestUserCommitted();
+    }
+
+    /**
+     * Deletes the test user (and any verification token) in its own committed transaction. A
+     * REQUIRES_NEW transaction is required because the class is {@code @Transactional}: a plain delete here
+     * would roll back with the test and never actually remove the committed registration row.
+     */
+    private void deleteTestUserCommitted() {
+        TransactionTemplate tx = new TransactionTemplate(transactionManager);
+        tx.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRES_NEW);
+        tx.executeWithoutResult(status -> {
+            User existing = userRepository.findByEmail(TEST_EMAIL);
+            if (existing != null) {
+                verificationTokenRepository.deleteByUser(existing);
+                userRepository.delete(existing);
+            }
+        });
+    }
+
     @Test
     @DisplayName("Should successfully register new user")
     void shouldRegisterNewUser() throws Exception {

src/test/java/com/digitalsanctuary/spring/user/api/UserApiIntegrationTestFixed.java

@@ -139,9 +139,9 @@ void getAuthoritiesFromUser_persistedUser_loadsAuthoritiesWithLazyLoading() {
                 Collection<? extends GrantedAuthority> authorities = authorityService
                                 .getAuthoritiesFromUser(fetchedUser);
 
-                // Then
-                assertThat(authorities).hasSize(3).extracting(GrantedAuthority::getAuthority).containsExactlyInAnyOrder(
-                                "LOGIN_PRIVILEGE",
+                // Then - as of 4.4.0 the role name itself is also granted (needed for hasRole() checks)
+                assertThat(authorities).hasSize(4).extracting(GrantedAuthority::getAuthority).containsExactlyInAnyOrder(
+                                "ROLE_USER", "LOGIN_PRIVILEGE",
                                 "UPDATE_OWN_USER_PRIVILEGE", "RESET_OWN_PASSWORD_PRIVILEGE");
         }
 
@@ -159,9 +159,10 @@ void getAuthoritiesFromUser_multipleRoles_combinesAllPrivileges() {
                 Collection<? extends GrantedAuthority> authorities = authorityService
                                 .getAuthoritiesFromUser(multiRoleUser);
 
-                // Then
-                assertThat(authorities).hasSize(6) // 3 from user role + 3 from manager role
-                                .extracting(GrantedAuthority::getAuthority).containsExactlyInAnyOrder("LOGIN_PRIVILEGE",
+                // Then - 6 privileges + the two role names (ROLE_USER, ROLE_MANAGER) granted as of 4.4.0
+                assertThat(authorities).hasSize(8) // 3 user + 3 manager privileges + 2 role names
+                                .extracting(GrantedAuthority::getAuthority).containsExactlyInAnyOrder("ROLE_USER",
+                                                "ROLE_MANAGER", "LOGIN_PRIVILEGE",
                                                 "UPDATE_OWN_USER_PRIVILEGE",
                                                 "RESET_OWN_PASSWORD_PRIVILEGE", "ADD_USER_TO_TEAM_PRIVILEGE",
                                                 "REMOVE_USER_FROM_TEAM_PRIVILEGE",
@@ -180,9 +181,9 @@ void getAuthoritiesFromUser_adminUser_hasAllAdminPrivileges() {
                 // When
                 Collection<? extends GrantedAuthority> authorities = authorityService.getAuthoritiesFromUser(adminUser);
 
-                // Then
-                assertThat(authorities).hasSize(5).extracting(GrantedAuthority::getAuthority).containsExactlyInAnyOrder(
-                                "ADMIN_PRIVILEGE",
+                // Then - 5 admin privileges + the ROLE_ADMIN role name granted as of 4.4.0
+                assertThat(authorities).hasSize(6).extracting(GrantedAuthority::getAuthority).containsExactlyInAnyOrder(
+                                "ROLE_ADMIN", "ADMIN_PRIVILEGE",
                                 "INVITE_USER_PRIVILEGE", "READ_USER_PRIVILEGE", "ASSIGN_MANAGER_PRIVILEGE",
                                 "RESET_ANY_USER_PASSWORD_PRIVILEGE");
         }
@@ -206,9 +207,9 @@ void getAuthoritiesFromRoles_sharedPrivileges_deduplicatesCorrectly() {
                 Collection<? extends GrantedAuthority> authorities = authorityService
                                 .getAuthoritiesFromRoles(Arrays.asList(role1, role2));
 
-                // Then - Should only have 4 unique privileges (3 from user + 1 shared)
-                assertThat(authorities).hasSize(4).extracting(GrantedAuthority::getAuthority).containsExactlyInAnyOrder(
-                                "LOGIN_PRIVILEGE",
+                // Then - 4 unique privileges (3 from user + 1 shared) + the two role names granted as of 4.4.0
+                assertThat(authorities).hasSize(6).extracting(GrantedAuthority::getAuthority).containsExactlyInAnyOrder(
+                                "ROLE_TEST1", "ROLE_TEST2", "LOGIN_PRIVILEGE",
                                 "UPDATE_OWN_USER_PRIVILEGE", "RESET_OWN_PASSWORD_PRIVILEGE", "SHARED_PRIVILEGE");
         }
 
@@ -247,9 +248,11 @@ void getAuthoritiesFromRoles_separatelyLoadedRoles_worksCorrectly() {
                 Collection<? extends GrantedAuthority> authorities = authorityService.getAuthoritiesFromRoles(
                                 Arrays.asList(loadedUserRole, loadedManagerRole, loadedAdminRole));
 
-                // Then - Should have all unique privileges from all three roles
-                assertThat(authorities).hasSize(11) // 3 + 3 + 5 unique privileges
+                // Then - all unique privileges from all three roles, plus the three role names granted as of 4.4.0
+                assertThat(authorities).hasSize(14) // 3 + 3 + 5 privileges + 3 role names
                                 .extracting(GrantedAuthority::getAuthority).contains(
+                                                // Role names
+                                                "ROLE_USER", "ROLE_MANAGER", "ROLE_ADMIN",
                                                 // User privileges
                                                 "LOGIN_PRIVILEGE", "UPDATE_OWN_USER_PRIVILEGE",
                                                 "RESET_OWN_PASSWORD_PRIVILEGE",