Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(permissions): use proper group for pods/deployments #689

Merged
merged 1 commit into from
Sep 27, 2021
Merged

fix(permissions): use proper group for pods/deployments #689

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 40 additions & 12 deletions src/main/java/io/cryostat/net/OpenShiftAuthManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@

public class OpenShiftAuthManager extends AbstractAuthManager {

private static final Set<String> PERMISSION_NOT_REQUIRED = Set.of("PERMISSION_NOT_REQUIRED");
private static final Set<GroupResource> PERMISSION_NOT_REQUIRED =
Set.of(GroupResource.PERMISSION_NOT_REQUIRED);

private final FileSystem fs;
private final Function<String, OpenShiftClient> clientProvider;
Expand Down Expand Up @@ -146,11 +147,10 @@ Future<Boolean> reviewToken(String token) {

private Stream<CompletableFuture<Void>> validateAction(
OpenShiftClient client, String namespace, ResourceAction resourceAction) {
Set<String> resources = map(resourceAction.getResource());
Set<GroupResource> 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()
Expand All @@ -164,8 +164,8 @@ private Stream<CompletableFuture<Void>> validateAction(
.withNewSpec()
.withNewResourceAttributes()
.withNamespace(namespace)
.withGroup(group)
.withResource(resource)
.withGroup(resource.getGroup())
.withResource(resource.getResource())
.withVerb(verb)
.endResourceAttributes()
.endSpec()
Expand All @@ -180,8 +180,8 @@ private Stream<CompletableFuture<Void>> validateAction(
return CompletableFuture.failedFuture(
new PermissionDeniedException(
namespace,
group,
resource,
resource.getGroup(),
resource.getResource(),
verb,
accessReview.getStatus().getReason()));
} else {
Expand Down Expand Up @@ -260,16 +260,17 @@ private String getServiceAccountToken() throws IOException {
.get();
}

private static Set<String> map(ResourceType resource) {
private static Set<GroupResource> 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:
Expand Down Expand Up @@ -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;
}
}
}
9 changes: 9 additions & 0 deletions src/test/java/io/cryostat/net/OpenShiftAuthManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,14 +258,19 @@ void shouldValidateExpectedPermissionsPerSecuredResource(ResourceAction resource
throw new IllegalArgumentException(resourceAction.getVerb().toString());
}

Set<String> expectedGroups;
Set<String> 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());
Expand Down Expand Up @@ -312,7 +317,9 @@ void shouldValidateExpectedPermissionsPerSecuredResource(ResourceAction resource
MatcherAssert.assertThat(
body.getSpec().getResourceAttributes().getVerb(), Matchers.equalTo(expectedVerb));

Set<String> actualGroups = new HashSet<>();
Set<String> 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++) {
Expand All @@ -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));
}

Expand Down