Skip to content

Commit

Permalink
fix: Revert the kubernetes Secrets role due to the security vulnerabi…
Browse files Browse the repository at this point in the history
…lity (#58)
  • Loading branch information
vinokurig authored Jul 20, 2021
1 parent 9435363 commit 7a81693
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 78 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ public abstract class AbstractWorkspaceServiceAccount<
public static final String EXEC_ROLE_NAME = "exec";
public static final String VIEW_ROLE_NAME = "workspace-view";
public static final String METRICS_ROLE_NAME = "workspace-metrics";
public static final String SECRETS_ROLE_NAME = "workspace-secrets";

protected final String namespace;
protected final String serviceAccountName;
Expand Down Expand Up @@ -134,15 +133,6 @@ private void createImplicitRolesWithBindings(Client k8sClient) {
singletonList("metrics.k8s.io"),
Arrays.asList("list", "get", "watch"),
serviceAccountName + "-metrics");

// secrets role
createRoleWithBinding(
k8sClient,
SECRETS_ROLE_NAME,
Arrays.asList("secrets"),
singletonList(""),
Arrays.asList("list", "create", "delete"),
serviceAccountName + "-secrets");
}

private void createRoleWithBinding(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import static org.eclipse.che.api.workspace.shared.Constants.WORKSPACE_INFRASTRUCTURE_NAMESPACE_ATTRIBUTE;
import static org.eclipse.che.workspace.infrastructure.kubernetes.api.shared.KubernetesNamespaceMeta.DEFAULT_ATTRIBUTE;
import static org.eclipse.che.workspace.infrastructure.kubernetes.api.shared.KubernetesNamespaceMeta.PHASE_ATTRIBUTE;
import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.AbstractWorkspaceServiceAccount.SECRETS_ROLE_NAME;
import static org.eclipse.che.workspace.infrastructure.kubernetes.namespace.KubernetesNamespaceFactory.NAMESPACE_TEMPLATE_ATTRIBUTE;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyMap;
Expand Down Expand Up @@ -44,7 +43,6 @@
import io.fabric8.kubernetes.api.model.ServiceAccountList;
import io.fabric8.kubernetes.api.model.Status;
import io.fabric8.kubernetes.api.model.rbac.ClusterRoleBuilder;
import io.fabric8.kubernetes.api.model.rbac.PolicyRule;
import io.fabric8.kubernetes.api.model.rbac.Role;
import io.fabric8.kubernetes.api.model.rbac.RoleBindingList;
import io.fabric8.kubernetes.api.model.rbac.RoleList;
Expand All @@ -59,7 +57,6 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
Expand Down Expand Up @@ -654,7 +651,7 @@ public void shouldBindToAllConfiguredClusterRoles() throws Exception {

RoleList roles = k8sClient.rbac().roles().inNamespace("workspace123").list();
assertEquals(
Sets.newHashSet("workspace-view", "workspace-metrics", "exec", "workspace-secrets"),
Sets.newHashSet("workspace-view", "workspace-metrics", "exec"),
roles.getItems().stream().map(r -> r.getMetadata().getName()).collect(Collectors.toSet()));
RoleBindingList bindings = k8sClient.rbac().roleBindings().inNamespace("workspace123").list();
assertEquals(
Expand All @@ -668,64 +665,7 @@ public void shouldBindToAllConfiguredClusterRoles() throws Exception {
"serviceAccount-cluster0",
"serviceAccount-cluster1",
"serviceAccount-view",
"serviceAccount-exec",
"serviceAccount-secrets"));
}

@Test
public void shouldCreateAndBindSecretsRole() throws Exception {
// given
namespaceFactory =
spy(
new KubernetesNamespaceFactory(
"serviceAccount",
"cr2, cr3",
"<username>-che",
true,
true,
NAMESPACE_LABELS,
NAMESPACE_ANNOTATIONS,
clientFactory,
cheClientFactory,
userManager,
preferenceManager,
pool));
KubernetesNamespace toReturnNamespace = mock(KubernetesNamespace.class);
when(toReturnNamespace.getWorkspaceId()).thenReturn("workspace123");
when(toReturnNamespace.getName()).thenReturn("workspace123");
doReturn(toReturnNamespace).when(namespaceFactory).doCreateNamespaceAccess(any(), any());
when(clientFactory.create(any())).thenReturn(k8sClient);

// when
RuntimeIdentity identity =
new RuntimeIdentityImpl("workspace123", null, USER_ID, "workspace123");
namespaceFactory.getOrCreate(identity);

// then
Optional<Role> roleOptional =
k8sClient
.rbac()
.roles()
.inNamespace("workspace123")
.list()
.getItems()
.stream()
.filter(r -> r.getMetadata().getName().equals(SECRETS_ROLE_NAME))
.findAny();
assertTrue(roleOptional.isPresent());
PolicyRule rule = roleOptional.get().getRules().get(0);
assertEquals(rule.getResources(), singletonList("secrets"));
assertEquals(rule.getApiGroups(), singletonList(""));
assertEquals(rule.getVerbs(), Arrays.asList("list", "create", "delete"));
assertTrue(
k8sClient
.rbac()
.roleBindings()
.inNamespace("workspace123")
.list()
.getItems()
.stream()
.anyMatch(rb -> rb.getMetadata().getName().equals("serviceAccount-secrets")));
"serviceAccount-exec"));
}

@Test
Expand Down Expand Up @@ -766,7 +706,7 @@ public void shouldCreateExecAndViewRolesAndBindings() throws Exception {

RoleList roles = k8sClient.rbac().roles().inNamespace("workspace123").list();
assertEquals(
Sets.newHashSet("workspace-view", "workspace-secrets", "workspace-metrics", "exec"),
Sets.newHashSet("workspace-view", "workspace-metrics", "exec"),
roles.getItems().stream().map(r -> r.getMetadata().getName()).collect(Collectors.toSet()));
Role role1 = roles.getItems().get(0);
Role role2 = roles.getItems().get(1);
Expand All @@ -783,11 +723,7 @@ public void shouldCreateExecAndViewRolesAndBindings() throws Exception {
.stream()
.map(r -> r.getMetadata().getName())
.collect(Collectors.toSet()),
Sets.newHashSet(
"serviceAccount-metrics",
"serviceAccount-view",
"serviceAccount-exec",
"serviceAccount-secrets"));
Sets.newHashSet("serviceAccount-metrics", "serviceAccount-view", "serviceAccount-exec"));
}

@Test
Expand Down

0 comments on commit 7a81693

Please sign in to comment.