Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Property "renew_token_without_revoking_existing" not being honored causing stuck threads #2630

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,8 @@ public final class OAuthConstants {

public static final String READ_AMR_VALUE_FROM_IDP = "OAuth.ReplaceDefaultAMRValuesWithIDPSentValues";

public static final String OAUTH_APP = "OAuthAppDO";

public static final String CNF = "cnf";
public static final String MTLS_AUTH_HEADER = "MutualTLS.ClientCertificateHeader";
public static final String BEGIN_CERT = "-----BEGIN CERTIFICATE-----";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,16 @@ public static class TokenBinderType {
public static final String CLIENT_REQUEST = "client-request";

}

/**
* Constants for token types.
*/
public static class TokenTypes {

public static final String OPAQUE = "Opaque";
public static final String JWT = "jwt";
}

public static final String GROUPS = "groups";
public static final String ENTITY_ID = "entity_id";
public static final String IS_CONSENTED = "is_consented";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,12 @@ public JWTTokenIssuer() throws IdentityOAuth2Exception {
signatureAlgorithm = mapSignatureAlgorithm(config.getSignatureAlgorithm());
}

@Override
public String getAccessTokenType() {

return JWT_TYP_HEADER_VALUE;
}

/**
* {@inheritDoc}
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
import org.wso2.carbon.identity.oauth2.IdentityOAuth2Exception;
import org.wso2.carbon.identity.oauth2.authz.OAuthAuthzReqMessageContext;

import static org.wso2.carbon.identity.oauth2.OAuth2Constants.TokenTypes.OPAQUE;

/**
* OAuth 2 access token issuer.
*/
Expand Down Expand Up @@ -99,4 +101,14 @@ default boolean usePersistedAccessTokenAlias() {
default String issueSubjectToken(OAuthAuthzReqMessageContext oauthAuthzMsgCtx) throws IdentityOAuth2Exception {
return StringUtils.EMPTY;
}

/**
* Default method to retrieve the access token type
*
* @return The type of the token (e.g., "JWT" or "Opaque").
*/
default String getAccessTokenType() {

return OPAQUE;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,13 @@
import java.util.UUID;
import java.util.function.Consumer;

import static org.wso2.carbon.identity.oauth.common.OAuthConstants.OAUTH_APP;
import static org.wso2.carbon.identity.oauth.common.OAuthConstants.RENEW_TOKEN_WITHOUT_REVOKING_EXISTING_ENABLE_CONFIG;
import static org.wso2.carbon.identity.oauth.common.OAuthConstants.TokenBindings.NONE;
import static org.wso2.carbon.identity.oauth.common.OAuthConstants.TokenStates.TOKEN_STATE_ACTIVE;
import static org.wso2.carbon.identity.oauth.config.OAuthServerConfiguration.JWT_TOKEN_TYPE;
import static org.wso2.carbon.identity.oauth2.util.OAuth2Util.EXTENDED_REFRESH_TOKEN_DEFAULT_TIME;
import static org.wso2.carbon.identity.oauth2.util.OAuth2Util.JWT;

/**
* Abstract authorization grant handler.
Expand Down Expand Up @@ -171,6 +174,29 @@ public OAuth2AccessTokenRespDTO issue(OAuthTokenReqMessageContext tokReqMsgCtx)

synchronized ((consumerKey + ":" + authorizedUserId + ":" + scope + ":" + tokenBindingReference).intern()) {
AccessTokenDO existingTokenBean = null;

OAuthAppDO oAuthAppDO = (OAuthAppDO) tokReqMsgCtx.getProperty(OAUTH_APP);
String tokenType = oauthTokenIssuer.getAccessTokenType();

/*
Check if the token type is JWT and renew without revoking existing tokens is enabled.
Additionally, ensure that the grant type used for the token request is allowed to renew without revoke,
based on the config.
*/
if (JWT.equalsIgnoreCase(tokenType) && getRenewWithoutRevokingExistingStatus() &&
OAuth2ServiceComponentHolder.getJwtRenewWithoutRevokeAllowedGrantTypes()
.contains(tokReqMsgCtx.getOauth2AccessTokenReqDTO().getGrantType())) {
/*
If the application does not have a token binding type (i.e., no specific binding type is set),
binding reference will be randomly generated UUID, in that case we can generate a new access token
without looking up the existing tokens in the token table.
*/
if (oAuthAppDO.getTokenBindingType() == null) {
return generateNewAccessToken(tokReqMsgCtx, scope, consumerKey, existingTokenBean,
false, oauthTokenIssuer);
}
}

if (isHashDisabled) {
existingTokenBean = getExistingToken(tokReqMsgCtx,
getOAuthCacheKey(scope, consumerKey, authorizedUserId, authenticatedIDP,
Expand Down Expand Up @@ -706,6 +732,12 @@ private void updateCacheIfEnabled(AccessTokenDO newTokenBean, String scope, Oaut
}
}

private boolean getRenewWithoutRevokingExistingStatus() {

return Boolean.parseBoolean(IdentityUtil.
getProperty(RENEW_TOKEN_WITHOUT_REVOKING_EXISTING_ENABLE_CONFIG));
}

private String getNewAccessToken(OAuthTokenReqMessageContext tokReqMsgCtx, OauthTokenIssuer oauthTokenIssuer)
throws IdentityOAuth2Exception {
try {
Expand Down Expand Up @@ -1137,7 +1169,8 @@ private boolean isTokenRenewalPerRequestConfigured() {
return OAuthServerConfiguration.getInstance().isTokenRenewalPerRequestEnabled();
}

private void clearExistingTokenFromCache(OAuthTokenReqMessageContext tokenMsgCtx, AccessTokenDO existingTokenBean) {
private void clearExistingTokenFromCache(OAuthTokenReqMessageContext tokenMsgCtx, AccessTokenDO existingTokenBean)
throws IdentityOAuth2Exception {

if (cacheEnabled) {
String tokenBindingReference = getTokenBindingReference(tokenMsgCtx);
Expand Down Expand Up @@ -1174,18 +1207,68 @@ private boolean hasValidationByApplicationScopeValidatorsFailed(OAuthTokenReqMes
* @param tokReqMsgCtx OAuthTokenReqMessageContext.
* @return token binding reference.
*/
protected String getTokenBindingReference(OAuthTokenReqMessageContext tokReqMsgCtx) {
protected String getTokenBindingReference(OAuthTokenReqMessageContext tokReqMsgCtx)
KD23243 marked this conversation as resolved.
Show resolved Hide resolved
throws IdentityOAuth2Exception {

if (tokReqMsgCtx.getTokenBinding() == null) {
if (log.isDebugEnabled()) {
log.debug("Token binding data is null.");
/**
* If OAuth.JWT.RenewTokenWithoutRevokingExisting is enabled from configurations, and current token
* binding is null,then we will add a new token binding (request binding) to the token binding with
* a value of a random UUID.
* The purpose of this new token binding type is to add a random value to the token binding so that
* "User, Application, Scope, Binding" combination will be unique for each token.
* Previously, if a token issue request come for the same combination of "User, Application, Scope, Binding",
* the existing JWT token will be revoked and issue a new token. but with this way, we can issue new tokens
* without revoking the old ones.
*
* Add following configuration to deployment.toml file to enable this feature.
* [oauth.jwt.renew_token_without_revoking_existing]
* enable = true
*
* By default, the allowed grant type for this feature is "client_credentials". If you need to enable for
* other grant types, add the following configuration to deployment.toml file.
* [oauth.jwt.renew_token_without_revoking_existing]
* enable = true
* allowed_grant_types = ["client_credentials","password", ...]
*/
String consumerKey = tokReqMsgCtx.getOauth2AccessTokenReqDTO().getClientId();
OauthTokenIssuer oauthTokenIssuer;

try {
oauthTokenIssuer = OAuth2Util.getOAuthTokenIssuerForOAuthApp(consumerKey);
} catch (InvalidOAuthClientException | IdentityOAuth2Exception e) {
throw new IdentityOAuth2Exception(
"Error while retrieving oauth issuer for the app with clientId: " + consumerKey, e);
}

String tokenType = oauthTokenIssuer.getAccessTokenType();

if (JWT.equalsIgnoreCase(tokenType)) {
if (getRenewWithoutRevokingExistingStatus()
&& tokReqMsgCtx != null && (tokReqMsgCtx.getTokenBinding() == null
|| StringUtils.isBlank(tokReqMsgCtx.getTokenBinding().getBindingReference()))) {
if (OAuth2ServiceComponentHolder.getJwtRenewWithoutRevokeAllowedGrantTypes()
.contains(tokReqMsgCtx.getOauth2AccessTokenReqDTO().getGrantType())) {
return UUID.randomUUID().toString();
}
return NONE;
}
return NONE;
}
if (StringUtils.isBlank(tokReqMsgCtx.getTokenBinding().getBindingReference())) {
return getExistingTokenBindingReference(tokReqMsgCtx);
}

/**
* Retrieves the existing token binding reference if available, otherwise returns NONE.
*
* @param tokReqMsgCtx OAuthTokenReqMessageContext.
* @return token binding reference.
*/
private String getExistingTokenBindingReference(OAuthTokenReqMessageContext tokReqMsgCtx) {

if (tokReqMsgCtx == null || tokReqMsgCtx.getTokenBinding() == null) {
return NONE;
}
return tokReqMsgCtx.getTokenBinding().getBindingReference();
String bindingReference = tokReqMsgCtx.getTokenBinding().getBindingReference();
return StringUtils.isBlank(bindingReference) ? NONE : bindingReference;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package org.wso2.carbon.identity.oauth2.token.handlers.grant;

import org.mockito.Mock;
import org.mockito.MockedStatic;
import org.mockito.MockitoAnnotations;
import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass;
Expand All @@ -36,6 +37,7 @@
import org.wso2.carbon.identity.common.testng.WithH2Database;
import org.wso2.carbon.identity.common.testng.WithRealmService;
import org.wso2.carbon.identity.core.util.IdentityTenantUtil;
import org.wso2.carbon.identity.core.util.IdentityUtil;
import org.wso2.carbon.identity.event.services.IdentityEventService;
import org.wso2.carbon.identity.oauth.IdentityOAuthAdminException;
import org.wso2.carbon.identity.oauth.common.GrantType;
Expand All @@ -47,11 +49,17 @@
import org.wso2.carbon.identity.oauth.internal.OAuthComponentServiceHolder;
import org.wso2.carbon.identity.oauth2.IdentityOAuth2Exception;
import org.wso2.carbon.identity.oauth2.TestConstants;
import org.wso2.carbon.identity.oauth2.dao.AccessTokenDAOImpl;
import org.wso2.carbon.identity.oauth2.dao.OAuthTokenPersistenceFactory;
import org.wso2.carbon.identity.oauth2.dto.OAuth2AccessTokenReqDTO;
import org.wso2.carbon.identity.oauth2.dto.OAuth2AccessTokenRespDTO;
import org.wso2.carbon.identity.oauth2.internal.OAuth2ServiceComponentHolder;
import org.wso2.carbon.identity.oauth2.model.AccessTokenDO;
import org.wso2.carbon.identity.oauth2.token.JWTTokenIssuer;
import org.wso2.carbon.identity.oauth2.token.OAuthTokenReqMessageContext;
import org.wso2.carbon.identity.oauth2.token.OauthTokenIssuer;
import org.wso2.carbon.identity.oauth2.token.bindings.TokenBinding;
import org.wso2.carbon.identity.oauth2.util.OAuth2Util;
import org.wso2.carbon.identity.oauth2.validators.OAuth2ScopeHandler;

import java.util.Collections;
Expand All @@ -63,7 +71,9 @@

import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyMap;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.mockStatic;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.testng.Assert.assertEquals;
Expand Down Expand Up @@ -142,6 +152,58 @@ public void tearDown() {
CentralLogMgtServiceComponentHolder.getInstance().setIdentityEventService(null);
}

@Test(dataProvider = "IssueWithRenewDataProvider", expectedExceptions = IdentityOAuth2Exception.class)
public void testIssueWithRenewWithoutRevokingExistingEnabled
(boolean cacheEnabled, boolean cacheEntryAvailable, long cachedTokenValidity,
long cachedRefreshTokenValidity, long dbTokenValidity, long dbRefreshTokenValidity,
boolean dbEntryAvailable, String dbTokenState, boolean tokenLoggable, boolean isIDPIdColumnEnabled,
boolean setBindingReference) throws Exception {

OAuth2ServiceComponentHolder.setIDPIdColumnEnabled(isIDPIdColumnEnabled);

Map<String, AuthorizationGrantHandler> supportedGrantTypes = new HashMap<>();
supportedGrantTypes.put("refresh_token", refreshGrantHandler);

OAuth2AccessTokenReqDTO oAuth2AccessTokenReqDTO = new OAuth2AccessTokenReqDTO();
oAuth2AccessTokenReqDTO.setClientId(clientId);
oAuth2AccessTokenReqDTO.setGrantType(PASSWORD_GRANT); // Ensure the grant type is valid for renewal

OAuthTokenReqMessageContext tokReqMsgCtx = new OAuthTokenReqMessageContext(oAuth2AccessTokenReqDTO);
tokReqMsgCtx.setAuthorizedUser(authenticatedUser);
tokReqMsgCtx.setScope(new String[]{"scope1", "scope2"});

tokReqMsgCtx.addProperty("OAuthAppDO", oAuthAppDO);

TokenBinding tokenBinding = new TokenBinding();
if (setBindingReference) {
tokenBinding.setBindingReference("bindingReference");
}
tokReqMsgCtx.setTokenBinding(tokenBinding);

// Mocking static methods using try-with-resources
try (MockedStatic<IdentityUtil> identityUtil = mockStatic(IdentityUtil.class);
MockedStatic<OAuth2Util> oauth2Util = mockStatic(OAuth2Util.class)) {

identityUtil.when(() -> IdentityUtil.getProperty(anyString()))
.thenReturn(Boolean.TRUE.toString());

OAuthComponentServiceHolder.getInstance().setActionExecutorService(mockActionExecutionService);
OAuthTokenPersistenceFactory persistenceFactory = mock(OAuthTokenPersistenceFactory.class);
when(persistenceFactory.getAccessTokenDAO()).thenReturn(new AccessTokenDAOImpl());

OauthTokenIssuer oauthTokenIssuer = mock(JWTTokenIssuer.class);
when(oauthTokenIssuer.getAccessTokenType()).thenReturn("jwt");
oauth2Util.when(() -> OAuth2Util.getOAuthTokenIssuerForOAuthApp(clientId)).thenReturn(oauthTokenIssuer);
oauth2Util.when(() -> OAuth2Util.getAppInformationByClientId(clientId)).thenReturn(oAuthAppDO);

// Set allowed grant types (ensure PASSWORD_GRANT is allowed for renewal)
OAuth2ServiceComponentHolder.setJwtRenewWithoutRevokeAllowedGrantTypes(
Collections.singletonList("password")); // This allows PASSWORD_GRANT

OAuth2AccessTokenRespDTO tokenRespDTO = handler.issue(tokReqMsgCtx);
}
}

@DataProvider(name = "IssueDataProvider")
public Object[][] issueDataProvider() {
return new Object[][] {
Expand Down Expand Up @@ -174,6 +236,14 @@ public Object[][] issueDataProvider() {
{false, true, 0L, 0L, -1L, 3600L, true, TOKEN_STATE_ACTIVE, true, false}};
}

@DataProvider(name = "IssueWithRenewDataProvider")
public Object[][] issueWithRenewDataProvider() {
return new Object[][]{
{true, true, 3600L, 3600L, 0L, 0L, false, TOKEN_STATE_ACTIVE, false, true, true},
{true, true, 3600L, 3600L, 0L, 0L, false, TOKEN_STATE_ACTIVE, false, true, false}
};
}

@Test(dataProvider = "IssueDataProvider")
public void testIssue(boolean cacheEnabled, boolean cacheEntryAvailable, long cachedTokenValidity,
long cachedRefreshTokenValidity, long dbTokenValidity, long dbRefreshTokenValidity,
Expand Down
Loading