Skip to content

Commit

Permalink
Add an ability to configure whether authentication via cookies is ena…
Browse files Browse the repository at this point in the history
…bled (#10763)
  • Loading branch information
sleshchenko authored Aug 15, 2018
1 parent 1176dc8 commit 43066dc
Show file tree
Hide file tree
Showing 7 changed files with 159 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,13 @@ public interface ServerConfig {
*/
String UNSECURED_PATHS_ATTRIBUTE = "unsecuredPaths";

/**
* {@link ServerConfig} and {@link Server} attribute name which indicates whether authentication
* with cookies is allowed or not. Attribute value {@code true} cookies authentication enabled,
* any other value or lack of the attribute denies to use cookies authentication.
*/
String SECURE_SERVER_COOKIES_AUTH_ENABLED_ATTRIBUTE = "cookiesAuthEnabled";

/**
* Port used by server.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,9 @@ public JwtProxyConfigBuilder(
this.ttl = ttl;
}

public void addVerifierProxy(Integer listenPort, String upstream, Set<String> excludes) {
verifierProxies.add(new VerifierProxy(listenPort, upstream, excludes));
public void addVerifierProxy(
Integer listenPort, String upstream, Set<String> excludes, Boolean cookiesAuthEnabled) {
verifierProxies.add(new VerifierProxy(listenPort, upstream, excludes, cookiesAuthEnabled));
}

public String build() throws InternalInfrastructureException {
Expand Down Expand Up @@ -102,6 +103,7 @@ public String build() throws InternalInfrastructureException {
workspaceId,
"public_key_path",
JWT_PROXY_CONFIG_FOLDER + '/' + JWT_PROXY_PUBLIC_KEY_FILE)))
.withCookiesEnabled(verifierProxy.cookiesAuthEnabled)
.withClaimsVerifier(
Collections.singleton(
new RegistrableComponentConfig()
Expand All @@ -113,7 +115,7 @@ public String build() throws InternalInfrastructureException {
verifierConfig.setExcludes(verifierProxy.excludes);
}

if (authPageUrl != null) {
if (verifierProxy.cookiesAuthEnabled && authPageUrl != null) {
verifierConfig.setAuthUrl(authPageUrl.toString());
}

Expand All @@ -137,11 +139,14 @@ private class VerifierProxy {
private Integer listenPort;
private String upstream;
private Set<String> excludes;
private boolean cookiesAuthEnabled;

VerifierProxy(Integer listenPort, String upstream, Set<String> excludes) {
VerifierProxy(
Integer listenPort, String upstream, Set<String> excludes, boolean cookiesAuthEnabled) {
this.listenPort = listenPort;
this.upstream = upstream;
this.excludes = excludes;
this.cookiesAuthEnabled = cookiesAuthEnabled;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,18 @@
*/
package org.eclipse.che.workspace.infrastructure.kubernetes.server.secure.jwtproxy;

import static java.lang.Boolean.parseBoolean;
import static java.util.Collections.emptyList;
import static java.util.Collections.emptyMap;
import static org.eclipse.che.api.core.model.workspace.config.ServerConfig.SECURE_SERVER_COOKIES_AUTH_ENABLED_ATTRIBUTE;
import static org.eclipse.che.api.core.model.workspace.config.ServerConfig.UNSECURED_PATHS_ATTRIBUTE;
import static org.eclipse.che.commons.lang.NameGenerator.generate;
import static org.eclipse.che.workspace.infrastructure.kubernetes.Constants.CHE_ORIGINAL_NAME_LABEL;
import static org.eclipse.che.workspace.infrastructure.kubernetes.server.KubernetesServerExposer.SERVER_PREFIX;
import static org.eclipse.che.workspace.infrastructure.kubernetes.server.KubernetesServerExposer.SERVER_UNIQUE_PART_SIZE;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableMap;
import com.google.inject.assistedinject.Assisted;
import io.fabric8.kubernetes.api.model.ConfigMap;
Expand Down Expand Up @@ -137,20 +140,39 @@ public ServicePort expose(
String protocol,
Map<String, ServerConfig> secureServers)
throws InfrastructureException {
Preconditions.checkArgument(
secureServers != null && !secureServers.isEmpty(), "Secure servers are missing");
ensureJwtProxyInjected(k8sEnv);

int listenPort = availablePort++;

Set<String> excludes = new HashSet<>();
Boolean cookiesAuthEnabled = null;
for (ServerConfig config : secureServers.values()) {
// accumulate unsecured paths
if (config.getAttributes().containsKey(UNSECURED_PATHS_ATTRIBUTE)) {
Collections.addAll(
excludes, config.getAttributes().get(UNSECURED_PATHS_ATTRIBUTE).split(","));
}

// calculate `cookiesAuthEnabled` attributes
Boolean serverCookiesAuthEnabled =
parseBoolean(config.getAttributes().get(SECURE_SERVER_COOKIES_AUTH_ENABLED_ATTRIBUTE));
if (cookiesAuthEnabled == null) {
cookiesAuthEnabled = serverCookiesAuthEnabled;
} else {
if (!cookiesAuthEnabled.equals(serverCookiesAuthEnabled)) {
throw new InfrastructureException(
"Secure servers which expose the same port should have the same `cookiesAuthEnabled` value.");
}
}
}

proxyConfigBuilder.addVerifierProxy(
listenPort, "http://" + backendServiceName + ":" + backendServicePort, excludes);
listenPort,
"http://" + backendServiceName + ":" + backendServicePort,
excludes,
cookiesAuthEnabled);
k8sEnv
.getConfigMaps()
.get(getConfigMapName())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ public class VerifierConfig {
@JsonProperty("auth_redirect_url")
private String authUrl;

@JsonProperty("auth_cookies_enabled")
private boolean cookiesEnabled;

private Set<String> excludes;

public String getAudience() {
Expand Down Expand Up @@ -163,4 +166,18 @@ public VerifierConfig withAuthUrl(String authUrl) {
this.authUrl = authUrl;
return this;
}

public boolean getCookiesEnabled() {
return cookiesEnabled;
}

public VerifierConfig setCookiesEnabled(boolean cookiesEnabled) {
this.cookiesEnabled = cookiesEnabled;
return this;
}

public VerifierConfig withCookiesEnabled(boolean cookiesEnabled) {
this.cookiesEnabled = cookiesEnabled;
return this;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,12 @@ public void setUp() {
public void shouldBuildJwtProxyConfigInYamlFormat() throws Exception {
// given
Set<String> excludes = new HashSet<>();
jwtProxyConfigBuilder.addVerifierProxy(8080, "http://tomcat:8080", new HashSet<>(excludes));
jwtProxyConfigBuilder.addVerifierProxy(
8080, "http://tomcat:8080", new HashSet<>(excludes), false);
excludes.add("/api/liveness");
excludes.add("/other/exclude");
jwtProxyConfigBuilder.addVerifierProxy(4101, "ws://terminal:4101", new HashSet<>(excludes));
jwtProxyConfigBuilder.addVerifierProxy(
4101, "ws://terminal:4101", new HashSet<>(excludes), true);

// when
String jwtProxyConfigYaml = jwtProxyConfigBuilder.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,23 @@
*/
package org.eclipse.che.workspace.infrastructure.kubernetes.server.secure.jwtproxy;

import static org.eclipse.che.api.core.model.workspace.config.ServerConfig.SECURE_SERVER_COOKIES_AUTH_ENABLED_ATTRIBUTE;
import static org.eclipse.che.workspace.infrastructure.kubernetes.server.KubernetesServerExposer.SERVER_PREFIX;
import static org.eclipse.che.workspace.infrastructure.kubernetes.server.KubernetesServerExposer.SERVER_UNIQUE_PART_SIZE;
import static org.eclipse.che.workspace.infrastructure.kubernetes.server.secure.jwtproxy.JwtProxyProvisioner.JWT_PROXY_CONFIG_FILE;
import static org.eclipse.che.workspace.infrastructure.kubernetes.server.secure.jwtproxy.JwtProxyProvisioner.JWT_PROXY_PUBLIC_KEY_FILE;
import static org.eclipse.che.workspace.infrastructure.kubernetes.server.secure.jwtproxy.JwtProxyProvisioner.PUBLIC_KEY_FOOTER;
import static org.eclipse.che.workspace.infrastructure.kubernetes.server.secure.jwtproxy.JwtProxyProvisioner.PUBLIC_KEY_HEADER;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertNotNull;
import static org.testng.Assert.assertTrue;

import com.google.common.collect.ImmutableMap;
import io.fabric8.kubernetes.api.model.ConfigMap;
import io.fabric8.kubernetes.api.model.Pod;
import io.fabric8.kubernetes.api.model.Service;
Expand All @@ -34,6 +39,8 @@
import java.util.regex.Pattern;
import org.eclipse.che.api.core.model.workspace.runtime.RuntimeIdentity;
import org.eclipse.che.api.workspace.server.model.impl.RuntimeIdentityImpl;
import org.eclipse.che.api.workspace.server.model.impl.ServerConfigImpl;
import org.eclipse.che.api.workspace.server.spi.InfrastructureException;
import org.eclipse.che.api.workspace.server.spi.environment.InternalMachineConfig;
import org.eclipse.che.multiuser.machine.authentication.server.signature.SignatureKeyManager;
import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment;
Expand All @@ -59,7 +66,6 @@ public class JwtProxyProvisionerTest {
new RuntimeIdentityImpl(WORKSPACE_ID, "env123", "owner123");

@Mock private SignatureKeyManager signatureKeyManager;
private KeyPair keyPair;
@Mock private PublicKey publicKey;
@Mock private JwtProxyConfigBuilderFactory configBuilderFactory;

Expand All @@ -68,8 +74,7 @@ public class JwtProxyProvisionerTest {

@BeforeMethod
public void setUp() {
keyPair = new KeyPair(publicKey, null);
when(signatureKeyManager.getKeyPair()).thenReturn(keyPair);
when(signatureKeyManager.getKeyPair()).thenReturn(new KeyPair(publicKey, null));
when(publicKey.getEncoded()).thenReturn("publickey".getBytes());

when(configBuilderFactory.create(any()))
Expand Down Expand Up @@ -102,8 +107,13 @@ public void shouldReturnGeneratedJwtProxyConfigMapName() {

@Test
public void shouldProvisionJwtProxyRelatedObjectsIntoKubernetesEnvironment() throws Exception {
// given
ServerConfigImpl secureServer =
new ServerConfigImpl("4401/tcp", "ws", "/", Collections.emptyMap());

// when
jwtProxyProvisioner.expose(k8sEnv, "terminal", 4401, "TCP", Collections.EMPTY_MAP);
jwtProxyProvisioner.expose(
k8sEnv, "terminal", 4401, "TCP", ImmutableMap.of("server", secureServer));

// then
InternalMachineConfig jwtProxyMachine =
Expand All @@ -125,4 +135,87 @@ public void shouldProvisionJwtProxyRelatedObjectsIntoKubernetesEnvironment() thr
Service jwtProxyService = k8sEnv.getServices().get(jwtProxyProvisioner.getServiceName());
assertNotNull(jwtProxyService);
}

@Test(
expectedExceptions = InfrastructureException.class,
expectedExceptionsMessageRegExp =
"Secure servers which expose the same port should have "
+ "the same `cookiesAuthEnabled` value\\.")
public void shouldThrowAnExceptionIsServersHaveDifferentValueForCookiesAuthEnabled()
throws Exception {
// given
ServerConfigImpl server1 =
new ServerConfigImpl(
"4401/tcp",
"ws",
"/",
ImmutableMap.of(SECURE_SERVER_COOKIES_AUTH_ENABLED_ATTRIBUTE, "true"));
ServerConfigImpl server2 =
new ServerConfigImpl(
"4401/tcp",
"http",
"/",
ImmutableMap.of(SECURE_SERVER_COOKIES_AUTH_ENABLED_ATTRIBUTE, "false"));
ServerConfigImpl server3 = new ServerConfigImpl("4401/tcp", "ws", "/", Collections.emptyMap());

// when
jwtProxyProvisioner.expose(
k8sEnv,
"terminal",
4401,
"TCP",
ImmutableMap.of("server1", server1, "server2", server2, "server3", server3));
}

@Test
public void shouldUseCookiesAuthEnabledFromServersConfigs() throws Exception {
// given
JwtProxyConfigBuilder configBuilder = mock(JwtProxyConfigBuilder.class);
when(configBuilderFactory.create(any())).thenReturn(configBuilder);

jwtProxyProvisioner =
new JwtProxyProvisioner(
signatureKeyManager, configBuilderFactory, "eclipse/che-jwtproxy", "128mb", runtimeId);

ServerConfigImpl server1 =
new ServerConfigImpl(
"4401/tcp",
"http",
"/",
ImmutableMap.of(SECURE_SERVER_COOKIES_AUTH_ENABLED_ATTRIBUTE, "true"));
ServerConfigImpl server2 =
new ServerConfigImpl(
"4401/tcp",
"ws",
"/",
ImmutableMap.of(SECURE_SERVER_COOKIES_AUTH_ENABLED_ATTRIBUTE, "true"));

// when
jwtProxyProvisioner.expose(
k8sEnv, "terminal", 4401, "TCP", ImmutableMap.of("server1", server1, "server2", server2));

// then
verify(configBuilder).addVerifierProxy(any(), any(), any(), eq(true));
}

@Test
public void shouldFalseValueAsDefaultForCookiesAuthEnabledAttribute() throws Exception {
// given
JwtProxyConfigBuilder configBuilder = mock(JwtProxyConfigBuilder.class);
when(configBuilderFactory.create(any())).thenReturn(configBuilder);

jwtProxyProvisioner =
new JwtProxyProvisioner(
signatureKeyManager, configBuilderFactory, "eclipse/che-jwtproxy", "128mb", runtimeId);

ServerConfigImpl server1 =
new ServerConfigImpl("4401/tcp", "http", "/", Collections.emptyMap());

// when
jwtProxyProvisioner.expose(
k8sEnv, "terminal", 4401, "TCP", ImmutableMap.of("server1", server1));

// then
verify(configBuilder).addVerifierProxy(any(), any(), any(), eq(false));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ jwtproxy:
- listen_addr: ":8080"
verifier:
audience: "workspace123"
auth_redirect_url: "http://che-host.com/app/loader.html"
auth_cookies_enabled: false
claims_verifiers:
- options:
iss: "wsmaster"
Expand All @@ -25,6 +25,7 @@ jwtproxy:
- listen_addr: ":4101"
verifier:
audience: "workspace123"
auth_cookies_enabled: true
auth_redirect_url: "http://che-host.com/app/loader.html"
claims_verifiers:
- options:
Expand Down

0 comments on commit 43066dc

Please sign in to comment.