diff --git a/webapp/src/main/java/com/box/l10n/mojito/security/ServiceDisambiguator.java b/webapp/src/main/java/com/box/l10n/mojito/security/ServiceDisambiguator.java index eaad1d8e3..f45849902 100644 --- a/webapp/src/main/java/com/box/l10n/mojito/security/ServiceDisambiguator.java +++ b/webapp/src/main/java/com/box/l10n/mojito/security/ServiceDisambiguator.java @@ -1,6 +1,7 @@ package com.box.l10n.mojito.security; import com.box.l10n.mojito.entity.security.user.User; +import java.util.ArrayList; import java.util.List; import java.util.Optional; import org.slf4j.Logger; @@ -25,7 +26,7 @@ public class ServiceDisambiguator { * @param services list of relevant services * @return the User or null if none match */ - public User findServiceWithCommonAncestor(List services, String servicePath) { + public User getServiceWithCommonAncestor(List services, String servicePath) { if (services == null || servicePath == null || services.isEmpty() || servicePath.isEmpty()) { logger.debug("No services found or no service path given"); return null; @@ -46,34 +47,22 @@ public User findServiceWithCommonAncestor(List services, String servicePat String[] currentPathElements = currentServicePath.split(headerSecurityConfig.serviceDelimiter); - StringBuilder commonAncestor = new StringBuilder(); - int minLength = Math.min(servicePathElements.length, currentPathElements.length); - - for (int i = 0; i < minLength; i++) { - if (servicePathElements[i].equals(currentPathElements[i])) { - commonAncestor.append(servicePathElements[i]); - if (i < minLength - 1) { - commonAncestor.append(headerSecurityConfig.serviceDelimiter); - } - } - } - - logger.debug("Common ancestor between services found: {}", commonAncestor); - - // Update the service which shares the shortest path - String mostCommonAncestor = commonAncestor.toString(); - if (mostCommonAncestor.isEmpty()) { + String commonAncestor = getCommonAncestorPath(servicePathElements, currentPathElements); + if (commonAncestor.isEmpty()) { continue; } - if (mostCommonAncestor.length() != currentServicePath.length()) { + // We ignore this service because it is a sibling (i.e different child service) + // example case: service doing auth -> test.com/infra/jenkins/agent3 + // currentServicePath -> test.com/infra/jenkins/agent1 + // commonAncestor -> test.com/infra/jenkins + if (commonAncestor.length() < currentServicePath.length()) { continue; } - if (longestAncestorPath == null - || mostCommonAncestor.length() > longestAncestorPath.length()) { - longestAncestorPath = mostCommonAncestor; + if (longestAncestorPath == null || commonAncestor.length() > longestAncestorPath.length()) { + longestAncestorPath = commonAncestor; closestService = currentService; } } @@ -83,4 +72,18 @@ public User findServiceWithCommonAncestor(List services, String servicePat Optional.ofNullable(closestService).map(User::getUsername).orElse("null")); return closestService; } + + private String getCommonAncestorPath(String[] servicePathElements, String[] currentPathElements) { + StringBuilder commonAncestor = new StringBuilder(); + int minLength = Math.min(servicePathElements.length, currentPathElements.length); + List matchedElements = new ArrayList<>(currentPathElements.length); + for (int i = 0; i < minLength; i++) { + if (servicePathElements[i].equals(currentPathElements[i])) { + matchedElements.add(servicePathElements[i]); + } + } + + logger.debug("Common ancestor between services found: {}", commonAncestor); + return String.join(headerSecurityConfig.serviceDelimiter, matchedElements); + } } diff --git a/webapp/src/main/java/com/box/l10n/mojito/service/security/user/UserService.java b/webapp/src/main/java/com/box/l10n/mojito/service/security/user/UserService.java index e6d09b36f..fd1dc6ca7 100644 --- a/webapp/src/main/java/com/box/l10n/mojito/service/security/user/UserService.java +++ b/webapp/src/main/java/com/box/l10n/mojito/service/security/user/UserService.java @@ -409,9 +409,8 @@ public Page findByUsernameOrName(String username, String search, Pageable /** * Gets a service by name and if it doesn't exist create a created user. * - *

All admin service accounts should already be created beforehand. - * Get the exact service account or any service account which is the - * ancestor service to the account + *

All admin service accounts should already be created beforehand. Get the exact service + * account or any service account which is the ancestor service to the account * * @param serviceName * @return @@ -423,7 +422,8 @@ public User getServiceAccountUser(String serviceName) { } Optional> users = - userRepository.findServicesByServiceNameAndPrefix(serviceName, headerSecurityConfig.getServicePrefix()); + userRepository.findServicesByServiceNameAndPrefix( + serviceName, headerSecurityConfig.getServicePrefix()); if (users.isEmpty()) { logger.debug("No matching services found as per DB query"); sendPagerDutyNotification(serviceName); @@ -433,8 +433,7 @@ public User getServiceAccountUser(String serviceName) { return null; } - User matchingUser = - serviceDisambiguator.findServiceWithCommonAncestor(users.get(), serviceName); + User matchingUser = serviceDisambiguator.getServiceWithCommonAncestor(users.get(), serviceName); if (matchingUser == null) { logger.debug("No matching services found as per ServiceDisambiguator"); sendPagerDutyNotification(serviceName); diff --git a/webapp/src/test/java/com/box/l10n/mojito/security/ServiceDisambiguatorTest.java b/webapp/src/test/java/com/box/l10n/mojito/security/ServiceDisambiguatorTest.java index 583cf7d60..66bcd9836 100644 --- a/webapp/src/test/java/com/box/l10n/mojito/security/ServiceDisambiguatorTest.java +++ b/webapp/src/test/java/com/box/l10n/mojito/security/ServiceDisambiguatorTest.java @@ -22,16 +22,16 @@ public ServiceDisambiguatorTest() { @Test public void testFindServiceWithShortestSharedAncestor_EdgeCases() { User sharedAncestor = - serviceDisambiguator.findServiceWithCommonAncestor( + serviceDisambiguator.getServiceWithCommonAncestor( new ArrayList<>(), "spiffe://test.com/container/service"); assertNull(sharedAncestor); - sharedAncestor = serviceDisambiguator.findServiceWithCommonAncestor(new ArrayList<>(), ""); + sharedAncestor = serviceDisambiguator.getServiceWithCommonAncestor(new ArrayList<>(), ""); assertNull(sharedAncestor); - sharedAncestor = serviceDisambiguator.findServiceWithCommonAncestor(new ArrayList<>(), null); + sharedAncestor = serviceDisambiguator.getServiceWithCommonAncestor(new ArrayList<>(), null); assertNull(sharedAncestor); - sharedAncestor = serviceDisambiguator.findServiceWithCommonAncestor(null, ""); + sharedAncestor = serviceDisambiguator.getServiceWithCommonAncestor(null, ""); assertNull(sharedAncestor); - sharedAncestor = serviceDisambiguator.findServiceWithCommonAncestor(null, "test"); + sharedAncestor = serviceDisambiguator.getServiceWithCommonAncestor(null, "test"); assertNull(sharedAncestor); } @@ -47,11 +47,11 @@ public void testFindServiceWithShortestSharedAncestor_ExactMatch() throws Except "spiffe://test.com/container1/imageService", "spiffe://test.com/container1/userService")); User result = - serviceDisambiguator.findServiceWithCommonAncestor( + serviceDisambiguator.getServiceWithCommonAncestor( services, "spiffe://test.com/container1/tagService"); assertEquals(result, services.get(3)); result = - serviceDisambiguator.findServiceWithCommonAncestor( + serviceDisambiguator.getServiceWithCommonAncestor( services, "spiffe://test.com/container1/imageService"); assertEquals(result, services.get(4)); } @@ -65,14 +65,14 @@ public void testFindServiceWithShortestSharedAncestor_SpecificExamples() { "spiffe://test.com/infra/jenkins/agent2", "spiffe://test.com/infra/jenkins")); User result = - serviceDisambiguator.findServiceWithCommonAncestor( + serviceDisambiguator.getServiceWithCommonAncestor( services, "spiffe://test.com/infra/jenkins/agent1"); assertEquals(result, services.getFirst()); result = - serviceDisambiguator.findServiceWithCommonAncestor( + serviceDisambiguator.getServiceWithCommonAncestor( services, "spiffe://test.com/infra/jenkins/agent3"); assertEquals(result, services.get(2)); - result = serviceDisambiguator.findServiceWithCommonAncestor(services, "spiffe://test.com"); + result = serviceDisambiguator.getServiceWithCommonAncestor(services, "spiffe://test.com"); assertNull(result); }