From eaa14e71012f520a61306b67f29e758814f66ff7 Mon Sep 17 00:00:00 2001 From: Rui Hu Date: Fri, 6 Jun 2025 16:31:25 +0800 Subject: [PATCH] feat(General): implement consul config permission mapping resolving with fallback to application yam Pull config from consul to resolve permission mapping dynamically if consul is not available or the value is corrupted, use settings from local value in application.yaml ISSUES CLOSED: clevermicro/user-management#23 --- pom.xml | 16 +++ .../config/PermissionMappingConfig.java | 6 + .../controller/AuthController.java | 42 +++--- .../service/PermissionMapLookupService.java | 26 ++++ .../FallbackPermissionMapLookupService.java | 126 ++++++++++++++++++ src/main/resources/application.yml | 15 ++- .../authservice/AuthControllerTest.java | 18 ++- ...allbackPermissionMapLookupServiceTest.java | 120 +++++++++++++++++ 8 files changed, 338 insertions(+), 31 deletions(-) create mode 100644 src/main/java/com/cleverthis/authservice/service/PermissionMapLookupService.java create mode 100644 src/main/java/com/cleverthis/authservice/service/impl/FallbackPermissionMapLookupService.java create mode 100644 src/test/java/com/cleverthis/authservice/service/impl/FallbackPermissionMapLookupServiceTest.java diff --git a/pom.xml b/pom.xml index 211786a..5c316b4 100644 --- a/pom.xml +++ b/pom.xml @@ -30,7 +30,19 @@ 21 com.cleverthis.authservice.AuthServiceApplication + 2024.0.1 + + + + org.springframework.cloud + spring-cloud-dependencies + ${spring-cloud.version} + pom + import + + + org.springframework.boot @@ -56,6 +68,10 @@ reactor-test test + + org.springframework.cloud + spring-cloud-starter-consul-config + diff --git a/src/main/java/com/cleverthis/authservice/config/PermissionMappingConfig.java b/src/main/java/com/cleverthis/authservice/config/PermissionMappingConfig.java index 3232fd9..c904fa9 100644 --- a/src/main/java/com/cleverthis/authservice/config/PermissionMappingConfig.java +++ b/src/main/java/com/cleverthis/authservice/config/PermissionMappingConfig.java @@ -4,7 +4,10 @@ import jakarta.validation.Valid; import jakarta.validation.constraints.NotBlank; import java.util.ArrayList; import java.util.List; +import lombok.AllArgsConstructor; +import lombok.Builder; import lombok.Data; +import lombok.NoArgsConstructor; import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.stereotype.Component; import org.springframework.validation.annotation.Validated; @@ -28,6 +31,9 @@ public class PermissionMappingConfig { * mapping request paths to Keycloak Client IDs. */ @Data + @Builder + @NoArgsConstructor + @AllArgsConstructor public static class Rule { @NotBlank(message = "Path prefix cannot be blank in permission mapping rule") private String pathPrefix; diff --git a/src/main/java/com/cleverthis/authservice/controller/AuthController.java b/src/main/java/com/cleverthis/authservice/controller/AuthController.java index fde3d4a..82c7277 100644 --- a/src/main/java/com/cleverthis/authservice/controller/AuthController.java +++ b/src/main/java/com/cleverthis/authservice/controller/AuthController.java @@ -7,6 +7,7 @@ import com.cleverthis.authservice.dto.TokenResponse; import com.cleverthis.authservice.dto.VerificationResponse; import com.cleverthis.authservice.exception.TokenVerificationException; import com.cleverthis.authservice.service.IdentityManagerService; +import com.cleverthis.authservice.service.PermissionMapLookupService; import jakarta.validation.Valid; import java.net.URI; import java.net.URISyntaxException; @@ -36,7 +37,7 @@ import reactor.core.publisher.Mono; */ @RestController @RequestMapping("/") -@Slf4j +@Slf4j public class AuthController { // --- Constants for Header Names --- @@ -53,13 +54,15 @@ public class AuthController { // private static final String HEADER_X_FORWARDED_PROTO = "X-Forwarded-Proto"; private final IdentityManagerService identityManagerService; - private final PermissionMappingConfig permissionMappingConfig; // Inject mapping config + private final PermissionMapLookupService permissionMapLookupService; @Autowired - public AuthController(IdentityManagerService identityManagerService, - PermissionMappingConfig permissionMappingConfig) { + public AuthController( + IdentityManagerService identityManagerService, + PermissionMapLookupService permissionMapLookupService + ) { this.identityManagerService = identityManagerService; - this.permissionMappingConfig = permissionMappingConfig; + this.permissionMapLookupService = permissionMapLookupService; } /** @@ -270,19 +273,16 @@ public class AuthController { String path = uri.getPath(); // Get path part, e.g., /cleverswarm/api/jobs/123 log.debug("Mapping URI path: {}", path); - for (PermissionMappingConfig.Rule rule : this.permissionMappingConfig.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()); - } + final var pathMatch = this.permissionMapLookupService.matchClientIdByPath(path); + if (pathMatch.isPresent()) { + return pathMatch; } - if (this.permissionMappingConfig.getDefaultKeycloakClientId() != null) { + final var defaultValue = this.permissionMapLookupService.getDefaultKeycloakClientId(); + if (defaultValue != null) { log.debug( - "No specific prefix matched for path '{}'," - + "using default Keycloak Client ID:{}", - path, this.permissionMappingConfig.getDefaultKeycloakClientId()); - return Optional.of(this.permissionMappingConfig.getDefaultKeycloakClientId()); + "No rule matched for path '{}', using default Keycloak Client ID:{}", + path, defaultValue); + return Optional.of(defaultValue); } } catch (URISyntaxException e) { log.error("Invalid URI syntax for X-Forwarded-Uri: {}", fullUriString, e); @@ -293,16 +293,16 @@ public class AuthController { private String extractServiceRelativePath(String fullUriString, String mappedKeycloakClientId) { // Find the rule that gave us this mappedKeycloakClientId to get its pathPrefix - Optional matchedRule = this.permissionMappingConfig.getRules() - .stream().filter(rule -> rule.getKeycloakClientId().equals(mappedKeycloakClientId) - && fullUriString.startsWith(rule.getPathPrefix())) - .findFirst(); + Optional matchedRule = + this.permissionMapLookupService.matchRuleByClientIdAndPath( + mappedKeycloakClientId, fullUriString + ); String pathPrefixToRemove = ""; if (matchedRule.isPresent()) { pathPrefixToRemove = matchedRule.get().getPathPrefix(); } else if (mappedKeycloakClientId - .equals(this.permissionMappingConfig.getDefaultKeycloakClientId())) { + .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 // needs definition. diff --git a/src/main/java/com/cleverthis/authservice/service/PermissionMapLookupService.java b/src/main/java/com/cleverthis/authservice/service/PermissionMapLookupService.java new file mode 100644 index 0000000..4b15363 --- /dev/null +++ b/src/main/java/com/cleverthis/authservice/service/PermissionMapLookupService.java @@ -0,0 +1,26 @@ +package com.cleverthis.authservice.service; + +import com.cleverthis.authservice.config.PermissionMappingConfig; +import java.util.Optional; + +/** + * Service for looking up the permission map. + */ +public interface PermissionMapLookupService { + /** + * The default client id when no rules matched. + * If not provided, might be null or empty. + */ + String getDefaultKeycloakClientId(); + + /** + * Given the path, find the client id of the first rule matching the path prefix. + */ + Optional matchClientIdByPath(String path); + + /** + * Given the client id and path, find the rule that both matches the client id exactly, + * and match the path prefix. + */ + Optional matchRuleByClientIdAndPath(String clientId, String path); +} diff --git a/src/main/java/com/cleverthis/authservice/service/impl/FallbackPermissionMapLookupService.java b/src/main/java/com/cleverthis/authservice/service/impl/FallbackPermissionMapLookupService.java new file mode 100644 index 0000000..c3575e2 --- /dev/null +++ b/src/main/java/com/cleverthis/authservice/service/impl/FallbackPermissionMapLookupService.java @@ -0,0 +1,126 @@ +package com.cleverthis.authservice.service.impl; + +import com.cleverthis.authservice.config.PermissionMappingConfig; +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.Optional; +import java.util.concurrent.atomic.AtomicReference; +import lombok.extern.slf4j.Slf4j; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.scheduling.annotation.EnableScheduling; +import org.springframework.scheduling.annotation.Scheduled; +import org.springframework.stereotype.Service; + +/** + * {@link PermissionMapLookupService} with fallback. + *

