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