Merge pull request #956 from cmolodo/spring-custom-auth-fix

Fix DelegatingNegotiateSecurityFilter use of custom auth
Waffle · Jun 11, 2020 · e5df07f · e5df07f
2 parents e6d9ca5 + 57f2af4
commit e5df07f
Show file tree

Hide file tree

Showing 4 changed files with 134 additions and 12 deletions.
diff --git a/...affle-spring-security4/src/main/java/waffle/spring/DelegatingNegotiateSecurityFilter.java b/...affle-spring-security4/src/main/java/waffle/spring/DelegatingNegotiateSecurityFilter.java
@@ -136,16 +136,15 @@ public void setAuthenticationFailureHandler(final AuthenticationFailureHandler v
     protected boolean setAuthentication(final HttpServletRequest request, final HttpServletResponse response,
             final Authentication authentication) {
         try {
+            Authentication delegateAuthentication = authentication;
             if (this.authenticationManager != null) {
                 DelegatingNegotiateSecurityFilter.LOGGER.debug("Delegating to custom authenticationmanager");
-                final Authentication customAuthentication = this.authenticationManager.authenticate(authentication);
-                SecurityContextHolder.getContext().setAuthentication(customAuthentication);
-            } else {
-                SecurityContextHolder.getContext().setAuthentication(authentication);
+                delegateAuthentication = this.authenticationManager.authenticate(authentication);
             }
+            SecurityContextHolder.getContext().setAuthentication(delegateAuthentication);
             if (this.authenticationSuccessHandler != null) {
                 try {
-                    this.authenticationSuccessHandler.onAuthenticationSuccess(request, response, authentication);
+                    this.authenticationSuccessHandler.onAuthenticationSuccess(request, response, delegateAuthentication);
                 } catch (final IOException | ServletException e) {
                     DelegatingNegotiateSecurityFilter.LOGGER.warn("Error calling authenticationSuccessHandler: {}",
                             e.getMessage());

diff --git a/...e-spring-security4/src/test/java/waffle/spring/DelegatingNegotiateSecurityFilterTest.java b/...e-spring-security4/src/test/java/waffle/spring/DelegatingNegotiateSecurityFilterTest.java
@@ -20,18 +20,21 @@
 import java.util.List;
 
 import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
 
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.support.AbstractApplicationContext;
 import org.springframework.context.support.ClassPathXmlApplicationContext;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.GrantedAuthority;
 import org.springframework.security.core.context.SecurityContextHolder;
-
+import org.springframework.security.web.authentication.AuthenticationSuccessHandler;
 import waffle.mock.http.SimpleFilterChain;
 import waffle.mock.http.SimpleHttpRequest;
 import waffle.mock.http.SimpleHttpResponse;
@@ -123,4 +126,63 @@ public void testNegotiate() throws IOException, ServletException {
         Assertions.assertEquals(0, response.getHeaderNamesSize());
     }
 
+    /**
+     * Tests that the same authentication token generated by the custom authentication manager is provided to both the
+     * SecurityContext and the custom authentication success handler.
+     *
+     * @throws IOException
+     *             Signals that an I/O exception has occurred.
+     * @throws ServletException
+     *             the servlet exception
+     */
+    @Test
+    public void testNegotiate_CustomAuth() throws IOException, ServletException {
+        final Authentication customToken = Mockito.mock(Authentication.class);
+        Mockito.when(customToken.getName()).thenReturn("Custom Token");
+        this.filter.setAuthenticationManager(authentication -> customToken);
+
+        final CustomAuthenticationSuccessHandler successHandler = new CustomAuthenticationSuccessHandler();
+        this.filter.setAuthenticationSuccessHandler(successHandler);
+
+        final String securityPackage = "Negotiate";
+        final SimpleFilterChain filterChain = new SimpleFilterChain();
+        final SimpleHttpRequest request = new SimpleHttpRequest();
+
+        final String clientToken = Base64.getEncoder()
+                                         .encodeToString(WindowsAccountImpl.getCurrentUsername().getBytes(StandardCharsets.UTF_8));
+        request.addHeader("Authorization", securityPackage + " " + clientToken);
+
+        final SimpleHttpResponse response = new SimpleHttpResponse();
+        this.filter.doFilter(request, response, filterChain);
+
+        final Authentication auth = SecurityContextHolder.getContext().getAuthentication();
+        Assertions.assertNotNull(auth);
+        Assertions.assertEquals(customToken, auth);
+        Assertions.assertEquals("Custom Token", auth.getName());
+
+        Assertions.assertEquals(customToken, successHandler.getAuthentication());
+        Assertions.assertEquals("Custom Token", successHandler.getAuthentication().getName());
+    }
+
+}
+
+/**
+ * Provides a basic implementation that simply stores the provided Authentication so it can be checked for testing.
+ *
+ * Class declared here rather than in the general handlers package because it should NOT be added to the overall
+ * filter configuration, but only to the specific filter instance testing its use.
+ */
+class CustomAuthenticationSuccessHandler implements AuthenticationSuccessHandler {
+
+    private Authentication authentication;
+
+    @Override
+    public void onAuthenticationSuccess(HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse,
+                                        Authentication authentication) throws IOException, ServletException {
+        this.authentication = authentication;
+    }
+
+    public Authentication getAuthentication() {
+        return authentication;
+    }
 }
diff --git a/...affle-spring-security5/src/main/java/waffle/spring/DelegatingNegotiateSecurityFilter.java b/...affle-spring-security5/src/main/java/waffle/spring/DelegatingNegotiateSecurityFilter.java
@@ -136,16 +136,15 @@ public void setAuthenticationFailureHandler(final AuthenticationFailureHandler v
     protected boolean setAuthentication(final HttpServletRequest request, final HttpServletResponse response,
             final Authentication authentication) {
         try {
+            Authentication delegateAuthentication = authentication;
             if (this.authenticationManager != null) {
                 DelegatingNegotiateSecurityFilter.LOGGER.debug("Delegating to custom authenticationmanager");
-                final Authentication customAuthentication = this.authenticationManager.authenticate(authentication);
-                SecurityContextHolder.getContext().setAuthentication(customAuthentication);
-            } else {
-                SecurityContextHolder.getContext().setAuthentication(authentication);
+                delegateAuthentication = this.authenticationManager.authenticate(authentication);
             }
+            SecurityContextHolder.getContext().setAuthentication(delegateAuthentication);
             if (this.authenticationSuccessHandler != null) {
                 try {
-                    this.authenticationSuccessHandler.onAuthenticationSuccess(request, response, authentication);
+                    this.authenticationSuccessHandler.onAuthenticationSuccess(request, response, delegateAuthentication);
                 } catch (final IOException | ServletException e) {
                     DelegatingNegotiateSecurityFilter.LOGGER.warn("Error calling authenticationSuccessHandler: {}",
                             e.getMessage());

diff --git a/...e-spring-security5/src/test/java/waffle/spring/DelegatingNegotiateSecurityFilterTest.java b/...e-spring-security5/src/test/java/waffle/spring/DelegatingNegotiateSecurityFilterTest.java
@@ -20,18 +20,21 @@
 import java.util.List;
 
 import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
 
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.support.AbstractApplicationContext;
 import org.springframework.context.support.ClassPathXmlApplicationContext;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.GrantedAuthority;
 import org.springframework.security.core.context.SecurityContextHolder;
-
+import org.springframework.security.web.authentication.AuthenticationSuccessHandler;
 import waffle.mock.http.SimpleFilterChain;
 import waffle.mock.http.SimpleHttpRequest;
 import waffle.mock.http.SimpleHttpResponse;
@@ -123,4 +126,63 @@ public void testNegotiate() throws IOException, ServletException {
         Assertions.assertEquals(0, response.getHeaderNamesSize());
     }
 
+    /**
+     * Tests that the same authentication token generated by the custom authentication manager is provided to both the
+     * SecurityContext and the custom authentication success handler.
+     *
+     * @throws IOException
+     *             Signals that an I/O exception has occurred.
+     * @throws ServletException
+     *             the servlet exception
+     */
+    @Test
+    public void testNegotiate_CustomAuth() throws IOException, ServletException {
+        final Authentication customToken = Mockito.mock(Authentication.class);
+        Mockito.when(customToken.getName()).thenReturn("Custom Token");
+        this.filter.setAuthenticationManager(authentication -> customToken);
+
+        final CustomAuthenticationSuccessHandler successHandler = new CustomAuthenticationSuccessHandler();
+        this.filter.setAuthenticationSuccessHandler(successHandler);
+
+        final String securityPackage = "Negotiate";
+        final SimpleFilterChain filterChain = new SimpleFilterChain();
+        final SimpleHttpRequest request = new SimpleHttpRequest();
+
+        final String clientToken = Base64.getEncoder()
+                                         .encodeToString(WindowsAccountImpl.getCurrentUsername().getBytes(StandardCharsets.UTF_8));
+        request.addHeader("Authorization", securityPackage + " " + clientToken);
+
+        final SimpleHttpResponse response = new SimpleHttpResponse();
+        this.filter.doFilter(request, response, filterChain);
+
+        final Authentication auth = SecurityContextHolder.getContext().getAuthentication();
+        Assertions.assertNotNull(auth);
+        Assertions.assertEquals(customToken, auth);
+        Assertions.assertEquals("Custom Token", auth.getName());
+
+        Assertions.assertEquals(customToken, successHandler.getAuthentication());
+        Assertions.assertEquals("Custom Token", successHandler.getAuthentication().getName());
+    }
+
+}
+
+/**
+ * Provides a basic implementation that simply stores the provided Authentication so it can be checked for testing.
+ *
+ * Class declared here rather than in the general handlers package because it should NOT be added to the overall
+ * filter configuration, but only to the specific filter instance testing its use.
+ */
+class CustomAuthenticationSuccessHandler implements AuthenticationSuccessHandler {
+
+    private Authentication authentication;
+
+    @Override
+    public void onAuthenticationSuccess(HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse,
+                                        Authentication authentication) throws IOException, ServletException {
+        this.authentication = authentication;
+    }
+
+    public Authentication getAuthentication() {
+        return authentication;
+    }
 }