diff --git a/src/main/java/io/cryostat/net/OpenShiftAuthManager.java b/src/main/java/io/cryostat/net/OpenShiftAuthManager.java index 72033b99a0..458ca95ac8 100644 --- a/src/main/java/io/cryostat/net/OpenShiftAuthManager.java +++ b/src/main/java/io/cryostat/net/OpenShiftAuthManager.java @@ -76,7 +76,8 @@ public class OpenShiftAuthManager extends AbstractAuthManager { - private static final Set PERMISSION_NOT_REQUIRED = Set.of("PERMISSION_NOT_REQUIRED"); + private static final Set PERMISSION_NOT_REQUIRED = + Set.of(GroupResource.PERMISSION_NOT_REQUIRED); private final FileSystem fs; private final Function clientProvider; @@ -146,11 +147,10 @@ Future reviewToken(String token) { private Stream> validateAction( OpenShiftClient client, String namespace, ResourceAction resourceAction) { - Set resources = map(resourceAction.getResource()); + Set resources = map(resourceAction.getResource()); if (PERMISSION_NOT_REQUIRED.equals(resources) || resources.isEmpty()) { return Stream.of(CompletableFuture.completedFuture(null)); } - String group = "operator.cryostat.io"; String verb = map(resourceAction.getVerb()); return resources .parallelStream() @@ -164,8 +164,8 @@ private Stream> validateAction( .withNewSpec() .withNewResourceAttributes() .withNamespace(namespace) - .withGroup(group) - .withResource(resource) + .withGroup(resource.getGroup()) + .withResource(resource.getResource()) .withVerb(verb) .endResourceAttributes() .endSpec() @@ -180,8 +180,8 @@ private Stream> validateAction( return CompletableFuture.failedFuture( new PermissionDeniedException( namespace, - group, - resource, + resource.getGroup(), + resource.getResource(), verb, accessReview.getStatus().getReason())); } else { @@ -260,16 +260,17 @@ private String getServiceAccountToken() throws IOException { .get(); } - private static Set map(ResourceType resource) { + private static Set map(ResourceType resource) { switch (resource) { case TARGET: - return Set.of("flightrecorders"); + return Set.of(GroupResource.FLIGHTRECORDERS); case RECORDING: - return Set.of("recordings"); + return Set.of(GroupResource.RECORDINGS); case CERTIFICATE: - return Set.of("deployments", "pods", "cryostats"); + return Set.of( + GroupResource.DEPLOYMENTS, GroupResource.PODS, GroupResource.CRYOSTATS); case CREDENTIALS: - return Set.of("cryostats"); + return Set.of(GroupResource.CRYOSTATS); case TEMPLATE: case REPORT: case RULE: @@ -342,4 +343,31 @@ public void setRequestSuccessful(boolean requestSuccessful) { this.requestSuccessful = requestSuccessful; } } + + // A pairing of a Kubernetes group name and resource name + private static enum GroupResource { + DEPLOYMENTS("apps", "deployments"), + PODS("", "pods"), + CRYOSTATS("operator.cryostat.io", "cryostats"), + FLIGHTRECORDERS("operator.cryostat.io", "flightrecorders"), + RECORDINGS("operator.cryostat.io", "recordings"), + PERMISSION_NOT_REQUIRED("", "PERMISSION_NOT_REQUIRED"), + ; + + private final String group; + private final String resource; + + private GroupResource(String group, String resource) { + this.group = group; + this.resource = resource; + } + + public String getGroup() { + return group; + } + + public String getResource() { + return resource; + } + } } diff --git a/src/test/java/io/cryostat/net/OpenShiftAuthManagerTest.java b/src/test/java/io/cryostat/net/OpenShiftAuthManagerTest.java index acf3c89834..f37ffbf878 100644 --- a/src/test/java/io/cryostat/net/OpenShiftAuthManagerTest.java +++ b/src/test/java/io/cryostat/net/OpenShiftAuthManagerTest.java @@ -258,14 +258,19 @@ void shouldValidateExpectedPermissionsPerSecuredResource(ResourceAction resource throw new IllegalArgumentException(resourceAction.getVerb().toString()); } + Set expectedGroups; Set expectedResources; if (resourceAction.getResource() == ResourceType.TARGET) { + expectedGroups = Set.of("operator.cryostat.io"); expectedResources = Set.of("flightrecorders"); } else if (resourceAction.getResource() == ResourceType.RECORDING) { + expectedGroups = Set.of("operator.cryostat.io"); expectedResources = Set.of("recordings"); } else if (resourceAction.getResource() == ResourceType.CERTIFICATE) { + expectedGroups = Set.of("apps", "", "operator.cryostat.io"); expectedResources = Set.of("deployments", "pods", "cryostats"); } else if (resourceAction.getResource() == ResourceType.CREDENTIALS) { + expectedGroups = Set.of("operator.cryostat.io"); expectedResources = Set.of("cryostats"); } else { throw new IllegalArgumentException(resourceAction.getResource().toString()); @@ -312,7 +317,9 @@ void shouldValidateExpectedPermissionsPerSecuredResource(ResourceAction resource MatcherAssert.assertThat( body.getSpec().getResourceAttributes().getVerb(), Matchers.equalTo(expectedVerb)); + Set actualGroups = new HashSet<>(); Set actualResources = new HashSet<>(); + actualGroups.add(body.getSpec().getResourceAttributes().getGroup()); actualResources.add(body.getSpec().getResourceAttributes().getResource()); // start at 1 because we've already checked the first request above for (int i = 1; i < expectedResources.size(); i++) { @@ -330,9 +337,11 @@ void shouldValidateExpectedPermissionsPerSecuredResource(ResourceAction resource MatcherAssert.assertThat( body.getSpec().getResourceAttributes().getVerb(), Matchers.equalTo(expectedVerb)); + actualGroups.add(body.getSpec().getResourceAttributes().getGroup()); actualResources.add(body.getSpec().getResourceAttributes().getResource()); } + MatcherAssert.assertThat(actualGroups, Matchers.equalTo(expectedGroups)); MatcherAssert.assertThat(actualResources, Matchers.equalTo(expectedResources)); }