SEC-3204: InMemoryUserDetailsManager NullPointerException

[spring-projects-issues]

Updated

We should provide better error messages when an invalid user format is provided

Original

Henrik Sachse (Migrated from SEC-3204) said:

The org.springframework.security.provisioning.InMemoryUserDetailsManager may throw a NullPointerException when an invalid user Properties file gets loaded (see attachments).

Can be reproduced with the following code having the attached file on the classpath:

String invalidPropertiesFile = "invalid.properties";
ClassPathResource classPathResource = new ClassPathResource(invalidPropertiesFile);
Properties userProperties = new Properties();
userProperties.load(classPathResource.getInputStream());
// NullPointerException thrown here
InMemoryUserDetailsManager inMemoryUserDetailsManager = new InMemoryUserDetailsManager(userProperties);

That is because the method org.springframework.security.core.userdetails.memory.UserAttributeEditor.setAsText(String) may invoke the java.beans.PropertyEditorSupport.setValue(Object) method with null in case its parameter is an empty string.

[spring-projects-issues]

Henrik Sachse said:

Please refer to the pull request #256

[ghillert]

@rwinch Was hitting this issue today -

The PR was closed. I actually have a minor use-case. A user may not necessarily have Authorities associated with their credentials. This works:

        users:
          bob: bobspassword, ROLE_ADMIN
          alice: alicepwd, ROLE_ADMIN

But this throws a null-pointer:

        users:
          bob: bobspassword
          alice: alicepwd

I believe the underlying UserAttributeEditor should be able to handle this gracefully without the null-pointer.

[rwinch]

@ghillert Thanks for the feedback.

I think that it is reasonable to expect you wouldn't get a NPE. However, #256 would simply skip the invalid lines so you would not have the users added and there would be no error. See #256 (comment)

In short, I agree we do not want to have the NPE but the PR did not solve this correctly. This is why the PR is closed and this issue is still open.

Additional feedback welcome!

[ghillert]

@rwinch Gotcha. I would expect, though, that if no roles are provided in the properties string that everything gets created without exception (or have that as an option, e.g. ignoreMissingRoles).

Just for completeness here is the stacktrace users might get:

org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'springSecurityFilterChain' defined in class path resource [org/springframework/security/config/annotation/web/configuration/WebSecurityConfiguration.class]: Bean instantiation via factory method failed; nested exception is org.springframework.beans.BeanInstantiationException: Failed to instantiate [javax.servlet.Filter]: Factory method 'springSecurityFilterChain' threw exception; nested exception is java.lang.NullPointerException
    at org.springframework.beans.factory.support.ConstructorResolver.instantiateUsingFactoryMethod(ConstructorResolver.java:599) ~[spring-beans-4.2.5.RELEASE.jar:4.2.5.RELEASE]
    at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.instantiateUsingFactoryMethod(AbstractAutowireCapableBeanFactory.java:1123) ~[spring-beans-4.2.5.RELEASE.jar:4.2.5.RELEASE]
    at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBeanInstance(AbstractAutowireCapableBeanFactory.java:1018) ~[spring-beans-4.2.5.RELEASE.jar:4.2.5.RELEASE]
    at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:510) ~[spring-beans-4.2.5.RELEASE.jar:4.2.5.RELEASE]
    at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:482) ~[spring-beans-4.2.5.RELEASE.jar:4.2.5.RELEASE]
    at org.springframework.beans.factory.support.AbstractBeanFactory$1.getObject(AbstractBeanFactory.java:306) ~[spring-beans-4.2.5.RELEASE.jar:4.2.5.RELEASE]
    at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:230) ~[spring-beans-4.2.5.RELEASE.jar:4.2.5.RELEASE]
    at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:302) ~[spring-beans-4.2.5.RELEASE.jar:4.2.5.RELEASE]
    at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:202) ~[spring-beans-4.2.5.RELEASE.jar:4.2.5.RELEASE]
    at org.springframework.context.support.AbstractApplicationContext.getBean(AbstractApplicationContext.java:1060) ~[spring-context-4.2.5.RELEASE.jar:4.2.5.RELEASE]
    at org.springframework.web.filter.DelegatingFilterProxy.initDelegate(DelegatingFilterProxy.java:326) ~[spring-web-4.2.6.RELEASE.jar:4.2.6.RELEASE]
    at org.springframework.web.filter.DelegatingFilterProxy.initFilterBean(DelegatingFilterProxy.java:235) ~[spring-web-4.2.6.RELEASE.jar:4.2.6.RELEASE]
    at org.springframework.web.filter.GenericFilterBean.init(GenericFilterBean.java:199) ~[spring-web-4.2.6.RELEASE.jar:4.2.6.RELEASE]
    at org.apache.catalina.core.ApplicationFilterConfig.initFilter(ApplicationFilterConfig.java:279) ~[tomcat-embed-core-8.0.33.jar:8.0.33]
    at org.apache.catalina.core.ApplicationFilterConfig.<init>(ApplicationFilterConfig.java:109) ~[tomcat-embed-core-8.0.33.jar:8.0.33]
    at org.apache.catalina.core.StandardContext.filterStart(StandardContext.java:4658) [tomcat-embed-core-8.0.33.jar:8.0.33]
    at org.apache.catalina.core.StandardContext.startInternal(StandardContext.java:5277) [tomcat-embed-core-8.0.33.jar:8.0.33]
    at org.apache.catalina.util.LifecycleBase.start(LifecycleBase.java:147) [tomcat-embed-core-8.0.33.jar:8.0.33]
    at org.apache.catalina.core.ContainerBase$StartChild.call(ContainerBase.java:1408) [tomcat-embed-core-8.0.33.jar:8.0.33]
    at org.apache.catalina.core.ContainerBase$StartChild.call(ContainerBase.java:1398) [tomcat-embed-core-8.0.33.jar:8.0.33]
    at java.util.concurrent.FutureTask.run(FutureTask.java:262) [na:1.7.0_60]
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145) [na:1.7.0_60]
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615) [na:1.7.0_60]
    at java.lang.Thread.run(Thread.java:745) [na:1.7.0_60]
