fix: fix permission check rule resolving

This commit fix the way for resolving the permission check rule. Now it respect the request path

instead of randomly picking one.

src/main/java/com/cleverthis/authservice/controller/AuthController.java

@@ -187,17 +187,19 @@ public class AuthController {
         log.info("Received forwardAuth request: Method='{}', Uri='{}'", originalMethod,
               originalUriString);
         // first detect the client id and service relative path
-        final var targetServiceClientIdOpt = getTargetServiceClientId(originalUriString);
+        final var optionalRule = getPermissionCheckRuleByUri(originalUriString);
-        if (targetServiceClientIdOpt.isEmpty()) {
+        if (optionalRule.isEmpty()) {
             log.warn("ForwardAuth: No Keycloak client mapping found for URI: {}",
                   originalUriString);
             return Mono.just(ResponseEntity.status(HttpStatus.FORBIDDEN).build());
         }
-        final var targetServiceClientId = targetServiceClientIdOpt.get();
+        final var rule = optionalRule.get();
+
+        final var targetServiceClientId = rule.getKeycloakClientId();
         final var serviceRelativePath =
-              extractServiceRelativePath(originalUriString, targetServiceClientId);
+              extractServiceRelativePath(originalUriString, rule);
         // then check pass-through rule
-        if (shouldPassThrough(targetServiceClientId, serviceRelativePath)) {
+        if (shouldPassThrough(rule, serviceRelativePath)) {
             return Mono.just(ResponseEntity.status(HttpStatus.NO_CONTENT).build());
         }
         if (authorizationHeader == null) {
@@ -276,16 +278,15 @@ public class AuthController {
     }
 
     private boolean shouldPassThrough(
-          final String serviceClientId, final String serviceRelativePath
+          final PermissionMappingConfigProperties.Rule rule, final String serviceRelativePath
     ) {
-        final var rules = this.permissionMapLookupService
+        final var passthroughs = rule.getPassthroughs();
-              .getPassThoughRulesByClientId(serviceClientId);
         try {
-            return rules.stream()
+            return passthroughs.stream()
                   .anyMatch(it -> executePassThroughRule(it, serviceRelativePath));
         } catch (final Throwable throwable) {
             log.error("Error executing pass-through rule for client {} : {}",
-                  serviceClientId, throwable.getMessage(), throwable);
+                  rule.getKeycloakClientId(), throwable.getMessage(), throwable);
             return false;
         }
     }
@@ -300,13 +301,15 @@ public class AuthController {
         };
     }
 
-    private Optional<String> getTargetServiceClientId(String fullUriString) {
+    private Optional<PermissionMappingConfigProperties.Rule> getPermissionCheckRuleByUri(
+          String fullUriString
+    ) {
         try {
             URI uri = new URI(fullUriString);
             String path = uri.getPath(); // Get path part, e.g., /cleverswarm/api/jobs/123
             log.debug("Mapping URI path: {}", path);
 
-            final var pathMatch = this.permissionMapLookupService.matchClientIdByPath(path);
+            final var pathMatch = this.permissionMapLookupService.matchRuleByPath(path);
             if (pathMatch.isPresent()) {
                 return pathMatch;
             }
@@ -315,7 +318,9 @@ public class AuthController {
                 log.debug(
                       "No rule matched for path '{}', using default Keycloak Client ID:{}",
                       path, defaultValue);
-                return Optional.of(defaultValue);
+                return Optional.of(PermissionMappingConfigProperties.Rule.builder()
+                      .keycloakClientId(defaultValue)
+                      .build());
             }
         } catch (URISyntaxException e) {
             log.error("Invalid URI syntax for X-Forwarded-Uri: {}", fullUriString, e);
@@ -324,17 +329,11 @@ public class AuthController {
         return Optional.empty();
     }
 
-    private String extractServiceRelativePath(String fullUriString, String mappedKeycloakClientId) {
+    private String extractServiceRelativePath(
-        // Find the rule that gave us this mappedKeycloakClientId to get its pathPrefix
+          String fullUriString, PermissionMappingConfigProperties.Rule rule
-        Optional<PermissionMappingConfigProperties.Rule> matchedRule =
+    ) {
-              this.permissionMapLookupService.matchRuleByClientIdAndPath(
+        String pathPrefixToRemove = rule.getPathPrefix();
-                    mappedKeycloakClientId, fullUriString
+        if (rule.getKeycloakClientId()
-              );
-
-        String pathPrefixToRemove = "";
-        if (matchedRule.isPresent()) {
-            pathPrefixToRemove = matchedRule.get().getPathPrefix();
-        } else if (mappedKeycloakClientId
               .equals(this.permissionMapLookupService.getDefaultKeycloakClientId())) {
             // If it's a default mapping, there's no prefix to remove, use the whole path.
             // Or, you might decide that default mappings always apply to root paths. This

src/main/java/com/cleverthis/authservice/service/PermissionMapLookupService.java

@@ -1,7 +1,6 @@
 package com.cleverthis.authservice.service;
 
 import com.cleverthis.authservice.config.PermissionMappingConfigProperties;
-import java.util.List;
 import java.util.Optional;
 
 /**
@@ -17,18 +16,5 @@ public interface PermissionMapLookupService {
     /**
      * Given the path, find the client id of the first rule matching the path prefix.
      */
-    Optional<String> matchClientIdByPath(String path);
+    Optional<PermissionMappingConfigProperties.Rule> matchRuleByPath(String path);
-
-    /**
-     * Given the client id and path, find the rule that both matches the client id exactly,
-     * and match the path prefix.
-     */
-    Optional<PermissionMappingConfigProperties.Rule> matchRuleByClientIdAndPath(
-          String clientId, String path);
-
-    /**
-     * Return the list of pass-through rules by client id.
-     */
-    List<PermissionMappingConfigProperties.PassThrough> getPassThoughRulesByClientId(
-          String clientId);
 }

src/main/java/com/cleverthis/authservice/service/impl/FallbackPermissionMapLookupService.java

@@ -5,7 +5,6 @@ import com.cleverthis.authservice.service.PermissionMapLookupService;
 import com.ecwid.consul.v1.ConsulClient;
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
-import java.util.List;
 import java.util.Optional;
 import java.util.concurrent.atomic.AtomicReference;
 import lombok.extern.slf4j.Slf4j;
@@ -102,38 +101,15 @@ public final class FallbackPermissionMapLookupService implements PermissionMapLo
     }
 
     @Override
-    public Optional<String> matchClientIdByPath(final String path) {
+    public Optional<PermissionMappingConfigProperties.Rule> matchRuleByPath(final String path) {
         final var config = getConfig();
         for (final var rule : config.getRules()) {
             if (path.startsWith(rule.getPathPrefix())) {
                 log.debug("Path '{}' matched prefix '{}', using Keycloak Client ID: {}",
                       path, rule.getPathPrefix(), rule.getKeycloakClientId());
-                return Optional.of(rule.getKeycloakClientId());
+                return Optional.of(rule);
             }
         }
         return Optional.empty();
     }
-
-    @Override
-    public Optional<PermissionMappingConfigProperties.Rule> matchRuleByClientIdAndPath(
-          final String clientId, final String path
-    ) {
-        final var config = getConfig();
-        return config.getRules().stream()
-              .filter(rule ->
-                    rule.getKeycloakClientId().equals(clientId)
-                          && path.startsWith(rule.getPathPrefix()))
-              .findFirst();
-    }
-
-    @Override
-    public List<PermissionMappingConfigProperties.PassThrough> getPassThoughRulesByClientId(
-          final String clientId
-    ) {
-        final var config = getConfig();
-        return config.getRules().stream()
-              .filter(rule -> rule.getKeycloakClientId().equals(clientId))
-              .flatMap(rule -> rule.getPassthroughs().stream())
-              .toList();
-    }
 }

src/test/java/com/cleverthis/authservice/service/impl/FallbackPermissionMapLookupServiceTest.java

@@ -55,7 +55,8 @@ class FallbackPermissionMapLookupServiceTest {
         assertNull(service.getConfig()); // fallback is null
         // has json content
         when(value.getDecodedValue())
-              .thenReturn(this.objectMapper.writeValueAsString(new PermissionMappingConfigProperties()));
+              .thenReturn(
+                    this.objectMapper.writeValueAsString(new PermissionMappingConfigProperties()));
         service.refreshConsul();
         assertEquals(new PermissionMappingConfigProperties(), service.getConfig());
     }
@@ -76,45 +77,22 @@ class FallbackPermissionMapLookupServiceTest {
     }
 
     @Test
-    void testMatchClientIdByPath() {
+    void testMatchRuleByPath() {
         final var defaultConfig = new PermissionMappingConfigProperties();
-        defaultConfig.setRules(List.of(
+        final var ruleA = PermissionMappingConfigProperties.Rule.builder()
-              PermissionMappingConfigProperties.Rule.builder()
+              .keycloakClientId("service-A").pathPrefix("/a/").build();
-                    .keycloakClientId("service-A").pathPrefix("/a/").build(),
+        final var ruleB = PermissionMappingConfigProperties.Rule.builder()
-              PermissionMappingConfigProperties.Rule.builder()
+              .keycloakClientId("service-B").pathPrefix("/b/").build();
-                    .keycloakClientId("service-B").pathPrefix("/b/").build()
+        defaultConfig.setRules(List.of(ruleA, ruleB));
-        ));
         final var service = new FallbackPermissionMapLookupService(
               defaultConfig, this.consulClient, this.objectMapper);
 
         assertEquals(
-              "service-B",
+              ruleB,
-              service.matchClientIdByPath("/b/items/123").orElseThrow());
+              service.matchRuleByPath("/b/items/123").orElseThrow());
         assertEquals(
               Optional.empty(),
-              service.matchClientIdByPath("/c/items/123"));
+              service.matchRuleByPath("/c/items/123"));
     }
 
-    @Test
-    void testMatchRuleByClientIdAndPath() {
-        final var defaultConfig = new PermissionMappingConfigProperties();
-        defaultConfig.setRules(List.of(
-              PermissionMappingConfigProperties.Rule.builder()
-                    .keycloakClientId("service-A").pathPrefix("/a/").build(),
-              PermissionMappingConfigProperties.Rule.builder()
-                    .keycloakClientId("service-B").pathPrefix("/b/").build()
-        ));
-        final var service = new FallbackPermissionMapLookupService(
-              defaultConfig, this.consulClient, this.objectMapper);
-
-        service.matchRuleByClientIdAndPath("service-A", "/a/items/123")
-              .ifPresentOrElse(
-                    rule -> assertEquals("service-A", rule.getKeycloakClientId()),
-                    () -> Assertions.fail("No rule matched"));
-        service.matchRuleByClientIdAndPath("service-B", "/a/items/123")
-              .ifPresentOrElse(
-                    it -> Assertions.fail("Rule matched but should not have"),
-                    () -> {}
-              );
-    }
 }

src/test/java/com/cleverthis/authservice/service/impl/KeycloakIdentityManagerServiceTest.java

@@ -28,6 +28,7 @@ import org.keycloak.representations.idm.GroupRepresentation;
 import org.keycloak.representations.idm.MemberRepresentation;
 import org.keycloak.representations.idm.OrganizationDomainRepresentation;
 import org.keycloak.representations.idm.OrganizationRepresentation;
+import org.keycloak.representations.idm.UserRepresentation;
 import org.mockito.Mockito;
 import org.springframework.web.reactive.function.client.WebClient;
 import reactor.core.publisher.Mono;
@@ -558,4 +559,76 @@ class KeycloakIdentityManagerServiceTest {
 
         Mockito.verify(this.adminClient).getGroupById(groupId);
     }
+
+    @Test
+    void listSubGroups_success() {
+        // Arrange
+        final var parentGroupId = "parent-group-id";
+        final var subGroups = List.of(
+              new GroupRepresentation() {{
+                  setId("sub-group-1");
+                  setName("SubGroup1");
+              }},
+              new GroupRepresentation() {{
+                  setId("sub-group-2");
+                  setName("SubGroup2");
+              }}
+        );
+
+        Mockito.when(this.adminClient.listSubGroups(parentGroupId))
+              .thenReturn(Mono.just(subGroups));
+
+        // Act
+        final var result = this.service.listSubGroups(parentGroupId);
+
+        // Assert
+        StepVerifier.create(result)
+              .expectNextMatches(groups -> groups.size() == 2 &&
+                    "SubGroup1".equals(groups.getFirst().getName()) &&
+                    "sub-group-2".equals(groups.get(1).getId()))
+              .verifyComplete();
+
+        Mockito.verify(this.adminClient).listSubGroups(parentGroupId);
+    }
+
+    @Test
+    void getGroupMembers_success() {
+        // Arrange
+        final var groupId = "test-group-id";
+        final var mockMembers = List.of(
+              new UserRepresentation() {{
+                  setId("user1");
+                  setUsername("user1Username");
+                  setFirstName("User1");
+                  setLastName("One");
+                  setEmail("user1@example.com");
+                  setEnabled(true);
+              }},
+              new UserRepresentation() {{
+                  setId("user2");
+                  setUsername("user2Username");
+                  setFirstName("User2");
+                  setLastName("Two");
+                  setEmail("user2@example.com");
+                  setEnabled(true);
+              }}
+        );
+
+        Mockito.when(this.adminClient.getGroupMembers(groupId))
+              .thenReturn(Mono.just(mockMembers));
+
+        // Act
+        final var result = this.service.getGroupMembers(groupId);
+
+        // Assert
+        StepVerifier.create(result)
+              .expectNextMatches(members -> members.size() == 2
+                    && "user1".equals(members.getFirst().getId())
+                    && "user1@example.com".equals(members.getFirst().getEmail())
+                    && "user2".equals(members.get(1).getId())
+                    && "user2@example.com".equals(members.get(1).getEmail()))
+              .verifyComplete();
+
+        Mockito.verify(this.adminClient).getGroupMembers(groupId);
+    }
 }