Skip to content

Commit

Permalink
Removed AT expiration in case of infinite token
Browse files Browse the repository at this point in the history 
Bump version to 1.3.6-cnaf-20231129
  • Loading branch information
@enricovianello
enricovianello committed Dec 1, 2023
1 parent da282b8 commit 29ec962
Show file tree
Hide file tree
Showing 8 changed files with 91 additions and 84 deletions.
16 changes: 1 addition & 15 deletions Jenkinsfile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ pipeline {

stage('deploy') {
steps {
sh "mvn -U -B clean deploy"
sh "mvn -U -B clean package deploy"
}
}

Expand All @@ -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>)"
}
}
}
}
}
2 changes: 1 addition & 1 deletion openid-connect-client/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
<parent>
<artifactId>openid-connect-parent</artifactId>
<groupId>org.mitre</groupId>
<version>1.3.6.cnaf-20231113</version>
<version>1.3.6.cnaf-20231129</version>
<relativePath>..</relativePath>
</parent>
<artifactId>openid-connect-client</artifactId>
Expand Down
2 changes: 1 addition & 1 deletion openid-connect-common/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
<parent>
<artifactId>openid-connect-parent</artifactId>
<groupId>org.mitre</groupId>
<version>1.3.6.cnaf-20231113</version>
<version>1.3.6.cnaf-20231129</version>
<relativePath>..</relativePath>
</parent>
<artifactId>openid-connect-common</artifactId>
Expand Down
39 changes: 21 additions & 18 deletions openid-connect-common/src/main/java/org/mitre/oauth2/service/OAuth2TokenEntityService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,37 +27,40 @@
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<OAuth2AccessTokenEntity> getAccessTokensForClient(ClientDetailsEntity client);
public List<OAuth2AccessTokenEntity> getAccessTokensForClient(ClientDetailsEntity client);

public List<OAuth2RefreshTokenEntity> getRefreshTokensForClient(ClientDetailsEntity client);
public List<OAuth2RefreshTokenEntity> 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<OAuth2AccessTokenEntity> getAllAccessTokensForUser(String name);
public Set<OAuth2AccessTokenEntity> getAllAccessTokensForUser(String name);

public Set<OAuth2RefreshTokenEntity> getAllRefreshTokensForUser(String name);
public Set<OAuth2RefreshTokenEntity> getAllRefreshTokensForUser(String name);

public OAuth2AccessTokenEntity getRegistrationAccessTokenForClient(ClientDetailsEntity client);

