Merge pull request #194 from devondragon/fix/hibernate-entity-management-issue

Fix/hibernate entity management issue, and other improvements

Author: devondragon

README.md

@@ -84,14 +84,14 @@ Check out the [Spring User Framework Demo Application](https://github.com/devond
 <dependency>
     <groupId>com.digitalsanctuary</groupId>
     <artifactId>ds-spring-user-framework</artifactId>
-    <version>3.2.2</version>
+    <version>3.3.0</version>
 </dependency>
 ```
 
 ### Gradle
 
 ```groovy
-implementation 'com.digitalsanctuary:ds-spring-user-framework:3.2.2'
+implementation 'com.digitalsanctuary:ds-spring-user-framework:3.3.0'
 ```
 
 ## Quick Start

gradle.properties

@@ -1,3 +1,3 @@
-version=3.2.4-SNAPSHOT
+version=3.3.0-SNAPSHOT
 mavenCentralPublishing=true
 mavenCentralAutomaticPublishing=true

src/main/java/com/digitalsanctuary/spring/user/listener/AuthenticationEventListener.java

@@ -3,7 +3,9 @@
 import org.springframework.context.event.EventListener;
 import org.springframework.security.authentication.event.AbstractAuthenticationFailureEvent;
 import org.springframework.security.authentication.event.AuthenticationSuccessEvent;
+import org.springframework.security.oauth2.core.user.OAuth2User;
 import org.springframework.stereotype.Component;
+import com.digitalsanctuary.spring.user.service.DSUserDetails;
 import com.digitalsanctuary.spring.user.service.LoginAttemptService;
 import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
@@ -22,28 +24,74 @@ public class AuthenticationEventListener {
 
     /**
      * This method listens for successful authentications and handles account lockout functionality.
+     * It properly handles different authentication types including form login, OAuth2, and OIDC.
      *
      * @param success the success event
      */
     @EventListener
     public void onSuccess(AuthenticationSuccessEvent success) {
-        // Handle successful authentication, e.g. logging or auditing
-        log.debug("Authentication success: " + success.getAuthentication().getName());
-        String username = success.getAuthentication().getName();
-        loginAttemptService.loginSucceeded(username);
+        // Extract username/email based on the principal type
+        String username = null;
+        Object principal = success.getAuthentication().getPrincipal();
+        
+        if (principal instanceof DSUserDetails) {
+            // Form login or custom authentication
+            username = ((DSUserDetails) principal).getUsername();
+            log.debug("Authentication success for DSUserDetails: {}", username);
+        } else if (principal instanceof OAuth2User) {
+            // OAuth2/OIDC authentication - try to get email
+            OAuth2User oauth2User = (OAuth2User) principal;
+            username = oauth2User.getAttribute("email");
+            if (username == null) {
+                // Fallback to name if email is not available
+                username = oauth2User.getName();
+            }
+            log.debug("Authentication success for OAuth2User: {}", username);
+        } else if (principal instanceof String) {
+            // Basic authentication or remember-me
+            username = (String) principal;
+            log.debug("Authentication success for String principal: {}", username);
+        } else {
+            // Fallback to getName() method for unknown principal types
+            username = success.getAuthentication().getName();
+            log.debug("Authentication success for unknown principal type {}: {}", 
+                     principal != null ? principal.getClass().getName() : "null", username);
+        }
+        
+        // Only process login success if we have a valid username
+        if (username != null && !username.trim().isEmpty()) {
+            loginAttemptService.loginSucceeded(username);
+        } else {
+            log.warn("Could not extract valid username from authentication event - not tracking");
+        }
     }
 
     /**
      * This method listens for authentication failures and handles account lockout functionality.
+     * It properly handles different authentication types including form login, OAuth2, and OIDC.
      *
      * @param failure the failure event
      */
     @EventListener
     public void onFailure(AbstractAuthenticationFailureEvent failure) {
-        // Handle unsuccessful authentication, e.g. logging or auditing
-        log.debug("Authentication failure: " + failure.getException().getMessage());
+        // For failures, try to get username from getName() first (the attempted username)
         String username = failure.getAuthentication().getName();
-        loginAttemptService.loginFailed(username);
+        
+        // If getName() is null/empty, try to extract from principal
+        if (username == null || username.trim().isEmpty()) {
+            Object principal = failure.getAuthentication().getPrincipal();
+            if (principal instanceof String) {
+                username = (String) principal;
+            }
+        }
+        
+        // Only process login failure if we have a valid username
+        if (username != null && !username.trim().isEmpty()) {
+            log.debug("Authentication failure for user '{}': {}", username, failure.getException().getMessage());
+            loginAttemptService.loginFailed(username);
+        } else {
+            log.warn("Could not extract valid username from authentication failure event - not tracking");
+        }
     }
 
 }

src/main/java/com/digitalsanctuary/spring/user/persistence/model/User.java

@@ -1,12 +1,17 @@
 package com.digitalsanctuary.spring.user.persistence.model;
 
+import java.util.ArrayList;
 import java.util.Date;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 import org.springframework.data.annotation.CreatedDate;
 import org.springframework.data.annotation.LastModifiedDate;
 import org.springframework.data.jpa.domain.support.AuditingEntityListener;
 import jakarta.persistence.*;
 import lombok.Data;
+import lombok.EqualsAndHashCode;
+import lombok.ToString;
 
 /**
  * The User Entity. Part of the basic User ->> Role ->> Privilege structure. This is the primary user data object. You can add to this, or add
@@ -92,11 +97,13 @@ public enum Provider {
 	@Temporal(TemporalType.TIMESTAMP)
 	private Date lockedDate;
 
-	/** The roles. */
+	/** The roles - stored as Set to avoid Hibernate immutable collection issues */
+	@ToString.Exclude
+	@EqualsAndHashCode.Exclude
 	@ManyToMany(cascade = {CascadeType.PERSIST, CascadeType.MERGE}, fetch = FetchType.EAGER)
 	@JoinTable(name = "users_roles", joinColumns = @JoinColumn(name = "user_id", referencedColumnName = "id"),
 			inverseJoinColumns = @JoinColumn(name = "role_id", referencedColumnName = "id"))
-	private List<Role> roles;
+	private Set<Role> roles = new HashSet<>();
 
 	/**
 	 * Instantiates a new user.
@@ -122,4 +129,43 @@ public void setLastActivityDate() {
 	public String getFullName() {
 		return firstName + " " + lastName;
 	}
+
+	/**
+	 * Gets the roles as a List for backward compatibility.
+	 * 
+	 * @return the roles as a List
+	 */
+	public List<Role> getRoles() {
+		return new ArrayList<>(roles);
+	}
+
+	/**
+	 * Sets the roles from a List for backward compatibility.
+	 * Creates a new HashSet to ensure the collection is mutable.
+	 * 
+	 * @param rolesList the roles to set
+	 */
+	public void setRoles(List<Role> rolesList) {
+		this.roles = new HashSet<>(rolesList != null ? rolesList : new ArrayList<>());
+	}
+
+	/**
+	 * Gets the roles as a Set (preferred method).
+	 * Returns a defensive copy to prevent external modification.
+	 * 
+	 * @return the roles as a Set
+	 */
+	public Set<Role> getRolesAsSet() {
+		return new HashSet<>(roles);
+	}
+
+	/**
+	 * Sets the roles from a Set (preferred method).
+	 * Creates a new HashSet to ensure the collection is mutable.
+	 * 
+	 * @param rolesSet the roles to set
+	 */
+	public void setRolesAsSet(Set<Role> rolesSet) {
+		this.roles = new HashSet<>(rolesSet != null ? rolesSet : new HashSet<>());
+	}
 }

src/main/java/com/digitalsanctuary/spring/user/security/WebSecurityConfig.java

@@ -266,7 +266,8 @@ public RoleHierarchy roleHierarchy() {
 			log.error("WebSecurityConfig.roleHierarchy: rolesAndPrivilegesConfig.getRoleHierarchyString() is null!");
 			return null;
 		}
-		RoleHierarchyImpl roleHierarchy = RoleHierarchyImpl.fromHierarchy(rolesAndPrivilegesConfig.getRoleHierarchyString());
+		RoleHierarchyImpl roleHierarchy = new RoleHierarchyImpl();
+		roleHierarchy.setHierarchy(rolesAndPrivilegesConfig.getRoleHierarchyString());
 		log.debug("WebSecurityConfig.roleHierarchy: roleHierarchy: {}", roleHierarchy.toString());
 		return roleHierarchy;
 	}

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

@@ -1,6 +1,8 @@
 package com.digitalsanctuary.spring.user.service;
 
+import java.util.Arrays;
 import com.digitalsanctuary.spring.user.persistence.model.User;
+import com.digitalsanctuary.spring.user.persistence.repository.RoleRepository;
 import com.digitalsanctuary.spring.user.persistence.repository.UserRepository;
 import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
@@ -41,6 +43,9 @@ public class DSOidcUserService implements OAuth2UserService<OidcUserRequest, Oid
 
     /** The user repository. */
     private final UserRepository userRepository;
+    
+    /** The role repository. */
+    private final RoleRepository roleRepository;
 
     OidcUserService defaultOidcUserService = new OidcUserService();
 
@@ -100,7 +105,7 @@ public User handleOidcLoginSuccess(String registrationId, OidcUser oidcUser) {
     private User registerNewOidcUser(String registrationId, User user) {
         User.Provider provider = User.Provider.valueOf(registrationId.toUpperCase());
         user.setProvider(provider);
-        // user.setRoles(Collections.singletonList(roleRepository.findByName(RoleName.ROLE_USER)));
+        user.setRoles(Arrays.asList(roleRepository.findByName("ROLE_USER")));
         // We will trust OAuth2 providers to provide us with a verified email address.
         user.setEnabled(true);
         return userRepository.save(user);