diff --git a/src/main/java/io/cryostat/net/OpenShiftAuthManager.java b/src/main/java/io/cryostat/net/OpenShiftAuthManager.java index f87292fd2e..72033b99a0 100644 --- a/src/main/java/io/cryostat/net/OpenShiftAuthManager.java +++ b/src/main/java/io/cryostat/net/OpenShiftAuthManager.java @@ -61,6 +61,8 @@ import io.cryostat.net.security.ResourceVerb; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import io.fabric8.kubernetes.api.model.authentication.TokenReview; +import io.fabric8.kubernetes.api.model.authentication.TokenReviewBuilder; import io.fabric8.kubernetes.api.model.authorization.v1.SelfSubjectAccessReview; import io.fabric8.kubernetes.api.model.authorization.v1.SelfSubjectAccessReviewBuilder; import io.fabric8.kubernetes.client.Config; @@ -99,7 +101,7 @@ public Future validateToken( return CompletableFuture.completedFuture(false); } if (resourceActions.isEmpty()) { - return CompletableFuture.completedFuture(true); + return reviewToken(token); } try (OpenShiftClient client = clientProvider.apply(token)) { @@ -126,6 +128,22 @@ public Future validateToken( } } + Future reviewToken(String token) { + try (OpenShiftClient client = clientProvider.apply(getServiceAccountToken())) { + TokenReview review = + new TokenReviewBuilder().withNewSpec().withToken(token).endSpec().build(); + review = client.tokenReviews().create(review); + Boolean authenticated = review.getStatus().getAuthenticated(); + return CompletableFuture.completedFuture(authenticated != null && authenticated); + } catch (KubernetesClientException e) { + logger.info(e); + return CompletableFuture.failedFuture(e); + } catch (Exception e) { + logger.error(e); + return CompletableFuture.failedFuture(e); + } + } + private Stream> validateAction( OpenShiftClient client, String namespace, ResourceAction resourceAction) { Set resources = map(resourceAction.getResource()); @@ -231,6 +249,17 @@ private String getNamespace() throws IOException { .get(); } + @SuppressFBWarnings( + value = "DMI_HARDCODED_ABSOLUTE_FILENAME", + justification = "Kubernetes serviceaccount file path is well-known and absolute") + private String getServiceAccountToken() throws IOException { + return fs.readFile(Paths.get(Config.KUBERNETES_SERVICE_ACCOUNT_TOKEN_PATH)) + .lines() + .filter(StringUtils::isNotBlank) + .findFirst() + .get(); + } + private static Set map(ResourceType resource) { switch (resource) { case TARGET: diff --git a/src/test/java/io/cryostat/net/OpenShiftAuthManagerTest.java b/src/test/java/io/cryostat/net/OpenShiftAuthManagerTest.java index af8a5d6b53..73ab7b381e 100644 --- a/src/test/java/io/cryostat/net/OpenShiftAuthManagerTest.java +++ b/src/test/java/io/cryostat/net/OpenShiftAuthManagerTest.java @@ -57,6 +57,8 @@ import io.cryostat.net.security.ResourceVerb; import com.google.gson.Gson; +import io.fabric8.kubernetes.api.model.authentication.TokenReview; +import io.fabric8.kubernetes.api.model.authentication.TokenReviewBuilder; import io.fabric8.kubernetes.api.model.authorization.v1.SelfSubjectAccessReview; import io.fabric8.kubernetes.api.model.authorization.v1.SelfSubjectAccessReviewBuilder; import io.fabric8.kubernetes.client.Config; @@ -87,8 +89,7 @@ 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"; + static final String TOKEN_REVIEW_API_PATH = "/apis/authentication.k8s.io/v1/tokenreviews"; OpenShiftAuthManager mgr; @Mock FileSystem fs; @@ -102,6 +103,7 @@ class OpenShiftAuthManagerTest { static void disableKubeConfig() { // FIXME Disable reading ~/.kube/config. Remove once updated to 5.5.0 or newer. System.setProperty(Config.KUBERNETES_AUTH_TRYKUBECONFIG_SYSTEM_PROPERTY, "false"); + System.setProperty(Config.KUBERNETES_AUTH_TRYSERVICEACCOUNT_SYSTEM_PROPERTY, "false"); } @BeforeEach @@ -124,9 +126,46 @@ void shouldNotValidateBlankToken(String tok) throws Exception { } @Test - void shouldValidateTokenWithNoPermissions() throws Exception { + void shouldValidateTokenWithNoRequiredPermissions() throws Exception { + TokenReview tokenReview = + new TokenReviewBuilder() + .withNewStatus() + .withAuthenticated(true) + .endStatus() + .build(); + server.expect() + .post() + .withPath(TOKEN_REVIEW_API_PATH) + .andReturn(HttpURLConnection.HTTP_CREATED, tokenReview) + .once(); + + Mockito.when(fs.readFile(Paths.get(Config.KUBERNETES_SERVICE_ACCOUNT_TOKEN_PATH))) + .thenReturn(new BufferedReader(new StringReader("serviceAccountToken"))); + + MatcherAssert.assertThat( + mgr.validateToken(() -> "userToken", ResourceAction.NONE).get(), Matchers.is(true)); + } + + @Test + void shouldNotValidateTokenWithNoRequiredPermissionsButNoTokenAccess() throws Exception { + TokenReview tokenReview = + new TokenReviewBuilder() + .withNewStatus() + .withAuthenticated(false) + .endStatus() + .build(); + server.expect() + .post() + .withPath(TOKEN_REVIEW_API_PATH) + .andReturn(HttpURLConnection.HTTP_CREATED, tokenReview) + .once(); + + Mockito.when(fs.readFile(Paths.get(Config.KUBERNETES_SERVICE_ACCOUNT_TOKEN_PATH))) + .thenReturn(new BufferedReader(new StringReader("serviceAccountToken"))); + MatcherAssert.assertThat( - mgr.validateToken(() -> "token", ResourceAction.NONE).get(), Matchers.is(true)); + mgr.validateToken(() -> "userToken", ResourceAction.NONE).get(), + Matchers.is(false)); } @Test @@ -143,7 +182,7 @@ void shouldValidateTokenWithSufficientPermissions() throws Exception { .andReturn(HttpURLConnection.HTTP_CREATED, accessReview) .once(); - Mockito.when(fs.readFile(Mockito.any())) + Mockito.when(fs.readFile(Paths.get(Config.KUBERNETES_NAMESPACE_PATH))) .thenReturn(new BufferedReader(new StringReader("mynamespace"))); MatcherAssert.assertThat( @@ -153,7 +192,8 @@ void shouldValidateTokenWithSufficientPermissions() throws Exception { ArgumentCaptor nsPathCaptor = ArgumentCaptor.forClass(Path.class); Mockito.verify(fs).readFile(nsPathCaptor.capture()); MatcherAssert.assertThat( - nsPathCaptor.getValue(), Matchers.equalTo(Paths.get(NAMESPACE_FS_PATH))); + nsPathCaptor.getValue(), + Matchers.equalTo(Paths.get(Config.KUBERNETES_NAMESPACE_PATH))); } @Test @@ -171,7 +211,7 @@ void shouldNotValidateTokenWithInsufficientPermissions() throws Exception { .once(); String namespace = "mynamespace"; - Mockito.when(fs.readFile(Mockito.any())) + Mockito.when(fs.readFile(Paths.get(Config.KUBERNETES_NAMESPACE_PATH))) .thenReturn(new BufferedReader(new StringReader(namespace))); ExecutionException ee = @@ -191,7 +231,8 @@ void shouldNotValidateTokenWithInsufficientPermissions() throws Exception { ArgumentCaptor nsPathCaptor = ArgumentCaptor.forClass(Path.class); Mockito.verify(fs).readFile(nsPathCaptor.capture()); MatcherAssert.assertThat( - nsPathCaptor.getValue(), Matchers.equalTo(Paths.get(NAMESPACE_FS_PATH))); + nsPathCaptor.getValue(), + Matchers.equalTo(Paths.get(Config.KUBERNETES_NAMESPACE_PATH))); } @ParameterizedTest @@ -200,7 +241,7 @@ void shouldNotValidateTokenWithInsufficientPermissions() throws Exception { names = "^([a-zA-Z]+_(RECORDING|TARGET|CERTIFICATE|CREDENTIALS))$") void shouldValidateExpectedPermissionsPerSecuredResource(ResourceAction resourceAction) throws Exception { - Mockito.when(fs.readFile(Mockito.any())) + Mockito.when(fs.readFile(Paths.get(Config.KUBERNETES_NAMESPACE_PATH))) .thenReturn(new BufferedReader(new StringReader("mynamespace"))); String expectedVerb; @@ -305,7 +346,7 @@ void shouldValidateExpectedPermissionsPerSecuredResource(ResourceAction resource }) void shouldValidateExpectedPermissionsForUnsecuredResources(ResourceAction resourceAction) throws Exception { - Mockito.when(fs.readFile(Mockito.any())) + Mockito.when(fs.readFile(Paths.get(Config.KUBERNETES_NAMESPACE_PATH))) .thenReturn(new BufferedReader(new StringReader("mynamespace"))); MatcherAssert.assertThat( mgr.validateToken(() -> "token", Set.of(resourceAction)).get(), Matchers.is(true));