Add proxy-headers option to the Keycloak CR (#27092)

Closes #25179

Signed-off-by: Václav Muzikář <vmuzikar@redhat.com>
This commit is contained in:
Václav Muzikář 2024-02-21 12:19:37 +01:00 committed by GitHub
parent 37805ffbb2
commit 33425dacd9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 175 additions and 40 deletions

View file

@ -174,6 +174,25 @@ The `--proxy` option has been deprecated and will be removed in a future release
NOTE: For hardened security, the `--proxy-headers` option does not allow selecting both `forwarded` and `xforwarded` values at the same time (as it was NOTE: For hardened security, the `--proxy-headers` option does not allow selecting both `forwarded` and `xforwarded` values at the same time (as it was
the case before for `--proxy edge` and `--proxy reencrypt`). the case before for `--proxy edge` and `--proxy reencrypt`).
WARNING: When using the proxy headers option, make sure your reverse proxy properly sets and overwrites the `Forwarded` or `X-Forwarded-*` headers respectively.
To set these headers, consult the documentation for your reverse proxy. Misconfiguration will leave {project_name} exposed to security vulnerabilities.
You can also set the proxy headers when using the Operator:
[source,yaml]
----
apiVersion: k8s.keycloak.org/v2alpha1
kind: Keycloak
metadata:
name: example-kc
spec:
...
proxy:
headers: forwarded|xforwarded
----
NOTE: If the `proxy.headers` field is not specified, the Operator falls back to the previous behaviour by implicitly setting
`proxy=passthrough` by default. This results in deprecation warnings in the server log. This fallback will be removed
in a future release.
= Changes to the user representation in both Admin API and Account contexts = Changes to the user representation in both Admin API and Account contexts
Both `org.keycloak.representations.idm.UserRepresentation` and `org.keycloak.representations.account.UserRepresentation` representation classes have changed Both `org.keycloak.representations.idm.UserRepresentation` and `org.keycloak.representations.account.UserRepresentation` representation classes have changed

View file

@ -152,6 +152,8 @@ spec:
tlsSecret: example-tls-secret tlsSecret: example-tls-secret
hostname: hostname:
hostname: test.keycloak.org hostname: test.keycloak.org
proxy:
headers: xforwarded # double check your reverse proxy sets and overwrites the X-Forwarded-* headers
---- ----
Apply the changes: Apply the changes:
@ -233,6 +235,34 @@ For debugging and development purposes, consider directly connecting to the {pro
kubectl port-forward service/example-kc-service 8443:8443 kubectl port-forward service/example-kc-service 8443:8443
---- ----
==== Configuring the reverse proxy settings matching your Ingress Controller
The Operator supports configuring which of the reverse proxy headers should be accepted by server, which includes
`Forwarded` and `X-Forwarded-*` headers.
If you Ingress implementation sets and overwrites either `Forwarded` or `X-Forwarded-*` headers, you can reflect that
in the Keycloak CR as follows:
[source,yaml]
----
apiVersion: k8s.keycloak.org/v2alpha1
kind: Keycloak
metadata:
name: example-kc
spec:
...
proxy:
headers: forwarded|xforwarded
----
NOTE: If the `proxy.headers` field is not specified, the Operator falls back to legacy behaviour by implicitly setting
`proxy=passthrough` by default. This results in deprecation warnings in the server log. This fallback will be removed
in a future release.
WARNING: When using the `proxy.headers` field, make sure your Ingress properly sets and overwrites the `Forwarded` or `X-Forwarded-*` headers respectively. To set these headers, consult the documentation for your Ingress Controller. Consider configuring it for
either reencrypt or edge TLS termination as passthrough TLS doesn't allow the Ingress to modify the requests headers.
Misconfiguration will leave {project_name} exposed to security vulnerabilities.
For more details refer to the <@links.server id="reverseproxy"/> guide.
=== Accessing the Admin Console === Accessing the Admin Console
When deploying {project_name}, the operator generates an arbitrary initial admin `username` and `password` and stores those credentials as a basic-auth Secret object in the same namespace as the CR. When deploying {project_name}, the operator generates an arbitrary initial admin `username` and `password` and stores those credentials as a basic-auth Secret object in the same namespace as the CR.

View file

@ -68,6 +68,7 @@ import java.util.stream.Stream;
import jakarta.inject.Inject; import jakarta.inject.Inject;
import static org.keycloak.operator.Utils.addResources; import static org.keycloak.operator.Utils.addResources;
import static org.keycloak.operator.controllers.KeycloakDistConfigurator.getKeycloakOptionEnvVarName;
import static org.keycloak.operator.crds.v2alpha1.CRDUtils.isTlsConfigured; import static org.keycloak.operator.crds.v2alpha1.CRDUtils.isTlsConfigured;
@KubernetesDependent(labelSelector = Constants.DEFAULT_LABELS_AS_STRING) @KubernetesDependent(labelSelector = Constants.DEFAULT_LABELS_AS_STRING)
@ -368,6 +369,13 @@ public class KeycloakDeploymentDependentResource extends CRUDKubernetesDependent
// include the kube CA if the user is not controlling KC_TRUSTSTORE_PATHS via the unsupported or the additional // include the kube CA if the user is not controlling KC_TRUSTSTORE_PATHS via the unsupported or the additional
varMap.putIfAbsent(KC_TRUSTSTORE_PATHS, new EnvVarBuilder().withName(KC_TRUSTSTORE_PATHS).withValue(truststores).build()); varMap.putIfAbsent(KC_TRUSTSTORE_PATHS, new EnvVarBuilder().withName(KC_TRUSTSTORE_PATHS).withValue(truststores).build());
// TODO remove this once the --proxy option is finally removed from Keycloak
// not strictly necessary as --proxy-headers take precedence over --proxy but at least removes the warning
// about deprecated --proxy option in use
if (varMap.containsKey(getKeycloakOptionEnvVarName("proxy-headers"))) {
varMap.remove(getKeycloakOptionEnvVarName("proxy"));
}
var envVars = new ArrayList<>(varMap.values()); var envVars = new ArrayList<>(varMap.values());
baseDeployment.getSpec().getTemplate().getSpec().getContainers().get(0).setEnv(envVars); baseDeployment.getSpec().getTemplate().getSpec().getContainers().get(0).setEnv(envVars);
@ -395,7 +403,7 @@ public class KeycloakDeploymentDependentResource extends CRUDKubernetesDependent
// set env vars // set env vars
List<EnvVar> envVars = serverConfigsList.stream() List<EnvVar> envVars = serverConfigsList.stream()
.map(v -> { .map(v -> {
var envBuilder = new EnvVarBuilder().withName(KeycloakDistConfigurator.getKeycloakOptionEnvVarName(v.getName())); var envBuilder = new EnvVarBuilder().withName(getKeycloakOptionEnvVarName(v.getName()));
var secret = v.getSecret(); var secret = v.getSecret();
if (secret != null) { if (secret != null) {
envBuilder.withValueFrom( envBuilder.withValueFrom(

View file

@ -32,6 +32,7 @@ import org.keycloak.operator.crds.v2alpha1.deployment.spec.DatabaseSpec;
import org.keycloak.operator.crds.v2alpha1.deployment.spec.FeatureSpec; import org.keycloak.operator.crds.v2alpha1.deployment.spec.FeatureSpec;
import org.keycloak.operator.crds.v2alpha1.deployment.spec.HostnameSpec; import org.keycloak.operator.crds.v2alpha1.deployment.spec.HostnameSpec;
import org.keycloak.operator.crds.v2alpha1.deployment.spec.HttpSpec; import org.keycloak.operator.crds.v2alpha1.deployment.spec.HttpSpec;
import org.keycloak.operator.crds.v2alpha1.deployment.spec.ProxySpec;
import org.keycloak.operator.crds.v2alpha1.deployment.spec.TransactionsSpec; import org.keycloak.operator.crds.v2alpha1.deployment.spec.TransactionsSpec;
import java.util.ArrayList; import java.util.ArrayList;
@ -68,6 +69,7 @@ public class KeycloakDistConfigurator {
configureHttp(); configureHttp();
configureDatabase(); configureDatabase();
configureCache(); configureCache();
configureProxy();
} }
/** /**
@ -130,6 +132,11 @@ public class KeycloakDistConfigurator {
.mapOption("db-pool-max-size", DatabaseSpec::getPoolMaxSize); .mapOption("db-pool-max-size", DatabaseSpec::getPoolMaxSize);
} }
void configureProxy() {
optionMapper(keycloakCR -> keycloakCR.getSpec().getProxySpec())
.mapOption("proxy-headers", ProxySpec::getHeaders);
}
/* ---------- END of configuration of first-class citizen fields ---------- */ /* ---------- END of configuration of first-class citizen fields ---------- */
/** /**

View file

@ -26,6 +26,7 @@ import org.keycloak.operator.crds.v2alpha1.deployment.spec.FeatureSpec;
import org.keycloak.operator.crds.v2alpha1.deployment.spec.HostnameSpec; import org.keycloak.operator.crds.v2alpha1.deployment.spec.HostnameSpec;
import org.keycloak.operator.crds.v2alpha1.deployment.spec.HttpSpec; import org.keycloak.operator.crds.v2alpha1.deployment.spec.HttpSpec;
import org.keycloak.operator.crds.v2alpha1.deployment.spec.IngressSpec; import org.keycloak.operator.crds.v2alpha1.deployment.spec.IngressSpec;
import org.keycloak.operator.crds.v2alpha1.deployment.spec.ProxySpec;
import org.keycloak.operator.crds.v2alpha1.deployment.spec.TransactionsSpec; import org.keycloak.operator.crds.v2alpha1.deployment.spec.TransactionsSpec;
import org.keycloak.operator.crds.v2alpha1.deployment.spec.Truststore; import org.keycloak.operator.crds.v2alpha1.deployment.spec.Truststore;
import org.keycloak.operator.crds.v2alpha1.deployment.spec.UnsupportedSpec; import org.keycloak.operator.crds.v2alpha1.deployment.spec.UnsupportedSpec;
@ -100,6 +101,10 @@ public class KeycloakSpec {
@JsonPropertyDescription("Compute Resources required by Keycloak container") @JsonPropertyDescription("Compute Resources required by Keycloak container")
private ResourceRequirements resourceRequirements; private ResourceRequirements resourceRequirements;
@JsonProperty("proxy")
@JsonPropertyDescription("In this section you can configure Keycloak's reverse proxy setting")
private ProxySpec proxySpec;
public HttpSpec getHttpSpec() { public HttpSpec getHttpSpec() {
return httpSpec; return httpSpec;
} }
@ -226,4 +231,11 @@ public class KeycloakSpec {
this.resourceRequirements = resourceRequirements; this.resourceRequirements = resourceRequirements;
} }
public ProxySpec getProxySpec() {
return proxySpec;
}
public void setProxySpec(ProxySpec proxySpec) {
this.proxySpec = proxySpec;
}
} }

View file

@ -0,0 +1,40 @@
/*
* Copyright 2024 Red Hat, Inc. and/or its affiliates
* and other contributors as indicated by the @author tags.
*
* Licensed 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.keycloak.operator.crds.v2alpha1.deployment.spec;
import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.annotation.JsonPropertyDescription;
import io.sundr.builder.annotations.Buildable;
/**
* @author Vaclav Muzikar <vmuzikar@redhat.com>
*/
@JsonInclude(JsonInclude.Include.NON_NULL)
@Buildable(editableEnabled = false, builderPackage = "io.fabric8.kubernetes.api.builder")
public class ProxySpec {
@JsonPropertyDescription("The proxy headers that should be accepted by the server. Misconfiguration might leave the server exposed to security vulnerabilities.")
private String headers;
public String getHeaders() {
return headers;
}
public void setHeaders(String headers) {
this.headers = headers;
}
}

View file

@ -158,20 +158,13 @@ public class KeycloakDeploymentTest extends BaseOperatorTest {
var defaultKCDeploy = getTestKeycloakDeployment(true); var defaultKCDeploy = getTestKeycloakDeployment(true);
var valueSecretHealthProp = new ValueOrSecret("health-enabled", "false"); var valueSecretHealthProp = new ValueOrSecret("health-enabled", "false");
var valueSecretProxyProp = new ValueOrSecret("proxy", "reencrypt");
var healthEnvVar = new EnvVarBuilder() var healthEnvVar = new EnvVarBuilder()
.withName(KeycloakDistConfigurator.getKeycloakOptionEnvVarName(valueSecretHealthProp.getName())) .withName(KeycloakDistConfigurator.getKeycloakOptionEnvVarName(valueSecretHealthProp.getName()))
.withValue(valueSecretHealthProp.getValue()) .withValue(valueSecretHealthProp.getValue())
.build(); .build();
var proxyEnvVar = new EnvVarBuilder()
.withName(KeycloakDistConfigurator.getKeycloakOptionEnvVarName(valueSecretProxyProp.getName()))
.withValue(valueSecretProxyProp.getValue())
.build();
defaultKCDeploy.getSpec().getAdditionalOptions().add(valueSecretHealthProp); defaultKCDeploy.getSpec().getAdditionalOptions().add(valueSecretHealthProp);
defaultKCDeploy.getSpec().getAdditionalOptions().add(valueSecretProxyProp);
deployKeycloak(k8sclient, defaultKCDeploy, false); deployKeycloak(k8sclient, defaultKCDeploy, false);
@ -183,14 +176,6 @@ public class KeycloakDeploymentTest extends BaseOperatorTest {
.getValue() .getValue()
).isEqualTo("true"); // just a sanity check default values did not change ).isEqualTo("true"); // just a sanity check default values did not change
assertThat(
Constants.DEFAULT_DIST_CONFIG_LIST.stream()
.filter(oneValueOrSecret -> oneValueOrSecret.getName().equalsIgnoreCase(valueSecretProxyProp.getName()))
.findFirst()
.get()
.getValue()
).isEqualTo("passthrough"); // just a sanity check default values did not change
Awaitility.await() Awaitility.await()
.ignoreExceptions() .ignoreExceptions()
.untilAsserted(() -> { .untilAsserted(() -> {
@ -213,10 +198,6 @@ public class KeycloakDeploymentTest extends BaseOperatorTest {
.filter(oneEnvVar -> oneEnvVar.getName().equalsIgnoreCase(healthEnvVar.getName()))) .filter(oneEnvVar -> oneEnvVar.getName().equalsIgnoreCase(healthEnvVar.getName())))
.containsExactly(healthEnvVar); .containsExactly(healthEnvVar);
assertThat(firstKCContainer.getEnv().stream()
.filter(oneEnvVar -> oneEnvVar.getName().equalsIgnoreCase(proxyEnvVar.getName())))
.containsExactly(proxyEnvVar);
}); });
} }

View file

@ -34,7 +34,6 @@ import org.keycloak.operator.crds.v2alpha1.deployment.Keycloak;
import org.keycloak.operator.crds.v2alpha1.deployment.spec.HostnameSpecBuilder; import org.keycloak.operator.crds.v2alpha1.deployment.spec.HostnameSpecBuilder;
import org.keycloak.operator.crds.v2alpha1.deployment.spec.IngressSpec; import org.keycloak.operator.crds.v2alpha1.deployment.spec.IngressSpec;
import org.keycloak.operator.crds.v2alpha1.deployment.spec.IngressSpecBuilder; import org.keycloak.operator.crds.v2alpha1.deployment.spec.IngressSpecBuilder;
import org.keycloak.operator.crds.v2alpha1.deployment.spec.UnsupportedSpecBuilder;
import org.keycloak.operator.testsuite.utils.K8sUtils; import org.keycloak.operator.testsuite.utils.K8sUtils;
import java.util.Map; import java.util.Map;
@ -57,22 +56,7 @@ public class KeycloakIngressTest extends BaseOperatorTest {
.withStrictBackchannel(false); .withStrictBackchannel(false);
if (isOpenShift) { if (isOpenShift) {
kc.getSpec().setIngressSpec(new IngressSpecBuilder().withIngressClassName(KeycloakController.OPENSHIFT_DEFAULT).build()); kc.getSpec().setIngressSpec(new IngressSpecBuilder().withIngressClassName(KeycloakController.OPENSHIFT_DEFAULT).build());
// see https://github.com/keycloak/keycloak/issues/14400#issuecomment-1659900081
kc.getSpec().setUnsupported(new UnsupportedSpecBuilder()
.withNewPodTemplate()
.withNewSpec()
.addNewContainer()
.addNewEnv()
.withName("KC_PROXY")
.withValue("edge")
.endEnv()
.endContainer()
.endSpec()
.endPodTemplate()
.build());
} }
kc.getSpec().setHostnameSpec(hostnameSpecBuilder.build()); kc.getSpec().setHostnameSpec(hostnameSpecBuilder.build());
K8sUtils.deployKeycloak(k8sclient, kc, true); K8sUtils.deployKeycloak(k8sclient, kc, true);
@ -91,7 +75,7 @@ public class KeycloakIngressTest extends BaseOperatorTest {
} }
@Test @Test
public void testIngressOnHTTPS() { public void testIngressOnHTTPSAndProxySettings() {
var kc = getTestKeycloakDeployment(false); var kc = getTestKeycloakDeployment(false);
var hostnameSpecBuilder = new HostnameSpecBuilder() var hostnameSpecBuilder = new HostnameSpecBuilder()
.withStrict(false) .withStrict(false)
@ -111,6 +95,45 @@ public class KeycloakIngressTest extends BaseOperatorTest {
} }
testIngressURLs("https://" + testHostname + ":443"); testIngressURLs("https://" + testHostname + ":443");
// just check we really have proxy set correctly
var envVars = k8sclient.apps().statefulSets().withName(kc.getMetadata().getName()).get().getSpec()
.getTemplate().getSpec().getContainers().get(0).getEnv();
assertThat(envVars)
.noneMatch(e -> "KC_PROXY".equals(e.getName()))
.anyMatch(e -> "KC_PROXY_HEADERS".equals(e.getName()) && "xforwarded".equals(e.getValue()));
}
// TODO remove this test once the --proxy option is finally removed from Keycloak
@Test
public void testFallbackToDefaultProxySettings() {
var kc = getTestKeycloakDeployment(false);
var hostnameSpecBuilder = new HostnameSpecBuilder()
.withStrict(false)
.withStrictBackchannel(false);
if (isOpenShift) {
kc.getSpec().setIngressSpec(new IngressSpecBuilder().withIngressClassName(KeycloakController.OPENSHIFT_DEFAULT).build());
}
kc.getSpec().setHostnameSpec(hostnameSpecBuilder.build());
kc.getSpec().setProxySpec(null);
K8sUtils.deployKeycloak(k8sclient, kc, true);
String testHostname;
if (isOpenShift) {
testHostname = k8sclient.resource(kc).get().getSpec().getHostnameSpec().getHostname();
} else {
testHostname = kubernetesIp;
}
testIngressURLs("https://" + testHostname + ":443");
// just check we really have proxy set correctly
var envVars = k8sclient.apps().statefulSets().withName(kc.getMetadata().getName()).get().getSpec()
.getTemplate().getSpec().getContainers().get(0).getEnv();
assertThat(envVars)
.anyMatch(e -> "KC_PROXY".equals(e.getName()) && "passthrough".equals(e.getValue()))
.noneMatch(e -> "KC_PROXY_HEADERS".equals(e.getName()));
} }
private void testIngressURLs(String baseUrl) { private void testIngressURLs(String baseUrl) {
@ -125,9 +148,10 @@ public class KeycloakIngressTest extends BaseOperatorTest {
.get(url) .get(url)
.body() .body()
.jsonPath() .jsonPath()
.getString("realm"); .getString("token-service");
assertEquals("master", output); // the Keycloak URL must be without port, otherwise proxy resolution doesn't work correctly
assertEquals(url.replaceAll(":\\d+", "") + "/protocol/openid-connect", output);
}); });
Awaitility.await() Awaitility.await()

View file

@ -135,6 +135,15 @@ public class KeycloakDistConfiguratorTest {
assertEnvVarNotPresent(envVars, "KC_HOSTNAME_STRICT_BACKCHANNEL"); assertEnvVarNotPresent(envVars, "KC_HOSTNAME_STRICT_BACKCHANNEL");
} }
@Test
public void proxy() {
final Map<String, String> expectedValues = Map.of(
"proxy-headers", "forwarded"
);
testFirstClassCitizen(expectedValues);
}
/* UTILS */ /* UTILS */
private void testFirstClassCitizen(Map<String, String> expectedValues) { private void testFirstClassCitizen(Map<String, String> expectedValues) {

View file

@ -16,4 +16,6 @@ spec:
http: http:
tlsSecret: example-tls-secret tlsSecret: example-tls-secret
hostname: hostname:
hostname: example.com hostname: example.com
proxy:
headers: xforwarded # default nginx ingress sets x-forwarded

View file

@ -63,6 +63,8 @@ spec:
limits: limits:
cpu: "2" cpu: "2"
memory: "1500M" memory: "1500M"
proxy:
headers: forwarded
unsupported: unsupported:
podTemplate: podTemplate:
metadata: metadata:

View file

@ -37,6 +37,7 @@ public class ConstantsDebugHostname {
"hostname-path", "hostname-path",
"hostname-port", "hostname-port",
"proxy", "proxy",
"proxy-headers",
"http-enabled", "http-enabled",
"http-relative-path", "http-relative-path",
"http-port", "http-port",