From 29ec962d010000f5de44eb562eefee485fc6a0e4 Mon Sep 17 00:00:00 2001 From: Enrico Vianello Date: Wed, 29 Nov 2023 20:00:12 +0100 Subject: [PATCH] Removed AT expiration in case of infinite token Bump version to 1.3.6-cnaf-20231129 --- Jenkinsfile | 16 +--- openid-connect-client/pom.xml | 2 +- openid-connect-common/pom.xml | 2 +- .../service/OAuth2TokenEntityService.java | 39 ++++---- openid-connect-server/pom.xml | 2 +- .../DefaultOAuth2ProviderTokenService.java | 22 ++--- ...TestDefaultOAuth2ProviderTokenService.java | 90 +++++++++++-------- pom.xml | 2 +- 8 files changed, 91 insertions(+), 84 deletions(-) diff --git a/Jenkinsfile b/Jenkinsfile index c6b558403..3b8ec1c6b 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -15,7 +15,7 @@ pipeline { stage('deploy') { steps { - sh "mvn -U -B clean deploy" + sh "mvn -U -B clean package deploy" } } @@ -27,18 +27,4 @@ pipeline { } } } - - post { - failure { - slackSend color: 'danger', message: "${env.JOB_NAME} - #${env.BUILD_NUMBER} Failure (<${env.BUILD_URL}|Open>)" - } - - changed { - script{ - if('SUCCESS'.equals(currentBuild.result)) { - slackSend color: 'good', message: "${env.JOB_NAME} - #${env.BUILD_NUMBER} Back to normal (<${env.BUILD_URL}|Open>)" - } - } - } - } } diff --git a/openid-connect-client/pom.xml b/openid-connect-client/pom.xml index efacd78c7..309c9e495 100644 --- a/openid-connect-client/pom.xml +++ b/openid-connect-client/pom.xml @@ -22,7 +22,7 @@ openid-connect-parent org.mitre - 1.3.6.cnaf-20231113 + 1.3.6.cnaf-20231129 .. openid-connect-client diff --git a/openid-connect-common/pom.xml b/openid-connect-common/pom.xml index 1da716777..836dc30eb 100644 --- a/openid-connect-common/pom.xml +++ b/openid-connect-common/pom.xml @@ -22,7 +22,7 @@ openid-connect-parent org.mitre - 1.3.6.cnaf-20231113 + 1.3.6.cnaf-20231129 .. openid-connect-common diff --git a/openid-connect-common/src/main/java/org/mitre/oauth2/service/OAuth2TokenEntityService.java b/openid-connect-common/src/main/java/org/mitre/oauth2/service/OAuth2TokenEntityService.java index c39ccd90d..bec398f9c 100644 --- a/openid-connect-common/src/main/java/org/mitre/oauth2/service/OAuth2TokenEntityService.java +++ b/openid-connect-common/src/main/java/org/mitre/oauth2/service/OAuth2TokenEntityService.java @@ -27,37 +27,40 @@ import org.springframework.security.oauth2.provider.OAuth2Authentication; import org.springframework.security.oauth2.provider.token.AuthorizationServerTokenServices; import org.springframework.security.oauth2.provider.token.ResourceServerTokenServices; -public interface OAuth2TokenEntityService extends AuthorizationServerTokenServices, ResourceServerTokenServices { +@SuppressWarnings("deprecation") +public interface OAuth2TokenEntityService + extends AuthorizationServerTokenServices, ResourceServerTokenServices { - @Override - public OAuth2AccessTokenEntity readAccessToken(String accessTokenValue); + @Override + public OAuth2AccessTokenEntity readAccessToken(String accessTokenValue); - public OAuth2RefreshTokenEntity getRefreshToken(String refreshTokenValue); + public OAuth2RefreshTokenEntity getRefreshToken(String refreshTokenValue); - public void revokeRefreshToken(OAuth2RefreshTokenEntity refreshToken); + public void revokeRefreshToken(OAuth2RefreshTokenEntity refreshToken); - public void revokeAccessToken(OAuth2AccessTokenEntity accessToken); + public void revokeAccessToken(OAuth2AccessTokenEntity accessToken); - public List getAccessTokensForClient(ClientDetailsEntity client); + public List getAccessTokensForClient(ClientDetailsEntity client); - public List getRefreshTokensForClient(ClientDetailsEntity client); + public List getRefreshTokensForClient(ClientDetailsEntity client); - public void clearExpiredTokens(); + public void clearExpiredTokens(); - public OAuth2AccessTokenEntity saveAccessToken(OAuth2AccessTokenEntity accessToken); + public OAuth2AccessTokenEntity saveAccessToken(OAuth2AccessTokenEntity accessToken); - public OAuth2RefreshTokenEntity saveRefreshToken(OAuth2RefreshTokenEntity refreshToken); + public OAuth2RefreshTokenEntity saveRefreshToken(OAuth2RefreshTokenEntity refreshToken); - @Override - public OAuth2AccessTokenEntity getAccessToken(OAuth2Authentication authentication); + @Override + public OAuth2AccessTokenEntity getAccessToken(OAuth2Authentication authentication); - public OAuth2AccessTokenEntity getAccessTokenById(Long id); + public OAuth2AccessTokenEntity getAccessTokenById(Long id); - public OAuth2RefreshTokenEntity getRefreshTokenById(Long id); + public OAuth2RefreshTokenEntity getRefreshTokenById(Long id); - public Set getAllAccessTokensForUser(String name); + public Set getAllAccessTokensForUser(String name); - public Set getAllRefreshTokensForUser(String name); + public Set getAllRefreshTokensForUser(String name); + + public OAuth2AccessTokenEntity getRegistrationAccessTokenForClient(ClientDetailsEntity client); - public OAuth2AccessTokenEntity getRegistrationAccessTokenForClient(ClientDetailsEntity client); } diff --git a/openid-connect-server/pom.xml b/openid-connect-server/pom.xml index 18b1592ca..45f853c03 100644 --- a/openid-connect-server/pom.xml +++ b/openid-connect-server/pom.xml @@ -23,7 +23,7 @@ org.mitre openid-connect-parent - 1.3.6.cnaf-20231113 + 1.3.6.cnaf-20231129 .. diff --git a/openid-connect-server/src/main/java/org/mitre/oauth2/service/impl/DefaultOAuth2ProviderTokenService.java b/openid-connect-server/src/main/java/org/mitre/oauth2/service/impl/DefaultOAuth2ProviderTokenService.java index 1aae3c6dc..e3dc32d56 100644 --- a/openid-connect-server/src/main/java/org/mitre/oauth2/service/impl/DefaultOAuth2ProviderTokenService.java +++ b/openid-connect-server/src/main/java/org/mitre/oauth2/service/impl/DefaultOAuth2ProviderTokenService.java @@ -75,6 +75,7 @@ import com.nimbusds.jwt.PlainJWT; * @author jricher * */ +@SuppressWarnings("deprecation") @Service("defaultOAuth2ProviderTokenService") public class DefaultOAuth2ProviderTokenService implements OAuth2TokenEntityService { @@ -163,7 +164,8 @@ public class DefaultOAuth2ProviderTokenService implements OAuth2TokenEntityServi @Override @Transactional(value = "defaultTransactionManager") public OAuth2AccessTokenEntity createAccessToken(OAuth2Authentication authentication) - throws AuthenticationException, InvalidClientException { + throws AuthenticationException { + if (authentication != null && authentication.getOAuth2Request() != null) { // look up our client OAuth2Request request = authentication.getOAuth2Request(); @@ -220,13 +222,12 @@ public class DefaultOAuth2ProviderTokenService implements OAuth2TokenEntityServi token.setScope(scopeService.toStrings(scopes)); // make it always expire - if (client.getAccessTokenValiditySeconds() != null && client.getAccessTokenValiditySeconds() > 0) { + if (client.getAccessTokenValiditySeconds() != null + && client.getAccessTokenValiditySeconds() > 0) { Date expiration = new Date(System.currentTimeMillis() + (client.getAccessTokenValiditySeconds() * 1000L)); token.setExpiration(expiration); - } else { - token.setExpiration(new Date(System.currentTimeMillis())); } // attach the authorization so that we can look it up later @@ -263,9 +264,7 @@ public class DefaultOAuth2ProviderTokenService implements OAuth2TokenEntityServi OAuth2AccessTokenEntity savedToken = saveAccessToken(enhancedToken); if (savedToken.getRefreshToken() != null) { - tokenRepository.saveRefreshToken(savedToken.getRefreshToken()); // make sure we save any - // changes that might have - // been enhanced + tokenRepository.saveRefreshToken(savedToken.getRefreshToken()); } return savedToken; @@ -281,8 +280,9 @@ public class DefaultOAuth2ProviderTokenService implements OAuth2TokenEntityServi JWTClaimsSet.Builder refreshClaims = new JWTClaimsSet.Builder(); - // make it expire if necessary - if (client.getRefreshTokenValiditySeconds() != null) { + // set RT's expiration value, otherwise leaves null + if (client.getRefreshTokenValiditySeconds() != null + && client.getRefreshTokenValiditySeconds() > 0) { Date expiration = new Date(System.currentTimeMillis() + (client.getRefreshTokenValiditySeconds() * 1000L)); refreshToken.setExpiration(expiration); @@ -386,7 +386,8 @@ public class DefaultOAuth2ProviderTokenService implements OAuth2TokenEntityServi token.setClient(client); - if (client.getAccessTokenValiditySeconds() != null) { + if (client.getAccessTokenValiditySeconds() != null + && client.getAccessTokenValiditySeconds() > 0) { Date expiration = new Date(System.currentTimeMillis() + (client.getAccessTokenValiditySeconds() * 1000L)); token.setExpiration(expiration); @@ -609,4 +610,5 @@ public class DefaultOAuth2ProviderTokenService implements OAuth2TokenEntityServi return null; } + } diff --git a/openid-connect-server/src/test/java/org/mitre/oauth2/service/impl/TestDefaultOAuth2ProviderTokenService.java b/openid-connect-server/src/test/java/org/mitre/oauth2/service/impl/TestDefaultOAuth2ProviderTokenService.java index eee4179a6..77ea50362 100644 --- a/openid-connect-server/src/test/java/org/mitre/oauth2/service/impl/TestDefaultOAuth2ProviderTokenService.java +++ b/openid-connect-server/src/test/java/org/mitre/oauth2/service/impl/TestDefaultOAuth2ProviderTokenService.java @@ -29,7 +29,7 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.AdditionalAnswers.returnsFirstArg; import static org.mockito.Matchers.any; -import static org.mockito.Matchers.anySet; +import static org.mockito.Matchers.anySetOf; import static org.mockito.Matchers.anyString; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.mock; @@ -54,6 +54,7 @@ import org.mitre.oauth2.repository.AuthenticationHolderRepository; import org.mitre.oauth2.repository.OAuth2TokenRepository; import org.mitre.oauth2.service.ClientDetailsEntityService; import org.mitre.oauth2.service.SystemScopeService; +import org.mitre.openid.connect.service.ApprovedSiteService; import org.mockito.InjectMocks; import org.mockito.Matchers; import org.mockito.Mock; @@ -75,18 +76,20 @@ import org.springframework.security.oauth2.provider.token.TokenEnhancer; * */ @RunWith(MockitoJUnitRunner.class) +@SuppressWarnings("deprecation") public class TestDefaultOAuth2ProviderTokenService { // Grace period for time-sensitive tests. private static final long DELTA = 100L; + private static final String clientId = "test_client"; + private static final String badClientId = "bad_client"; + private static final Set scope = + newHashSet("openid", "profile", "email", "offline_access"); + // Test Fixture: - private OAuth2Authentication authentication; private ClientDetailsEntity client; private ClientDetailsEntity badClient; - private String clientId = "test_client"; - private String badClientId = "bad_client"; - private Set scope = newHashSet("openid", "profile", "email", "offline_access"); private OAuth2RefreshTokenEntity refreshToken; private OAuth2AccessTokenEntity accessToken; private String refreshTokenValue = "refresh_token_value"; @@ -99,6 +102,9 @@ public class TestDefaultOAuth2ProviderTokenService { private AuthenticationHolderEntity storedAuthHolder; private Set storedScope; + @Mock + private OAuth2Authentication authentication; + @Mock private OAuth2TokenRepository tokenRepository; @@ -114,6 +120,9 @@ public class TestDefaultOAuth2ProviderTokenService { @Mock private SystemScopeService scopeService; + @Mock + private ApprovedSiteService approvedSiteService; + @InjectMocks private DefaultOAuth2ProviderTokenService service; @@ -122,9 +131,10 @@ public class TestDefaultOAuth2ProviderTokenService { */ @Before public void prepare() { - reset(tokenRepository, authenticationHolderRepository, clientDetailsService, tokenEnhancer); - authentication = Mockito.mock(OAuth2Authentication.class); + reset(tokenRepository, authenticationHolderRepository, clientDetailsService, tokenEnhancer, + scopeService, approvedSiteService, authentication); + OAuth2Request clientAuth = new OAuth2Request(null, clientId, null, true, scope, null, null, null, null); when(authentication.getOAuth2Request()).thenReturn(clientAuth); @@ -165,21 +175,24 @@ public class TestDefaultOAuth2ProviderTokenService { when(authenticationHolderRepository.save(any(AuthenticationHolderEntity.class))) .thenReturn(storedAuthHolder); - when(scopeService.fromStrings(anySet())).thenAnswer(new Answer>() { - @Override - public Set answer(InvocationOnMock invocation) throws Throwable { - Object[] args = invocation.getArguments(); - Set input = (Set) args[0]; - Set output = new HashSet<>(); - for (String scope : input) { - output.add(new SystemScope(scope)); + when(scopeService.fromStrings(anySetOf(String.class))) + .thenAnswer(new Answer>() { + @Override + @SuppressWarnings("unchecked") + public Set answer(InvocationOnMock invocation) throws Throwable { + Object[] args = invocation.getArguments(); + Set input = (Set) args[0]; + Set output = new HashSet<>(); + for (String scope : input) { + output.add(new SystemScope(scope)); + } + return output; } - return output; - } - }); + }); - when(scopeService.toStrings(anySet())).thenAnswer(new Answer>() { + when(scopeService.toStrings(anySetOf(SystemScope.class))).thenAnswer(new Answer>() { @Override + @SuppressWarnings("unchecked") public Set answer(InvocationOnMock invocation) throws Throwable { Object[] args = invocation.getArguments(); Set input = (Set) args[0]; @@ -191,19 +204,22 @@ public class TestDefaultOAuth2ProviderTokenService { } }); - when(scopeService.scopesMatch(anySet(), anySet())).thenAnswer(new Answer() { - @Override - public Boolean answer(InvocationOnMock invocation) throws Throwable { - Object[] args = invocation.getArguments(); - Set expected = (Set) args[0]; - Set actual = (Set) args[1]; - return expected.containsAll(actual); - } - }); - + when(scopeService.scopesMatch(anySetOf(String.class), anySetOf(String.class))) + .thenAnswer(new Answer() { + @Override + @SuppressWarnings("unchecked") + public Boolean answer(InvocationOnMock invocation) throws Throwable { + Object[] args = invocation.getArguments(); + Set expected = (Set) args[0]; + Set actual = (Set) args[1]; + return expected.containsAll(actual); + } + }); + // we're not testing restricted or reserved scopes here, just pass through - when(scopeService.removeReservedScopes(anySet())).then(returnsFirstArg()); - when(scopeService.removeRestrictedAndReservedScopes(anySet())).then(returnsFirstArg()); + when(scopeService.removeReservedScopes(anySetOf(SystemScope.class))).then(returnsFirstArg()); + when(scopeService.removeRestrictedAndReservedScopes(anySetOf(SystemScope.class))) + .then(returnsFirstArg()); when(tokenEnhancer.enhance(any(OAuth2AccessTokenEntity.class), any(OAuth2Authentication.class))) .thenAnswer(new Answer() { @@ -281,7 +297,7 @@ public class TestDefaultOAuth2ProviderTokenService { verify(authenticationHolderRepository).save(any(AuthenticationHolderEntity.class)); verify(tokenEnhancer).enhance(any(OAuth2AccessTokenEntity.class), Matchers.eq(authentication)); verify(tokenRepository).saveAccessToken(any(OAuth2AccessTokenEntity.class)); - verify(scopeService, atLeastOnce()).removeReservedScopes(anySet()); + verify(scopeService, atLeastOnce()).removeReservedScopes(anySetOf(SystemScope.class)); verify(tokenRepository, Mockito.never()).saveRefreshToken(any(OAuth2RefreshTokenEntity.class)); @@ -303,7 +319,7 @@ public class TestDefaultOAuth2ProviderTokenService { // Note: a refactor may be appropriate to only save refresh tokens once to the repository during // creation. verify(tokenRepository, atLeastOnce()).saveRefreshToken(any(OAuth2RefreshTokenEntity.class)); - verify(scopeService, atLeastOnce()).removeReservedScopes(anySet()); + verify(scopeService, atLeastOnce()).removeReservedScopes(anySetOf(SystemScope.class)); assertThat(token.getRefreshToken(), is(notNullValue())); } @@ -330,7 +346,7 @@ public class TestDefaultOAuth2ProviderTokenService { Date lowerBoundRefreshTokens = new Date(start + (refreshTokenValiditySeconds * 1000L) - DELTA); Date upperBoundRefreshTokens = new Date(end + (refreshTokenValiditySeconds * 1000L) + DELTA); - verify(scopeService, atLeastOnce()).removeReservedScopes(anySet()); + verify(scopeService, atLeastOnce()).removeReservedScopes(anySetOf(SystemScope.class)); assertTrue(token.getExpiration().after(lowerBoundAccessTokens) && token.getExpiration().before(upperBoundAccessTokens)); @@ -342,7 +358,7 @@ public class TestDefaultOAuth2ProviderTokenService { public void createAccessToken_checkClient() { OAuth2AccessTokenEntity token = service.createAccessToken(authentication); - verify(scopeService, atLeastOnce()).removeReservedScopes(anySet()); + verify(scopeService, atLeastOnce()).removeReservedScopes(anySetOf(SystemScope.class)); assertThat(token.getClient().getClientId(), equalTo(clientId)); } @@ -351,7 +367,7 @@ public class TestDefaultOAuth2ProviderTokenService { public void createAccessToken_checkScopes() { OAuth2AccessTokenEntity token = service.createAccessToken(authentication); - verify(scopeService, atLeastOnce()).removeReservedScopes(anySet()); + verify(scopeService, atLeastOnce()).removeReservedScopes(anySetOf(SystemScope.class)); assertThat(token.getScope(), equalTo(scope)); } @@ -368,7 +384,7 @@ public class TestDefaultOAuth2ProviderTokenService { assertThat(token.getAuthenticationHolder().getAuthentication(), equalTo(authentication)); verify(authenticationHolderRepository).save(any(AuthenticationHolderEntity.class)); - verify(scopeService, atLeastOnce()).removeReservedScopes(anySet()); + verify(scopeService, atLeastOnce()).removeReservedScopes(anySetOf(SystemScope.class)); } @Test(expected = InvalidTokenException.class) diff --git a/pom.xml b/pom.xml index 8433e8fc2..12a4fbfb4 100644 --- a/pom.xml +++ b/pom.xml @@ -20,7 +20,7 @@ 4.0.0 org.mitre openid-connect-parent - 1.3.6.cnaf-20231113 + 1.3.6.cnaf-20231129 MITREid Connect pom