From c730d4540583d86e456db1f733bb19c2c438ef6d Mon Sep 17 00:00:00 2001 From: Johannes Passing Date: Thu, 14 Dec 2023 14:52:09 +1100 Subject: [PATCH 1/6] Convert DeviceInfo to record --- .../jitaccess/core/adapters/LogAdapter.java | 4 +-- .../jitaccess/core/data/DeviceInfo.java | 26 ++----------------- .../jitaccess/web/TestIapAssertion.java | 8 +++--- 3 files changed, 8 insertions(+), 30 deletions(-) diff --git a/sources/src/main/java/com/google/solutions/jitaccess/core/adapters/LogAdapter.java b/sources/src/main/java/com/google/solutions/jitaccess/core/adapters/LogAdapter.java index 2ae0e2e87..11c2cc126 100644 --- a/sources/src/main/java/com/google/solutions/jitaccess/core/adapters/LogAdapter.java +++ b/sources/src/main/java/com/google/solutions/jitaccess/core/adapters/LogAdapter.java @@ -123,9 +123,9 @@ private LogEntry( if (principal != null) { this.labels.put("user", principal.getId().email); this.labels.put("user_id", principal.getId().id); - this.labels.put("device_id", principal.getDevice().getDeviceId()); + this.labels.put("device_id", principal.getDevice().deviceId()); this.labels.put("device_access_levels", - String.join(", ", principal.getDevice().getAccessLevels())); + String.join(", ", principal.getDevice().accessLevels())); } } diff --git a/sources/src/main/java/com/google/solutions/jitaccess/core/data/DeviceInfo.java b/sources/src/main/java/com/google/solutions/jitaccess/core/data/DeviceInfo.java index c8931eeeb..e79751204 100644 --- a/sources/src/main/java/com/google/solutions/jitaccess/core/data/DeviceInfo.java +++ b/sources/src/main/java/com/google/solutions/jitaccess/core/data/DeviceInfo.java @@ -24,33 +24,16 @@ import com.google.common.base.Preconditions; import java.util.List; -import java.util.Objects; /** * Information about the device of a user. */ -public class DeviceInfo { +public record DeviceInfo(String deviceId, List accessLevels) { public static final DeviceInfo UNKNOWN = new DeviceInfo("unknown", List.of()); - private final String deviceId; - private final List accessLevels; - public DeviceInfo( - String deviceId, - List accessLevels - ) { + public DeviceInfo { Preconditions.checkNotNull(deviceId, "deviceId"); Preconditions.checkNotNull(accessLevels, "accessLevels"); - - this.deviceId = deviceId; - this.accessLevels = accessLevels; - } - - public String getDeviceId() { - return this.deviceId; - } - - public List getAccessLevels() { - return accessLevels; } @Override @@ -69,9 +52,4 @@ public boolean equals(Object o) { var that = (DeviceInfo) o; return deviceId.equals(that.deviceId) && accessLevels.equals(that.accessLevels); } - - @Override - public int hashCode() { - return Objects.hash(deviceId, accessLevels); - } } diff --git a/sources/src/test/java/com/google/solutions/jitaccess/web/TestIapAssertion.java b/sources/src/test/java/com/google/solutions/jitaccess/web/TestIapAssertion.java index 9e333a35a..82c5960db 100644 --- a/sources/src/test/java/com/google/solutions/jitaccess/web/TestIapAssertion.java +++ b/sources/src/test/java/com/google/solutions/jitaccess/web/TestIapAssertion.java @@ -69,8 +69,8 @@ public void whenGoogleClaimSet_ThenGetDeviceInfoReturnsDevice() { var assertion = new IapAssertion(new JsonWebToken.Payload() .set("google", Map.of("device_id", "device-1"))); - assertEquals("device-1", assertion.getDeviceInfo().getDeviceId()); - assertEquals(List.of(), assertion.getDeviceInfo().getAccessLevels()); + assertEquals("device-1", assertion.getDeviceInfo().deviceId()); + assertEquals(List.of(), assertion.getDeviceInfo().accessLevels()); } @Test @@ -80,7 +80,7 @@ public void whenGoogleClaimContainsAccessLevelsSet_ThenGetDeviceInfoReturnsDevic "device_id", "device-1", "access_levels", List.of("level-1", "level-2")))); - assertEquals("device-1", assertion.getDeviceInfo().getDeviceId()); - assertEquals(List.of("level-1", "level-2"), assertion.getDeviceInfo().getAccessLevels()); + assertEquals("device-1", assertion.getDeviceInfo().deviceId()); + assertEquals(List.of("level-1", "level-2"), assertion.getDeviceInfo().accessLevels()); } } From 5353a801500abaa32daf05376b0f18ccfb83e50a Mon Sep 17 00:00:00 2001 From: Johannes Passing Date: Thu, 14 Dec 2023 14:54:12 +1100 Subject: [PATCH 2/6] Convert ProjectId to record --- .../core/adapters/ResourceManagerAdapter.java | 2 +- .../solutions/jitaccess/core/data/ProjectId.java | 14 ++------------ .../solutions/jitaccess/web/ApiResource.java | 12 ++++++------ .../core/adapters/IntegrationTestEnvironment.java | 4 ++-- .../jitaccess/core/adapters/TestPubSubAdapter.java | 4 ++-- 5 files changed, 13 insertions(+), 23 deletions(-) diff --git a/sources/src/main/java/com/google/solutions/jitaccess/core/adapters/ResourceManagerAdapter.java b/sources/src/main/java/com/google/solutions/jitaccess/core/adapters/ResourceManagerAdapter.java index a0a7a2c8e..4d73ce9a6 100644 --- a/sources/src/main/java/com/google/solutions/jitaccess/core/adapters/ResourceManagerAdapter.java +++ b/sources/src/main/java/com/google/solutions/jitaccess/core/adapters/ResourceManagerAdapter.java @@ -112,7 +112,7 @@ public void addProjectIamBinding( var policy = service .projects() .getIamPolicy( - String.format("projects/%s", projectId.id), + String.format("projects/%s", projectId.id()), new GetIamPolicyRequest() .setOptions(new GetPolicyOptions().setRequestedPolicyVersion(3))) .execute(); diff --git a/sources/src/main/java/com/google/solutions/jitaccess/core/data/ProjectId.java b/sources/src/main/java/com/google/solutions/jitaccess/core/data/ProjectId.java index c24a3fcd3..7c5034c05 100644 --- a/sources/src/main/java/com/google/solutions/jitaccess/core/data/ProjectId.java +++ b/sources/src/main/java/com/google/solutions/jitaccess/core/data/ProjectId.java @@ -23,21 +23,16 @@ import com.google.common.base.Preconditions; -import java.util.Objects; - /** * Project ID for a Google Cloud project. */ -public class ProjectId implements Comparable { +public record ProjectId(String id) implements Comparable { private static final String PROJECT_RESOURCE_NAME_PREFIX = "//cloudresourcemanager.googleapis.com/projects/"; - public final String id; - - public ProjectId(String id) { + public ProjectId { Preconditions.checkNotNull(id, "id"); assert !id.startsWith("//"); - this.id = id; } @Override @@ -90,11 +85,6 @@ public boolean equals(Object o) { return this.id.equals(projectId.id); } - @Override - public int hashCode() { - return Objects.hash(this.id); - } - @Override public int compareTo(ProjectId o) { return this.id.compareTo(o.id); diff --git a/sources/src/main/java/com/google/solutions/jitaccess/web/ApiResource.java b/sources/src/main/java/com/google/solutions/jitaccess/web/ApiResource.java index 11e5ff651..3200cfc77 100644 --- a/sources/src/main/java/com/google/solutions/jitaccess/web/ApiResource.java +++ b/sources/src/main/java/com/google/solutions/jitaccess/web/ApiResource.java @@ -168,7 +168,7 @@ public ProjectsResponse listProjects( var projects = this.roleDiscoveryService.listAvailableProjects(iapPrincipal.getId()); return new ProjectsResponse(projects - .stream().map(p -> p.id) + .stream().map(p -> p.id()) .collect(Collectors.toSet())); } catch (Exception e) { @@ -694,7 +694,7 @@ private static LogAdapter.LogEntry addLabels( return entry .addLabel("role", roleBinding.role) .addLabel("resource", roleBinding.fullResourceName) - .addLabel("project_id", ProjectId.fromFullResourceName(roleBinding.fullResourceName).id); + .addLabel("project_id", ProjectId.fromFullResourceName(roleBinding.fullResourceName).id()); } private static LogAdapter.LogEntry addLabels( @@ -708,7 +708,7 @@ private static LogAdapter.LogEntry addLabels( LogAdapter.LogEntry entry, ProjectId project ) { - return entry.addLabel("project", project.id); + return entry.addLabel("project", project.id()); } // ------------------------------------------------------------------------- @@ -856,7 +856,7 @@ private ActivationStatus( assert startTime < endTime; this.activationId = activationId.toString(); - this.projectId = ProjectId.fromFullResourceName(roleBinding.fullResourceName).id; + this.projectId = ProjectId.fromFullResourceName(roleBinding.fullResourceName).id(); this.roleBinding = roleBinding; this.status = status; this.startTime = startTime; @@ -895,7 +895,7 @@ protected RequestActivationNotification( String.format( "%s requests access to project %s", request.beneficiary, - ProjectId.fromFullResourceName(request.roleBinding.fullResourceName).id)); + ProjectId.fromFullResourceName(request.roleBinding.fullResourceName).id())); this.properties.put("BENEFICIARY", request.beneficiary); this.properties.put("REVIEWERS", request.reviewers); @@ -930,7 +930,7 @@ protected ActivationApprovedNotification( String.format( "%s requests access to project %s", request.beneficiary, - ProjectId.fromFullResourceName(request.roleBinding.fullResourceName).id)); + ProjectId.fromFullResourceName(request.roleBinding.fullResourceName).id())); this.properties.put("APPROVER", approver.email); this.properties.put("BENEFICIARY", request.beneficiary); diff --git a/sources/src/test/java/com/google/solutions/jitaccess/core/adapters/IntegrationTestEnvironment.java b/sources/src/test/java/com/google/solutions/jitaccess/core/adapters/IntegrationTestEnvironment.java index 1f182d881..bb87a6451 100644 --- a/sources/src/test/java/com/google/solutions/jitaccess/core/adapters/IntegrationTestEnvironment.java +++ b/sources/src/test/java/com/google/solutions/jitaccess/core/adapters/IntegrationTestEnvironment.java @@ -91,14 +91,14 @@ public void refresh() { APPLICATION_CREDENTIALS = GoogleCredentials .getApplicationDefault() - .createWithQuotaProject(PROJECT_ID.id); + .createWithQuotaProject(PROJECT_ID.id()); NO_ACCESS_CREDENTIALS = impersonate(APPLICATION_CREDENTIALS, NO_ACCESS_USER.email); TEMPORARY_ACCESS_CREDENTIALS = impersonate(APPLICATION_CREDENTIALS, TEMPORARY_ACCESS_USER.email); var topicName = getOptional(settings, "test.topic", ""); if (!Strings.isNullOrEmpty(topicName)) { - PUBSUB_TOPIC = new Topic(PROJECT_ID.id, topicName); + PUBSUB_TOPIC = new Topic(PROJECT_ID.id(), topicName); } else { PUBSUB_TOPIC = null; diff --git a/sources/src/test/java/com/google/solutions/jitaccess/core/adapters/TestPubSubAdapter.java b/sources/src/test/java/com/google/solutions/jitaccess/core/adapters/TestPubSubAdapter.java index efba2b18d..e826d359b 100644 --- a/sources/src/test/java/com/google/solutions/jitaccess/core/adapters/TestPubSubAdapter.java +++ b/sources/src/test/java/com/google/solutions/jitaccess/core/adapters/TestPubSubAdapter.java @@ -43,7 +43,7 @@ public void whenUnauthenticated_ThenPublishThrowsException() { assertThrows( NotAuthenticatedException.class, () -> adapter.publish( - new Topic(IntegrationTestEnvironment.PROJECT_ID.id, "topic-1"), + new Topic(IntegrationTestEnvironment.PROJECT_ID.id(), "topic-1"), new PubsubMessage())); } @@ -55,7 +55,7 @@ public void whenCallerLacksPermission_ThenAddProjectIamBindingThrowsException() assertThrows( AccessDeniedException.class, () -> adapter.publish( - new Topic(IntegrationTestEnvironment.PROJECT_ID.id, "topic-1"), + new Topic(IntegrationTestEnvironment.PROJECT_ID.id(), "topic-1"), new PubsubMessage())); } From 013a4adaeee25371674984e4c54f80c4c4d61d5c Mon Sep 17 00:00:00 2001 From: Johannes Passing Date: Thu, 14 Dec 2023 14:57:59 +1100 Subject: [PATCH 3/6] Convert ProjectRole to record --- .../jitaccess/core/data/ProjectRole.java | 34 ++++++++--------- .../core/services/RoleActivationService.java | 6 +-- .../core/services/RoleDiscoveryService.java | 8 ++-- .../solutions/jitaccess/web/ApiResource.java | 10 ++--- .../services/TestRoleActivationService.java | 8 ++-- .../services/TestRoleDiscoveryService.java | 38 +++++++++---------- 6 files changed, 52 insertions(+), 52 deletions(-) diff --git a/sources/src/main/java/com/google/solutions/jitaccess/core/data/ProjectRole.java b/sources/src/main/java/com/google/solutions/jitaccess/core/data/ProjectRole.java index 999eff8c8..488c7ed95 100644 --- a/sources/src/main/java/com/google/solutions/jitaccess/core/data/ProjectRole.java +++ b/sources/src/main/java/com/google/solutions/jitaccess/core/data/ProjectRole.java @@ -23,22 +23,19 @@ import com.google.common.base.Preconditions; -import java.util.Objects; - /** * Represents an eligible role on a project. */ -public class ProjectRole implements Comparable { - public final RoleBinding roleBinding; - public final Status status; +public record ProjectRole( + RoleBinding roleBinding, + ProjectRole.Status status +) implements Comparable { - public ProjectRole(RoleBinding roleBinding, Status status) { + public ProjectRole { Preconditions.checkNotNull(roleBinding); Preconditions.checkNotNull(status); Preconditions.checkArgument(ProjectId.isProjectFullResourceName(roleBinding.fullResourceName)); - this.roleBinding = roleBinding; - this.status = status; } @Override @@ -71,11 +68,6 @@ public boolean equals(Object o) { return this.roleBinding.equals(that.roleBinding) && this.status.equals(that.status); } - @Override - public int hashCode() { - return Objects.hash(this.roleBinding, this.status); - } - @Override public int compareTo(ProjectRole o) { return this.roleBinding.compareTo(o.roleBinding); @@ -86,16 +78,24 @@ public int compareTo(ProjectRole o) { // ------------------------------------------------------------------------- public enum Status { - /** Role binding can be activated using self-approval ("JIT approval") */ + /** + * Role binding can be activated using self-approval ("JIT approval") + */ ELIGIBLE_FOR_JIT, - /** Role binding can be activated using multi party-approval ("MPA approval") */ + /** + * Role binding can be activated using multi party-approval ("MPA approval") + */ ELIGIBLE_FOR_MPA, - /** Eligible role binding has been activated */ + /** + * Eligible role binding has been activated + */ ACTIVATED, - /** Approval pending */ + /** + * Approval pending + */ ACTIVATION_PENDING } } diff --git a/sources/src/main/java/com/google/solutions/jitaccess/core/services/RoleActivationService.java b/sources/src/main/java/com/google/solutions/jitaccess/core/services/RoleActivationService.java index c6ed0af55..07abcc523 100644 --- a/sources/src/main/java/com/google/solutions/jitaccess/core/services/RoleActivationService.java +++ b/sources/src/main/java/com/google/solutions/jitaccess/core/services/RoleActivationService.java @@ -74,8 +74,8 @@ private static boolean canActivateProjectRole( ActivationType activationType ) { switch (activationType) { - case JIT: return projectRole.status == ProjectRole.Status.ELIGIBLE_FOR_JIT; - case MPA: return projectRole.status == ProjectRole.Status.ELIGIBLE_FOR_MPA; + case JIT: return projectRole.status() == ProjectRole.Status.ELIGIBLE_FOR_JIT; + case MPA: return projectRole.status() == ProjectRole.Status.ELIGIBLE_FOR_MPA; default: return false; } } @@ -100,7 +100,7 @@ private void checkUserCanActivateProjectRole( ProjectRole.Status.ELIGIBLE_FOR_MPA)) .getItems() .stream() - .filter(pr -> pr.roleBinding.equals(roleBinding)) + .filter(pr -> pr.roleBinding().equals(roleBinding)) .filter(pr -> canActivateProjectRole(pr, activationType)) .findAny() .isEmpty()) { diff --git a/sources/src/main/java/com/google/solutions/jitaccess/core/services/RoleDiscoveryService.java b/sources/src/main/java/com/google/solutions/jitaccess/core/services/RoleDiscoveryService.java index cc0141e14..df7563960 100644 --- a/sources/src/main/java/com/google/solutions/jitaccess/core/services/RoleDiscoveryService.java +++ b/sources/src/main/java/com/google/solutions/jitaccess/core/services/RoleDiscoveryService.java @@ -275,7 +275,7 @@ public Result listEligibleProjectRoles( allEligibleRoles.addAll(jitEligibleRoles); allEligibleRoles.addAll(mpaEligibleRoles .stream() - .filter(r -> !jitEligibleRoles.stream().anyMatch(a -> a.roleBinding.equals(r.roleBinding))) + .filter(r -> !jitEligibleRoles.stream().anyMatch(a -> a.roleBinding().equals(r.roleBinding()))) .collect(Collectors.toList())); // @@ -286,7 +286,7 @@ public Result listEligibleProjectRoles( // var consolidatedRoles = allEligibleRoles .stream() - .filter(r -> !activatedRoles.stream().anyMatch(a -> a.roleBinding.equals(r.roleBinding))) + .filter(r -> !activatedRoles.stream().anyMatch(a -> a.roleBinding().equals(r.roleBinding()))) .collect(Collectors.toList()); consolidatedRoles.addAll(activatedRoles); @@ -320,8 +320,8 @@ public Set listEligibleUsersForProjectRole( if (eligibleRoles .getItems() .stream() - .filter(pr -> pr.roleBinding.equals(roleBinding)) - .filter(pr -> pr.status == ProjectRole.Status.ELIGIBLE_FOR_MPA) + .filter(pr -> pr.roleBinding().equals(roleBinding)) + .filter(pr -> pr.status() == ProjectRole.Status.ELIGIBLE_FOR_MPA) .findAny() .isEmpty()) { throw new AccessDeniedException( diff --git a/sources/src/main/java/com/google/solutions/jitaccess/web/ApiResource.java b/sources/src/main/java/com/google/solutions/jitaccess/web/ApiResource.java index 3200cfc77..39e08b99e 100644 --- a/sources/src/main/java/com/google/solutions/jitaccess/web/ApiResource.java +++ b/sources/src/main/java/com/google/solutions/jitaccess/web/ApiResource.java @@ -667,7 +667,7 @@ private static LogAdapter.LogEntry addLabels( .addLabel("activation_id", activation.id.toString()) .addLabel("activation_start", activation.startTime.atOffset(ZoneOffset.UTC).toString()) .addLabel("activation_end", activation.endTime.atOffset(ZoneOffset.UTC).toString()) - .addLabels(e -> addLabels(e, activation.projectRole.roleBinding)); + .addLabels(e -> addLabels(e, activation.projectRole.roleBinding())); } private static LogAdapter.LogEntry addLabels( @@ -866,8 +866,8 @@ private ActivationStatus( private ActivationStatus(RoleActivationService.Activation activation) { this( activation.id, - activation.projectRole.roleBinding, - activation.projectRole.status, + activation.projectRole.roleBinding(), + activation.projectRole.status(), activation.startTime.getEpochSecond(), activation.endTime.getEpochSecond()); } @@ -968,12 +968,12 @@ protected ActivationSelfApprovedNotification( List.of(), String.format( "Activated role '%s' on '%s'", - activation.projectRole.roleBinding, + activation.projectRole.roleBinding(), activation.projectRole.getProjectId())); this.properties.put("BENEFICIARY", beneficiary); this.properties.put("PROJECT_ID", activation.projectRole.getProjectId()); - this.properties.put("ROLE", activation.projectRole.roleBinding.role); + this.properties.put("ROLE", activation.projectRole.roleBinding().role); this.properties.put("START_TIME", activation.startTime); this.properties.put("END_TIME", activation.endTime); this.properties.put("JUSTIFICATION", justification); diff --git a/sources/src/test/java/com/google/solutions/jitaccess/core/services/TestRoleActivationService.java b/sources/src/test/java/com/google/solutions/jitaccess/core/services/TestRoleActivationService.java index a92883e05..8adffea18 100644 --- a/sources/src/test/java/com/google/solutions/jitaccess/core/services/TestRoleActivationService.java +++ b/sources/src/test/java/com/google/solutions/jitaccess/core/services/TestRoleActivationService.java @@ -198,8 +198,8 @@ public void whenCallerIsJitEligible_ThenActivateProjectRoleForSelfAddsBinding() activationTimeout); assertTrue(activation.id.toString().startsWith("jit-")); - assertEquals(activation.projectRole.roleBinding, roleBinding); - assertEquals(ProjectRole.Status.ACTIVATED, activation.projectRole.status); + assertEquals(activation.projectRole.roleBinding(), roleBinding); + assertEquals(ProjectRole.Status.ACTIVATED, activation.projectRole.status()); assertTrue(activation.endTime.isAfter(activation.startTime)); assertTrue(activation.endTime.isAfter(Instant.now().minusSeconds(60))); assertTrue(activation.endTime.isBefore(Instant.now().plus(activationTimeout).plusSeconds(60))); @@ -458,8 +458,8 @@ public void whenPeerAndCallerEligible_ThenActivateProjectRoleAddsBinding() throw assertNotNull(activation); assertEquals(request.id, activation.id); - assertEquals(ProjectRole.Status.ACTIVATED, activation.projectRole.status); - assertEquals(roleBinding, activation.projectRole.roleBinding); + assertEquals(ProjectRole.Status.ACTIVATED, activation.projectRole.status()); + assertEquals(roleBinding, activation.projectRole.roleBinding()); assertEquals(request.startTime, activation.startTime); assertEquals(request.endTime, activation.endTime); diff --git a/sources/src/test/java/com/google/solutions/jitaccess/core/services/TestRoleDiscoveryService.java b/sources/src/test/java/com/google/solutions/jitaccess/core/services/TestRoleDiscoveryService.java index 5c803017e..eb4a40452 100644 --- a/sources/src/test/java/com/google/solutions/jitaccess/core/services/TestRoleDiscoveryService.java +++ b/sources/src/test/java/com/google/solutions/jitaccess/core/services/TestRoleDiscoveryService.java @@ -361,8 +361,8 @@ public void whenAnalysisContainsJitEligibleBinding_ThenListEligibleProjectRolesR var role = roles.getItems().stream().findFirst().get(); assertEquals(SAMPLE_PROJECT_ID_1, role.getProjectId()); - assertEquals(SAMPLE_ROLE, role.roleBinding.role); - assertEquals(ProjectRole.Status.ELIGIBLE_FOR_JIT, role.status); + assertEquals(SAMPLE_ROLE, role.roleBinding().role); + assertEquals(ProjectRole.Status.ELIGIBLE_FOR_JIT, role.status()); } @Test @@ -409,8 +409,8 @@ public void whenAnalysisContainsDuplicateJitEligibleBinding_ThenListEligibleProj var role = roles.getItems().stream().findFirst().get(); assertEquals(SAMPLE_PROJECT_ID_1, role.getProjectId()); - assertEquals(SAMPLE_ROLE, role.roleBinding.role); - assertEquals(ProjectRole.Status.ELIGIBLE_FOR_JIT, role.status); + assertEquals(SAMPLE_ROLE, role.roleBinding().role); + assertEquals(ProjectRole.Status.ELIGIBLE_FOR_JIT, role.status()); } @Test @@ -450,8 +450,8 @@ public void whenAnalysisContainsMpaEligibleBinding_ThenListEligibleProjectRolesR var role = roles.getItems().stream().findFirst().get(); assertEquals(SAMPLE_PROJECT_ID_1, role.getProjectId()); - assertEquals(SAMPLE_ROLE, role.roleBinding.role); - assertEquals(ProjectRole.Status.ELIGIBLE_FOR_MPA, role.status); + assertEquals(SAMPLE_ROLE, role.roleBinding().role); + assertEquals(ProjectRole.Status.ELIGIBLE_FOR_MPA, role.status()); } @Test @@ -498,8 +498,8 @@ public void whenAnalysisContainsDuplicateMpaEligibleBinding_ThenListEligibleProj var role = roles.getItems().stream().findFirst().get(); assertEquals(SAMPLE_PROJECT_ID_1, role.getProjectId()); - assertEquals(SAMPLE_ROLE, role.roleBinding.role); - assertEquals(ProjectRole.Status.ELIGIBLE_FOR_MPA, role.status); + assertEquals(SAMPLE_ROLE, role.roleBinding().role); + assertEquals(ProjectRole.Status.ELIGIBLE_FOR_MPA, role.status()); } @Test @@ -548,13 +548,13 @@ public void whenAnalysisContainsMpaEligibleBindingAndJitEligibleBindingForDiffer var role = roles.getItems().stream().findFirst().get(); assertEquals(SAMPLE_PROJECT_ID_1, role.getProjectId()); - assertEquals(SAMPLE_ROLE, role.roleBinding.role); - assertEquals(ProjectRole.Status.ELIGIBLE_FOR_JIT, role.status); + assertEquals(SAMPLE_ROLE, role.roleBinding().role); + assertEquals(ProjectRole.Status.ELIGIBLE_FOR_JIT, role.status()); role = roles.getItems().stream().skip(1).findFirst().get(); assertEquals(SAMPLE_PROJECT_ID_1, role.getProjectId()); - assertEquals(SAMPLE_ROLE_2, role.roleBinding.role); - assertEquals(ProjectRole.Status.ELIGIBLE_FOR_MPA, role.status); + assertEquals(SAMPLE_ROLE_2, role.roleBinding().role); + assertEquals(ProjectRole.Status.ELIGIBLE_FOR_MPA, role.status()); } @Test @@ -604,8 +604,8 @@ public void whenAnalysisContainsMpaEligibleBindingAndJitEligibleBindingForSameRo // Only the JIT-eligible binding is retained. var role = roles.getItems().stream().findFirst().get(); assertEquals(SAMPLE_PROJECT_ID_1, role.getProjectId()); - assertEquals(SAMPLE_ROLE, role.roleBinding.role); - assertEquals(ProjectRole.Status.ELIGIBLE_FOR_JIT, role.status); + assertEquals(SAMPLE_ROLE, role.roleBinding().role); + assertEquals(ProjectRole.Status.ELIGIBLE_FOR_JIT, role.status()); } @Test @@ -666,8 +666,8 @@ public void whenAnalysisContainsActivatedBinding_ThenListEligibleProjectRolesRet var role = roles.getItems().stream().findFirst().get(); assertEquals(SAMPLE_PROJECT_ID_1, role.getProjectId()); - assertEquals(SAMPLE_ROLE, role.roleBinding.role); - assertEquals(ProjectRole.Status.ACTIVATED, role.status); + assertEquals(SAMPLE_ROLE, role.roleBinding().role); + assertEquals(ProjectRole.Status.ACTIVATED, role.status()); } @Test @@ -838,7 +838,7 @@ public void whenStatusSetToJitOnly_ThenListEligibleProjectRolesOnlyReturnsJitEli var role = roles.getItems().stream().findFirst().get(); assertEquals(SAMPLE_PROJECT_ID_1, role.getProjectId()); - assertEquals(ProjectRole.Status.ELIGIBLE_FOR_JIT, role.status); + assertEquals(ProjectRole.Status.ELIGIBLE_FOR_JIT, role.status()); } @Test @@ -901,7 +901,7 @@ public void whenStatusSetToMpaOnly_ThenListEligibleProjectRolesOnlyReturnsMpaEli var role = roles.getItems().stream().findFirst().get(); assertEquals(SAMPLE_PROJECT_ID_1, role.getProjectId()); - assertEquals(ProjectRole.Status.ELIGIBLE_FOR_MPA, role.status); + assertEquals(ProjectRole.Status.ELIGIBLE_FOR_MPA, role.status()); } @Test @@ -964,7 +964,7 @@ public void whenStatusSetToActivatedOnly_ThenListEligibleProjectRolesOnlyReturns var role = roles.getItems().stream().findFirst().get(); assertEquals(SAMPLE_PROJECT_ID_1, role.getProjectId()); - assertEquals(ProjectRole.Status.ACTIVATED, role.status); + assertEquals(ProjectRole.Status.ACTIVATED, role.status()); } // --------------------------------------------------------------------- From e2b50d1db74b0467a559bc942a37c90bb1deba0f Mon Sep 17 00:00:00 2001 From: Johannes Passing Date: Thu, 14 Dec 2023 15:04:36 +1100 Subject: [PATCH 4/6] Cleanup --- .../jitaccess/core/data/ProjectId.java | 16 +------- .../jitaccess/core/data/RoleBinding.java | 40 +++---------------- 2 files changed, 6 insertions(+), 50 deletions(-) diff --git a/sources/src/main/java/com/google/solutions/jitaccess/core/data/ProjectId.java b/sources/src/main/java/com/google/solutions/jitaccess/core/data/ProjectId.java index 7c5034c05..506afbbfc 100644 --- a/sources/src/main/java/com/google/solutions/jitaccess/core/data/ProjectId.java +++ b/sources/src/main/java/com/google/solutions/jitaccess/core/data/ProjectId.java @@ -68,23 +68,9 @@ public static boolean isProjectFullResourceName(String fullResourceName) { } // ------------------------------------------------------------------------- - // Equality. + // Comparable. // ------------------------------------------------------------------------- - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - - if (o == null || getClass() != o.getClass()) { - return false; - } - - ProjectId projectId = (ProjectId) o; - return this.id.equals(projectId.id); - } - @Override public int compareTo(ProjectId o) { return this.id.compareTo(o.id); diff --git a/sources/src/main/java/com/google/solutions/jitaccess/core/data/RoleBinding.java b/sources/src/main/java/com/google/solutions/jitaccess/core/data/RoleBinding.java index 81589a9fd..bf8f35875 100644 --- a/sources/src/main/java/com/google/solutions/jitaccess/core/data/RoleBinding.java +++ b/sources/src/main/java/com/google/solutions/jitaccess/core/data/RoleBinding.java @@ -21,26 +21,15 @@ package com.google.solutions.jitaccess.core.data; -import com.google.common.base.Preconditions; - import java.util.Comparator; -import java.util.Objects; /** * Represents a role that has been granted on a resource. */ -public class RoleBinding implements Comparable { - public final String fullResourceName; - public final String role; - - public RoleBinding(String fullResourceName, String role) { - Preconditions.checkNotNull(fullResourceName); - Preconditions.checkNotNull(role); - - this.fullResourceName = fullResourceName; - this.role = role; - } - +public record RoleBinding ( + String fullResourceName, + String role +) implements Comparable { public RoleBinding(ProjectId project, String role) { this(project.getFullResourceName(), role); } @@ -51,28 +40,9 @@ public String toString() { } // ------------------------------------------------------------------------- - // Equality. + // Comparable. // ------------------------------------------------------------------------- - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - - if (o == null || getClass() != o.getClass()) { - return false; - } - - var that = (RoleBinding) o; - return this.fullResourceName.equals(that.fullResourceName) && this.role.equals(that.role); - } - - @Override - public int hashCode() { - return Objects.hash(this.fullResourceName, this.role); - } - @Override public int compareTo(RoleBinding o) { return Comparator.comparing((RoleBinding r) -> r.fullResourceName) From 40b07d9681201bdeae7f327528346667246e3fe0 Mon Sep 17 00:00:00 2001 From: Johannes Passing Date: Thu, 14 Dec 2023 15:11:45 +1100 Subject: [PATCH 5/6] Cleanup --- .../jitaccess/core/data/ProjectRole.java | 4 +- .../core/services/RoleActivationService.java | 20 ++++---- .../core/services/RoleDiscoveryService.java | 10 ++-- .../solutions/jitaccess/web/ApiResource.java | 46 +++++++++---------- .../services/TestRoleDiscoveryService.java | 16 +++---- .../jitaccess/web/TestApiResource.java | 8 ++-- 6 files changed, 52 insertions(+), 52 deletions(-) diff --git a/sources/src/main/java/com/google/solutions/jitaccess/core/data/ProjectRole.java b/sources/src/main/java/com/google/solutions/jitaccess/core/data/ProjectRole.java index 488c7ed95..6b322c27d 100644 --- a/sources/src/main/java/com/google/solutions/jitaccess/core/data/ProjectRole.java +++ b/sources/src/main/java/com/google/solutions/jitaccess/core/data/ProjectRole.java @@ -34,7 +34,7 @@ public record ProjectRole( public ProjectRole { Preconditions.checkNotNull(roleBinding); Preconditions.checkNotNull(status); - Preconditions.checkArgument(ProjectId.isProjectFullResourceName(roleBinding.fullResourceName)); + Preconditions.checkArgument(ProjectId.isProjectFullResourceName(roleBinding.fullResourceName())); } @@ -47,7 +47,7 @@ public String toString() { * Return the unqualified project ID. */ public ProjectId getProjectId() { - return ProjectId.fromFullResourceName(this.roleBinding.fullResourceName); + return ProjectId.fromFullResourceName(this.roleBinding.fullResourceName()); } // ------------------------------------------------------------------------- diff --git a/sources/src/main/java/com/google/solutions/jitaccess/core/services/RoleActivationService.java b/sources/src/main/java/com/google/solutions/jitaccess/core/services/RoleActivationService.java index 07abcc523..edbff106b 100644 --- a/sources/src/main/java/com/google/solutions/jitaccess/core/services/RoleActivationService.java +++ b/sources/src/main/java/com/google/solutions/jitaccess/core/services/RoleActivationService.java @@ -94,7 +94,7 @@ private void checkUserCanActivateProjectRole( // if (this.roleDiscoveryService.listEligibleProjectRoles( user, - ProjectId.fromFullResourceName(roleBinding.fullResourceName), + ProjectId.fromFullResourceName(roleBinding.fullResourceName()), EnumSet.of( ProjectRole.Status.ELIGIBLE_FOR_JIT, ProjectRole.Status.ELIGIBLE_FOR_MPA)) @@ -108,7 +108,7 @@ private void checkUserCanActivateProjectRole( String.format( "The user %s is not allowed to activate the role %s", user, - roleBinding.role)); + roleBinding.role())); } } @@ -139,7 +139,7 @@ public Activation activateProjectRoleForSelf( Preconditions.checkNotNull(caller, "caller"); Preconditions.checkNotNull(roleBinding, "roleBinding"); Preconditions.checkNotNull(justification, "justification"); - Preconditions.checkArgument(ProjectId.isProjectFullResourceName(roleBinding.fullResourceName)); + Preconditions.checkArgument(ProjectId.isProjectFullResourceName(roleBinding.fullResourceName())); Preconditions.checkArgument(activationTimeout.toMinutes() >= Options.MIN_ACTIVATION_TIMEOUT_MINUTES, "The activation timeout is too short"); Preconditions.checkArgument(activationTimeout.toMinutes() <= this.options.maxActivationTimeout.toMinutes(), @@ -179,14 +179,14 @@ public Activation activateProjectRoleForSelf( var binding = new Binding() .setMembers(List.of("user:" + caller)) - .setRole(roleBinding.role) + .setRole(roleBinding.role()) .setCondition(new com.google.api.services.cloudresourcemanager.v3.model.Expr() .setTitle(JitConstraints.ACTIVATION_CONDITION_TITLE) .setDescription(bindingDescription) .setExpression(IamTemporaryAccessConditions.createExpression(activationTime, expiryTime))); this.resourceManagerAdapter.addProjectIamBinding( - ProjectId.fromFullResourceName(roleBinding.fullResourceName), + ProjectId.fromFullResourceName(roleBinding.fullResourceName()), binding, EnumSet.of(ResourceManagerAdapter.IamBindingOptions.PURGE_EXISTING_TEMPORARY_BINDINGS), justification); @@ -253,7 +253,7 @@ public Activation activateProjectRoleForPeer( var binding = new Binding() .setMembers(List.of("user:" + request.beneficiary.email)) - .setRole(request.roleBinding.role) + .setRole(request.roleBinding.role()) .setCondition(new com.google.api.services.cloudresourcemanager.v3.model.Expr() .setTitle(JitConstraints.ACTIVATION_CONDITION_TITLE) .setDescription(bindingDescription) @@ -262,7 +262,7 @@ public Activation activateProjectRoleForPeer( request.endTime))); this.resourceManagerAdapter.addProjectIamBinding( - ProjectId.fromFullResourceName(request.roleBinding.fullResourceName), + ProjectId.fromFullResourceName(request.roleBinding.fullResourceName()), binding, EnumSet.of( ResourceManagerAdapter.IamBindingOptions.PURGE_EXISTING_TEMPORARY_BINDINGS, @@ -291,7 +291,7 @@ public ActivationRequest createActivationRequestForPeer( Preconditions.checkNotNull(roleBinding, "roleBinding"); Preconditions.checkNotNull(justification, "justification"); - Preconditions.checkArgument(ProjectId.isProjectFullResourceName(roleBinding.fullResourceName)); + Preconditions.checkArgument(ProjectId.isProjectFullResourceName(roleBinding.fullResourceName())); Preconditions.checkArgument(reviewers != null && reviewers.size() >= this.options.minNumberOfReviewersPerActivationRequest, "At least " + this.options.minNumberOfReviewersPerActivationRequest + " reviewers must be specified"); Preconditions.checkArgument(reviewers.size() <= this.options.maxNumberOfReviewersPerActivationRequest, @@ -478,8 +478,8 @@ protected JsonWebToken.Payload toJsonWebTokenPayload() { .setJwtId(this.id.toString()) .set("beneficiary", this.beneficiary.email) .set("reviewers", this.reviewers.stream().map(id -> id.email).collect(Collectors.toList())) - .set("resource", this.roleBinding.fullResourceName) - .set("role", this.roleBinding.role) + .set("resource", this.roleBinding.fullResourceName()) + .set("role", this.roleBinding.role()) .set("justification", this.justification) .set("start", this.startTime.getEpochSecond()) .set("end", this.endTime.getEpochSecond()); diff --git a/sources/src/main/java/com/google/solutions/jitaccess/core/services/RoleDiscoveryService.java b/sources/src/main/java/com/google/solutions/jitaccess/core/services/RoleDiscoveryService.java index df7563960..4a89864ab 100644 --- a/sources/src/main/java/com/google/solutions/jitaccess/core/services/RoleDiscoveryService.java +++ b/sources/src/main/java/com/google/solutions/jitaccess/core/services/RoleDiscoveryService.java @@ -149,7 +149,7 @@ public Set listAvailableProjects( return roleBindings .stream() - .map(b -> ProjectId.fromFullResourceName(b.fullResourceName)) + .map(b -> ProjectId.fromFullResourceName(b.fullResourceName())) .collect(Collectors.toCollection(TreeSet::new)); } else { @@ -308,13 +308,13 @@ public Set listEligibleUsersForProjectRole( Preconditions.checkNotNull(callerUserId, "callerUserId"); Preconditions.checkNotNull(roleBinding, "roleBinding"); - assert ProjectId.isProjectFullResourceName(roleBinding.fullResourceName); + assert ProjectId.isProjectFullResourceName(roleBinding.fullResourceName()); // // Check that the (calling) user is really allowed to request approval // this role. // - var projectId = ProjectId.fromFullResourceName(roleBinding.fullResourceName); + var projectId = ProjectId.fromFullResourceName(roleBinding.fullResourceName()); var eligibleRoles = listEligibleProjectRoles(callerUserId, projectId); if (eligibleRoles @@ -333,8 +333,8 @@ public Set listEligibleUsersForProjectRole( // var analysisResult = this.assetInventoryAdapter.findPermissionedPrincipalsByResource( this.options.scope, - roleBinding.fullResourceName, - roleBinding.role); + roleBinding.fullResourceName(), + roleBinding.role()); return Stream.ofNullable(analysisResult.getAnalysisResults()) .flatMap(Collection::stream) diff --git a/sources/src/main/java/com/google/solutions/jitaccess/web/ApiResource.java b/sources/src/main/java/com/google/solutions/jitaccess/web/ApiResource.java index 39e08b99e..355bcc362 100644 --- a/sources/src/main/java/com/google/solutions/jitaccess/web/ApiResource.java +++ b/sources/src/main/java/com/google/solutions/jitaccess/web/ApiResource.java @@ -341,8 +341,8 @@ public ActivationStatusResponse selfApproveActivation( String.format( "User %s activated role '%s' on '%s' for themselves for %d minutes", iapPrincipal.getId(), - roleBinding.role, - roleBinding.fullResourceName, + roleBinding.role(), + roleBinding.fullResourceName(), requestedRoleBindingDuration)) .addLabels(le -> addLabels(le, activation)) .addLabel("justification", request.justification) @@ -355,8 +355,8 @@ public ActivationStatusResponse selfApproveActivation( String.format( "User %s failed to activate role '%s' on '%s' for themselves for %d minutes: %s", iapPrincipal.getId(), - roleBinding.role, - roleBinding.fullResourceName, + roleBinding.role(), + roleBinding.fullResourceName(), requestedRoleBindingDuration, Exceptions.getFullMessage(e))) .addLabels(le -> addLabels(le, projectId)) @@ -476,8 +476,8 @@ public ActivationStatusResponse requestActivation( String.format( "User %s requested role '%s' on '%s' for %d minutes", iapPrincipal.getId(), - roleBinding.role, - roleBinding.fullResourceName, + roleBinding.role(), + roleBinding.fullResourceName(), requestedRoleBindingDuration )) .addLabels(le -> addLabels(le, projectId)) @@ -496,8 +496,8 @@ public ActivationStatusResponse requestActivation( String.format( "User %s failed to request role '%s' on '%s' for %d minutes: %s", iapPrincipal.getId(), - roleBinding.role, - roleBinding.fullResourceName, + roleBinding.role(), + roleBinding.fullResourceName(), requestedRoleBindingDuration, Exceptions.getFullMessage(e))) .addLabels(le -> addLabels(le, projectId)) @@ -617,8 +617,8 @@ public ActivationStatusResponse approveActivationRequest( String.format( "User %s approved role '%s' on '%s' for %s", iapPrincipal.getId(), - activationRequest.roleBinding.role, - activationRequest.roleBinding.fullResourceName, + activationRequest.roleBinding.role(), + activationRequest.roleBinding.fullResourceName(), activationRequest.beneficiary)) .addLabels(le -> addLabels(le, activationRequest)) .write(); @@ -638,8 +638,8 @@ public ActivationStatusResponse approveActivationRequest( String.format( "User %s failed to activate role '%s' on '%s' for %s: %s", iapPrincipal.getId(), - activationRequest.roleBinding.role, - activationRequest.roleBinding.fullResourceName, + activationRequest.roleBinding.role(), + activationRequest.roleBinding.fullResourceName(), activationRequest.beneficiary, Exceptions.getFullMessage(e))) .addLabels(le -> addLabels(le, activationRequest)) @@ -692,9 +692,9 @@ private static LogAdapter.LogEntry addLabels( RoleBinding roleBinding ) { return entry - .addLabel("role", roleBinding.role) - .addLabel("resource", roleBinding.fullResourceName) - .addLabel("project_id", ProjectId.fromFullResourceName(roleBinding.fullResourceName).id()); + .addLabel("role", roleBinding.role()) + .addLabel("resource", roleBinding.fullResourceName()) + .addLabel("project_id", ProjectId.fromFullResourceName(roleBinding.fullResourceName()).id()); } private static LogAdapter.LogEntry addLabels( @@ -856,7 +856,7 @@ private ActivationStatus( assert startTime < endTime; this.activationId = activationId.toString(); - this.projectId = ProjectId.fromFullResourceName(roleBinding.fullResourceName).id(); + this.projectId = ProjectId.fromFullResourceName(roleBinding.fullResourceName()).id(); this.roleBinding = roleBinding; this.status = status; this.startTime = startTime; @@ -895,12 +895,12 @@ protected RequestActivationNotification( String.format( "%s requests access to project %s", request.beneficiary, - ProjectId.fromFullResourceName(request.roleBinding.fullResourceName).id())); + ProjectId.fromFullResourceName(request.roleBinding.fullResourceName()).id())); this.properties.put("BENEFICIARY", request.beneficiary); this.properties.put("REVIEWERS", request.reviewers); - this.properties.put("PROJECT_ID", ProjectId.fromFullResourceName(request.roleBinding.fullResourceName)); - this.properties.put("ROLE", request.roleBinding.role); + this.properties.put("PROJECT_ID", ProjectId.fromFullResourceName(request.roleBinding.fullResourceName())); + this.properties.put("ROLE", request.roleBinding.role()); this.properties.put("START_TIME", request.startTime); this.properties.put("END_TIME", request.endTime); this.properties.put("REQUEST_EXPIRY_TIME", requestExpiryTime); @@ -930,13 +930,13 @@ protected ActivationApprovedNotification( String.format( "%s requests access to project %s", request.beneficiary, - ProjectId.fromFullResourceName(request.roleBinding.fullResourceName).id())); + ProjectId.fromFullResourceName(request.roleBinding.fullResourceName()).id())); this.properties.put("APPROVER", approver.email); this.properties.put("BENEFICIARY", request.beneficiary); this.properties.put("REVIEWERS", request.reviewers); - this.properties.put("PROJECT_ID", ProjectId.fromFullResourceName(request.roleBinding.fullResourceName)); - this.properties.put("ROLE", request.roleBinding.role); + this.properties.put("PROJECT_ID", ProjectId.fromFullResourceName(request.roleBinding.fullResourceName())); + this.properties.put("ROLE", request.roleBinding.role()); this.properties.put("START_TIME", request.startTime); this.properties.put("END_TIME", request.endTime); this.properties.put("JUSTIFICATION", request.justification); @@ -973,7 +973,7 @@ protected ActivationSelfApprovedNotification( this.properties.put("BENEFICIARY", beneficiary); this.properties.put("PROJECT_ID", activation.projectRole.getProjectId()); - this.properties.put("ROLE", activation.projectRole.roleBinding().role); + this.properties.put("ROLE", activation.projectRole.roleBinding().role()); this.properties.put("START_TIME", activation.startTime); this.properties.put("END_TIME", activation.endTime); this.properties.put("JUSTIFICATION", justification); diff --git a/sources/src/test/java/com/google/solutions/jitaccess/core/services/TestRoleDiscoveryService.java b/sources/src/test/java/com/google/solutions/jitaccess/core/services/TestRoleDiscoveryService.java index eb4a40452..cc918ffe4 100644 --- a/sources/src/test/java/com/google/solutions/jitaccess/core/services/TestRoleDiscoveryService.java +++ b/sources/src/test/java/com/google/solutions/jitaccess/core/services/TestRoleDiscoveryService.java @@ -361,7 +361,7 @@ public void whenAnalysisContainsJitEligibleBinding_ThenListEligibleProjectRolesR var role = roles.getItems().stream().findFirst().get(); assertEquals(SAMPLE_PROJECT_ID_1, role.getProjectId()); - assertEquals(SAMPLE_ROLE, role.roleBinding().role); + assertEquals(SAMPLE_ROLE, role.roleBinding().role()); assertEquals(ProjectRole.Status.ELIGIBLE_FOR_JIT, role.status()); } @@ -409,7 +409,7 @@ public void whenAnalysisContainsDuplicateJitEligibleBinding_ThenListEligibleProj var role = roles.getItems().stream().findFirst().get(); assertEquals(SAMPLE_PROJECT_ID_1, role.getProjectId()); - assertEquals(SAMPLE_ROLE, role.roleBinding().role); + assertEquals(SAMPLE_ROLE, role.roleBinding().role()); assertEquals(ProjectRole.Status.ELIGIBLE_FOR_JIT, role.status()); } @@ -450,7 +450,7 @@ public void whenAnalysisContainsMpaEligibleBinding_ThenListEligibleProjectRolesR var role = roles.getItems().stream().findFirst().get(); assertEquals(SAMPLE_PROJECT_ID_1, role.getProjectId()); - assertEquals(SAMPLE_ROLE, role.roleBinding().role); + assertEquals(SAMPLE_ROLE, role.roleBinding().role()); assertEquals(ProjectRole.Status.ELIGIBLE_FOR_MPA, role.status()); } @@ -498,7 +498,7 @@ public void whenAnalysisContainsDuplicateMpaEligibleBinding_ThenListEligibleProj var role = roles.getItems().stream().findFirst().get(); assertEquals(SAMPLE_PROJECT_ID_1, role.getProjectId()); - assertEquals(SAMPLE_ROLE, role.roleBinding().role); + assertEquals(SAMPLE_ROLE, role.roleBinding().role()); assertEquals(ProjectRole.Status.ELIGIBLE_FOR_MPA, role.status()); } @@ -548,12 +548,12 @@ public void whenAnalysisContainsMpaEligibleBindingAndJitEligibleBindingForDiffer var role = roles.getItems().stream().findFirst().get(); assertEquals(SAMPLE_PROJECT_ID_1, role.getProjectId()); - assertEquals(SAMPLE_ROLE, role.roleBinding().role); + assertEquals(SAMPLE_ROLE, role.roleBinding().role()); assertEquals(ProjectRole.Status.ELIGIBLE_FOR_JIT, role.status()); role = roles.getItems().stream().skip(1).findFirst().get(); assertEquals(SAMPLE_PROJECT_ID_1, role.getProjectId()); - assertEquals(SAMPLE_ROLE_2, role.roleBinding().role); + assertEquals(SAMPLE_ROLE_2, role.roleBinding().role()); assertEquals(ProjectRole.Status.ELIGIBLE_FOR_MPA, role.status()); } @@ -604,7 +604,7 @@ public void whenAnalysisContainsMpaEligibleBindingAndJitEligibleBindingForSameRo // Only the JIT-eligible binding is retained. var role = roles.getItems().stream().findFirst().get(); assertEquals(SAMPLE_PROJECT_ID_1, role.getProjectId()); - assertEquals(SAMPLE_ROLE, role.roleBinding().role); + assertEquals(SAMPLE_ROLE, role.roleBinding().role()); assertEquals(ProjectRole.Status.ELIGIBLE_FOR_JIT, role.status()); } @@ -666,7 +666,7 @@ public void whenAnalysisContainsActivatedBinding_ThenListEligibleProjectRolesRet var role = roles.getItems().stream().findFirst().get(); assertEquals(SAMPLE_PROJECT_ID_1, role.getProjectId()); - assertEquals(SAMPLE_ROLE, role.roleBinding().role); + assertEquals(SAMPLE_ROLE, role.roleBinding().role()); assertEquals(ProjectRole.Status.ACTIVATED, role.status()); } diff --git a/sources/src/test/java/com/google/solutions/jitaccess/web/TestApiResource.java b/sources/src/test/java/com/google/solutions/jitaccess/web/TestApiResource.java index 7c5decf54..65dde9d70 100644 --- a/sources/src/test/java/com/google/solutions/jitaccess/web/TestApiResource.java +++ b/sources/src/test/java/com/google/solutions/jitaccess/web/TestApiResource.java @@ -271,7 +271,7 @@ public void whenPeerDiscoveryReturnsNoPeers_ThenListPeersReturnsEmptyList() thro when(this.resource.roleDiscoveryService .listEligibleUsersForProjectRole( eq(SAMPLE_USER), - argThat(r -> r.role.equals("roles/browser")))) + argThat(r -> r.role().equals("roles/browser")))) .thenReturn(Set.of()); var response = new RestDispatcher<>(this.resource, SAMPLE_USER) @@ -289,7 +289,7 @@ public void whenPeerDiscoveryReturnsProjects_ThenListPeersReturnsList() throws E when(this.resource.roleDiscoveryService .listEligibleUsersForProjectRole( eq(SAMPLE_USER), - argThat(r -> r.role.equals("roles/browser")))) + argThat(r -> r.role().equals("roles/browser")))) .thenReturn(Set.of(new UserId("peer-1@example.com"), new UserId("peer-2@example.com"))); var response = new RestDispatcher<>(this.resource, SAMPLE_USER) @@ -826,7 +826,7 @@ public void whenRequestValid_ThenRequestActivationSendsNotification() throws Exc .createActivationRequestForPeer( eq(SAMPLE_USER), eq(Set.of(SAMPLE_USER_2)), - argThat(r -> r.role.equals("roles/mock")), + argThat(r -> r.role().equals("roles/mock")), eq("justification"), eq(Duration.ofMinutes(5)))) .thenReturn(RoleActivationService.ActivationRequest.createForTestingOnly( @@ -873,7 +873,7 @@ public void whenRequestValid_ThenRequestActivationReturnsSuccessResponse() throw .createActivationRequestForPeer( eq(SAMPLE_USER), eq(Set.of(SAMPLE_USER_2)), - argThat(r -> r.role.equals("roles/mock")), + argThat(r -> r.role().equals("roles/mock")), eq("justification"), eq(Duration.ofMinutes(5)))) .thenReturn(RoleActivationService.ActivationRequest.createForTestingOnly( From 8aa5dbf6f0b81776d2aa32b170a34f654f739c1c Mon Sep 17 00:00:00 2001 From: Johannes Passing Date: Thu, 14 Dec 2023 15:18:01 +1100 Subject: [PATCH 6/6] Add preconditions --- .../google/solutions/jitaccess/core/data/ProjectRole.java | 1 - .../google/solutions/jitaccess/core/data/RoleBinding.java | 7 +++++++ .../com/google/solutions/jitaccess/core/data/Topic.java | 8 ++++++++ 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/sources/src/main/java/com/google/solutions/jitaccess/core/data/ProjectRole.java b/sources/src/main/java/com/google/solutions/jitaccess/core/data/ProjectRole.java index 6b322c27d..43951c0d7 100644 --- a/sources/src/main/java/com/google/solutions/jitaccess/core/data/ProjectRole.java +++ b/sources/src/main/java/com/google/solutions/jitaccess/core/data/ProjectRole.java @@ -35,7 +35,6 @@ public record ProjectRole( Preconditions.checkNotNull(roleBinding); Preconditions.checkNotNull(status); Preconditions.checkArgument(ProjectId.isProjectFullResourceName(roleBinding.fullResourceName())); - } @Override diff --git a/sources/src/main/java/com/google/solutions/jitaccess/core/data/RoleBinding.java b/sources/src/main/java/com/google/solutions/jitaccess/core/data/RoleBinding.java index bf8f35875..b2c32f728 100644 --- a/sources/src/main/java/com/google/solutions/jitaccess/core/data/RoleBinding.java +++ b/sources/src/main/java/com/google/solutions/jitaccess/core/data/RoleBinding.java @@ -21,6 +21,8 @@ package com.google.solutions.jitaccess.core.data; +import com.google.common.base.Preconditions; + import java.util.Comparator; /** @@ -30,6 +32,11 @@ public record RoleBinding ( String fullResourceName, String role ) implements Comparable { + + public RoleBinding { + Preconditions.checkNotNull(fullResourceName, "fullResourceName"); + Preconditions.checkNotNull(role, "role"); + } public RoleBinding(ProjectId project, String role) { this(project.getFullResourceName(), role); } diff --git a/sources/src/main/java/com/google/solutions/jitaccess/core/data/Topic.java b/sources/src/main/java/com/google/solutions/jitaccess/core/data/Topic.java index 81d8d2ada..03eacaea6 100644 --- a/sources/src/main/java/com/google/solutions/jitaccess/core/data/Topic.java +++ b/sources/src/main/java/com/google/solutions/jitaccess/core/data/Topic.java @@ -21,7 +21,15 @@ package com.google.solutions.jitaccess.core.data; +import com.google.common.base.Preconditions; + public record Topic(String projectId, String topicName) { + + public Topic { + Preconditions.checkNotNull(projectId, "projectId"); + Preconditions.checkNotNull(topicName, "topicName"); + } + @Override public String toString() { return getFullResourceName();