Pitfalls of Reissuing Tokens in Spring Security OAuth

Cause & Environment#

Environment:

Java 8
Spring Security Oauth 2.0.20.BUILD-SNAPSHOT Click here to go to the original code repository

While working, I encountered a problem: why do the same account use the same token in different places? This is not reasonable under normal circumstances. If there is a new login, we might clear the original token to prevent others from using it.

Investigation#

First, I opened the OAuth2 authorization server code and found that there wasn't much logic related to token generation; only this piece of code in AuthorizationServerConfiguration:

@Bean
@Primary
public DefaultTokenServices tokenServices() {
	DefaultTokenServices defaultTokenServices = new DefaultTokenServices();
	defaultTokenServices.setTokenStore(tokenStore());
	defaultTokenServices.setClientDetailsService(clientDetailsService);
	defaultTokenServices.setAccessTokenValiditySeconds(7200);
	defaultTokenServices.setSupportRefreshToken(false);
	return defaultTokenServices;
}

Hmm? Token service, and the only part related to tokens is this, so we need to continue investigating and look directly at the content of the DefaultTokenServices class, where I found this piece of code:

	@Transactional
	public OAuth2AccessToken createAccessToken(OAuth2Authentication authentication) throws AuthenticationException {
		OAuth2AccessToken existingAccessToken = tokenStore.getAccessToken(authentication);
		OAuth2RefreshToken refreshToken = null;
		if (existingAccessToken != null) {
			if (existingAccessToken.isExpired()) {
				if (existingAccessToken.getRefreshToken() != null) {
					refreshToken = existingAccessToken.getRefreshToken();
					// The token store could remove the refresh token when the
					// access token is removed, but we want to
					// be sure...
					tokenStore.removeRefreshToken(refreshToken);
				}
				tokenStore.removeAccessToken(existingAccessToken);
			}
			else {
				// Re-store the access token in case the authentication has changed
				tokenStore.storeAccessToken(existingAccessToken, authentication);
				return existingAccessToken;
			}
		}
		// Only create a new refresh token if there wasn't an existing one
		// associated with an expired access token.
		// Clients might be holding existing refresh tokens, so we re-use it in
		// the case that the old access token
		// expired.
		if (refreshToken == null) {
			refreshToken = createRefreshToken(authentication);
		}
		// But the refresh token itself might need to be re-issued if it has
		// expired.
		else if (refreshToken instanceof ExpiringOAuth2RefreshToken) {
			ExpiringOAuth2RefreshToken expiring = (ExpiringOAuth2RefreshToken) refreshToken;
			if (System.currentTimeMillis() > expiring.getExpiration().getTime()) {
				refreshToken = createRefreshToken(authentication);
			}
		}
		OAuth2AccessToken accessToken = createAccessToken(authentication, refreshToken);
		tokenStore.storeAccessToken(accessToken, authentication);
		// In case it was modified
		refreshToken = accessToken.getRefreshToken();
		if (refreshToken != null) {
			tokenStore.storeRefreshToken(refreshToken, authentication);
		}
		return accessToken;
	}

It contains a line // Re-store the access token in case the authentication has changed, which translates to re-storing the previously issued non-expired token as if issuing a new token. Good, good, good, that's how it's written.

Then we need to rewrite this piece of code to make it invalid and issue a new token; we just need to change this part:

	@Transactional
	public OAuth2AccessToken createAccessToken(OAuth2Authentication authentication) throws AuthenticationException {
		OAuth2AccessToken existingAccessToken = tokenStore.getAccessToken(authentication);
		OAuth2RefreshToken refreshToken = null;
		if (existingAccessToken != null) {
			if (existingAccessToken.isExpired()) {
				if (existingAccessToken.getRefreshToken() != null) {
					refreshToken = existingAccessToken.getRefreshToken();
					// The token store could remove the refresh token when the
					// access token is removed, but we want to
					// be sure...
					tokenStore.removeRefreshToken(refreshToken);
				}
				tokenStore.removeAccessToken(existingAccessToken);
			}
			else {
				// Modify these lines here
				// Re-store the access token in case the authentication has changed
				tokenStore.storeAccessToken(existingAccessToken, authentication);
				return existingAccessToken;
			}
		}
		// Other code remains unchanged unless there are other modifications
	}

Change it to the following code:

				tokenStore.removeAccessToken(existingAccessToken);
				// And remove the return statement

This way, logging in will not directly issue the same token.