[nifi] branch main updated: NIFI-10871 Skipped CSRF processing for replicated HTTP requests

[tp...@apache.org]

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

tpalfy 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 d55fb91b0f NIFI-10871 Skipped CSRF processing for replicated HTTP requests
d55fb91b0f is described below

commit d55fb91b0f2f62b081a8732e66dbbd3ea66961a6
Author: exceptionfactory <ex...@apache.org>
AuthorDate: Wed Nov 23 16:30:20 2022 -0600

    NIFI-10871 Skipped CSRF processing for replicated HTTP requests
    
    - Updated Security Filter Configuration to avoid unnecessary CSRF Request Token generation for requests replicated between cluster nodes
    
    This closes #6715.
    
    Signed-off-by: Tamas Palfy <tp...@apache.org>
---
 .../nifi/web/NiFiWebApiSecurityConfiguration.java  |   2 +
 .../security/csrf/SkipReplicatedCsrfFilter.java    |  61 +++++++++++
 .../csrf/SkipReplicatedCsrfFilterTest.java         | 113 +++++++++++++++++++++
 3 files changed, 176 insertions(+)

diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/NiFiWebApiSecurityConfiguration.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/NiFiWebApiSecurityConfiguration.java
index d43824d122..2108b09abe 100644
--- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/NiFiWebApiSecurityConfiguration.java
+++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/NiFiWebApiSecurityConfiguration.java
@@ -20,6 +20,7 @@ import org.apache.nifi.util.NiFiProperties;
 import org.apache.nifi.web.security.StandardAuthenticationEntryPoint;
 import org.apache.nifi.web.security.anonymous.NiFiAnonymousAuthenticationFilter;
 import org.apache.nifi.web.security.csrf.CsrfCookieRequestMatcher;
+import org.apache.nifi.web.security.csrf.SkipReplicatedCsrfFilter;
 import org.apache.nifi.web.security.csrf.StandardCookieCsrfTokenRepository;
 import org.apache.nifi.web.security.knox.KnoxAuthenticationFilter;
 import org.apache.nifi.web.security.log.AuthenticationUserFilter;
@@ -109,6 +110,7 @@ public class NiFiWebApiSecurityConfiguration {
                         ).permitAll()
                         .anyRequest().authenticated()
                 )
