From 2cc051346df2cca7ed438db852a52a7123740600 Mon Sep 17 00:00:00 2001 From: rmartinc Date: Sat, 11 May 2024 18:35:34 +0200 Subject: [PATCH] Allow empty CSP header in headers provider Closes #29458 Signed-off-by: rmartinc --- .../DefaultSecurityHeadersProvider.java | 21 ++++++++++--------- .../oauth/LoginStatusIframeEndpointTest.java | 20 ++++++++++++++++++ 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/services/src/main/java/org/keycloak/headers/DefaultSecurityHeadersProvider.java b/services/src/main/java/org/keycloak/headers/DefaultSecurityHeadersProvider.java index b10b2b28df..3854464356 100644 --- a/services/src/main/java/org/keycloak/headers/DefaultSecurityHeadersProvider.java +++ b/services/src/main/java/org/keycloak/headers/DefaultSecurityHeadersProvider.java @@ -106,24 +106,25 @@ public class DefaultSecurityHeadersProvider implements SecurityHeadersProvider { // TODO This will be refactored as part of introducing a more strict CSP header if (options != null) { - ContentSecurityPolicyBuilder csp = ContentSecurityPolicyBuilder.create( - headers.getFirst(CONTENT_SECURITY_POLICY.getHeaderName()).toString()); - if (options.isAllowAnyFrameAncestor()) { headers.remove(BrowserSecurityHeaders.X_FRAME_OPTIONS.getHeaderName()); + } - if (csp.isDefaultFrameAncestors()) { + Object cspVal = headers.getFirst(CONTENT_SECURITY_POLICY.getHeaderName()); + if (cspVal != null) { + ContentSecurityPolicyBuilder csp = ContentSecurityPolicyBuilder.create(cspVal.toString()); + if (options.isAllowAnyFrameAncestor() && csp.isDefaultFrameAncestors()) { // only remove frame ancestors if defined to default 'self' csp.frameAncestors(null); } - } - String allowedFrameSrc = options.getAllowedFrameSrc(); - if (allowedFrameSrc != null) { - csp.addFrameSrc(allowedFrameSrc); - } + String allowedFrameSrc = options.getAllowedFrameSrc(); + if (allowedFrameSrc != null) { + csp.addFrameSrc(allowedFrameSrc); + } - headers.putSingle(CONTENT_SECURITY_POLICY.getHeaderName(), csp.build()); + headers.putSingle(CONTENT_SECURITY_POLICY.getHeaderName(), csp.build()); + } } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/LoginStatusIframeEndpointTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/LoginStatusIframeEndpointTest.java index 86298bee50..528377eda9 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/LoginStatusIframeEndpointTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/LoginStatusIframeEndpointTest.java @@ -40,7 +40,12 @@ import org.keycloak.testsuite.AbstractKeycloakTest; import org.keycloak.testsuite.ActionURIUtils; import org.keycloak.testsuite.oidc.PkceGenerator; import org.keycloak.testsuite.runonserver.ServerVersion; +import org.keycloak.testsuite.updaters.RealmAttributeUpdater; +import org.keycloak.testsuite.util.AdminClientUtil; +import org.keycloak.testsuite.util.RealmBuilder; +import jakarta.ws.rs.client.Client; +import jakarta.ws.rs.core.Response; import java.io.IOException; import java.net.URLEncoder; import java.util.Collections; @@ -200,8 +205,23 @@ public class LoginStatusIframeEndpointTest extends AbstractKeycloakTest { } } + @Test + public void checkEmptyCsp() throws Exception { + try (RealmAttributeUpdater realmUpdater = new RealmAttributeUpdater(adminClient.realm("test")) + .setBrowserSecurityHeader(BrowserSecurityHeaders.CONTENT_SECURITY_POLICY.getKey(), "") + .update(); + Client client = AdminClientUtil.createResteasyClient(); + Response response = client.target(suiteContext.getAuthServerInfo().getContextRoot() + + "/auth/realms/test/protocol/openid-connect/login-status-iframe.html").request().get()) { + assertEquals(Response.Status.OK.getStatusCode(), response.getStatus()); + assertNull(response.getHeaderString(BrowserSecurityHeaders.CONTENT_SECURITY_POLICY.getKey())); + assertNull(response.getHeaderString(BrowserSecurityHeaders.X_FRAME_OPTIONS.getHeaderName())); + } + } + @Override public void addTestRealms(List testRealms) { + testRealms.add(RealmBuilder.create().name("test").build()); } }