public OAuth2AccessTokenEntity getRegistrationAccessTokenForClient(ClientDetailsEntity client);
}
2 changes: 1 addition & 1 deletion openid-connect-server/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
<parent>
<groupId>org.mitre</groupId>
<artifactId>openid-connect-parent</artifactId>
<version>1.3.6.cnaf-20231113</version>
<version>1.3.6.cnaf-20231129</version>
<relativePath>..</relativePath>
</parent>
<build>
Expand Down
22 changes: 12 additions & 10 deletions ...server/src/main/java/org/mitre/oauth2/service/impl/DefaultOAuth2ProviderTokenService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@
* @author jricher
*
*/
@SuppressWarnings("deprecation")
@Service("defaultOAuth2ProviderTokenService")
public class DefaultOAuth2ProviderTokenService implements OAuth2TokenEntityService {

Expand Down Expand Up @@ -163,7 +164,8 @@ private OAuth2RefreshTokenEntity clearExpiredRefreshToken(OAuth2RefreshTokenEnti
@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();
Expand Down Expand Up @@ -220,13 +222,12 @@ public OAuth2AccessTokenEntity createAccessToken(OAuth2Authentication authentica
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
Expand Down Expand Up @@ -263,9 +264,7 @@ public OAuth2AccessTokenEntity createAccessToken(OAuth2Authentication authentica
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;
Expand All @@ -281,8 +280,9 @@ private OAuth2RefreshTokenEntity createRefreshToken(ClientDetailsEntity client,
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);
Expand Down Expand Up @@ -386,7 +386,8 @@ public OAuth2AccessTokenEntity refreshAccessToken(String refreshTokenValue,

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);
Expand Down Expand Up @@ -609,4 +610,5 @@ public OAuth2AccessTokenEntity getRegistrationAccessTokenForClient(ClientDetails

return null;
}

}
90 changes: 53 additions & 37 deletions ...er/src/test/java/org/mitre/oauth2/service/impl/TestDefaultOAuth2ProviderTokenService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
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;
Expand All @@ -54,6 +54,7 @@
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;
Expand All @@ -75,18 +76,20 @@
*
*/
@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<String> 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<String> scope = newHashSet("openid", "profile", "email", "offline_access");
private OAuth2RefreshTokenEntity refreshToken;
private OAuth2AccessTokenEntity accessToken;
private String refreshTokenValue = "refresh_token_value";
Expand All @@ -99,6 +102,9 @@ public class TestDefaultOAuth2ProviderTokenService {
private AuthenticationHolderEntity storedAuthHolder;
private Set<String> storedScope;

@Mock
private OAuth2Authentication authentication;

@Mock
private OAuth2TokenRepository tokenRepository;

Expand All @@ -114,6 +120,9 @@ public class TestDefaultOAuth2ProviderTokenService {
@Mock
private SystemScopeService scopeService;

@Mock
private ApprovedSiteService approvedSiteService;

@InjectMocks
private DefaultOAuth2ProviderTokenService service;

Expand All @@ -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);
Expand Down Expand Up @@ -165,21 +175,24 @@ public void prepare() {
when(authenticationHolderRepository.save(any(AuthenticationHolderEntity.class)))
.thenReturn(storedAuthHolder);

when(scopeService.fromStrings(anySet())).thenAnswer(new Answer<Set<SystemScope>>() {
@Override
public Set<SystemScope> answer(InvocationOnMock invocation) throws Throwable {
Object[] args = invocation.getArguments();
Set<String> input = (Set<String>) args[0];
Set<SystemScope> output = new HashSet<>();
for (String scope : input) {
output.add(new SystemScope(scope));
when(scopeService.fromStrings(anySetOf(String.class)))
.thenAnswer(new Answer<Set<SystemScope>>() {
@Override
@SuppressWarnings("unchecked")
public Set<SystemScope> answer(InvocationOnMock invocation) throws Throwable {
Object[] args = invocation.getArguments();
Set<String> input = (Set<String>) args[0];
Set<SystemScope> output = new HashSet<>();
for (String scope : input) {
output.add(new SystemScope(scope));
}
return output;
}
return output;
}
});
});

when(scopeService.toStrings(anySet())).thenAnswer(new Answer<Set<String>>() {
when(scopeService.toStrings(anySetOf(SystemScope.class))).thenAnswer(new Answer<Set<String>>() {
@Override
@SuppressWarnings("unchecked")
public Set<String> answer(InvocationOnMock invocation) throws Throwable {
Object[] args = invocation.getArguments();
Set<SystemScope> input = (Set<SystemScope>) args[0];
Expand All @@ -191,19 +204,22 @@ public Set<String> answer(InvocationOnMock invocation) throws Throwable {
}
});

when(scopeService.scopesMatch(anySet(), anySet())).thenAnswer(new Answer<Boolean>() {
@Override
public Boolean answer(InvocationOnMock invocation) throws Throwable {
Object[] args = invocation.getArguments();
Set<String> expected = (Set<String>) args[0];
Set<String> actual = (Set<String>) args[1];
return expected.containsAll(actual);
}
});

when(scopeService.scopesMatch(anySetOf(String.class), anySetOf(String.class)))
.thenAnswer(new Answer<Boolean>() {
@Override
@SuppressWarnings("unchecked")
public Boolean answer(InvocationOnMock invocation) throws Throwable {
Object[] args = invocation.getArguments();
Set<String> expected = (Set<String>) args[0];
Set<String> actual = (Set<String>) 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<OAuth2AccessTokenEntity>() {
Expand Down Expand Up @@ -281,7 +297,7 @@ public void createAccessToken_noRefresh() {
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));

Expand All @@ -303,7 +319,7 @@ public void createAccessToken_yesRefresh() {
// 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()));
}
Expand All @@ -330,7 +346,7 @@ public void createAccessToken_expiration() {
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));
Expand All @@ -342,7 +358,7 @@ public void createAccessToken_expiration() {
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));
}
Expand All @@ -351,7 +367,7 @@ public void createAccessToken_checkClient() {
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));
}
Expand All @@ -368,7 +384,7 @@ public void createAccessToken_checkAttachedAuthentication() {

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)
Expand Down
2 changes: 1 addition & 1 deletion pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
<modelVersion>4.0.0</modelVersion>
<groupId>org.mitre</groupId>
<artifactId>openid-connect-parent</artifactId>
<version>1.3.6.cnaf-20231113</version>
<version>1.3.6.cnaf-20231129</version>
<name>MITREid Connect</name>
<packaging>pom</packaging>
<parent>
Expand Down

0 comments on commit 29ec962

Please sign in to comment.