From 13688cff586ebf3278193aba0c29eb94bb494a19 Mon Sep 17 00:00:00 2001 From: amontenegro Date: Wed, 27 Dec 2023 12:42:37 -0600 Subject: [PATCH] Add unit tests --- .../OrcidAuthorizationCodeTokenGranter.java | 2 +- .../OrcidOauth2TokenDetailServiceImpl.java | 10 +++ .../core/utils/cache/redis/RedisClient.java | 1 - .../orcid/core/manager/WorkManagerTest.java | 6 -- .../OrcidOauth2TokenDetailServiceTest.java | 77 +++++++++++++++++++ .../dao/OrcidOauth2TokenDetailDao.java | 4 +- .../impl/OrcidOauth2TokenDetailDaoImpl.java | 12 ++- 7 files changed, 101 insertions(+), 11 deletions(-) diff --git a/orcid-core/src/main/java/org/orcid/core/oauth/OrcidAuthorizationCodeTokenGranter.java b/orcid-core/src/main/java/org/orcid/core/oauth/OrcidAuthorizationCodeTokenGranter.java index a223c68dce8..6756da51e4f 100644 --- a/orcid-core/src/main/java/org/orcid/core/oauth/OrcidAuthorizationCodeTokenGranter.java +++ b/orcid-core/src/main/java/org/orcid/core/oauth/OrcidAuthorizationCodeTokenGranter.java @@ -84,7 +84,7 @@ protected OAuth2Authentication getOAuth2Authentication(ClientDetails client, Tok OrcidOauth2AuthoriziationCodeDetail codeDetails = orcidOauth2AuthoriziationCodeDetailDao.find(authorizationCode); if(codeDetails == null) { int numDisabled = orcidOauthTokenDetailService.disableAccessTokenByCodeAndClient(authorizationCode, tokenRequest.getClientId(), RevokeReason.AUTH_CODE_REUSED); - if (numDisabled >0){ + if (numDisabled > 0) { throw new InvalidGrantException("Reused authorization code: " + authorizationCode); } throw new InvalidGrantException("Invalid authorization code: " + authorizationCode); diff --git a/orcid-core/src/main/java/org/orcid/core/oauth/service/OrcidOauth2TokenDetailServiceImpl.java b/orcid-core/src/main/java/org/orcid/core/oauth/service/OrcidOauth2TokenDetailServiceImpl.java index d4e48b8bc0d..ab98087e127 100644 --- a/orcid-core/src/main/java/org/orcid/core/oauth/service/OrcidOauth2TokenDetailServiceImpl.java +++ b/orcid-core/src/main/java/org/orcid/core/oauth/service/OrcidOauth2TokenDetailServiceImpl.java @@ -234,6 +234,16 @@ public boolean doesClientKnowUser(String clientId, String userOrcid) { @Override @Transactional public int disableAccessTokenByCodeAndClient(String authorizationCode, String clientID, RevokeReason reason) { + // Find the tokens to disable + List tokensToDisable = orcidOauth2TokenDetailDao.findAccessTokenByCodeAndClient(authorizationCode, clientID); + // Remove them from the cache + for(String accessToken : tokensToDisable) { + LOGGER.info("Token {} will be disabled because auth code {} was reused", accessToken, authorizationCode); + if(isTokenCacheEnabled) { + redisClient.remove(accessToken); + } + } + // Disable them return orcidOauth2TokenDetailDao.disableAccessTokenByCodeAndClient(authorizationCode, clientID, reason.name()); } diff --git a/orcid-core/src/main/java/org/orcid/core/utils/cache/redis/RedisClient.java b/orcid-core/src/main/java/org/orcid/core/utils/cache/redis/RedisClient.java index bc09b7f234d..07c9672ba45 100644 --- a/orcid-core/src/main/java/org/orcid/core/utils/cache/redis/RedisClient.java +++ b/orcid-core/src/main/java/org/orcid/core/utils/cache/redis/RedisClient.java @@ -11,7 +11,6 @@ import org.orcid.utils.alerting.SlackManager; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.springframework.beans.factory.annotation.Value; import redis.clients.jedis.DefaultJedisClientConfig; import redis.clients.jedis.HostAndPort; diff --git a/orcid-core/src/test/java/org/orcid/core/manager/WorkManagerTest.java b/orcid-core/src/test/java/org/orcid/core/manager/WorkManagerTest.java index 26189e200e3..9242c477939 100644 --- a/orcid-core/src/test/java/org/orcid/core/manager/WorkManagerTest.java +++ b/orcid-core/src/test/java/org/orcid/core/manager/WorkManagerTest.java @@ -26,14 +26,12 @@ import org.junit.AfterClass; import org.junit.Before; import org.junit.BeforeClass; -import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.Mockito; import org.orcid.core.BaseTest; import org.orcid.core.exception.ExceedMaxNumberOfPutCodesException; -import org.orcid.core.togglz.Features; import org.orcid.core.utils.DateFieldsOnBaseEntityUtils; import org.orcid.jaxb.model.common_v2.Country; import org.orcid.jaxb.model.common_v2.CreatedDate; @@ -74,7 +72,6 @@ import org.springframework.beans.factory.annotation.Value; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.util.ReflectionTestUtils; -import org.togglz.junit.TogglzRule; @RunWith(OrcidJUnit4ClassRunner.class) @ContextConfiguration(locations = { "classpath:test-orcid-core-context.xml" }) @@ -105,9 +102,6 @@ public class WorkManagerTest extends BaseTest { @Value("${org.orcid.core.work.contributors.ui.max:50}") private int maxContributorsForUI; - @Rule - public TogglzRule togglzRule = TogglzRule.allDisabled(Features.class); - @BeforeClass public static void initDBUnitData() throws Exception { initDBUnitData(DATA_FILES); diff --git a/orcid-core/src/test/java/org/orcid/core/oauth/service/OrcidOauth2TokenDetailServiceTest.java b/orcid-core/src/test/java/org/orcid/core/oauth/service/OrcidOauth2TokenDetailServiceTest.java index b0d6a187c6a..6982f76b55a 100644 --- a/orcid-core/src/test/java/org/orcid/core/oauth/service/OrcidOauth2TokenDetailServiceTest.java +++ b/orcid-core/src/test/java/org/orcid/core/oauth/service/OrcidOauth2TokenDetailServiceTest.java @@ -7,9 +7,12 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import java.util.Arrays; import java.util.Date; @@ -19,16 +22,22 @@ import javax.persistence.NoResultException; import org.junit.AfterClass; +import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.orcid.core.common.manager.EmailFrequencyManager; import org.orcid.core.constants.RevokeReason; import org.orcid.core.oauth.OrcidOauth2TokenDetailService; +import org.orcid.core.utils.cache.redis.RedisClient; import org.orcid.persistence.dao.OrcidOauth2TokenDetailDao; import org.orcid.persistence.jpa.entities.OrcidOauth2TokenDetail; import org.orcid.persistence.jpa.entities.ProfileEntity; import org.orcid.test.DBUnitTest; import org.orcid.test.OrcidJUnit4ClassRunner; +import org.orcid.test.TargetProxyHelper; import org.springframework.test.context.ContextConfiguration; /** @@ -50,12 +59,23 @@ public class OrcidOauth2TokenDetailServiceTest extends DBUnitTest { @Resource(name="orcidOauth2TokenDetailDao") private OrcidOauth2TokenDetailDao orcidOauth2TokenDetailDao; + @Mock + private RedisClient redisClientMock; + @BeforeClass public static void initDBUnitData() throws Exception { initDBUnitData(Arrays.asList("/data/SubjectEntityData.xml", "/data/SourceClientDetailsEntityData.xml", "/data/ProfileEntityData.xml")); } + @Before + public void before() { + MockitoAnnotations.initMocks(this); + // Enable the cache + TargetProxyHelper.injectIntoProxy(orcidOauth2TokenDetailService, "isTokenCacheEnabled", true); + TargetProxyHelper.injectIntoProxy(orcidOauth2TokenDetailService, "redisClient", redisClientMock); + } + @AfterClass public static void removeDBUnitData() throws Exception { removeDBUnitData(Arrays.asList("/data/ProfileEntityData.xml", "/data/SourceClientDetailsEntityData.xml", @@ -216,6 +236,57 @@ public void disableAccessTokenByUserOrcidTest() { } } + @Test + public void disableAccessTokenByCodeAndClientTest() { + Date date = new Date(System.currentTimeMillis() + 100000); + String authCode = "auth-code-1"; + OrcidOauth2TokenDetail dbt1 = createToken(CLIENT_ID_1, "token-1", USER_ORCID, date, "/activities/update", false, authCode); + OrcidOauth2TokenDetail dbt2 = createToken(CLIENT_ID_1, "token-2", USER_ORCID, date, "/activities/update", false, authCode); + OrcidOauth2TokenDetail dbt3 = createToken(CLIENT_ID_1, "token-3", USER_ORCID, date, "/activities/update", false, authCode); + OrcidOauth2TokenDetail dbt4 = createToken(CLIENT_ID_1, "token-4", USER_ORCID_2, date, "/activities/update", false, authCode); + OrcidOauth2TokenDetail dbt5 = createToken(CLIENT_ID_2, "token-5", USER_ORCID_3, date, "/activities/update", false, authCode); + OrcidOauth2TokenDetail dbt6 = createToken(CLIENT_ID_2, "token-6", USER_ORCID, date, "/activities/update", false, authCode); + + // Disable tokens with authCode and CLIENT_ID_1 + orcidOauth2TokenDetailService.disableAccessTokenByCodeAndClient(authCode, CLIENT_ID_1, RevokeReason.AUTH_CODE_REUSED); + + verify(redisClientMock, times(1)).remove("token-1"); + verify(redisClientMock, times(1)).remove("token-2"); + verify(redisClientMock, times(1)).remove("token-3"); + verify(redisClientMock, times(1)).remove("token-4"); + + // Tokens 1, 2, 3 and 4 should be revoked + OrcidOauth2TokenDetail t1 = orcidOauth2TokenDetailService.findIgnoringDisabledByTokenValue("token-1"); + assertNotNull(t1.getRevocationDate()); + assertEquals(RevokeReason.AUTH_CODE_REUSED.toString(), t1.getRevokeReason()); + OrcidOauth2TokenDetail t2 = orcidOauth2TokenDetailService.findIgnoringDisabledByTokenValue("token-2"); + assertNotNull(t2.getRevocationDate()); + assertEquals(RevokeReason.AUTH_CODE_REUSED.toString(), t2.getRevokeReason()); + OrcidOauth2TokenDetail t3 = orcidOauth2TokenDetailService.findIgnoringDisabledByTokenValue("token-3"); + assertNotNull(t3.getRevocationDate()); + assertEquals(RevokeReason.AUTH_CODE_REUSED.toString(), t3.getRevokeReason()); + // This case is never possible, the client used the same auth code to create a token on other user + OrcidOauth2TokenDetail t4 = orcidOauth2TokenDetailService.findIgnoringDisabledByTokenValue("token-4"); + assertNotNull(t4.getRevocationDate()); + assertEquals(RevokeReason.AUTH_CODE_REUSED.toString(), t4.getRevokeReason()); + + // Tokens 5 and 6 should be active + OrcidOauth2TokenDetail t5 = orcidOauth2TokenDetailService.findIgnoringDisabledByTokenValue("token-5"); + assertNull(t5.getRevocationDate()); + assertNull(t5.getRevokeReason()); + OrcidOauth2TokenDetail t6 = orcidOauth2TokenDetailService.findIgnoringDisabledByTokenValue("token-6"); + assertNull(t6.getRevocationDate()); + assertNull(t6.getRevokeReason()); + + // Cleanup + orcidOauth2TokenDetailDao.remove(dbt1.getId()); + orcidOauth2TokenDetailDao.remove(dbt2.getId()); + orcidOauth2TokenDetailDao.remove(dbt3.getId()); + orcidOauth2TokenDetailDao.remove(dbt4.getId()); + orcidOauth2TokenDetailDao.remove(dbt5.getId()); + orcidOauth2TokenDetailDao.remove(dbt6.getId()); + } + @Test public void updateScopesTest() { String tokenValue = "TOKEN123"; @@ -261,6 +332,10 @@ public void updateScopesTest() { } private OrcidOauth2TokenDetail createToken(String clientId, String tokenValue, String userOrcid, Date expirationDate, String scopes, boolean disabled) { + return createToken(clientId, tokenValue, userOrcid, expirationDate, scopes, disabled, null); + } + + private OrcidOauth2TokenDetail createToken(String clientId, String tokenValue, String userOrcid, Date expirationDate, String scopes, boolean disabled, String authCode) { OrcidOauth2TokenDetail token = new OrcidOauth2TokenDetail(); token.setApproved(true); token.setClientDetailsId(clientId); @@ -270,7 +345,9 @@ private OrcidOauth2TokenDetail createToken(String clientId, String tokenValue, S token.setTokenExpiration(expirationDate); token.setTokenType("bearer"); token.setTokenValue(tokenValue); + token.setAuthorizationCode(authCode); orcidOauth2TokenDetailDao.persist(token); return token; } + } diff --git a/orcid-persistence/src/main/java/org/orcid/persistence/dao/OrcidOauth2TokenDetailDao.java b/orcid-persistence/src/main/java/org/orcid/persistence/dao/OrcidOauth2TokenDetailDao.java index d2240760f0e..95ca4ef8fc8 100644 --- a/orcid-persistence/src/main/java/org/orcid/persistence/dao/OrcidOauth2TokenDetailDao.java +++ b/orcid-persistence/src/main/java/org/orcid/persistence/dao/OrcidOauth2TokenDetailDao.java @@ -37,7 +37,9 @@ public interface OrcidOauth2TokenDetailDao extends GenericDao findAccessTokenByCodeAndClient(String authorizationCode, String clientId); void disableAccessTokenByUserOrcid(String userOrcid, String reason); diff --git a/orcid-persistence/src/main/java/org/orcid/persistence/dao/impl/OrcidOauth2TokenDetailDaoImpl.java b/orcid-persistence/src/main/java/org/orcid/persistence/dao/impl/OrcidOauth2TokenDetailDaoImpl.java index 9f4c7dad529..c6cdd0c1ccf 100644 --- a/orcid-persistence/src/main/java/org/orcid/persistence/dao/impl/OrcidOauth2TokenDetailDaoImpl.java +++ b/orcid-persistence/src/main/java/org/orcid/persistence/dao/impl/OrcidOauth2TokenDetailDaoImpl.java @@ -169,8 +169,16 @@ public int disableAccessTokenByCodeAndClient(String authorizationCode, String cl query.setParameter("authorizationCode", authorizationCode); query.setParameter("clientId", clientId); query.setParameter("reason", reason); - int count = query.executeUpdate(); - return count; + return query.executeUpdate(); + } + + @Override + public List findAccessTokenByCodeAndClient(String authorizationCode, String clientId) { + Query query = entityManager.createQuery("select tokenValue from OrcidOauth2TokenDetail where clientDetailsId = :clientId and authorizationCode = :authorizationCode"); + query.setParameter("authorizationCode", authorizationCode); + query.setParameter("clientId", clientId); + + return query.getResultList(); } @Override