+                .addFilterBefore(new SkipReplicatedCsrfFilter(), CsrfFilter.class)
                 .csrf(csrf -> csrf
                         .csrfTokenRepository(
                                 new StandardCookieCsrfTokenRepository()
diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/main/java/org/apache/nifi/web/security/csrf/SkipReplicatedCsrfFilter.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/main/java/org/apache/nifi/web/security/csrf/SkipReplicatedCsrfFilter.java
new file mode 100644
index 0000000000..e3637fdb55
--- /dev/null
+++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/main/java/org/apache/nifi/web/security/csrf/SkipReplicatedCsrfFilter.java
@@ -0,0 +1,61 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.web.security.csrf;
+
+import org.springframework.security.web.csrf.CsrfFilter;
+import org.springframework.security.web.util.matcher.AndRequestMatcher;
+import org.springframework.security.web.util.matcher.NegatedRequestMatcher;
+import org.springframework.security.web.util.matcher.RequestHeaderRequestMatcher;
+import org.springframework.security.web.util.matcher.RequestMatcher;
+import org.springframework.web.filter.OncePerRequestFilter;
+
+import javax.servlet.FilterChain;
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import java.io.IOException;
+
+/**
+ * Skip Replicated Cross-Site Request Forgery Filter disables subsequent filtering for matched requests
+ */
+public class SkipReplicatedCsrfFilter extends OncePerRequestFilter {
+    /** RequestReplicator.REQUEST_TRANSACTION_ID_HEADER applied to replicated cluster requests */
+    protected static final String REPLICATED_REQUEST_HEADER = "X-RequestTransactionId";
+
+    /** Requests containing replicated header and not containing authorization cookies will be skipped */
+    private static final RequestMatcher REQUEST_MATCHER = new AndRequestMatcher(
+            new RequestHeaderRequestMatcher(REPLICATED_REQUEST_HEADER),
+            new NegatedRequestMatcher(new CsrfCookieRequestMatcher())
+    );
+
+    /**
+     * Set request attribute to disable standard CSRF Filter when request matches
+     *
+     * @param request HTTP Servlet Request
+     * @param response HTTP Servlet Response
+     * @param filterChain Filter Chain
+     * @throws ServletException Thrown on FilterChain.doFilter()
+     * @throws IOException Thrown on FilterChain.doFilter()
+     */
+    @Override
+    protected void doFilterInternal(final HttpServletRequest request, final HttpServletResponse response, final FilterChain filterChain) throws ServletException, IOException {
+        if (REQUEST_MATCHER.matches(request)) {
+            CsrfFilter.skipRequest(request);
+        }
+        filterChain.doFilter(request, response);
+    }
+}
diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/test/java/org/apache/nifi/web/security/csrf/SkipReplicatedCsrfFilterTest.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/test/java/org/apache/nifi/web/security/csrf/SkipReplicatedCsrfFilterTest.java
new file mode 100644
index 0000000000..a2e0727c44
--- /dev/null
+++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/test/java/org/apache/nifi/web/security/csrf/SkipReplicatedCsrfFilterTest.java
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.web.security.csrf;
+
+import org.apache.nifi.web.security.http.SecurityCookieName;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+import org.springframework.http.HttpMethod;
+import org.springframework.mock.web.MockHttpServletRequest;
+import org.springframework.mock.web.MockHttpServletResponse;
+import org.springframework.security.web.csrf.CsrfFilter;
+
+import javax.servlet.FilterChain;
+import javax.servlet.ServletException;
+import javax.servlet.http.Cookie;
+
+import java.io.IOException;
+import java.util.UUID;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.verify;
+
+@ExtendWith(MockitoExtension.class)
+class SkipReplicatedCsrfFilterTest {
+    private static final String SHOULD_NOT_FILTER = String.format("SHOULD_NOT_FILTER%s", CsrfFilter.class.getName());
+
+    MockHttpServletRequest httpServletRequest;
+
+    MockHttpServletResponse httpServletResponse;
+
+    @Mock
+    FilterChain filterChain;
+
+    SkipReplicatedCsrfFilter filter;
+
+    @BeforeEach
+    void setHandler() {
+        filter = new SkipReplicatedCsrfFilter();
+        httpServletRequest = new MockHttpServletRequest();
+        httpServletResponse = new MockHttpServletResponse();
+    }
+
+    @Test
+    void testDoFilterInternalNotSkipped() throws ServletException, IOException {
+        httpServletRequest.setMethod(HttpMethod.GET.name());
+
+        filter.doFilterInternal(httpServletRequest, httpServletResponse, filterChain);
+
+        assertCsrfFilterNotSkipped();
+    }
+
+    @Test
+    void testDoFilterInternalBearerCookieNotSkipped() throws ServletException, IOException {
+        httpServletRequest.setMethod(HttpMethod.GET.name());
+        final Cookie bearerCookie = new Cookie(SecurityCookieName.AUTHORIZATION_BEARER.getName(), UUID.randomUUID().toString());
+        httpServletRequest.setCookies(bearerCookie);
+
+        filter.doFilterInternal(httpServletRequest, httpServletResponse, filterChain);
+
+        assertCsrfFilterNotSkipped();
+    }
+
+    @Test
+    void testDoFilterInternalReplicatedHeaderAndBearerCookieNotSkipped() throws ServletException, IOException {
+        httpServletRequest.setMethod(HttpMethod.GET.name());
+        httpServletRequest.addHeader(SkipReplicatedCsrfFilter.REPLICATED_REQUEST_HEADER, UUID.randomUUID().toString());
+        final Cookie bearerCookie = new Cookie(SecurityCookieName.AUTHORIZATION_BEARER.getName(), UUID.randomUUID().toString());
+        httpServletRequest.setCookies(bearerCookie);
+
+        filter.doFilterInternal(httpServletRequest, httpServletResponse, filterChain);
+
+        assertCsrfFilterNotSkipped();
+    }
+
+    @Test
+    void testDoFilterInternalReplicatedHeaderSkipped() throws ServletException, IOException {
+        httpServletRequest.setMethod(HttpMethod.GET.name());
+        httpServletRequest.addHeader(SkipReplicatedCsrfFilter.REPLICATED_REQUEST_HEADER, UUID.randomUUID().toString());
+
+        filter.doFilterInternal(httpServletRequest, httpServletResponse, filterChain);
+
+        final Object shouldNotFilter = httpServletRequest.getAttribute(SHOULD_NOT_FILTER);
+
+        assertEquals(Boolean.TRUE, shouldNotFilter);
+        verify(filterChain).doFilter(eq(httpServletRequest), eq(httpServletResponse));
+    }
+
+    private void assertCsrfFilterNotSkipped() throws ServletException, IOException {
+        final Object shouldNotFilter = httpServletRequest.getAttribute(SHOULD_NOT_FILTER);
+
+        assertNotEquals(Boolean.TRUE, shouldNotFilter);
+        verify(filterChain).doFilter(eq(httpServletRequest), eq(httpServletResponse));
+    }
+}