You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nifi.apache.org by th...@apache.org on 2022/08/09 20:10:38 UTC

[nifi] branch main updated: NIFI-10322 Corrected Cookie path when removing Bearer Token

This is an automated email from the ASF dual-hosted git repository.

thenatog pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/nifi.git


The following commit(s) were added to refs/heads/main by this push:
     new 77033ec11a NIFI-10322 Corrected Cookie path when removing Bearer Token
77033ec11a is described below

commit 77033ec11aa7e738aaa32e80957b4a1538923f18
Author: exceptionfactory <ex...@apache.org>
AuthorDate: Mon Aug 8 22:57:29 2022 -0500

    NIFI-10322 Corrected Cookie path when removing Bearer Token
    
    - Appended root path to Cookie path attribute when removing Bearer Tokens as part of unauthorized response handling
    - Updated Saml2AuthenticationSuccessHandler to follow standard Cookie path building strategy
    
    Signed-off-by: Nathan Gough <th...@gmail.com>
    
    This closes #6278.
---
 .../security/StandardAuthenticationEntryPoint.java |  4 +-
 .../Saml2AuthenticationSuccessHandler.java         |  4 +-
 .../StandardAuthenticationEntryPointTest.java      | 43 ++++++++++++++++--
 .../Saml2AuthenticationSuccessHandlerTest.java     | 51 +++++++++++++++++++---
 4 files changed, 92 insertions(+), 10 deletions(-)

diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/main/java/org/apache/nifi/web/security/StandardAuthenticationEntryPoint.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/main/java/org/apache/nifi/web/security/StandardAuthenticationEntryPoint.java
index dbe7eea195..ebf2da74d1 100644
--- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/main/java/org/apache/nifi/web/security/StandardAuthenticationEntryPoint.java
+++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/main/java/org/apache/nifi/web/security/StandardAuthenticationEntryPoint.java
@@ -45,6 +45,8 @@ public class StandardAuthenticationEntryPoint implements AuthenticationEntryPoin
 
     protected static final String UNAUTHORIZED = "Unauthorized";
 
+    private static final String ROOT_PATH = "/";
+
     private static final ApplicationCookieService applicationCookieService = new StandardApplicationCookieService();
 
     private final BearerTokenAuthenticationEntryPoint bearerTokenAuthenticationEntryPoint;
@@ -91,7 +93,7 @@ public class StandardAuthenticationEntryPoint implements AuthenticationEntryPoin
     private void removeAuthorizationBearerCookie(final HttpServletRequest request, final HttpServletResponse response) {
         final Optional<String> authorizationBearer = applicationCookieService.getCookieValue(request, ApplicationCookieName.AUTHORIZATION_BEARER);
         if (authorizationBearer.isPresent()) {
-            final URI uri = RequestUriBuilder.fromHttpServletRequest(request).build();
+            final URI uri = RequestUriBuilder.fromHttpServletRequest(request).path(ROOT_PATH).build();
             applicationCookieService.removeCookie(uri, response, ApplicationCookieName.AUTHORIZATION_BEARER);
         }
     }
diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/main/java/org/apache/nifi/web/security/saml2/web/authentication/Saml2AuthenticationSuccessHandler.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/main/java/org/apache/nifi/web/security/saml2/web/authentication/Saml2AuthenticationSuccessHandler.java
index 9e3b2ce1c6..646a5e5ab6 100644
--- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/main/java/org/apache/nifi/web/security/saml2/web/authentication/Saml2AuthenticationSuccessHandler.java
+++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/main/java/org/apache/nifi/web/security/saml2/web/authentication/Saml2AuthenticationSuccessHandler.java
@@ -47,6 +47,8 @@ import java.util.stream.Collectors;
 public class Saml2AuthenticationSuccessHandler extends SimpleUrlAuthenticationSuccessHandler {
     private static final String UI_PATH = "/nifi/";
 
+    private static final String ROOT_PATH = "/";
+
     private final ApplicationCookieService applicationCookieService = new StandardApplicationCookieService();
 
     private final BearerTokenProvider bearerTokenProvider;
@@ -108,7 +110,7 @@ public class Saml2AuthenticationSuccessHandler extends SimpleUrlAuthenticationSu
      */
     @Override
     public String determineTargetUrl(final HttpServletRequest request, final HttpServletResponse response, final Authentication authentication) {
-        final URI resourceUri = RequestUriBuilder.fromHttpServletRequest(request).build();
+        final URI resourceUri = RequestUriBuilder.fromHttpServletRequest(request).path(ROOT_PATH).build();
         processAuthentication(response, authentication, resourceUri);
 
         final URI targetUri = RequestUriBuilder.fromHttpServletRequest(request).path(UI_PATH).build();
diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/test/java/org/apache/nifi/web/security/StandardAuthenticationEntryPointTest.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/test/java/org/apache/nifi/web/security/StandardAuthenticationEntryPointTest.java
index 2047a748dc..8dccdfa966 100644
--- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/test/java/org/apache/nifi/web/security/StandardAuthenticationEntryPointTest.java
+++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/test/java/org/apache/nifi/web/security/StandardAuthenticationEntryPointTest.java
@@ -17,6 +17,7 @@
 package org.apache.nifi.web.security;
 
 import org.apache.nifi.web.security.cookie.ApplicationCookieName;
+import org.apache.nifi.web.util.WebUtils;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.springframework.mock.web.MockHttpServletRequest;
@@ -26,10 +27,12 @@ import org.springframework.security.core.AuthenticationException;
 import org.springframework.security.oauth2.core.OAuth2AuthenticationException;
 import org.springframework.security.oauth2.server.resource.web.BearerTokenAuthenticationEntryPoint;
 
+import javax.servlet.ServletContext;
 import javax.servlet.ServletException;
 import javax.servlet.http.Cookie;
 import javax.servlet.http.HttpServletResponse;
 import java.io.IOException;
+import java.io.UnsupportedEncodingException;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
@@ -41,6 +44,14 @@ class StandardAuthenticationEntryPointTest {
 
     static final String BEARER_TOKEN = "Bearer Token";
 
+    static final String ROOT_PATH = "/";
+
+    static final String FORWARDED_PATH = "/forwarded";
+
+    static final String FORWARDED_COOKIE_PATH = String.format("%s/", FORWARDED_PATH);
+
+    private static final String ALLOWED_CONTEXT_PATHS_PARAMETER = "allowedContextPaths";
+
     MockHttpServletRequest request;
 
     MockHttpServletResponse response;
@@ -101,12 +112,38 @@ class StandardAuthenticationEntryPointTest {
         request.setCookies(cookie);
         authenticationEntryPoint.commence(request, response, exception);
 
-        assertEquals(HttpServletResponse.SC_UNAUTHORIZED, response.getStatus());
+        assertResponseStatusUnauthorized();
+        assertBearerCookieRemoved(ROOT_PATH);
+    }
 
-        final Cookie responseCookie = response.getCookie(ApplicationCookieName.AUTHORIZATION_BEARER.getCookieName());
-        assertNotNull(responseCookie);
+    @Test
+    void testCommenceRemoveCookieForwardedPath() throws ServletException, IOException {
+        final AuthenticationException exception = new AuthenticationServiceException(FAILED);
+
+        final ServletContext servletContext = request.getServletContext();
+        servletContext.setInitParameter(ALLOWED_CONTEXT_PATHS_PARAMETER, FORWARDED_PATH);
+
+        request.addHeader(WebUtils.FORWARDED_PREFIX_HTTP_HEADER, FORWARDED_PATH);
+
+        final Cookie cookie = new Cookie(ApplicationCookieName.AUTHORIZATION_BEARER.getCookieName(), BEARER_TOKEN);
+        request.setCookies(cookie);
+        authenticationEntryPoint.commence(request, response, exception);
+
+        assertResponseStatusUnauthorized();
+        assertBearerCookieRemoved(FORWARDED_COOKIE_PATH);
+    }
+
+    void assertResponseStatusUnauthorized() throws UnsupportedEncodingException {
+        assertEquals(HttpServletResponse.SC_UNAUTHORIZED, response.getStatus());
 
         final String content = response.getContentAsString();
         assertEquals(StandardAuthenticationEntryPoint.UNAUTHORIZED, content);
     }
+
+    void assertBearerCookieRemoved(final String expectedCookiePath) {
+        final Cookie responseCookie = response.getCookie(ApplicationCookieName.AUTHORIZATION_BEARER.getCookieName());
+
+        assertNotNull(responseCookie);
+        assertEquals(expectedCookiePath, responseCookie.getPath());
+    }
 }
diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/test/java/org/apache/nifi/web/security/saml2/web/authentication/Saml2AuthenticationSuccessHandlerTest.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/test/java/org/apache/nifi/web/security/saml2/web/authentication/Saml2AuthenticationSuccessHandlerTest.java
index 97cda5ff31..0b0f72ab6d 100644
--- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/test/java/org/apache/nifi/web/security/saml2/web/authentication/Saml2AuthenticationSuccessHandlerTest.java
+++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/test/java/org/apache/nifi/web/security/saml2/web/authentication/Saml2AuthenticationSuccessHandlerTest.java
@@ -21,6 +21,7 @@ import org.apache.nifi.authorization.util.IdentityMapping;
 import org.apache.nifi.idp.IdpType;
 import org.apache.nifi.web.security.cookie.ApplicationCookieName;
 import org.apache.nifi.web.security.jwt.provider.BearerTokenProvider;
+import org.apache.nifi.web.util.WebUtils;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
@@ -32,6 +33,7 @@ import org.springframework.security.authentication.TestingAuthenticationToken;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.GrantedAuthority;
 
+import javax.servlet.ServletContext;
 import javax.servlet.http.Cookie;
 import java.time.Duration;
 import java.util.Collections;
@@ -58,14 +60,28 @@ class Saml2AuthenticationSuccessHandlerTest {
 
     private static final String REQUEST_URI = "/nifi-api";
 
+    private static final String UI_PATH = "/nifi/";
+
     private static final int SERVER_PORT = 8080;
 
-    private static final String TARGET_URL = "http://localhost:8080/nifi/";
+    private static final String LOCALHOST_URL = "http://localhost:8080";
+
+    private static final String TARGET_URL = String.format("%s%s", LOCALHOST_URL, UI_PATH);
+
+    static final String FORWARDED_PATH = "/forwarded";
+
+    static final String FORWARDED_COOKIE_PATH = String.format("%s/", FORWARDED_PATH);
+
+    private static final String FORWARDED_TARGET_URL = String.format("%s%s%s", LOCALHOST_URL, FORWARDED_PATH, UI_PATH);
 
     private static final String FIRST_GROUP = "$1";
 
     private static final Pattern MATCH_PATTERN = Pattern.compile("(.*)");
 
+    static final String ROOT_PATH = "/";
+
+    private static final String ALLOWED_CONTEXT_PATHS_PARAMETER = "allowedContextPaths";
+
     private static final IdentityMapping UPPER_IDENTITY_MAPPING = new IdentityMapping(
             IdentityMapping.Transform.UPPER.toString(),
             MATCH_PATTERN,
@@ -111,15 +127,40 @@ class Saml2AuthenticationSuccessHandlerTest {
     void testDetermineTargetUrl() {
         httpServletRequest.setRequestURI(REQUEST_URI);
 
+        assertTargetUrlEquals(TARGET_URL);
+        assertBearerCookieAdded(ROOT_PATH);
+        assertReplaceUserGroupsInvoked();
+    }
+
+    @Test
+    void testDetermineTargetUrlForwardedPath() {
+        final ServletContext servletContext = httpServletRequest.getServletContext();
+        servletContext.setInitParameter(ALLOWED_CONTEXT_PATHS_PARAMETER, FORWARDED_PATH);
+        httpServletRequest.addHeader(WebUtils.FORWARDED_PREFIX_HTTP_HEADER, FORWARDED_PATH);
+
+        httpServletRequest.setRequestURI(REQUEST_URI);
+
+        assertTargetUrlEquals(FORWARDED_TARGET_URL);
+        assertBearerCookieAdded(FORWARDED_COOKIE_PATH);
+        assertReplaceUserGroupsInvoked();
+    }
+
+    void assertReplaceUserGroupsInvoked() {
+        verify(idpUserGroupService).replaceUserGroups(eq(IDENTITY_UPPER), eq(IdpType.SAML), eq(Collections.singleton(AUTHORITY_LOWER)));
+    }
+
+    void assertTargetUrlEquals(final String expectedTargetUrl) {
         final Authentication authentication = new TestingAuthenticationToken(IDENTITY, IDENTITY, AUTHORITY);
 
         final String targetUrl = handler.determineTargetUrl(httpServletRequest, httpServletResponse, authentication);
 
-        assertEquals(TARGET_URL, targetUrl);
+        assertEquals(expectedTargetUrl, targetUrl);
+    }
 
-        verify(idpUserGroupService).replaceUserGroups(eq(IDENTITY_UPPER), eq(IdpType.SAML), eq(Collections.singleton(AUTHORITY_LOWER)));
+    void assertBearerCookieAdded(final String expectedCookiePath) {
+        final Cookie responseCookie = httpServletResponse.getCookie(ApplicationCookieName.AUTHORIZATION_BEARER.getCookieName());
 
-        final Cookie bearerCookie = httpServletResponse.getCookie(ApplicationCookieName.AUTHORIZATION_BEARER.getCookieName());
-        assertNotNull(bearerCookie);
+        assertNotNull(responseCookie);
+        assertEquals(expectedCookiePath, responseCookie.getPath());
     }
 }