+ * It will first look up consul, if not available, + * fallback to {@code application.yaml} + *

+ */ +@Service +@Slf4j +@EnableScheduling +public final class FallbackPermissionMapLookupService implements PermissionMapLookupService { + /** + * The consul kv key for permission mapping. + */ + static final String PERMISSION_MAPPING_CONSUL_KEY = + "cleverthis/clevermicro/auth-service/config/permission-mapping"; + private final PermissionMappingConfig permissionMappingConfig; + private final ConsulClient consulClient; + private final ObjectMapper objectMapper; + + /** + * Create an instance of {@link FallbackPermissionMapLookupService}. + * + * @param permissionMappingConfig the local config, used as fallback. + * @param consulClient the consul client, used for query consul value. + * @param objectMapper for deserializing the value from consul. + */ + @Autowired + public FallbackPermissionMapLookupService( + final PermissionMappingConfig permissionMappingConfig, + final ConsulClient consulClient, + final ObjectMapper objectMapper + ) { + this.permissionMappingConfig = permissionMappingConfig; + this.consulClient = consulClient; + this.objectMapper = objectMapper; + refreshConsul(); // initial refresh + } + + private final AtomicReference consulValue = new AtomicReference<>(); + + @Scheduled(initialDelay = 1000, fixedRate = 1000) + void refreshConsul() { + try { + // clear cache + this.consulValue.set(null); + // fetch value + final var resp = this.consulClient.getKVValue(PERMISSION_MAPPING_CONSUL_KEY); + if (resp == null) { + return; // skip if no response + } + final var kvValue = resp.getValue(); + if (kvValue.getDecodedValue() == null) { + // skip if no value provided + return; + } + try { // try decoding + final var config = this.objectMapper.readValue( + kvValue.getDecodedValue(), PermissionMappingConfig.class); + this.consulValue.set(config); + } catch (final JsonProcessingException ex) { + // got a corrupted value, leaving the cache empty + // will fall back to local value when query + log.error("Corrupted config value from consul for key {}: {}", + PERMISSION_MAPPING_CONSUL_KEY, kvValue.getValue(), ex); + } + } catch (final Throwable ex) { + // catch-all + log.error("Unknown error when updating consul value for permission mapping", ex); + } + } + + PermissionMappingConfig getConfig() { + final var consulValue = this.consulValue.get(); + if (consulValue != null) { + return consulValue; + } else { + return this.permissionMappingConfig; + } + } + + @Override + public String getDefaultKeycloakClientId() { + return getConfig().getDefaultKeycloakClientId(); + } + + @Override + public Optional matchClientIdByPath(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.empty(); + } + + @Override + public Optional 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(); + } +} diff --git a/src/main/resources/application.yml b/src/main/resources/application.yml index abe7d5d..0efb49e 100644 --- a/src/main/resources/application.yml +++ b/src/main/resources/application.yml @@ -5,6 +5,13 @@ server: spring: application: name: auth-service + cloud: + consul: + host: ${CONSUL_HOST:localhost} + port: ${CONSUL_PORT:8500} + config: + import-check: + enabled: false # Keycloak Configuration (adjust values as needed) keycloak: @@ -26,12 +33,8 @@ auth-service: # The Keycloak Client ID is the one you set in Keycloak (e.g., "cleverswarm-app") # The 'pathPrefix' will be matched against the X-Forwarded-Uri rules: - - pathPrefix: "/cleverswarm/" # If X-Forwarded-Uri starts with /cleverswarm/ - keycloakClientId: "cleverswarm-client" # ...then permissions are checked against this Keycloak client - - pathPrefix: "/cleverbrag/" - keycloakClientId: "cleverbrag-client" - - pathPrefix: "/test-service/" - keycloakClientId: "auth-service" + - pathPrefix: "/auth-service/" # If X-Forwarded-Uri starts with /cleverswarm/ + keycloakClientId: "auth-service" # ...then permissions are checked against this Keycloak client # Add more rules as needed # Default Keycloak Client ID if no prefix matches (optional) # defaultKeycloakClientId: "general-api-client" diff --git a/src/test/java/com/cleverthis/authservice/AuthControllerTest.java b/src/test/java/com/cleverthis/authservice/AuthControllerTest.java index 48366c6..5f91933 100644 --- a/src/test/java/com/cleverthis/authservice/AuthControllerTest.java +++ b/src/test/java/com/cleverthis/authservice/AuthControllerTest.java @@ -1,6 +1,6 @@ -package com.cleverthis.authservice; // Assuming correct package +package com.cleverthis.authservice; -import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasItems; import static org.mockito.ArgumentMatchers.eq; @@ -17,6 +17,8 @@ import com.cleverthis.authservice.exception.GlobalExceptionHandler; import com.cleverthis.authservice.exception.LogoutException; import com.cleverthis.authservice.exception.TokenVerificationException; import com.cleverthis.authservice.service.IdentityManagerService; +import com.cleverthis.authservice.service.impl.FallbackPermissionMapLookupService; +import com.ecwid.consul.v1.ConsulClient; import java.util.ArrayList; import java.util.List; import org.junit.jupiter.api.BeforeEach; @@ -26,7 +28,7 @@ import org.mockito.Captor; import org.mockito.Mockito; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.web.reactive.WebFluxTest; -import org.springframework.context.annotation.Import; // Import annotation +import org.springframework.context.annotation.Import; import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; import org.springframework.test.context.bean.override.mockito.MockitoBean; @@ -38,7 +40,10 @@ import reactor.core.publisher.Mono; * controller logic and request/response handling. */ @WebFluxTest(AuthController.class) -@Import({ GlobalExceptionHandler.class, PermissionMappingConfig.class }) class AuthControllerTest { +@Import({GlobalExceptionHandler.class, + PermissionMappingConfig.class, + FallbackPermissionMapLookupService.class}) +class AuthControllerTest { @Autowired private WebTestClient webTestClient; @@ -52,6 +57,11 @@ import reactor.core.publisher.Mono; @Captor private ArgumentCaptor> candidateRolesCaptor; + // required by the fallback permission mapping lookup service + // provide a mocked one + @MockitoBean + private ConsulClient consulClient; + private TokenResponse mockTokenResponse; private VerificationResponse mockUserVerificationResponse; diff --git a/src/test/java/com/cleverthis/authservice/service/impl/FallbackPermissionMapLookupServiceTest.java b/src/test/java/com/cleverthis/authservice/service/impl/FallbackPermissionMapLookupServiceTest.java new file mode 100644 index 0000000..6dc3e92 --- /dev/null +++ b/src/test/java/com/cleverthis/authservice/service/impl/FallbackPermissionMapLookupServiceTest.java @@ -0,0 +1,120 @@ +package com.cleverthis.authservice.service.impl; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.mockito.Mockito.when; + +import com.cleverthis.authservice.config.PermissionMappingConfig; +import com.ecwid.consul.v1.ConsulClient; +import com.ecwid.consul.v1.Response; +import com.ecwid.consul.v1.kv.model.GetValue; +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import java.util.List; +import java.util.Optional; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; + +class FallbackPermissionMapLookupServiceTest { + private final ObjectMapper objectMapper = new ObjectMapper(); + private final ConsulClient consulClient = Mockito.mock(ConsulClient.class); + + /** + * Test {@link FallbackPermissionMapLookupService#refreshConsul()}} + * and {@link FallbackPermissionMapLookupService#getConfig()} + * to see if the refresh and fallback is working. + */ + @Test + void testRefreshConsulAndGetConfigFallBack() throws JsonProcessingException { + final var service = new FallbackPermissionMapLookupService( + null, this.consulClient, this.objectMapper); + Mockito.reset(this.consulClient); + // null response from consul client + service.refreshConsul(); + Mockito.verify(this.consulClient) + .getKVValue(FallbackPermissionMapLookupService.PERMISSION_MAPPING_CONSUL_KEY); + assertNull(service.getConfig()); // fallback is null + // have response but null value + final var resp = Mockito.mock(Response.class); + when(this.consulClient.getKVValue( + FallbackPermissionMapLookupService.PERMISSION_MAPPING_CONSUL_KEY)) + .thenReturn(resp); + service.refreshConsul(); + Mockito.verify(resp).getValue(); + assertNull(service.getConfig()); // fallback is null + // has value but null content + final var value = Mockito.mock(GetValue.class); + when(resp.getValue()).thenReturn(value); + service.refreshConsul(); + Mockito.verify(value).getDecodedValue(); + assertNull(service.getConfig()); // fallback is null + // has content, but not valid json + when(value.getDecodedValue()).thenReturn("not-json"); + Assertions.assertDoesNotThrow(service::refreshConsul); + assertNull(service.getConfig()); // fallback is null + // has json content + when(value.getDecodedValue()) + .thenReturn(this.objectMapper.writeValueAsString(new PermissionMappingConfig())); + service.refreshConsul(); + assertEquals(new PermissionMappingConfig(), service.getConfig()); + } + + /** + * Test {@link FallbackPermissionMapLookupService#getDefaultKeycloakClientId()} + * to ensure it returns the correct default Keycloak client ID when the config is set. + */ + @Test + void testGetDefaultKeycloakClientId() { + final var defaultConfig = new PermissionMappingConfig(); + defaultConfig.setDefaultKeycloakClientId("default-client-id"); + + final var service = new FallbackPermissionMapLookupService( + defaultConfig, this.consulClient, this.objectMapper); + + assertEquals("default-client-id", service.getDefaultKeycloakClientId()); + } + + @Test + void testMatchClientIdByPath() { + final var defaultConfig = new PermissionMappingConfig(); + defaultConfig.setRules(List.of( + PermissionMappingConfig.Rule.builder() + .keycloakClientId("service-A").pathPrefix("/a/").build(), + PermissionMappingConfig.Rule.builder() + .keycloakClientId("service-B").pathPrefix("/b/").build() + )); + final var service = new FallbackPermissionMapLookupService( + defaultConfig, this.consulClient, this.objectMapper); + + assertEquals( + "service-B", + service.matchClientIdByPath("/b/items/123").orElseThrow()); + assertEquals( + Optional.empty(), + service.matchClientIdByPath("/c/items/123")); + } + + @Test + void testMatchRuleByClientIdAndPath() { + final var defaultConfig = new PermissionMappingConfig(); + defaultConfig.setRules(List.of( + PermissionMappingConfig.Rule.builder() + .keycloakClientId("service-A").pathPrefix("/a/").build(), + PermissionMappingConfig.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"), + () -> {} + ); + } +} \ No newline at end of file -- 2.52.0