Skip to content

Commit

Permalink
Add unit tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@amontenegro
amontenegro committed Dec 27, 2023
1 parent 4a7a617 commit 13688cf
Show file tree
Hide file tree
Showing 7 changed files with 101 additions and 11 deletions.
2 changes: 1 addition & 1 deletion orcid-core/src/main/java/org/orcid/core/oauth/OrcidAuthorizationCodeTokenGranter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
10 changes: 10 additions & 0 deletions orcid-core/src/main/java/org/orcid/core/oauth/service/OrcidOauth2TokenDetailServiceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> 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());
}

Expand Down
1 change: 0 additions & 1 deletion orcid-core/src/main/java/org/orcid/core/utils/cache/redis/RedisClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 0 additions & 6 deletions orcid-core/src/test/java/org/orcid/core/manager/WorkManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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" })
Expand Down Expand Up @@ -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);
Expand Down
77 changes: 77 additions & 0 deletions orcid-core/src/test/java/org/orcid/core/oauth/service/OrcidOauth2TokenDetailServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

/**
Expand All @@ -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",
Expand Down Expand Up @@ -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";
Expand Down Expand Up @@ -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);
Expand All @@ -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;
}

}
4 changes: 3 additions & 1 deletion orcid-persistence/src/main/java/org/orcid/persistence/dao/OrcidOauth2TokenDetailDao.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@ public interface OrcidOauth2TokenDetailDao extends GenericDao<OrcidOauth2TokenDe

boolean hasToken(String userName);

int disableAccessTokenByCodeAndClient(String authorizationCode, String clientID, String reason);
int disableAccessTokenByCodeAndClient(String authorizationCode, String clientId, String reason);

List<String> findAccessTokenByCodeAndClient(String authorizationCode, String clientId);

void disableAccessTokenByUserOrcid(String userOrcid, String reason);

Expand Down
12 changes: 10 additions & 2 deletions ...rsistence/src/main/java/org/orcid/persistence/dao/impl/OrcidOauth2TokenDetailDaoImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> 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
Expand Down

0 comments on commit 13688cf

Please sign in to comment.