This is an automated email from the ASF dual-hosted git repository.
more pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/knox.git
The following commit(s) were added to refs/heads/master by this push:
new 06c3f8c4c KNOX-3134: Set Secure and HttpOnly attributes for
pac4jCsrfToken Cookie (#1029)
06c3f8c4c is described below
commit 06c3f8c4c28b63b060528272dc8e6c023598416d
Author: hanicz <[email protected]>
AuthorDate: Tue Apr 29 19:37:31 2025 +0200
KNOX-3134: Set Secure and HttpOnly attributes for pac4jCsrfToken Cookie
(#1029)
* KNOX-3134: Set Secure and HttpOnly attributes for pac4jCsrfToken Cookie
* KNOX-3134: Pac4jProviderTest nullpointer fix
---
.../pac4j/filter/Pac4jDispatcherFilter.java | 27 +++++++-
.../knox/gateway/pac4j/Pac4jProviderTest.java | 9 +++
.../pac4j/filter/Pac4jDispatcherFilterTest.java | 76 ++++++++++++++++++++--
3 files changed, 106 insertions(+), 6 deletions(-)
diff --git
a/gateway-provider-security-pac4j/src/main/java/org/apache/knox/gateway/pac4j/filter/Pac4jDispatcherFilter.java
b/gateway-provider-security-pac4j/src/main/java/org/apache/knox/gateway/pac4j/filter/Pac4jDispatcherFilter.java
index 7f1670ed0..d0b7521d9 100644
---
a/gateway-provider-security-pac4j/src/main/java/org/apache/knox/gateway/pac4j/filter/Pac4jDispatcherFilter.java
+++
b/gateway-provider-security-pac4j/src/main/java/org/apache/knox/gateway/pac4j/filter/Pac4jDispatcherFilter.java
@@ -57,8 +57,10 @@ import javax.servlet.ServletContext;
import javax.servlet.ServletException;
import javax.servlet.ServletRequest;
import javax.servlet.ServletResponse;
+import javax.servlet.http.Cookie;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.HttpServletResponseWrapper;
import java.io.IOException;
import java.util.Enumeration;
@@ -128,6 +130,9 @@ public class Pac4jDispatcherFilter implements Filter,
SessionInvalidator {
/* default value is same is KNOXSSO token ttl default */
private static final String PAC4J_COOKIE_MAX_AGE_DEFAULT = "-1";
+ private static final String PAC4J_CSRF_TOKEN = "pac4jCsrfToken";
+ private static boolean SSL_ENABLED = true;
+
private CallbackFilter callbackFilter;
private SecurityFilter securityFilter;
@@ -250,6 +255,8 @@ public class Pac4jDispatcherFilter implements Filter,
SessionInvalidator {
SessionInvalidators.KNOX_SSO_INVALIDATOR.registerSessionInvalidator(this);
+ GatewayConfig gatewayConfig = (GatewayConfig)
context.getAttribute(GatewayConfig.GATEWAY_CONFIG_ATTRIBUTE);
+ SSL_ENABLED = gatewayConfig.isSSLEnabled();
}
/**
@@ -320,7 +327,6 @@ public class Pac4jDispatcherFilter implements Filter,
SessionInvalidator {
@Override
public void doFilter( ServletRequest servletRequest, ServletResponse
servletResponse, FilterChain filterChain) throws IOException, ServletException {
-
final HttpServletRequest request = (HttpServletRequest) servletRequest;
request.setAttribute(PAC4J_CONFIG, securityFilter.getSharedConfig());
@@ -333,7 +339,7 @@ public class Pac4jDispatcherFilter implements Filter,
SessionInvalidator {
} else {
// otherwise just apply security and requires authentication
// apply RequiresAuthenticationFilter
- securityFilter.doFilter(servletRequest, servletResponse, filterChain);
+ securityFilter.doFilter(servletRequest, new
Pac4jSetCookieResponseWrapper((HttpServletResponse) servletResponse),
filterChain);
}
}
@@ -350,4 +356,21 @@ public class Pac4jDispatcherFilter implements Filter,
SessionInvalidator {
SessionInvalidators.KNOX_SSO_INVALIDATOR.unregisterSessionInvalidator(this);
}
+ static class Pac4jSetCookieResponseWrapper extends
HttpServletResponseWrapper {
+
+ Pac4jSetCookieResponseWrapper(HttpServletResponse response) {
+ super(response);
+ }
+
+ @Override
+ public void addCookie(Cookie cookie) {
+ if(cookie.getName().equals(PAC4J_CSRF_TOKEN)) {
+ cookie.setHttpOnly(true);
+ if(SSL_ENABLED) {
+ cookie.setSecure(true);
+ }
+ }
+ super.addCookie(cookie);
+ }
+ }
}
diff --git
a/gateway-provider-security-pac4j/src/test/java/org/apache/knox/gateway/pac4j/Pac4jProviderTest.java
b/gateway-provider-security-pac4j/src/test/java/org/apache/knox/gateway/pac4j/Pac4jProviderTest.java
index e73d8feab..de4b7a5c9 100644
---
a/gateway-provider-security-pac4j/src/test/java/org/apache/knox/gateway/pac4j/Pac4jProviderTest.java
+++
b/gateway-provider-security-pac4j/src/test/java/org/apache/knox/gateway/pac4j/Pac4jProviderTest.java
@@ -20,6 +20,7 @@ package org.apache.knox.gateway.pac4j;
import org.apache.knox.gateway.audit.api.AuditContext;
import org.apache.knox.gateway.audit.api.AuditService;
import org.apache.knox.gateway.audit.api.Auditor;
+import org.apache.knox.gateway.config.GatewayConfig;
import org.apache.knox.gateway.pac4j.filter.Pac4jDispatcherFilter;
import org.apache.knox.gateway.pac4j.filter.Pac4jIdentityAdapter;
import org.apache.knox.gateway.pac4j.session.KnoxSessionStore;
@@ -80,6 +81,8 @@ public class Pac4jProviderTest {
EasyMock.replay(services);
final ServletContext context =
EasyMock.createNiceMock(ServletContext.class);
+ GatewayConfig gatewayConfig =
EasyMock.createNiceMock(GatewayConfig.class);
+
EasyMock.expect(context.getAttribute(GatewayConfig.GATEWAY_CONFIG_ATTRIBUTE)).andReturn(gatewayConfig);
EasyMock.expect(context.getAttribute(GatewayServices.GATEWAY_SERVICES_ATTRIBUTE)).andReturn(services);
EasyMock.expect(context.getAttribute(GatewayServices.GATEWAY_CLUSTER_ATTRIBUTE)).andReturn(CLUSTER_NAME);
EasyMock.replay(context);
@@ -182,6 +185,8 @@ public class Pac4jProviderTest {
EasyMock.replay(services);
final ServletContext context =
EasyMock.createNiceMock(ServletContext.class);
+ GatewayConfig gatewayConfig =
EasyMock.createNiceMock(GatewayConfig.class);
+
EasyMock.expect(context.getAttribute(GatewayConfig.GATEWAY_CONFIG_ATTRIBUTE)).andReturn(gatewayConfig);
EasyMock.expect(context.getAttribute(GatewayServices.GATEWAY_SERVICES_ATTRIBUTE)).andReturn(services);
EasyMock.expect(context.getAttribute(GatewayServices.GATEWAY_CLUSTER_ATTRIBUTE)).andReturn(CLUSTER_NAME);
EasyMock.replay(context);
@@ -284,6 +289,8 @@ public class Pac4jProviderTest {
EasyMock.replay(services);
final ServletContext context =
EasyMock.createNiceMock(ServletContext.class);
+ GatewayConfig gatewayConfig =
EasyMock.createNiceMock(GatewayConfig.class);
+
EasyMock.expect(context.getAttribute(GatewayConfig.GATEWAY_CONFIG_ATTRIBUTE)).andReturn(gatewayConfig);
EasyMock.expect(context.getAttribute(GatewayServices.GATEWAY_SERVICES_ATTRIBUTE)).andReturn(services);
EasyMock.expect(context.getAttribute(GatewayServices.GATEWAY_CLUSTER_ATTRIBUTE)).andReturn(CLUSTER_NAME);
EasyMock.replay(context);
@@ -386,8 +393,10 @@ public class Pac4jProviderTest {
EasyMock.replay(services);
ServletContext context = EasyMock.createNiceMock(ServletContext.class);
+ GatewayConfig gatewayConfig =
EasyMock.createNiceMock(GatewayConfig.class);
EasyMock.expect(context.getAttribute(GatewayServices.GATEWAY_SERVICES_ATTRIBUTE)).andReturn(services);
EasyMock.expect(context.getAttribute(GatewayServices.GATEWAY_CLUSTER_ATTRIBUTE)).andReturn(CLUSTER_NAME);
+
EasyMock.expect(context.getAttribute(GatewayConfig.GATEWAY_CONFIG_ATTRIBUTE)).andReturn(gatewayConfig);
EasyMock.replay(context);
new Pac4jDispatcherFilter().init(
diff --git
a/gateway-provider-security-pac4j/src/test/java/org/apache/knox/gateway/pac4j/filter/Pac4jDispatcherFilterTest.java
b/gateway-provider-security-pac4j/src/test/java/org/apache/knox/gateway/pac4j/filter/Pac4jDispatcherFilterTest.java
index 19d20f427..c93d969be 100644
---
a/gateway-provider-security-pac4j/src/test/java/org/apache/knox/gateway/pac4j/filter/Pac4jDispatcherFilterTest.java
+++
b/gateway-provider-security-pac4j/src/test/java/org/apache/knox/gateway/pac4j/filter/Pac4jDispatcherFilterTest.java
@@ -17,6 +17,7 @@
*/
package org.apache.knox.gateway.pac4j.filter;
+import org.apache.knox.gateway.config.GatewayConfig;
import org.apache.knox.gateway.pac4j.session.KnoxSessionStore;
import org.apache.knox.gateway.services.GatewayServices;
import org.apache.knox.gateway.services.ServiceType;
@@ -30,12 +31,16 @@ import org.junit.Test;
import javax.servlet.FilterConfig;
import javax.servlet.ServletContext;
+import javax.servlet.http.Cookie;
+import javax.servlet.http.HttpServletResponse;
import java.security.KeyStore;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Map;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
import static
org.pac4j.config.client.PropertiesConstants.SAML_IDENTITY_PROVIDER_METADATA_PATH;
import static org.pac4j.config.client.PropertiesConstants.SAML_KEYSTORE_PATH;
@@ -53,6 +58,7 @@ public class Pac4jDispatcherFilterTest {
MasterService masterService;
FilterConfig filterConfig;
KeyStore ks;
+ GatewayConfig gatewayConfig;
}
private TestMocks createMocks() throws Exception {
@@ -65,6 +71,7 @@ public class Pac4jDispatcherFilterTest {
mocks.keystoreService = EasyMock.createNiceMock(KeystoreService.class);
mocks.masterService = EasyMock.createNiceMock(MasterService.class);
mocks.filterConfig = EasyMock.createNiceMock(FilterConfig.class);
+ mocks.gatewayConfig = EasyMock.createNiceMock(GatewayConfig.class);
return mocks;
}
@@ -94,6 +101,7 @@ public class Pac4jDispatcherFilterTest {
EasyMock.expect(mocks.filterConfig.getInitParameter("clientName")).andReturn("SAML2Client").anyTimes();
EasyMock.expect(mocks.aliasService.getPasswordFromAliasForCluster(TEST_CLUSTER_NAME,
KnoxSessionStore.PAC4J_PASSWORD, true))
.andReturn(KnoxSessionStore.PAC4J_PASSWORD.toCharArray()).anyTimes();
+
EasyMock.expect(mocks.context.getAttribute(GatewayConfig.GATEWAY_CONFIG_ATTRIBUTE)).andReturn(mocks.gatewayConfig).anyTimes();
}
private void verifyCookiemaxAge(FilterConfig filterConfig, String
expectedCookieMaxAge) throws Exception {
@@ -119,12 +127,12 @@ public class Pac4jDispatcherFilterTest {
setupCommonExpectations(mocks, additionalParams);
EasyMock.expect(mocks.filterConfig.getInitParameter(Pac4jDispatcherFilter.PAC4J_COOKIE_MAX_AGE)).andReturn(expectedCookieMaxAge).anyTimes();
- EasyMock.replay(mocks.context, mocks.services, mocks.cryptoService,
mocks.aliasService, mocks.keystoreService, mocks.masterService,
mocks.filterConfig);
+ EasyMock.replay(mocks.context, mocks.services, mocks.cryptoService,
mocks.aliasService, mocks.keystoreService, mocks.masterService,
mocks.filterConfig, mocks.gatewayConfig);
verifyCookiemaxAge(mocks.filterConfig, expectedCookieMaxAge);
// Verify all mock interactions
- EasyMock.verify(mocks.context, mocks.services, mocks.cryptoService,
mocks.aliasService, mocks.keystoreService, mocks.masterService,
mocks.filterConfig);
+ EasyMock.verify(mocks.context, mocks.services, mocks.cryptoService,
mocks.aliasService, mocks.keystoreService, mocks.masterService,
mocks.filterConfig, mocks.gatewayConfig);
}
/**
@@ -142,11 +150,71 @@ public class Pac4jDispatcherFilterTest {
TestMocks mocks = createMocks();
setupCommonExpectations(mocks, Collections.EMPTY_LIST);
- EasyMock.replay(mocks.context, mocks.services, mocks.cryptoService,
mocks.aliasService, mocks.keystoreService, mocks.masterService,
mocks.filterConfig);
+ EasyMock.replay(mocks.context, mocks.services, mocks.cryptoService,
mocks.aliasService, mocks.keystoreService, mocks.masterService,
mocks.filterConfig, mocks.gatewayConfig);
verifyCookiemaxAge(mocks.filterConfig, expectedCookieMaxAge);
// Verify all mock interactions
- EasyMock.verify(mocks.context, mocks.services, mocks.cryptoService,
mocks.aliasService, mocks.keystoreService, mocks.masterService,
mocks.filterConfig);
+ EasyMock.verify(mocks.context, mocks.services, mocks.cryptoService,
mocks.aliasService, mocks.keystoreService, mocks.masterService,
mocks.filterConfig, mocks.gatewayConfig);
+ }
+
+ @Test
+ public void testPAC4JSetCookieResponseWrapperStaysTrue() throws Exception {
+ HttpServletResponse response =
EasyMock.createNiceMock(HttpServletResponse.class);
+ Cookie cookie = new Cookie("pac4jCsrfToken", "pac4j_value");
+ cookie.setSecure(true);
+ cookie.setHttpOnly(true);
+ TestMocks mocks = createMocks();
+ setupCommonExpectations(mocks, Collections.EMPTY_LIST);
+
EasyMock.expect(mocks.gatewayConfig.isSSLEnabled()).andReturn(false).anyTimes();
+ EasyMock.replay(mocks.context, mocks.services, mocks.cryptoService,
mocks.aliasService, mocks.keystoreService, mocks.masterService,
mocks.filterConfig, mocks.gatewayConfig);
+
+ Pac4jDispatcherFilter filter = new Pac4jDispatcherFilter();
+ filter.init(mocks.filterConfig);
+ Pac4jDispatcherFilter.Pac4jSetCookieResponseWrapper wrapper = new
Pac4jDispatcherFilter.Pac4jSetCookieResponseWrapper(response);
+ wrapper.addCookie(cookie);
+
+ assertTrue(cookie.getSecure());
+ assertTrue(cookie.isHttpOnly());
+ }
+
+ @Test
+ public void testPAC4JSetCookieResponseWrapperFalseToTrue() throws
Exception {
+ HttpServletResponse response =
EasyMock.createNiceMock(HttpServletResponse.class);
+ Cookie cookie = new Cookie("pac4jCsrfToken", "pac4j_value");
+ cookie.setSecure(false);
+ cookie.setHttpOnly(false);
+ TestMocks mocks = createMocks();
+ setupCommonExpectations(mocks, Collections.EMPTY_LIST);
+
EasyMock.expect(mocks.gatewayConfig.isSSLEnabled()).andReturn(true).anyTimes();
+ EasyMock.replay(mocks.context, mocks.services, mocks.cryptoService,
mocks.aliasService, mocks.keystoreService, mocks.masterService,
mocks.filterConfig, mocks.gatewayConfig);
+
+ Pac4jDispatcherFilter filter = new Pac4jDispatcherFilter();
+ filter.init(mocks.filterConfig);
+ Pac4jDispatcherFilter.Pac4jSetCookieResponseWrapper wrapper = new
Pac4jDispatcherFilter.Pac4jSetCookieResponseWrapper(response);
+ wrapper.addCookie(cookie);
+
+ assertTrue(cookie.getSecure());
+ assertTrue(cookie.isHttpOnly());
+ }
+
+ @Test
+ public void testPAC4JSetCookieResponseWrapperNoSSL() throws Exception {
+ HttpServletResponse response =
EasyMock.createNiceMock(HttpServletResponse.class);
+ Cookie cookie = new Cookie("pac4jCsrfToken", "pac4j_value");
+ cookie.setSecure(false);
+ cookie.setHttpOnly(false);
+ TestMocks mocks = createMocks();
+ setupCommonExpectations(mocks, Collections.EMPTY_LIST);
+
EasyMock.expect(mocks.gatewayConfig.isSSLEnabled()).andReturn(false).anyTimes();
+ EasyMock.replay(mocks.context, mocks.services, mocks.cryptoService,
mocks.aliasService, mocks.keystoreService, mocks.masterService,
mocks.filterConfig, mocks.gatewayConfig);
+
+ Pac4jDispatcherFilter filter = new Pac4jDispatcherFilter();
+ filter.init(mocks.filterConfig);
+ Pac4jDispatcherFilter.Pac4jSetCookieResponseWrapper wrapper = new
Pac4jDispatcherFilter.Pac4jSetCookieResponseWrapper(response);
+ wrapper.addCookie(cookie);
+
+ assertFalse(cookie.getSecure());
+ assertTrue(cookie.isHttpOnly());
}
}
