Polish UserDetailsResourceFactoryBean Support

Issues: gh-4380 gh-4381 gh-4382

config/src/main/java/org/springframework/security/config/core/userdetails/UserDetailsRepositoryResourceFactoryBean.java

@@ -56,32 +56,32 @@ public class UserDetailsRepositoryResourceFactoryBean implements ResourceLoaderA
 	/**
 	 * Sets a the location of a Resource that is a Properties file in the format defined in {@link UserDetailsResourceFactoryBean}
 	 *
-	 * @param propertiesResourceLocation the location of the properties file that contains the users (i.e. "classpath:users.properties")
+	 * @param resourceLocation the location of the properties file that contains the users (i.e. "classpath:users.properties")
 	 * @return the UserDetailsResourceFactoryBean
 	 */
-	public void setPropertiesResourceLocation(String propertiesResourceLocation) {
+	public void setResourceLocation(String resourceLocation) {
-		this.userDetails.setPropertiesResourceLocation(propertiesResourceLocation);
+		this.userDetails.setResourceLocation(resourceLocation);
 	}
 
 	/**
 	 * Sets a a Resource that is a Properties file in the format defined in {@link UserDetailsResourceFactoryBean}
 	 *
-	 * @param propertiesResource the Resource to use
+	 * @param resource the Resource to use
 	 */
-	public void setPropertiesResource(Resource propertiesResource) {
+	public void setResource(Resource resource) {
-		this.userDetails.setPropertiesResource(propertiesResource);
+		this.userDetails.setResource(resource);
 	}
 
 	/**
 	 * Create a UserDetailsRepositoryResourceFactoryBean with the location of a Resource that is a Properties file in the
 	 * format defined in {@link UserDetailsResourceFactoryBean}
 	 *
-	 * @param propertiesResourceLocation the location of the properties file that contains the users (i.e. "classpath:users.properties")
+	 * @param resourceLocatiton the location of the properties file that contains the users (i.e. "classpath:users.properties")
 	 * @return the UserDetailsResourceFactoryBean
 	 */
-	public static UserDetailsRepositoryResourceFactoryBean usersFromResourceLocation(String propertiesResourceLocation) {
+	public static UserDetailsRepositoryResourceFactoryBean fromResourceLocation(String resourceLocatiton) {
 		UserDetailsRepositoryResourceFactoryBean result = new UserDetailsRepositoryResourceFactoryBean();
-		result.setPropertiesResourceLocation(propertiesResourceLocation);
+		result.setResourceLocation(resourceLocatiton);
 		return result;
 	}
 
@@ -92,9 +92,9 @@ public class UserDetailsRepositoryResourceFactoryBean implements ResourceLoaderA
 	 * @param propertiesResource the Resource that is a properties file that contains the users
 	 * @return the UserDetailsResourceFactoryBean
 	 */
-	public static UserDetailsRepositoryResourceFactoryBean usersFromResource(Resource propertiesResource) {
+	public static UserDetailsRepositoryResourceFactoryBean fromResource(Resource propertiesResource) {
 		UserDetailsRepositoryResourceFactoryBean result = new UserDetailsRepositoryResourceFactoryBean();
-		result.setPropertiesResource(propertiesResource);
+		result.setResource(propertiesResource);
 		return result;
 	}
 }

config/src/main/java/org/springframework/security/config/core/userdetails/UserDetailsResourceFactoryBean.java

@@ -20,6 +20,7 @@ package org.springframework.security.config.core.userdetails;
 
 import org.springframework.beans.factory.FactoryBean;
 import org.springframework.context.ResourceLoaderAware;
+import org.springframework.core.io.DefaultResourceLoader;
 import org.springframework.core.io.Resource;
 import org.springframework.core.io.ResourceLoader;
 import org.springframework.security.core.userdetails.User;
@@ -28,8 +29,6 @@ import org.springframework.security.core.userdetails.memory.UserAttribute;
 import org.springframework.security.core.userdetails.memory.UserAttributeEditor;
 import org.springframework.util.Assert;
 
-import java.io.File;
-import java.io.FileInputStream;
 import java.io.InputStream;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -55,14 +54,15 @@ import java.util.Properties;
  * @since 5.0
  */
 public class UserDetailsResourceFactoryBean implements ResourceLoaderAware, FactoryBean<Collection<UserDetails>> {
-	private ResourceLoader resourceLoader;
+	private ResourceLoader resourceLoader = new DefaultResourceLoader();
 
-	private String propertiesResourceLocation;
+	private String resourceLocation;
 
-	private Resource propertiesResource;
+	private Resource resource;
 
 	@Override
 	public void setResourceLoader(ResourceLoader resourceLoader) {
+		Assert.notNull(resourceLoader,"resourceLoader cannot be null");
 		this.resourceLoader = resourceLoader;
 	}
 
@@ -104,29 +104,53 @@ public class UserDetailsResourceFactoryBean implements ResourceLoaderAware, Fact
 	/**
 	 * Sets a the location of a Resource that is a Properties file in the format defined in {@link UserDetailsResourceFactoryBean}
 	 *
-	 * @param propertiesResourceLocation the location of the properties file that contains the users (i.e. "classpath:users.properties")
+	 * @param resourceLocation the location of the properties file that contains the users (i.e. "classpath:users.properties")
 	 */
-	public void setPropertiesResourceLocation(String propertiesResourceLocation) {
+	public void setResourceLocation(String resourceLocation) {
-		this.propertiesResourceLocation = propertiesResourceLocation;
+		this.resourceLocation = resourceLocation;
 	}
 
 	/**
 	 * Sets a a Resource that is a Properties file in the format defined in {@link UserDetailsResourceFactoryBean}
 	 *
-	 * @param propertiesResource the Resource to use
+	 * @param resource the Resource to use
 	 */
-	public void setPropertiesResource(Resource propertiesResource) {
+	public void setResource(Resource resource) {
-		this.propertiesResource = propertiesResource;
+		this.resource = resource;
 	}
 
 	private Resource getProperitesResource() {
-		if(propertiesResource != null) {
+		Resource result = resource;
-			return propertiesResource;
+		if(result == null && resourceLocation != null) {
+			result = resourceLoader.getResource(resourceLocation);
 		}
-		if(propertiesResourceLocation != null) {
+		Assert.notNull(result, "resource cannot be null if resourceLocation is null");
-			Assert.notNull(resourceLoader, "resourceLoader cannot be null if propertiesResource is null");
+		return result;
-			return resourceLoader.getResource(propertiesResourceLocation);
 	}
-		throw new IllegalStateException("Either propertiesResource cannot be null or both resourceLoader and propertiesResourceLocation cannot be null");
+
+	/**
+	 * Create a UserDetailsResourceFactoryBean with the location of a Resource that is a Properties file in the
+	 * format defined in {@link UserDetailsResourceFactoryBean}
+	 *
+	 * @param resourceLocatiton the location of the properties file that contains the users (i.e. "classpath:users.properties")
+	 * @return the UserDetailsResourceFactoryBean
+	 */
+	public static UserDetailsResourceFactoryBean fromResourceLocation(String resourceLocatiton) {
+		UserDetailsResourceFactoryBean result = new UserDetailsResourceFactoryBean();
+		result.setResourceLocation(resourceLocatiton);
+		return result;
+	}
+
+	/**
+	 * Create a UserDetailsResourceFactoryBean with a Resource that is a Properties file in the
+	 * format defined in {@link UserDetailsResourceFactoryBean}
+	 *
+	 * @param propertiesResource the Resource that is a properties file that contains the users
+	 * @return the UserDetailsResourceFactoryBean
+	 */
+	public static UserDetailsResourceFactoryBean fromResource(Resource propertiesResource) {
+		UserDetailsResourceFactoryBean result = new UserDetailsResourceFactoryBean();
+		result.setResource(propertiesResource);
+		return result;
 	}
 }

config/src/main/java/org/springframework/security/config/provisioning/UserDetailsManagerResourceFactoryBean.java

@@ -24,7 +24,6 @@ import org.springframework.core.io.Resource;
 import org.springframework.core.io.ResourceLoader;
 import org.springframework.security.config.core.userdetails.UserDetailsResourceFactoryBean;
 import org.springframework.security.core.userdetails.UserDetails;
-import org.springframework.security.core.userdetails.UserDetailsService;
 import org.springframework.security.provisioning.InMemoryUserDetailsManager;
 
 import java.util.Collection;
@@ -58,32 +57,32 @@ public class UserDetailsManagerResourceFactoryBean implements ResourceLoaderAwar
 	/**
 	 * Sets a the location of a Resource that is a Properties file in the format defined in {@link UserDetailsResourceFactoryBean}
 	 *
-	 * @param propertiesResourceLocation the location of the properties file that contains the users (i.e. "classpath:users.properties")
+	 * @param resourceLocation the location of the properties file that contains the users (i.e. "classpath:users.properties")
 	 * @return the UserDetailsResourceFactoryBean
 	 */
-	public void setPropertiesResourceLocation(String propertiesResourceLocation) {
+	public void setResourceLocation(String resourceLocation) {
-		this.userDetails.setPropertiesResourceLocation(propertiesResourceLocation);
+		this.userDetails.setResourceLocation(resourceLocation);
 	}
 
 	/**
 	 * Sets a a Resource that is a Properties file in the format defined in {@link UserDetailsResourceFactoryBean}
 	 *
-	 * @param propertiesResource the Resource to use
+	 * @param resource the Resource to use
 	 */
-	public void setPropertiesResource(Resource propertiesResource) {
+	public void setResource(Resource resource) {
-		this.userDetails.setPropertiesResource(propertiesResource);
+		this.userDetails.setResource(resource);
 	}
 
 	/**
 	 * Create a UserDetailsServiceResourceFactoryBean with the location of a Resource that is a Properties file in the
 	 * format defined in {@link UserDetailsResourceFactoryBean}
 	 *
-	 * @param propertiesResourceLocation the location of the properties file that contains the users (i.e. "classpath:users.properties")
+	 * @param resourceLocation the location of the properties file that contains the users (i.e. "classpath:users.properties")
 	 * @return the UserDetailsResourceFactoryBean
 	 */
-	public static UserDetailsManagerResourceFactoryBean usersFromResourceLocation(String propertiesResourceLocation) {
+	public static UserDetailsManagerResourceFactoryBean fromResourceLocation(String resourceLocation) {
 		UserDetailsManagerResourceFactoryBean result = new UserDetailsManagerResourceFactoryBean();
-		result.setPropertiesResourceLocation(propertiesResourceLocation);
+		result.setResourceLocation(resourceLocation);
 		return result;
 	}
 
@@ -91,12 +90,12 @@ public class UserDetailsManagerResourceFactoryBean implements ResourceLoaderAwar
 	 * Create a UserDetailsServiceResourceFactoryBean with a Resource that is a Properties file in the
 	 * format defined in {@link UserDetailsResourceFactoryBean}
 	 *
-	 * @param propertiesResource the Resource that is a properties file that contains the users
+	 * @param resource the Resource that is a properties file that contains the users
 	 * @return the UserDetailsResourceFactoryBean
 	 */
-	public static UserDetailsManagerResourceFactoryBean usersFromResource(Resource propertiesResource) {
+	public static UserDetailsManagerResourceFactoryBean fromResource(Resource resource) {
 		UserDetailsManagerResourceFactoryBean result = new UserDetailsManagerResourceFactoryBean();
-		result.setPropertiesResource(propertiesResource);
+		result.setResource(resource);
 		return result;
 	}
 }

config/src/test/java/org/springframework/security/config/core/userdetails/UserDetailsRepositoryResourceFactoryBeanPropertiesResourceITests.java

@@ -48,7 +48,7 @@ public class UserDetailsRepositoryResourceFactoryBeanPropertiesResourceITests {
 	static class Config {
 		@Bean
 		public UserDetailsRepositoryResourceFactoryBean userDetailsService() {
-			return UserDetailsRepositoryResourceFactoryBean.usersFromResource(new InMemoryResource("user=password,ROLE_USER"));
+			return UserDetailsRepositoryResourceFactoryBean.fromResource(new InMemoryResource("user=password,ROLE_USER"));
 		}
 	}
 }

config/src/test/java/org/springframework/security/config/core/userdetails/UserDetailsRepositoryResourceFactoryBeanPropertiesResourceLocationITests.java

@@ -47,7 +47,7 @@ public class UserDetailsRepositoryResourceFactoryBeanPropertiesResourceLocationI
 	static class Config {
 		@Bean
 		public UserDetailsRepositoryResourceFactoryBean userDetailsService() {
-			return UserDetailsRepositoryResourceFactoryBean.usersFromResourceLocation("classpath:users.properties");
+			return UserDetailsRepositoryResourceFactoryBean.fromResourceLocation("classpath:users.properties");
 		}
 	}
 }

config/src/test/java/org/springframework/security/config/core/userdetails/UserDetailsResourceFactoryBeanTest.java

@@ -22,23 +22,17 @@ import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
 import org.mockito.runners.MockitoJUnitRunner;
-import org.springframework.core.io.Resource;
 import org.springframework.core.io.ResourceLoader;
 import org.springframework.security.core.userdetails.User;
 import org.springframework.security.core.userdetails.UserDetails;
 import org.springframework.security.util.InMemoryResource;
 
-import java.io.ByteArrayInputStream;
-import java.io.IOException;
-import java.io.InputStream;
-import java.nio.charset.Charset;
 import java.util.Collection;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
 
 
 /**
@@ -47,20 +41,16 @@ import static org.mockito.Mockito.when;
  */
 @RunWith(MockitoJUnitRunner.class)
 public class UserDetailsResourceFactoryBeanTest {
-	String location = "classpath:users.properties";
-
 	@Mock
 	ResourceLoader resourceLoader;
 
 	UserDetailsResourceFactoryBean factory = new UserDetailsResourceFactoryBean();
 
 	@Test
-	public void getObjectWhenResourceLoaderNullThenThrowsIllegalStateException() throws Exception {
+	public void setResourceLoaderWhenNullThenThrowsException() throws Exception {
-		factory.setPropertiesResourceLocation(location);
+		assertThatThrownBy(() -> factory.setResourceLoader(null) )
-
-		assertThatThrownBy(() -> factory.getObject() )
 			.isInstanceOf(IllegalArgumentException.class)
-			.hasStackTraceContaining("resourceLoader cannot be null if propertiesResource is null");
+			.hasStackTraceContaining("resourceLoader cannot be null");
 	}
 
 	@Test
@@ -68,40 +58,29 @@ public class UserDetailsResourceFactoryBeanTest {
 		factory.setResourceLoader(resourceLoader);
 
 		assertThatThrownBy(() -> factory.getObject() )
-			.isInstanceOf(IllegalStateException.class)
+			.isInstanceOf(IllegalArgumentException.class)
-			.hasStackTraceContaining("Either propertiesResource cannot be null or both resourceLoader and propertiesResourceLocation cannot be null");
+			.hasStackTraceContaining("resource cannot be null if resourceLocation is null");
 	}
 
 	@Test
 	public void getObjectWhenPropertiesResourceLocationSingleUserThenThrowsGetsSingleUser() throws Exception {
-		setResource("user=password,ROLE_USER");
+		factory.setResourceLocation("classpath:users.properties");
 
 		Collection<UserDetails> users = factory.getObject();
 
-		UserDetails expectedUser = User.withUsername("user")
+		assertLoaded();
-			.password("password")
-			.authorities("ROLE_USER")
-			.build();
-		assertThat(users).containsExactly(expectedUser);
 	}
 
 	@Test
 	public void getObjectWhenPropertiesResourceSingleUserThenThrowsGetsSingleUser() throws Exception {
-		factory.setPropertiesResource(new InMemoryResource("user=password,ROLE_USER"));
+		factory.setResource(new InMemoryResource("user=password,ROLE_USER"));
 
-		Collection<UserDetails> users = factory.getObject();
+		assertLoaded();
-
-		UserDetails expectedUser = User.withUsername("user")
-			.password("password")
-			.authorities("ROLE_USER")
-			.build();
-		assertThat(users).containsExactly(expectedUser);
 	}
 
 	@Test
 	public void getObjectWhenInvalidUserThenThrowsMeaningfulException() throws Exception {
-		setResource("user=invalidFormatHere");
+		factory.setResource(new InMemoryResource("user=invalidFormatHere"));
-
 
 		assertThatThrownBy(() -> factory.getObject() )
 			.isInstanceOf(IllegalStateException.class)
@@ -109,11 +88,13 @@ public class UserDetailsResourceFactoryBeanTest {
 			.hasStackTraceContaining("invalidFormatHere");
 	}
 
-	private void setResource(String contents) throws IOException {
+	private void assertLoaded() throws Exception {
-		Resource resource = new InMemoryResource(contents);
+		Collection<UserDetails> users = factory.getObject();
-		when(resourceLoader.getResource(location)).thenReturn(resource);
 
-		factory.setPropertiesResourceLocation(location);
+		UserDetails expectedUser = User.withUsername("user")
-		factory.setResourceLoader(resourceLoader);
+			.password("password")
+			.authorities("ROLE_USER")
+			.build();
+		assertThat(users).containsExactly(expectedUser);
 	}
 }

config/src/test/java/org/springframework/security/config/provisioning/UserDetailsManagerResourceFactoryBeanPropertiesResourceITests.java

@@ -48,7 +48,7 @@ public class UserDetailsManagerResourceFactoryBeanPropertiesResourceITests {
 	static class Config {
 		@Bean
 		public UserDetailsManagerResourceFactoryBean userDetailsService() {
-			return UserDetailsManagerResourceFactoryBean.usersFromResource(new InMemoryResource("user=password,ROLE_USER"));
+			return UserDetailsManagerResourceFactoryBean.fromResource(new InMemoryResource("user=password,ROLE_USER"));
 		}
 	}
 }

config/src/test/java/org/springframework/security/config/provisioning/UserDetailsManagerResourceFactoryBeanPropertiesResourceLocationITests.java

@@ -47,7 +47,7 @@ public class UserDetailsManagerResourceFactoryBeanPropertiesResourceLocationITes
 	static class Config {
 		@Bean
 		public UserDetailsManagerResourceFactoryBean userDetailsService() {
-			return UserDetailsManagerResourceFactoryBean.usersFromResourceLocation("classpath:users.properties");
+			return UserDetailsManagerResourceFactoryBean.fromResourceLocation("classpath:users.properties");
 		}
 	}
 }