From fd8c3399ec5b04e9ab5fd4c0ae3d3e4e3ebad977 Mon Sep 17 00:00:00 2001 From: Abdulla Abdurakhmanov Date: Thu, 7 Dec 2023 15:12:51 +0100 Subject: [PATCH 1/7] Sorting Project IDs and roles. Adjasting UI stepper width --- .../jitaccess/core/adapters/ResourceManagerAdapter.java | 4 ++-- .../jitaccess/core/services/RoleDiscoveryService.java | 8 ++++---- sources/src/main/resources/META-INF/resources/styles.css | 4 ++++ 3 files changed, 10 insertions(+), 6 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 082dd1a1e..a0a7a2c8e 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 @@ -252,7 +252,7 @@ public List testIamPermissions( } } - public List searchProjectIds(String query) throws NotAuthenticatedException, IOException { + public Set searchProjectIds(String query) throws NotAuthenticatedException, IOException { try { var client = createClient(); @@ -287,7 +287,7 @@ public List searchProjectIds(String query) throws NotAuthenticatedExc return allProjects.stream() .map(p -> new ProjectId(p.getProjectId())) - .collect(Collectors.toList()); + .collect(Collectors.toCollection(TreeSet::new)); } catch (GoogleJsonResponseException e) { switch (e.getStatusCode()) { 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 d96870616..5108c6ab3 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 @@ -150,11 +150,11 @@ public Set listAvailableProjects( return roleBindings .stream() .map(b -> ProjectId.fromFullResourceName(b.fullResourceName)) - .collect(Collectors.toSet()); + .collect(Collectors.toCollection(TreeSet::new)); } else { // Used as alternative option if availableProjectsQuery is set and the main approach with Asset API is not working fast enough. - return new HashSet<>(resourceManagerAdapter.searchProjectIds(this.options.availableProjectsQuery)); + return resourceManagerAdapter.searchProjectIds(this.options.availableProjectsQuery); } } @@ -297,7 +297,7 @@ public Result listEligibleProjectRoles( Stream.ofNullable(analysisResult.getNonCriticalErrors()) .flatMap(Collection::stream) .map(e -> e.getCause()) - .collect(Collectors.toSet())); + .collect(Collectors.toCollection(TreeSet::new))); } /** @@ -353,7 +353,7 @@ public Set listEligibleUsersForProjectRole( // Remove the caller. .filter(user -> !user.equals(callerUserId)) - .collect(Collectors.toSet()); + .collect(Collectors.toCollection(TreeSet::new)); } // ------------------------------------------------------------------------- diff --git a/sources/src/main/resources/META-INF/resources/styles.css b/sources/src/main/resources/META-INF/resources/styles.css index 7db6ab323..100d5bbef 100644 --- a/sources/src/main/resources/META-INF/resources/styles.css +++ b/sources/src/main/resources/META-INF/resources/styles.css @@ -24,6 +24,10 @@ a { height: auto; } +.mdl-stepper { + max-width: 1024px; +} + .mdl-step__transient { height: calc(100% - 32px); } From 3306c52ff01c0b11a3f69e8db1d3de37de0061c1 Mon Sep 17 00:00:00 2001 From: Abdulla Abdurakhmanov Date: Thu, 7 Dec 2023 15:33:59 +0100 Subject: [PATCH 2/7] Fixed tests --- .../jitaccess/core/services/TestRoleDiscoveryService.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) 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 bc3733eb9..5c803017e 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 @@ -1101,8 +1101,7 @@ public void whenResourceManagerEmpty_ThenListAvailableProjectsReturnsEmptyList() var assetAdapter = Mockito.mock(AssetInventoryAdapter.class); var resourceManagerAdapter = Mockito.mock(ResourceManagerAdapter.class); - when(resourceManagerAdapter.searchProjectIds(eq("parent:folder/0"))) - .thenReturn(List.of()); + when(resourceManagerAdapter.searchProjectIds(eq("parent:folder/0"))).thenReturn(Set.of()); var service = new RoleDiscoveryService( assetAdapter, @@ -1118,7 +1117,7 @@ public void whenResourceManagerEmpty_ThenListAvailableProjectsReturnsEmptyList() public void whenResourceManagerReturnsList_ThenListAvailableProjectsReturnsTheSameList() throws Exception { var assetAdapter = Mockito.mock(AssetInventoryAdapter.class); var resourceManagerAdapter = Mockito.mock(ResourceManagerAdapter.class); - var expectedProjectIds = List.of(new ProjectId("project-1"), new ProjectId("project-2")); + var expectedProjectIds = Set.of(new ProjectId("project-1"), new ProjectId("project-2")); when(resourceManagerAdapter.searchProjectIds(eq("parent:folder/0"))) .thenReturn(expectedProjectIds); From 7f796a2a40dce2c4ffed3eabb8930076685a7e10 Mon Sep 17 00:00:00 2001 From: Abdulla Abdurakhmanov Date: Fri, 8 Dec 2023 07:20:07 +0100 Subject: [PATCH 3/7] Comparable in models --- .../google/solutions/jitaccess/core/data/ProjectId.java | 7 ++++++- .../google/solutions/jitaccess/core/data/ProjectRole.java | 7 ++++++- .../google/solutions/jitaccess/core/data/RoleBinding.java | 7 ++++++- .../com/google/solutions/jitaccess/core/data/UserId.java | 7 ++++++- 4 files changed, 24 insertions(+), 4 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 bed74bf86..c24a3fcd3 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 @@ -28,7 +28,7 @@ /** * Project ID for a Google Cloud project. */ -public class ProjectId { +public class ProjectId implements Comparable { private static final String PROJECT_RESOURCE_NAME_PREFIX = "//cloudresourcemanager.googleapis.com/projects/"; public final String id; @@ -94,4 +94,9 @@ public boolean equals(Object o) { 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/core/data/ProjectRole.java b/sources/src/main/java/com/google/solutions/jitaccess/core/data/ProjectRole.java index 3cf9cfa28..999eff8c8 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 @@ -28,7 +28,7 @@ /** * Represents an eligible role on a project. */ -public class ProjectRole { +public class ProjectRole implements Comparable { public final RoleBinding roleBinding; public final Status status; @@ -76,6 +76,11 @@ public int hashCode() { return Objects.hash(this.roleBinding, this.status); } + @Override + public int compareTo(ProjectRole o) { + return this.roleBinding.compareTo(o.roleBinding); + } + // ------------------------------------------------------------------------- // Inner classes. // ------------------------------------------------------------------------- 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 e6c05369c..254d07962 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 @@ -28,7 +28,7 @@ /** * Represents a role that has been granted on a resource. */ -public class RoleBinding { +public class RoleBinding implements Comparable { public final String fullResourceName; public final String role; @@ -71,4 +71,9 @@ public boolean equals(Object o) { public int hashCode() { return Objects.hash(this.fullResourceName, this.role); } + + @Override + public int compareTo(RoleBinding o) { + return this.fullResourceName.compareTo(o.fullResourceName); + } } diff --git a/sources/src/main/java/com/google/solutions/jitaccess/core/data/UserId.java b/sources/src/main/java/com/google/solutions/jitaccess/core/data/UserId.java index 54a25dad3..fbc8ceb34 100644 --- a/sources/src/main/java/com/google/solutions/jitaccess/core/data/UserId.java +++ b/sources/src/main/java/com/google/solutions/jitaccess/core/data/UserId.java @@ -25,7 +25,7 @@ import java.util.Objects; -public class UserId { +public class UserId implements Comparable { public final transient String id; public final String email; @@ -67,4 +67,9 @@ public boolean equals(Object o) { public int hashCode() { return Objects.hash(email); } + + @Override + public int compareTo(UserId o) { + return this.email.compareTo(o.email); + } } From ce016bdddc77d3aa75dcd9d63a3262295012aa82 Mon Sep 17 00:00:00 2001 From: Abdulla Abdurakhmanov Date: Fri, 8 Dec 2023 07:20:38 +0100 Subject: [PATCH 4/7] Fixed RoleDiscoveryService --- .../solutions/jitaccess/core/services/RoleDiscoveryService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 5108c6ab3..3ddc41219 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 @@ -297,7 +297,7 @@ public Result listEligibleProjectRoles( Stream.ofNullable(analysisResult.getNonCriticalErrors()) .flatMap(Collection::stream) .map(e -> e.getCause()) - .collect(Collectors.toCollection(TreeSet::new))); + .collect(Collectors.toSet())); } /** From 9bf517b83a6ed34c7caa2c692c8e9915f5cec253 Mon Sep 17 00:00:00 2001 From: Abdulla Abdurakhmanov Date: Fri, 8 Dec 2023 07:38:25 +0100 Subject: [PATCH 5/7] Update tests --- .../jitaccess/core/data/RoleBinding.java | 5 +++- .../core/services/RoleDiscoveryService.java | 4 +-- .../jitaccess/core/data/TestProjectId.java | 14 +++++++++++ .../jitaccess/core/data/TestProjectRole.java | 25 +++++++++++++++++++ 4 files changed, 44 insertions(+), 4 deletions(-) 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 254d07962..81589a9fd 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 @@ -23,6 +23,7 @@ import com.google.common.base.Preconditions; +import java.util.Comparator; import java.util.Objects; /** @@ -74,6 +75,8 @@ public int hashCode() { @Override public int compareTo(RoleBinding o) { - return this.fullResourceName.compareTo(o.fullResourceName); + return Comparator.comparing((RoleBinding r) -> r.fullResourceName) + .thenComparing(r -> r.role) + .compare(this, o); } } 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 3ddc41219..94975879b 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 @@ -291,9 +291,7 @@ public Result listEligibleProjectRoles( consolidatedRoles.addAll(activatedRoles); return new Result<>( - consolidatedRoles.stream() - .sorted(Comparator.comparing(r -> r.roleBinding.fullResourceName)) - .collect(Collectors.toList()), + consolidatedRoles.stream().sorted().collect(Collectors.toList()), Stream.ofNullable(analysisResult.getNonCriticalErrors()) .flatMap(Collection::stream) .map(e -> e.getCause()) diff --git a/sources/src/test/java/com/google/solutions/jitaccess/core/data/TestProjectId.java b/sources/src/test/java/com/google/solutions/jitaccess/core/data/TestProjectId.java index 1fc5adea1..fa1d36712 100644 --- a/sources/src/test/java/com/google/solutions/jitaccess/core/data/TestProjectId.java +++ b/sources/src/test/java/com/google/solutions/jitaccess/core/data/TestProjectId.java @@ -23,6 +23,10 @@ import org.junit.jupiter.api.Test; +import java.util.List; +import java.util.TreeSet; +import java.util.stream.Collectors; + import static org.junit.jupiter.api.Assertions.*; public class TestProjectId { @@ -104,4 +108,14 @@ public void whenObjectIsDifferentType_ThenEqualsReturnsFalse() { assertFalse(id1.equals("")); } + + @Test + public void whenInTreeSet_ThenReturnsInExpectedOrder() { + var projects = List.of(new ProjectId("project-3"),new ProjectId("project-1"), new ProjectId("project-2")); + var sortedProjects = new TreeSet<>(projects); + var sortedIter = sortedProjects.iterator(); + assertEquals(sortedIter.next(), new ProjectId("project-1")); + assertEquals(sortedIter.next(), new ProjectId("project-2")); + assertEquals(sortedIter.next(), new ProjectId("project-3")); + } } diff --git a/sources/src/test/java/com/google/solutions/jitaccess/core/data/TestProjectRole.java b/sources/src/test/java/com/google/solutions/jitaccess/core/data/TestProjectRole.java index 22c3dc8fa..2c585eb61 100644 --- a/sources/src/test/java/com/google/solutions/jitaccess/core/data/TestProjectRole.java +++ b/sources/src/test/java/com/google/solutions/jitaccess/core/data/TestProjectRole.java @@ -23,6 +23,9 @@ import org.junit.jupiter.api.Test; +import java.util.List; +import java.util.TreeSet; + import static org.junit.jupiter.api.Assertions.*; public class TestProjectRole { @@ -133,4 +136,26 @@ public void equalsNullIsFalse() { assertFalse(role.equals(null)); } + + @Test + public void whenInTreeSet_ThenReturnsInExpectedOrder() { + var role1 = new ProjectRole( + new RoleBinding("//cloudresourcemanager.googleapis.com/projects/project-1", "role/sample1"), + ProjectRole.Status.ELIGIBLE_FOR_JIT + ); + var role2 = new ProjectRole( + new RoleBinding("//cloudresourcemanager.googleapis.com/projects/project-1", "role/sample2"), + ProjectRole.Status.ELIGIBLE_FOR_JIT + ); + var role3 = new ProjectRole( + new RoleBinding("//cloudresourcemanager.googleapis.com/projects/project-2", "role/sample1"), + ProjectRole.Status.ELIGIBLE_FOR_JIT + ); + var roles = List.of(role3,role1,role2); + var sorted = new TreeSet<>(roles); + var sortedIter = sorted.iterator(); + assertEquals(sortedIter.next(), role1); + assertEquals(sortedIter.next(), role2); + assertEquals(sortedIter.next(), role3); + } } \ No newline at end of file From 09b676704d993be2acd11d1d1d40ef7d28737987 Mon Sep 17 00:00:00 2001 From: Abdulla Abdurakhmanov Date: Tue, 12 Dec 2023 00:33:55 +0100 Subject: [PATCH 6/7] Addressing consolidatedRoles sorting --- .../jitaccess/core/services/RoleDiscoveryService.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 94975879b..28b011bf8 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 @@ -221,7 +221,7 @@ public Result listEligibleProjectRoles( evalResult -> "TRUE".equalsIgnoreCase(evalResult)) .stream() .map(binding -> new ProjectRole(binding, ProjectRole.Status.ACTIVATED)) - .collect(Collectors.toSet()); + .collect(Collectors.toCollection(TreeSet::new)); } else { activatedRoles = Set.of(); @@ -240,7 +240,7 @@ public Result listEligibleProjectRoles( evalResult -> "CONDITIONAL".equalsIgnoreCase(evalResult)) .stream() .map(binding -> new ProjectRole(binding, ProjectRole.Status.ELIGIBLE_FOR_JIT)) - .collect(Collectors.toSet()); + .collect(Collectors.toCollection(TreeSet::new)); } else { jitEligibleRoles = Set.of(); @@ -259,7 +259,7 @@ public Result listEligibleProjectRoles( evalResult -> "CONDITIONAL".equalsIgnoreCase(evalResult)) .stream() .map(binding -> new ProjectRole(binding, ProjectRole.Status.ELIGIBLE_FOR_MPA)) - .collect(Collectors.toSet()); + .collect(Collectors.toCollection(TreeSet::new)); } else { mpaEligibleRoles = Set.of(); From 344557c337b40b914055692e1adc2edad75c4a7c Mon Sep 17 00:00:00 2001 From: Abdulla Abdurakhmanov Date: Tue, 12 Dec 2023 09:56:11 +0100 Subject: [PATCH 7/7] Removed sorting consolidatedRoles --- .../solutions/jitaccess/core/services/RoleDiscoveryService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 28b011bf8..cc0141e14 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 @@ -291,7 +291,7 @@ public Result listEligibleProjectRoles( consolidatedRoles.addAll(activatedRoles); return new Result<>( - consolidatedRoles.stream().sorted().collect(Collectors.toList()), + consolidatedRoles, Stream.ofNullable(analysisResult.getNonCriticalErrors()) .flatMap(Collection::stream) .map(e -> e.getCause())