Sfoglia il codice sorgente

SEC-2020: Set eraseCredentialsAfterAuthentication when using http@authentication-manager-ref

Previously the namespace configuration did not properly set the eraseCredentialsAfterAuthentication
property on the parent AuthenticationProvider when using http@authentication-manager-ref.

Now the ProviderManager that is created by the namespace consults the original
AuthenticationManager to determine if eraseCredentialsAfterAuthentication should
be set on the wrapped instance. If the original is not a ProviderManager the
eraseCredentialsAfterAuthentication is set to false since we should not "magically"
add behavior to the custom AuthenticationManager without knowing the desired behavior.
Rob Winch 13 anni fa
parent
commit
a19cc8f1c7

+ 38 - 1
config/src/main/java/org/springframework/security/config/http/HttpSecurityBeanDefinitionParser.java

@@ -32,6 +32,7 @@ import org.springframework.beans.factory.xml.BeanDefinitionParser;
 import org.springframework.beans.factory.xml.ParserContext;
 import org.springframework.core.OrderComparator;
 import org.springframework.core.Ordered;
+import org.springframework.security.authentication.AuthenticationManager;
 import org.springframework.security.authentication.ProviderManager;
 import org.springframework.security.config.BeanIds;
 import org.springframework.security.config.Elements;
@@ -231,7 +232,13 @@ public class HttpSecurityBeanDefinitionParser implements BeanDefinitionParser {
         authManager.addConstructorArgValue(authenticationProviders);
 
         if (StringUtils.hasText(parentMgrRef)) {
-            authManager.addConstructorArgValue(new RuntimeBeanReference(parentMgrRef));
+            RuntimeBeanReference parentAuthManager = new RuntimeBeanReference(parentMgrRef);
+            authManager.addConstructorArgValue(parentAuthManager);
+            RootBeanDefinition clearCredentials = new RootBeanDefinition(ClearCredentialsMethodInvokingFactoryBean.class);
+            clearCredentials.getPropertyValues().addPropertyValue("targetObject", parentAuthManager);
+            clearCredentials.getPropertyValues().addPropertyValue("targetMethod", "isEraseCredentialsAfterAuthentication");
+
+            authManager.addPropertyValue("eraseCredentialsAfterAuthentication", clearCredentials);
         } else {
             RootBeanDefinition amfb = new RootBeanDefinition(AuthenticationManagerFactoryBean.class);
             amfb.setRole(BeanDefinition.ROLE_INFRASTRUCTURE);
@@ -363,3 +370,33 @@ class OrderDecorator implements Ordered {
     }
 }
 
+/**
+ * Custom {@link MethodInvokingFactoryBean} that is specifically used for looking up the child {@link ProviderManager}
+ * value for {@link ProviderManager#setEraseCredentialsAfterAuthentication(boolean)} given the parent
+ * {@link AuthenticationManager}. This is necessary because the parent {@link AuthenticationManager} might not be a
+ * {@link ProviderManager}.
+ *
+ * @author Rob Winch
+ */
+final class ClearCredentialsMethodInvokingFactoryBean extends MethodInvokingFactoryBean {
+    public void afterPropertiesSet() throws Exception {
+        boolean isTargetProviderManager = getTargetObject() instanceof ProviderManager;
+        if(!isTargetProviderManager) {
+            setTargetObject(this);
+        }
+        super.afterPropertiesSet();
+    }
+
+    /**
+     * The default value if the target object is not a ProviderManager is false. We use false because this feature is
+     * associated with {@link ProviderManager} not {@link AuthenticationManager}. If the user wants to leverage
+     * {@link ProviderManager#setEraseCredentialsAfterAuthentication(boolean)} their original
+     * {@link AuthenticationManager} must be a {@link ProviderManager} (we should not magically add this functionality
+     * to their implementation since we cannot determine if it should be on or off).
+     *
+     * @return
+     */
+    public boolean isEraseCredentialsAfterAuthentication() {
+        return false;
+    }
+}

+ 23 - 0
config/src/test/groovy/org/springframework/security/config/http/MiscHttpConfigTests.groovy

@@ -624,6 +624,29 @@ class MiscHttpConfigTests extends AbstractHttpConfigTests {
         !getFilter(UsernamePasswordAuthenticationFilter).authenticationManager.eraseCredentialsAfterAuthentication
     }
 
+    def 'SEC-2020 authentication-manager@erase-credentials with http@authentication-manager-ref'() {
+        xml.http('authentication-manager-ref':'authMgr') {
+            'form-login'()
+        }
+        createAppContext("<authentication-manager id='authMgr' erase-credentials='false' />");
+        expect:
+        def authManager = getFilter(UsernamePasswordAuthenticationFilter).authenticationManager
+        !authManager.eraseCredentialsAfterAuthentication
+        !authManager.parent.eraseCredentialsAfterAuthentication
+    }
+
+    def 'authentication-manager@erase-credentials with http@authentication-manager-ref not ProviderManager'() {
+        xml.http('authentication-manager-ref':'authMgr') {
+            'form-login'()
+        }
+        xml.'b:bean'(id: 'authMgr', 'class': MockAuthenticationManager.class.name)
+        createAppContext()
+        expect:
+        def authManager = getFilter(UsernamePasswordAuthenticationFilter).authenticationManager
+        !authManager.eraseCredentialsAfterAuthentication
+        authManager.parent instanceof MockAuthenticationManager
+    }
+
     def jeeFilterExtractsExpectedRoles() {
         xml.http() {
             jee('mappable-roles': 'admin,user,a,b,c')