diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/api/client/service/DefaultClientService.java b/iam-login-service/src/main/java/it/infn/mw/iam/api/client/service/DefaultClientService.java index 1f6383cb0..b28e54efd 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/api/client/service/DefaultClientService.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/api/client/service/DefaultClientService.java @@ -23,12 +23,10 @@ import org.mitre.oauth2.model.ClientDetailsEntity; import org.mitre.oauth2.model.OAuth2AccessTokenEntity; import org.mitre.oauth2.service.OAuth2TokenEntityService; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.cache.annotation.CacheEvict; import org.springframework.context.ApplicationEventPublisher; import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; -import org.springframework.security.oauth2.provider.OAuth2RequestValidator; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; @@ -42,7 +40,6 @@ @Service @Transactional -@SuppressWarnings("deprecation") public class DefaultClientService implements ClientService { private final Clock clock; @@ -55,10 +52,8 @@ public class DefaultClientService implements ClientService { private OAuth2TokenEntityService tokenService; - @Autowired public DefaultClientService(Clock clock, IamClientRepository clientRepo, - IamAccountClientRepository accountClientRepo, ApplicationEventPublisher eventPublisher, - OAuth2RequestValidator requestValidator, OAuth2TokenEntityService tokenService) { + IamAccountClientRepository accountClientRepo, ApplicationEventPublisher eventPublisher, OAuth2TokenEntityService tokenService) { this.clock = clock; this.clientRepo = clientRepo; this.accountClientRepo = accountClientRepo; diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/config/CacheConfig.java b/iam-login-service/src/main/java/it/infn/mw/iam/config/CacheConfig.java index b50fad3a9..b212c1197 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/config/CacheConfig.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/config/CacheConfig.java @@ -16,9 +16,10 @@ package it.infn.mw.iam.config; import org.springframework.boot.autoconfigure.cache.RedisCacheManagerBuilderCustomizer; -import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.boot.autoconfigure.condition.ConditionalOnExpression; import org.springframework.cache.CacheManager; import org.springframework.cache.concurrent.ConcurrentMapCacheManager; +import org.springframework.cache.support.NoOpCacheManager; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.data.redis.cache.RedisCacheConfiguration; @@ -30,26 +31,31 @@ public class CacheConfig { @Bean - @ConditionalOnProperty(name = "redis-cache.enabled", havingValue = "false") - public CacheManager localCacheManager() { + @ConditionalOnExpression("${cache.enabled} == false") + CacheManager fakeCacheManager(CacheProperties props) { + return new NoOpCacheManager(); + } + + @Bean + @ConditionalOnExpression("${cache.enabled} == true and ${cache.redis.enabled} == false") + CacheManager localCacheManager(CacheProperties props) { return new ConcurrentMapCacheManager(IamWellKnownInfoProvider.CACHE_KEY, DefaultScopeMatcherRegistry.SCOPE_CACHE_KEY); } @Bean - @ConditionalOnProperty(name = "redis-cache.enabled", havingValue = "true") - public RedisCacheManagerBuilderCustomizer redisCacheManagerBuilderCustomizer() { + @ConditionalOnExpression("${cache.enabled} == true and ${cache.redis.enabled} == true") + RedisCacheManagerBuilderCustomizer redisCacheManagerBuilderCustomizer() { return builder -> builder .withCacheConfiguration(IamWellKnownInfoProvider.CACHE_KEY, RedisCacheConfiguration.defaultCacheConfig()) .withCacheConfiguration(DefaultScopeMatcherRegistry.SCOPE_CACHE_KEY, RedisCacheConfiguration.defaultCacheConfig()); - } @Bean - @ConditionalOnProperty(name = "redis-cache.enabled", havingValue = "true") - public RedisCacheConfiguration redisCacheConfiguration() { + @ConditionalOnExpression("${cache.enabled} == true and ${cache.redis.enabled} == true") + RedisCacheConfiguration redisCacheConfiguration() { return RedisCacheConfiguration.defaultCacheConfig().disableCachingNullValues(); } diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/config/RedisCacheProperties.java b/iam-login-service/src/main/java/it/infn/mw/iam/config/CacheProperties.java similarity index 65% rename from iam-login-service/src/main/java/it/infn/mw/iam/config/RedisCacheProperties.java rename to iam-login-service/src/main/java/it/infn/mw/iam/config/CacheProperties.java index 1751a2579..e8ffd455d 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/config/RedisCacheProperties.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/config/CacheProperties.java @@ -18,11 +18,27 @@ import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.context.annotation.Configuration; -@ConfigurationProperties("redis-cache") +@ConfigurationProperties("cache") @Configuration -public class RedisCacheProperties { +public class CacheProperties { - private boolean enabled = false; + public class RedisProperties { + + private boolean enabled = false; + + public boolean isEnabled() { + return enabled; + } + + public void setEnabled(boolean enable) { + this.enabled = enable; + } + + } + + private boolean enabled = true; + + private RedisProperties redis = new RedisProperties(); public boolean isEnabled() { return enabled; @@ -32,4 +48,13 @@ public void setEnabled(boolean enable) { this.enabled = enable; } + public RedisProperties getRedis() { + return redis; + } + + public void setRedis(RedisProperties redis) { + this.redis = redis; + } + + } diff --git a/iam-login-service/src/main/java/it/infn/mw/iam/core/web/wellknown/IamWellKnownInfoProvider.java b/iam-login-service/src/main/java/it/infn/mw/iam/core/web/wellknown/IamWellKnownInfoProvider.java index 9254a1551..05c60d158 100644 --- a/iam-login-service/src/main/java/it/infn/mw/iam/core/web/wellknown/IamWellKnownInfoProvider.java +++ b/iam-login-service/src/main/java/it/infn/mw/iam/core/web/wellknown/IamWellKnownInfoProvider.java @@ -164,7 +164,7 @@ private String getIssuerWithTrailingSlash() { } @Override - @Cacheable(CACHE_KEY) + @Cacheable(value = CACHE_KEY) public Map getWellKnownInfo() { Map result = newHashMap(); diff --git a/iam-login-service/src/main/resources/application-redis-test.yml b/iam-login-service/src/main/resources/application-redis-test.yml index 9a5b06f54..6fad69368 100644 --- a/iam-login-service/src/main/resources/application-redis-test.yml +++ b/iam-login-service/src/main/resources/application-redis-test.yml @@ -25,5 +25,6 @@ spring: flush-mode: immediate namespace: iam:session -redis-cache: - enabled: true \ No newline at end of file +cache: + redis: + enabled: true \ No newline at end of file diff --git a/iam-login-service/src/main/resources/application.yml b/iam-login-service/src/main/resources/application.yml index 0e126065b..76083b81d 100644 --- a/iam-login-service/src/main/resources/application.yml +++ b/iam-login-service/src/main/resources/application.yml @@ -191,8 +191,10 @@ iam: client: track-last-used: ${IAM_CLIENT_TRACK_LAST_USED:false} -redis-cache: - enabled: ${IAM_REDIS_CACHE_ENABLED:false} +cache: + enabled: ${IAM_CACHE_ENABLED:true} + redis: + enabled: ${IAM_CACHE_REDIS_ENABLED:false} x509: trustAnchorsDir: ${IAM_X509_TRUST_ANCHORS_DIR:/etc/grid-security/certificates} diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/EndpointsTestUtils.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/EndpointsTestUtils.java index 7f28b6620..81aaf7146 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/EndpointsTestUtils.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/EndpointsTestUtils.java @@ -22,6 +22,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.security.oauth2.common.DefaultOAuth2AccessToken; import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.MvcResult; import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder; import com.fasterxml.jackson.databind.ObjectMapper; @@ -103,7 +104,14 @@ public AccessTokenGetter audience(String audience) { return this; } - public String performTokenRequest() throws Exception { + public String performSuccessfulTokenRequest() throws Exception { + + return performTokenRequest(200) + .getResponse() + .getContentAsString(); + } + + public MvcResult performTokenRequest(int statusCode) throws Exception { MockHttpServletRequestBuilder req = post("/token").param("grant_type", grantType) .param("client_id", clientId) .param("client_secret", clientSecret); @@ -120,18 +128,14 @@ public String performTokenRequest() throws Exception { req.param("aud", audience); } - String response = mvc.perform(req) - .andExpect(status().isOk()) - .andReturn() - .getResponse() - .getContentAsString(); - - return response; + return mvc.perform(req) + .andExpect(status().is(statusCode)) + .andReturn(); } public DefaultOAuth2AccessToken getTokenResponseObject() throws Exception { - String response = performTokenRequest(); + String response = performSuccessfulTokenRequest(); // This is incorrectly named in spring security OAuth, what they call OAuth2AccessToken // is a TokenResponse object diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/scope/matchers/ScopeMatcherCacheTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/scope/matchers/ScopeMatcherCacheTests.java index 2ba38ac82..0d78278b0 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/scope/matchers/ScopeMatcherCacheTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/scope/matchers/ScopeMatcherCacheTests.java @@ -28,6 +28,7 @@ import org.mitre.oauth2.model.ClientDetailsEntity; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.cache.CacheManager; +import org.springframework.cache.concurrent.ConcurrentMapCacheManager; import org.springframework.test.context.TestPropertySource; import org.springframework.test.context.junit4.SpringRunner; @@ -36,8 +37,8 @@ import com.nimbusds.jwt.JWTParser; import it.infn.mw.iam.api.client.service.ClientService; -import it.infn.mw.iam.config.CacheConfig; -import it.infn.mw.iam.config.RedisCacheProperties; +import it.infn.mw.iam.config.CacheProperties; +import it.infn.mw.iam.persistence.repository.client.IamClientRepository; import it.infn.mw.iam.test.oauth.EndpointsTestUtils; import it.infn.mw.iam.test.util.annotation.IamMockMvcIntegrationTest; @@ -53,10 +54,13 @@ public class ScopeMatcherCacheTests extends EndpointsTestUtils { private ClientService clientService; @Autowired - private CacheConfig cacheConfig; - + private IamClientRepository clientRepository; + + @Autowired + private CacheManager localCacheManager; + @Autowired - private RedisCacheProperties redisCacheProperties; + private CacheProperties cacheProperties; private String getAccessTokenForClient(String scopes) throws Exception { @@ -67,10 +71,19 @@ private String getAccessTokenForClient(String scopes) throws Exception { .getAccessTokenValue(); } + private void getAccessTokenForClientFailWithStatusCode(String scopes, int statusCode) throws Exception { + + new AccessTokenGetter().grantType("client_credentials") + .clientId(CLIENT_ID) + .clientSecret(CLIENT_SECRET) + .scope(scopes) + .performTokenRequest(statusCode); + } + @Test - public void ensureRedisCashIsDisabled() { - assertFalse(redisCacheProperties.isEnabled()); - assertThat(cacheConfig.localCacheManager(), instanceOf(CacheManager.class)); + public void ensureRedisCacheIsDisabled() { + assertFalse(cacheProperties.getRedis().isEnabled()); + assertThat(localCacheManager, instanceOf(ConcurrentMapCacheManager.class)); } @Test @@ -87,6 +100,8 @@ public void updatingClientScopesInvalidatesCache() throws ParseException, Except assertThat("scim:read", not(in(token.getJWTClaimsSet().getClaim("scope").toString().split(" ")))); client.setScope(Sets.newHashSet("openid", "profile", "email", "scim:read")); + clientRepository.save(client); + getAccessTokenForClientFailWithStatusCode("openid profile email scim:read", 400); clientService.updateClient(client); token = JWTParser.parse(getAccessTokenForClient("openid profile email scim:read")); assertThat("scim:read", in(token.getJWTClaimsSet().getClaim("scope").toString().split(" "))); @@ -94,4 +109,5 @@ public void updatingClientScopesInvalidatesCache() throws ParseException, Except clientService.deleteClient(client); } } + } diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/scope/matchers/ScopeMatcherExternalCacheTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/scope/matchers/ScopeMatcherExternalCacheTests.java index efcedaa85..56af564ff 100644 --- a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/scope/matchers/ScopeMatcherExternalCacheTests.java +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/scope/matchers/ScopeMatcherExternalCacheTests.java @@ -27,11 +27,11 @@ import org.junit.jupiter.api.Test; import org.mitre.oauth2.model.ClientDetailsEntity; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.boot.autoconfigure.cache.RedisCacheManagerBuilderCustomizer; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.context.SpringBootTest.WebEnvironment; import org.springframework.boot.web.server.LocalServerPort; -import org.springframework.data.redis.cache.RedisCacheConfiguration; +import org.springframework.cache.CacheManager; +import org.springframework.data.redis.cache.RedisCacheManager; import org.springframework.test.context.DynamicPropertyRegistry; import org.springframework.test.context.DynamicPropertySource; import org.testcontainers.junit.jupiter.Container; @@ -44,8 +44,7 @@ import io.restassured.RestAssured; import it.infn.mw.iam.api.client.service.ClientService; -import it.infn.mw.iam.config.CacheConfig; -import it.infn.mw.iam.config.RedisCacheProperties; +import it.infn.mw.iam.config.CacheProperties; import it.infn.mw.iam.test.TestUtils; import it.infn.mw.iam.test.oauth.EndpointsTestUtils; import it.infn.mw.iam.test.util.annotation.IamMockMvcIntegrationTest; @@ -54,7 +53,7 @@ @Testcontainers @IamMockMvcIntegrationTest @SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT, - properties = {"iam.access_token.include_scope=true", "redis-cache.enabled=true"}) + properties = {"iam.access_token.include_scope=true", "cache.redis.enabled=true"}) public class ScopeMatcherExternalCacheTests extends EndpointsTestUtils { private static final String CLIENT_ID = "cache-client"; @@ -68,10 +67,7 @@ public class ScopeMatcherExternalCacheTests extends EndpointsTestUtils { private ClientService clientService; @Autowired - private CacheConfig cacheConfig; - - @Autowired - private RedisCacheProperties redisCacheProperties; + private CacheProperties redisCacheProperties; @LocalServerPort private Integer iamPort; @@ -79,6 +75,9 @@ public class ScopeMatcherExternalCacheTests extends EndpointsTestUtils { @Autowired ObjectMapper mapper; + @Autowired + CacheManager cacheManager; + @Container private static final RedisContainer REDIS = new RedisContainer(); @@ -93,10 +92,7 @@ public void setup() { TestUtils.initRestAssured(); RestAssured.port = iamPort; assertTrue(redisCacheProperties.isEnabled()); - assertThat(cacheConfig.redisCacheConfiguration(), instanceOf(RedisCacheConfiguration.class)); - assertThat(cacheConfig.redisCacheManagerBuilderCustomizer(), - instanceOf(RedisCacheManagerBuilderCustomizer.class)); - + assertThat(cacheManager, instanceOf(RedisCacheManager.class)); } private String getAccessTokenForClient(String scopes) throws Exception { @@ -106,7 +102,6 @@ private String getAccessTokenForClient(String scopes) throws Exception { .clientSecret(CLIENT_SECRET) .scope(scopes) .getAccessTokenValue(); - } @Test diff --git a/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/scope/matchers/ScopeMatcherNoCacheTests.java b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/scope/matchers/ScopeMatcherNoCacheTests.java new file mode 100644 index 000000000..85ad1a3a4 --- /dev/null +++ b/iam-login-service/src/test/java/it/infn/mw/iam/test/oauth/scope/matchers/ScopeMatcherNoCacheTests.java @@ -0,0 +1,93 @@ +/** + * Copyright (c) Istituto Nazionale di Fisica Nucleare (INFN). 2016-2021 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package it.infn.mw.iam.test.oauth.scope.matchers; + +import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.in; +import static org.hamcrest.Matchers.not; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mitre.oauth2.model.ClientDetailsEntity; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.cache.CacheManager; +import org.springframework.cache.concurrent.ConcurrentMapCacheManager; +import org.springframework.cache.support.NoOpCacheManager; +import org.springframework.data.redis.cache.RedisCacheManager; +import org.springframework.test.context.TestPropertySource; +import org.springframework.test.context.junit4.SpringRunner; + +import com.google.common.collect.Sets; +import com.nimbusds.jwt.JWT; +import com.nimbusds.jwt.JWTParser; + +import it.infn.mw.iam.persistence.repository.client.IamClientRepository; +import it.infn.mw.iam.test.oauth.EndpointsTestUtils; +import it.infn.mw.iam.test.util.annotation.IamMockMvcIntegrationTest; + +@RunWith(SpringRunner.class) +@IamMockMvcIntegrationTest +@TestPropertySource(properties = {"cache.enabled=false", "iam.access_token.include_scope=true"}) +public class ScopeMatcherNoCacheTests extends EndpointsTestUtils { + + private static final String CLIENT_ID = "cache-client"; + private static final String CLIENT_SECRET = "secret"; + + @Autowired + private IamClientRepository clientRepo; + + @Autowired + private CacheManager cacheManager; + + private String getAccessTokenForClient(String scopes) throws Exception { + + return new AccessTokenGetter().grantType("client_credentials") + .clientId(CLIENT_ID) + .clientSecret(CLIENT_SECRET) + .scope(scopes) + .getAccessTokenValue(); + } + + @Test + public void ensureRedisCacheIsDisabled() { + assertThat(cacheManager, instanceOf(NoOpCacheManager.class)); + assertThat(cacheManager, not(instanceOf(ConcurrentMapCacheManager.class))); + assertThat(cacheManager, not(instanceOf(RedisCacheManager.class))); + } + + @Test + public void updatingClientScopesWithNoCache() throws Exception { + + ClientDetailsEntity client = new ClientDetailsEntity(); + client.setClientId(CLIENT_ID); + client.setClientSecret(CLIENT_SECRET); + client.setScope(Sets.newHashSet("openid", "profile", "email")); + clientRepo.save(client); + + try { + JWT token = JWTParser.parse(getAccessTokenForClient("openid profile email")); + assertThat("scim:read", + not(in(token.getJWTClaimsSet().getClaim("scope").toString().split(" ")))); + client.setScope(Sets.newHashSet("openid", "profile", "email", "scim:read")); + clientRepo.save(client); + token = JWTParser.parse(getAccessTokenForClient("openid profile email scim:read")); + assertThat("scim:read", in(token.getJWTClaimsSet().getClaim("scope").toString().split(" "))); + } finally { + clientRepo.delete(client); + } + } +}