diff --git a/pom.xml b/pom.xml
index 779b367131..00f1231a4d 100644
--- a/pom.xml
+++ b/pom.xml
@@ -201,6 +201,18 @@
${com.github.spotbugs.version}
provided
+
+ io.fabric8
+ kubernetes-server-mock
+ ${io.fabric8.client.version}
+ test
+
+
+ io.fabric8
+ openshift-server-mock
+ ${io.fabric8.client.version}
+ test
+
diff --git a/src/main/java/io/cryostat/net/NetworkModule.java b/src/main/java/io/cryostat/net/NetworkModule.java
index 3f8a3b7abd..48ed6f9db0 100644
--- a/src/main/java/io/cryostat/net/NetworkModule.java
+++ b/src/main/java/io/cryostat/net/NetworkModule.java
@@ -55,6 +55,8 @@
import dagger.Module;
import dagger.Provides;
import dagger.multibindings.IntoSet;
+import io.fabric8.openshift.client.DefaultOpenShiftClient;
+import io.fabric8.openshift.client.OpenShiftConfigBuilder;
import io.vertx.core.Vertx;
import io.vertx.ext.web.client.WebClient;
import io.vertx.ext.web.client.WebClientOptions;
@@ -159,7 +161,12 @@ static BasicAuthManager provideBasicAuthManager(Logger logger, FileSystem fs) {
@Provides
@Singleton
static OpenShiftAuthManager provideOpenShiftAuthManager(Logger logger, FileSystem fs) {
- return new OpenShiftAuthManager(logger, fs);
+ return new OpenShiftAuthManager(
+ logger,
+ fs,
+ token ->
+ new DefaultOpenShiftClient(
+ new OpenShiftConfigBuilder().withOauthToken(token).build()));
}
@Binds
diff --git a/src/main/java/io/cryostat/net/OpenShiftAuthManager.java b/src/main/java/io/cryostat/net/OpenShiftAuthManager.java
index 37735c2327..3ddfa4c5ac 100644
--- a/src/main/java/io/cryostat/net/OpenShiftAuthManager.java
+++ b/src/main/java/io/cryostat/net/OpenShiftAuthManager.java
@@ -47,40 +47,42 @@
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
+import java.util.function.Function;
import java.util.function.Supplier;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
-import org.apache.commons.lang3.StringUtils;
-
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import io.cryostat.core.log.Logger;
import io.cryostat.core.sys.FileSystem;
import io.cryostat.net.security.ResourceAction;
import io.cryostat.net.security.ResourceType;
import io.cryostat.net.security.ResourceVerb;
+
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import io.fabric8.kubernetes.api.model.authorization.v1.SelfSubjectAccessReview;
import io.fabric8.kubernetes.api.model.authorization.v1.SelfSubjectAccessReviewBuilder;
import io.fabric8.kubernetes.client.Config;
import io.fabric8.kubernetes.client.KubernetesClientException;
-import io.fabric8.openshift.client.DefaultOpenShiftClient;
import io.fabric8.openshift.client.OpenShiftClient;
-import io.fabric8.openshift.client.OpenShiftConfigBuilder;
import jdk.jfr.Category;
import jdk.jfr.Event;
import jdk.jfr.Label;
import jdk.jfr.Name;
+import org.apache.commons.lang3.StringUtils;
public class OpenShiftAuthManager extends AbstractAuthManager {
private static final String PERMISSION_NOT_REQUIRED = "PERMISSION_NOT_REQUIRED";
private final FileSystem fs;
+ private final Function clientProvider;
- public OpenShiftAuthManager(Logger logger, FileSystem fs) {
+ OpenShiftAuthManager(
+ Logger logger, FileSystem fs, Function clientProvider) {
super(logger);
this.fs = fs;
+ this.clientProvider = clientProvider;
}
@Override
@@ -96,9 +98,7 @@ public Future validateToken(
return CompletableFuture.completedFuture(false);
}
- try (OpenShiftClient authClient =
- new DefaultOpenShiftClient(
- new OpenShiftConfigBuilder().withOauthToken(token).build())) {
+ try (OpenShiftClient client = clientProvider.apply(token)) {
List> results =
resourceActions
.parallelStream()
@@ -106,7 +106,7 @@ public Future validateToken(
resourceAction -> {
try {
return CompletableFuture.completedFuture(
- validateAction(authClient, resourceAction));
+ validateAction(client, resourceAction));
} catch (IOException | PermissionDeniedException e) {
return CompletableFuture.failedFuture(e);
}
@@ -142,7 +142,7 @@ public Future validateToken(
}
}
- private boolean validateAction(OpenShiftClient authClient, ResourceAction resourceAction)
+ private boolean validateAction(OpenShiftClient client, ResourceAction resourceAction)
throws IOException, PermissionDeniedException {
AuthRequest evt = new AuthRequest();
evt.begin();
@@ -167,7 +167,7 @@ private boolean validateAction(OpenShiftClient authClient, ResourceAction resour
.endSpec()
.build();
accessReview =
- authClient.authorization().v1().selfSubjectAccessReview().create(accessReview);
+ client.authorization().v1().selfSubjectAccessReview().create(accessReview);
boolean allowed = accessReview.getStatus().getAllowed();
evt.setRequestSuccessful(true);
if (allowed) {
@@ -184,25 +184,6 @@ private boolean validateAction(OpenShiftClient authClient, ResourceAction resour
}
}
- @Name("io.cryostat.net.OpenShiftAuthManager.AuthRequest")
- @Label("AuthRequest")
- @Category("Cryostat")
- @SuppressFBWarnings(
- value = "URF_UNREAD_FIELD",
- justification = "Event fields are recorded with JFR instead of accessed directly")
- public static class AuthRequest extends Event {
-
- boolean requestSuccessful;
-
- public AuthRequest() {
- this.requestSuccessful = false;
- }
-
- public void setRequestSuccessful(boolean requestSuccessful) {
- this.requestSuccessful = requestSuccessful;
- }
- }
-
@Override
public Future validateHttpHeader(
Supplier headerProvider, Set resourceActions) {
@@ -254,7 +235,7 @@ private String getNamespace() throws IOException {
.get();
}
- static String map(ResourceType resource) {
+ private static String map(ResourceType resource) {
switch (resource) {
case TARGET:
return "flightrecorders";
@@ -270,7 +251,7 @@ static String map(ResourceType resource) {
}
}
- static String map(ResourceVerb verb) {
+ private static String map(ResourceVerb verb) {
switch (verb) {
case CREATE:
return "create";
@@ -288,12 +269,50 @@ static String map(ResourceVerb verb) {
@SuppressWarnings("serial")
public static class PermissionDeniedException extends Exception {
+ private final String namespace;
+ private final String resource;
+ private final String verb;
+
public PermissionDeniedException(
String namespace, String group, String resource, String verb, String reason) {
super(
String.format(
"Requesting client in namespace \"%s\" cannot %s %s.%s: %s",
namespace, verb, resource, group, reason));
+ this.namespace = namespace;
+ this.resource = resource;
+ this.verb = verb;
+ }
+
+ public String getNamespace() {
+ return namespace;
+ }
+
+ public String getResourceType() {
+ return resource;
+ }
+
+ public String getVerb() {
+ return verb;
+ }
+ }
+
+ @Name("io.cryostat.net.OpenShiftAuthManager.AuthRequest")
+ @Label("AuthRequest")
+ @Category("Cryostat")
+ @SuppressFBWarnings(
+ value = "URF_UNREAD_FIELD",
+ justification = "Event fields are recorded with JFR instead of accessed directly")
+ public static class AuthRequest extends Event {
+
+ boolean requestSuccessful;
+
+ public AuthRequest() {
+ this.requestSuccessful = false;
+ }
+
+ public void setRequestSuccessful(boolean requestSuccessful) {
+ this.requestSuccessful = requestSuccessful;
}
}
}
diff --git a/src/test/java/io/cryostat/net/OpenShiftAuthManagerTest.java b/src/test/java/io/cryostat/net/OpenShiftAuthManagerTest.java
new file mode 100644
index 0000000000..868abc6dde
--- /dev/null
+++ b/src/test/java/io/cryostat/net/OpenShiftAuthManagerTest.java
@@ -0,0 +1,274 @@
+/*
+ * Copyright The Cryostat Authors
+ *
+ * The Universal Permissive License (UPL), Version 1.0
+ *
+ * Subject to the condition set forth below, permission is hereby granted to any
+ * person obtaining a copy of this software, associated documentation and/or data
+ * (collectively the "Software"), free of charge and under any and all copyright
+ * rights in the Software, and any and all patent rights owned or freely
+ * licensable by each licensor hereunder covering either (i) the unmodified
+ * Software as contributed to or provided by such licensor, or (ii) the Larger
+ * Works (as defined below), to deal in both
+ *
+ * (a) the Software, and
+ * (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if
+ * one is included with the Software (each a "Larger Work" to which the Software
+ * is contributed by such licensors),
+ *
+ * without restriction, including without limitation the rights to copy, create
+ * derivative works of, display, perform, and distribute the Software and make,
+ * use, sell, offer for sale, import, export, have made, and have sold the
+ * Software and the Larger Work(s), and to sublicense the foregoing rights on
+ * either these or other terms.
+ *
+ * This license is subject to the following condition:
+ * The above copyright notice and either this complete permission notice or at
+ * a minimum a reference to the UPL must be included in all copies or
+ * substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+package io.cryostat.net;
+
+import java.io.BufferedReader;
+import java.io.StringReader;
+import java.net.HttpURLConnection;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Set;
+import java.util.concurrent.ExecutionException;
+import java.util.function.Function;
+
+import io.cryostat.MainModule;
+import io.cryostat.core.log.Logger;
+import io.cryostat.core.sys.FileSystem;
+import io.cryostat.net.OpenShiftAuthManager.PermissionDeniedException;
+import io.cryostat.net.security.ResourceAction;
+import io.cryostat.net.security.ResourceType;
+import io.cryostat.net.security.ResourceVerb;
+
+import com.google.gson.Gson;
+import io.fabric8.kubernetes.api.model.authorization.v1.SelfSubjectAccessReview;
+import io.fabric8.kubernetes.api.model.authorization.v1.SelfSubjectAccessReviewBuilder;
+import io.fabric8.openshift.client.OpenShiftClient;
+import io.fabric8.openshift.client.server.mock.EnableOpenShiftMockClient;
+import io.fabric8.openshift.client.server.mock.OpenShiftMockServer;
+import io.fabric8.openshift.client.server.mock.OpenShiftMockServerExtension;
+import okhttp3.mockwebserver.RecordedRequest;
+import org.apache.commons.lang3.exception.ExceptionUtils;
+import org.hamcrest.MatcherAssert;
+import org.hamcrest.Matchers;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.EnumSource;
+import org.junit.jupiter.params.provider.NullAndEmptySource;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+@ExtendWith({MockitoExtension.class, OpenShiftMockServerExtension.class})
+@EnableOpenShiftMockClient(https = false, crud = false)
+class OpenShiftAuthManagerTest {
+
+ static final String SUBJECT_REVIEW_API_PATH =
+ "/apis/authorization.k8s.io/v1/selfsubjectaccessreviews";
+ static final String NAMESPACE_FS_PATH =
+ "/var/run/secrets/kubernetes.io/serviceaccount/namespace";
+
+ OpenShiftAuthManager mgr;
+ @Mock FileSystem fs;
+ @Mock Logger logger;
+ OpenShiftClient client;
+ OpenShiftMockServer server;
+ TokenProvider tokenProvider;
+ Gson gson = MainModule.provideGson(logger);
+
+ @BeforeEach
+ void setup() {
+ client = Mockito.spy(client);
+ tokenProvider = new TokenProvider(client);
+ mgr = new OpenShiftAuthManager(logger, fs, tokenProvider);
+ }
+
+ @Test
+ void shouldHandleBearerAuthentication() {
+ MatcherAssert.assertThat(mgr.getScheme(), Matchers.equalTo(AuthenticationScheme.BEARER));
+ }
+
+ @ParameterizedTest
+ @NullAndEmptySource
+ void shouldNotValidateBlankToken(String tok) throws Exception {
+ MatcherAssert.assertThat(
+ mgr.validateToken(() -> tok, ResourceAction.NONE).get(), Matchers.is(false));
+ }
+
+ @Test
+ void shouldValidateTokenWithNoPermissions() throws Exception {
+ MatcherAssert.assertThat(
+ mgr.validateToken(() -> "token", ResourceAction.NONE).get(), Matchers.is(true));
+ }
+
+ @Test
+ void shouldValidateTokenWithSufficientPermissions() throws Exception {
+ SelfSubjectAccessReview accessReview =
+ new SelfSubjectAccessReviewBuilder()
+ .withNewStatus()
+ .withAllowed(true)
+ .endStatus()
+ .build();
+ server.expect()
+ .post()
+ .withPath(SUBJECT_REVIEW_API_PATH)
+ .andReturn(HttpURLConnection.HTTP_CREATED, accessReview)
+ .once();
+
+ Mockito.when(fs.readFile(Mockito.any()))
+ .thenReturn(new BufferedReader(new StringReader("mynamespace")));
+
+ MatcherAssert.assertThat(
+ mgr.validateToken(() -> "token", Set.of(ResourceAction.READ_TARGET)).get(),
+ Matchers.is(true));
+
+ ArgumentCaptor nsPathCaptor = ArgumentCaptor.forClass(Path.class);
+ Mockito.verify(fs).readFile(nsPathCaptor.capture());
+ MatcherAssert.assertThat(
+ nsPathCaptor.getValue(), Matchers.equalTo(Paths.get(NAMESPACE_FS_PATH)));
+ }
+
+ @Test
+ void shouldNotValidateTokenWithSufficientPermissions() throws Exception {
+ SelfSubjectAccessReview accessReview =
+ new SelfSubjectAccessReviewBuilder()
+ .withNewStatus()
+ .withAllowed(false)
+ .endStatus()
+ .build();
+ server.expect()
+ .post()
+ .withPath(SUBJECT_REVIEW_API_PATH)
+ .andReturn(HttpURLConnection.HTTP_CREATED, accessReview)
+ .once();
+
+ String namespace = "mynamespace";
+ Mockito.when(fs.readFile(Mockito.any()))
+ .thenReturn(new BufferedReader(new StringReader(namespace)));
+
+ ExecutionException ee =
+ Assertions.assertThrows(
+ ExecutionException.class,
+ () ->
+ mgr.validateToken(() -> "token", Set.of(ResourceAction.READ_TARGET))
+ .get());
+ MatcherAssert.assertThat(
+ ExceptionUtils.getRootCause(ee),
+ Matchers.instanceOf(PermissionDeniedException.class));
+ PermissionDeniedException pde = (PermissionDeniedException) ExceptionUtils.getRootCause(ee);
+ MatcherAssert.assertThat(pde.getNamespace(), Matchers.equalTo(namespace));
+ MatcherAssert.assertThat(pde.getResourceType(), Matchers.equalTo("flightrecorders"));
+ MatcherAssert.assertThat(pde.getVerb(), Matchers.equalTo("get"));
+
+ ArgumentCaptor nsPathCaptor = ArgumentCaptor.forClass(Path.class);
+ Mockito.verify(fs).readFile(nsPathCaptor.capture());
+ MatcherAssert.assertThat(
+ nsPathCaptor.getValue(), Matchers.equalTo(Paths.get(NAMESPACE_FS_PATH)));
+ }
+
+ @ParameterizedTest
+ @EnumSource(mode = EnumSource.Mode.MATCH_ANY, names = "^([a-zA-Z]+_(RECORDING|TARGET))$")
+ void shouldValidateExpectedPermissionsPerSecuredResource(ResourceAction resourceAction)
+ throws Exception {
+ Mockito.when(fs.readFile(Mockito.any()))
+ .thenReturn(new BufferedReader(new StringReader("mynamespace")));
+
+ SelfSubjectAccessReview accessReview =
+ new SelfSubjectAccessReviewBuilder()
+ .withNewStatus()
+ .withAllowed(true)
+ .endStatus()
+ .build();
+ server.expect()
+ .post()
+ .withPath(SUBJECT_REVIEW_API_PATH)
+ .andReturn(HttpURLConnection.HTTP_CREATED, accessReview)
+ .once();
+
+ String token = "abcd1234";
+ MatcherAssert.assertThat(
+ mgr.validateToken(() -> token, Set.of(resourceAction)).get(), Matchers.is(true));
+
+ RecordedRequest req = server.getLastRequest();
+ MatcherAssert.assertThat(req.getPath(), Matchers.equalTo(SUBJECT_REVIEW_API_PATH));
+ MatcherAssert.assertThat(tokenProvider.token, Matchers.equalTo(token));
+ MatcherAssert.assertThat(req.getMethod(), Matchers.equalTo("POST"));
+
+ SelfSubjectAccessReview body =
+ gson.fromJson(req.getBody().readUtf8(), SelfSubjectAccessReview.class);
+
+ String expectedVerb;
+ if (resourceAction.getVerb() == ResourceVerb.CREATE) {
+ expectedVerb = "create";
+ } else if (resourceAction.getVerb() == ResourceVerb.READ) {
+ expectedVerb = "get";
+ } else if (resourceAction.getVerb() == ResourceVerb.UPDATE) {
+ expectedVerb = "patch";
+ } else if (resourceAction.getVerb() == ResourceVerb.DELETE) {
+ expectedVerb = "delete";
+ } else {
+ throw new IllegalArgumentException(resourceAction.getVerb().toString());
+ }
+ MatcherAssert.assertThat(
+ body.getSpec().getResourceAttributes().getVerb(), Matchers.equalTo(expectedVerb));
+
+ String expectedResource;
+ if (resourceAction.getResource() == ResourceType.TARGET) {
+ expectedResource = "flightrecorders";
+ } else if (resourceAction.getResource() == ResourceType.RECORDING) {
+ expectedResource = "recordings";
+ } else {
+ throw new IllegalArgumentException(resourceAction.getResource().toString());
+ }
+ MatcherAssert.assertThat(
+ body.getSpec().getResourceAttributes().getResource(),
+ Matchers.equalTo(expectedResource));
+ }
+
+ @ParameterizedTest
+ @EnumSource(
+ mode = EnumSource.Mode.MATCH_ALL,
+ names = {"^[a-zA-Z]+_(?!TARGET).*$", "^[a-zA-Z]+_(?!RECORDING).*$"})
+ void shouldValidateExpectedPermissionsForUnsecuredResources(ResourceAction resourceAction)
+ throws Exception {
+ MatcherAssert.assertThat(
+ mgr.validateToken(() -> "token", Set.of(resourceAction)).get(), Matchers.is(true));
+ }
+
+ private static class TokenProvider implements Function {
+
+ private final OpenShiftClient osc;
+ String token;
+
+ TokenProvider(OpenShiftClient osc) {
+ this.osc = osc;
+ }
+
+ @Override
+ public OpenShiftClient apply(String token) {
+ if (this.token != null) {
+ throw new IllegalStateException("Token was already set!");
+ }
+ this.token = token;
+ return osc;
+ }
+ }
+}