Caused by: org.springframework.beans.BeanInstantiationException: Failed to instantiate [javax.servlet.Filter]: Factory method 'springSecurityFilterChain' threw exception; nested exception is java.lang.NullPointerException
    at org.springframework.beans.factory.support.SimpleInstantiationStrategy.instantiate(SimpleInstantiationStrategy.java:189) ~[spring-beans-4.2.5.RELEASE.jar:4.2.5.RELEASE]
    at org.springframework.beans.factory.support.ConstructorResolver.instantiateUsingFactoryMethod(ConstructorResolver.java:588) ~[spring-beans-4.2.5.RELEASE.jar:4.2.5.RELEASE]
    ... 23 common frames omitted
Caused by: java.lang.NullPointerException: null
    at org.springframework.security.provisioning.InMemoryUserDetailsManager.<init>(InMemoryUserDetailsManager.java:54) ~[spring-security-core-4.0.4.RELEASE.jar:4.0.4.RELEASE]
    at org.springframework.cloud.dataflow.server.config.security.FileAuthenticationConfiguration.init(FileAuthenticationConfiguration.java:50) ~[classes/:na]
    at org.springframework.security.config.annotation.authentication.configurers.GlobalAuthenticationConfigurerAdapter.init(GlobalAuthenticationConfigurerAdapter.java:33) ~[spring-security-config-4.0.4.RELEASE.jar:4.0.4.RELEASE]
    at org.springframework.cloud.dataflow.server.config.security.FileAuthenticationConfiguration$$EnhancerBySpringCGLIB$$962ac1cf.init(<generated>) ~[FileAuthenticationConfiguration$$EnhancerBySpringCGLIB$$962ac1cf.class:na]
    at org.springframework.security.config.annotation.AbstractConfiguredSecurityBuilder.init(AbstractConfiguredSecurityBuilder.java:370) ~[spring-security-config-4.0.4.RELEASE.jar:4.0.4.RELEASE]
    at org.springframework.security.config.annotation.AbstractConfiguredSecurityBuilder.doBuild(AbstractConfiguredSecurityBuilder.java:324) ~[spring-security-config-4.0.4.RELEASE.jar:4.0.4.RELEASE]
    at org.springframework.security.config.annotation.AbstractSecurityBuilder.build(AbstractSecurityBuilder.java:41) ~[spring-security-config-4.0.4.RELEASE.jar:4.0.4.RELEASE]
    at org.springframework.security.config.annotation.authentication.configuration.AuthenticationConfiguration.getAuthenticationManager(AuthenticationConfiguration.java:81) ~[spring-security-config-4.0.4.RELEASE.jar:4.0.4.RELEASE]
    at org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter.authenticationManager(WebSecurityConfigurerAdapter.java:237) ~[spring-security-config-4.0.4.RELEASE.jar:4.0.4.RELEASE]
    at org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter.getHttp(WebSecurityConfigurerAdapter.java:175) ~[spring-security-config-4.0.4.RELEASE.jar:4.0.4.RELEASE]
    at org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter.init(WebSecurityConfigurerAdapter.java:290) ~[spring-security-config-4.0.4.RELEASE.jar:4.0.4.RELEASE]
    at org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter.init(WebSecurityConfigurerAdapter.java:67) ~[spring-security-config-4.0.4.RELEASE.jar:4.0.4.RELEASE]
    at org.springframework.cloud.dataflow.server.config.security.SecurityConfiguration$$EnhancerBySpringCGLIB$$6b529863.init(<generated>) ~[SecurityConfiguration$$EnhancerBySpringCGLIB$$6b529863.class:na]
    at org.springframework.security.config.annotation.AbstractConfiguredSecurityBuilder.init(AbstractConfiguredSecurityBuilder.java:370) ~[spring-security-config-4.0.4.RELEASE.jar:4.0.4.RELEASE]
    at org.springframework.security.config.annotation.AbstractConfiguredSecurityBuilder.doBuild(AbstractConfiguredSecurityBuilder.java:324) ~[spring-security-config-4.0.4.RELEASE.jar:4.0.4.RELEASE]
    at org.springframework.security.config.annotation.AbstractSecurityBuilder.build(AbstractSecurityBuilder.java:41) ~[spring-security-config-4.0.4.RELEASE.jar:4.0.4.RELEASE]
    at org.springframework.security.config.annotation.web.configuration.WebSecurityConfiguration.springSecurityFilterChain(WebSecurityConfiguration.java:105) ~[spring-security-config-4.0.4.RELEASE.jar:4.0.4.RELEASE]
    at org.springframework.security.config.annotation.web.configuration.WebSecurityConfiguration$$EnhancerBySpringCGLIB$$73bd2b2.CGLIB$springSecurityFilterChain$4(<generated>) ~[spring-security-config-4.0.4.RELEASE.jar:4.0.4.RELEASE]
    at org.springframework.security.config.annotation.web.configuration.WebSecurityConfiguration$$EnhancerBySpringCGLIB$$73bd2b2$$FastClassBySpringCGLIB$$fa8a2559.invoke(<generated>) ~[spring-security-config-4.0.4.RELEASE.jar:4.0.4.RELEASE]
    at org.springframework.cglib.proxy.MethodProxy.invokeSuper(MethodProxy.java:228) ~[spring-core-4.2.5.RELEASE.jar:4.2.5.RELEASE]
    at org.springframework.context.annotation.ConfigurationClassEnhancer$BeanMethodInterceptor.intercept(ConfigurationClassEnhancer.java:355) ~[spring-context-4.2.5.RELEASE.jar:4.2.5.RELEASE]
    at org.springframework.security.config.annotation.web.configuration.WebSecurityConfiguration$$EnhancerBySpringCGLIB$$73bd2b2.springSecurityFilterChain(<generated>) ~[spring-security-config-4.0.4.RELEASE.jar:4.0.4.RELEASE]
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[na:1.7.0_60]
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) ~[na:1.7.0_60]
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[na:1.7.0_60]
    at java.lang.reflect.Method.invoke(Method.java:606) ~[na:1.7.0_60]
    at org.springframework.beans.factory.support.SimpleInstantiationStrategy.instantiate(SimpleInstantiationStrategy.java:162) ~[spring-beans-4.2.5.RELEASE.jar:4.2.5.RELEASE]
    ... 24 common frames omitted

[rwinch]

@ghillert

Gotcha. I would expect, though, that if no roles are provided in the properties string that everything gets created without exception

That seems like a possible change we could make. Could you create an addition ticket for this? I view this ticket as better error message for invalid properties where as you appear to be requesting changing what is valid.