Browse Source

InMemoryUserDetailsManager preserve user type

Closes gh-3192
MrJovanovic13 1 year ago
parent
commit
6d657ea3da

+ 19 - 3
core/src/main/java/org/springframework/security/provisioning/InMemoryUserDetailsManager.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2022 the original author or authors.
+ * Copyright 2002-2024 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -30,6 +30,7 @@ import org.springframework.security.access.AccessDeniedException;
 import org.springframework.security.authentication.AuthenticationManager;
 import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
 import org.springframework.security.core.Authentication;
+import org.springframework.security.core.CredentialsContainer;
 import org.springframework.security.core.context.SecurityContextHolder;
 import org.springframework.security.core.context.SecurityContextHolderStrategy;
 import org.springframework.security.core.userdetails.User;
@@ -96,7 +97,13 @@ public class InMemoryUserDetailsManager implements UserDetailsManager, UserDetai
 	@Override
 	public void createUser(UserDetails user) {
 		Assert.isTrue(!userExists(user.getUsername()), "user should not exist");
-		this.users.put(user.getUsername().toLowerCase(), new MutableUser(user));
+
+		if (user instanceof MutableUserDetails mutable) {
+			this.users.put(user.getUsername().toLowerCase(), mutable);
+		}
+		else {
+			this.users.put(user.getUsername().toLowerCase(), new MutableUser(user));
+		}
 	}
 
 	@Override
@@ -107,7 +114,13 @@ public class InMemoryUserDetailsManager implements UserDetailsManager, UserDetai
 	@Override
 	public void updateUser(UserDetails user) {
 		Assert.isTrue(userExists(user.getUsername()), "user should exist");
-		this.users.put(user.getUsername().toLowerCase(), new MutableUser(user));
+
+		if (user instanceof MutableUserDetails mutable) {
+			this.users.put(user.getUsername().toLowerCase(), mutable);
+		}
+		else {
+			this.users.put(user.getUsername().toLowerCase(), new MutableUser(user));
+		}
 	}
 
 	@Override
@@ -154,6 +167,9 @@ public class InMemoryUserDetailsManager implements UserDetailsManager, UserDetai
 		if (user == null) {
 			throw new UsernameNotFoundException(username);
 		}
+		if (user instanceof CredentialsContainer) {
+			return user;
+		}
 		return new User(user.getUsername(), user.getPassword(), user.isEnabled(), user.isAccountNonExpired(),
 				user.isCredentialsNonExpired(), user.isAccountNonLocked(), user.getAuthorities());
 	}

+ 70 - 0
core/src/test/java/org/springframework/security/provisioning/InMemoryUserDetailsManagerTests.java

@@ -16,12 +16,15 @@
 
 package org.springframework.security.provisioning;
 
+import java.util.Collection;
 import java.util.Properties;
 
 import org.junit.jupiter.api.Test;
 
 import org.springframework.security.authentication.TestAuthentication;
 import org.springframework.security.core.Authentication;
+import org.springframework.security.core.CredentialsContainer;
+import org.springframework.security.core.GrantedAuthority;
 import org.springframework.security.core.context.SecurityContextHolderStrategy;
 import org.springframework.security.core.context.SecurityContextImpl;
 import org.springframework.security.core.userdetails.PasswordEncodedUser;
@@ -105,6 +108,20 @@ public class InMemoryUserDetailsManagerTests {
 			.withMessage("user should not exist");
 	}
 
+	@Test
+	public void createUserWhenInstanceOfMutableUserDetailsThenChangePasswordWorks() {
+		InMemoryUserDetailsManager manager = new InMemoryUserDetailsManager();
+		CustomUser user = new CustomUser(User.withUserDetails(PasswordEncodedUser.user()).build());
+		Authentication authentication = TestAuthentication.authenticated(user);
+		SecurityContextHolderStrategy strategy = mock(SecurityContextHolderStrategy.class);
+		given(strategy.getContext()).willReturn(new SecurityContextImpl(authentication));
+		manager.setSecurityContextHolderStrategy(strategy);
+		manager.createUser(user);
+		String newPassword = "newPassword";
+		manager.changePassword(user.getPassword(), newPassword);
+		assertThat(manager.loadUserByUsername(user.getUsername()).getPassword()).isEqualTo(newPassword);
+	}
+
 	@Test
 	public void updateUserWhenUserDoesNotExistThenException() {
 		InMemoryUserDetailsManager manager = new InMemoryUserDetailsManager();
@@ -119,4 +136,57 @@ public class InMemoryUserDetailsManagerTests {
 			.isThrownBy(() -> manager.loadUserByUsername(this.user.getUsername()));
 	}
 
+	@Test
+	public void loadUserByUsernameWhenNotInstanceOfCredentialsContainerThenReturnInstanceOfCredentialsContainer() {
+		MutableUser user = new MutableUser(User.withUserDetails(PasswordEncodedUser.user()).build());
+		InMemoryUserDetailsManager manager = new InMemoryUserDetailsManager(user);
+		assertThat(user).isNotInstanceOf(CredentialsContainer.class);
+		assertThat(manager.loadUserByUsername(user.getUsername())).isInstanceOf(CredentialsContainer.class);
+	}
+
+	@Test
+	public void loadUserByUsernameWhenInstanceOfCredentialsContainerThenReturnInstance() {
+		CustomUser user = new CustomUser(User.withUserDetails(PasswordEncodedUser.user()).build());
+		InMemoryUserDetailsManager manager = new InMemoryUserDetailsManager(user);
+		assertThat(manager.loadUserByUsername(user.getUsername())).isSameAs(user);
+	}
+
+	static class CustomUser implements MutableUserDetails, CredentialsContainer {
+
+		private final UserDetails delegate;
+
+		private String password;
+
+		CustomUser(UserDetails user) {
+			this.delegate = user;
+			this.password = user.getPassword();
+		}
+
+		@Override
+		public Collection<? extends GrantedAuthority> getAuthorities() {
+			return this.delegate.getAuthorities();
+		}
+
+		@Override
+		public String getPassword() {
+			return this.password;
+		}
+
+		@Override
+		public void setPassword(final String password) {
+			this.password = password;
+		}
+
+		@Override
+		public String getUsername() {
+			return this.delegate.getUsername();
+		}
+
+		@Override
+		public void eraseCredentials() {
+			this.password = null;
+		}
+
+	}
+
 }