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.

Sign up for GitHub

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

Jump to bottom

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

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
94e241e
Fix Property "renew_token_without_revoking_existing" not being honore…
KD23243 Nov 12, 2024
480cd3f
Add unit tests for AbstractAuthorizationGrantHandler
KD23243 Nov 13, 2024
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
2 changes: 2 additions & 0 deletions ...tity.oauth.common/src/main/java/org/wso2/carbon/identity/oauth/common/OAuthConstants.java
View file
Open in desktop
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
26 changes: 23 additions & 3 deletions ...y.oauth/src/main/java/org/wso2/carbon/identity/oauth/config/OAuthServerConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ public class OAuthServerConfiguration {
private String oauthTokenGeneratorClassName;
private OAuthIssuer oauthTokenGenerator;
private String oauthIdentityTokenGeneratorClassName;
private String oauthDefaultAccessTokenGeneratorType = null;
private String clientIdValidationRegex = "[a-zA-Z0-9_]{15,30}";
private String persistAccessTokenAlias;
private String retainOldAccessTokens;
Expand Down Expand Up @@ -2569,8 +2570,14 @@ private void parseOAuthTokenIssuerConfig(OMElement oauthConfigElem) {

OMElement tokenIssuerClassConfigElem = oauthConfigElem
.getFirstChildWithName(getQNameWithIdentityNS(ConfigElements.IDENTITY_OAUTH_TOKEN_GENERATOR));
OMElement tokenIssuerTypeConfigElem = oauthConfigElem
.getFirstChildWithName(getQNameWithIdentityNS(ConfigElements.TOKEN_TYPE));
if (tokenIssuerClassConfigElem != null && !"".equals(tokenIssuerClassConfigElem.getText().trim())) {
oauthIdentityTokenGeneratorClassName = tokenIssuerClassConfigElem.getText().trim();
if (tokenIssuerTypeConfigElem != null && StringUtils.isNotEmpty(tokenIssuerTypeConfigElem.getText())) {
oauthDefaultAccessTokenGeneratorType = tokenIssuerTypeConfigElem.getText();
}

if (log.isDebugEnabled()) {
log.debug("Identity OAuth token generator is set to : " + oauthIdentityTokenGeneratorClassName);
}
Expand Down Expand Up @@ -2786,6 +2793,14 @@ private void parseSupportedTokenTypesConfig(OMElement oauthConfigElem) {
tokenTypeName = tokenTypeNameElement.getText();
}

OMElement tokenTypeElement = supportedTokenTypeElement
.getFirstChildWithName(getQNameWithIdentityNS(ConfigElements.TOKEN_TYPE));

String accessTokenType = null;
if (tokenTypeElement != null) {
accessTokenType = tokenTypeElement.getText();
}

OMElement tokenTypeImplClassElement = supportedTokenTypeElement
.getFirstChildWithName(getQNameWithIdentityNS(ConfigElements.TOKEN_TYPE_IMPL_CLASS));

Expand All @@ -2807,6 +2822,7 @@ private void parseSupportedTokenTypesConfig(OMElement oauthConfigElem) {
if (StringUtils.isNotEmpty(tokenTypeImplClass)) {
tokenIssuerDO.setTokenType(tokenTypeName);
tokenIssuerDO.setTokenImplClass(tokenTypeImplClass);
tokenIssuerDO.setAccessTokenType(accessTokenType);
}

if (StringUtils.isNotEmpty(persistAccessTokenAlias)) {
Expand Down Expand Up @@ -2838,9 +2854,12 @@ private void parseSupportedTokenTypesConfig(OMElement oauthConfigElem) {

// If a server level <IdentityOAuthTokenGenerator> is defined, that will be our first choice for the
// "Default" token type issuer implementation.
supportedTokenIssuers.put(DEFAULT_TOKEN_TYPE,
new TokenIssuerDO(DEFAULT_TOKEN_TYPE, oauthIdentityTokenGeneratorClassName,
isPersistTokenAlias));
TokenIssuerDO tokenIssuerDO = new TokenIssuerDO(DEFAULT_TOKEN_TYPE, oauthIdentityTokenGeneratorClassName,
isPersistTokenAlias);
if (StringUtils.isNotEmpty(oauthDefaultAccessTokenGeneratorType)) {
tokenIssuerDO.setAccessTokenType(oauthDefaultAccessTokenGeneratorType);
}
supportedTokenIssuers.put(DEFAULT_TOKEN_TYPE, tokenIssuerDO);
}

// Adding default token types if not added in the configuration.
Expand Down Expand Up @@ -4216,6 +4235,7 @@ private class ConfigElements {
private static final String SUPPORTED_TOKEN_TYPES = "SupportedTokenTypes";
private static final String SUPPORTED_TOKEN_TYPE = "SupportedTokenType";
private static final String TOKEN_TYPE_NAME = "TokenTypeName";
private static final String TOKEN_TYPE = "TokenType";

private static final String USER_CONSENT_ENABLED_GRANT_TYPES = "UserConsentEnabledGrantTypes";
private static final String USER_CONSENT_ENABLED_GRANT_TYPE = "UserConsentEnabledGrantType";
Expand Down
33 changes: 33 additions & 0 deletions ...bon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/model/TokenIssuerDO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
public class TokenIssuerDO {

private String tokenType;
private String accessTokenType;
private String tokenImplClass;
private boolean persistAccessTokenAlias;

Expand All @@ -49,14 +50,46 @@ public TokenIssuerDO(String tokenType, String tokenImplClass) {
public TokenIssuerDO() {
}

/**
* @deprecated Use {@link #getAccessTokenType()} instead.
*/
@Deprecated
public String getTokenType() {
return tokenType;
}

/**
* @deprecated Use {@link #setAccessTokenType(String)} instead.
*/
@Deprecated
public void setTokenType(String tokenType) {
this.tokenType = tokenType;
}

/**
* Get the token issuer name.
* @return the token issuer name
*/
public String getTokenIssuerName() {
return tokenType;
}

/**
* Set the token issuer name.
* @param tokenIssuerName the token issuer name to set
*/
public void setTokenIssuerName(String tokenIssuerName) {
this.tokenType = tokenIssuerName;
}

public String getAccessTokenType() {
return accessTokenType;
}

public void setAccessTokenType(String accessTokenType) {
this.accessTokenType = accessTokenType;
}

public String getTokenImplClass() {
return tokenImplClass;
}
Expand Down
98 changes: 92 additions & 6 deletions ...g/wso2/carbon/identity/oauth2/token/handlers/grant/AbstractAuthorizationGrantHandler.java
View file
Open in desktop
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,37 @@ public OAuth2AccessTokenRespDTO issue(OAuthTokenReqMessageContext tokReqMsgCtx)

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

OAuthAppDO oAuthAppDO = (OAuthAppDO) tokReqMsgCtx.getProperty(OAUTH_APP);
String tokenIssuerName = (oAuthAppDO != null) ? oAuthAppDO.getTokenType() : null;
String tokenType = null;
if (tokenIssuerName != null) {
tokenType = OAuthServerConfiguration.getInstance().getSupportedTokenIssuers()
.get(tokenIssuerName).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.
*/
boolean isJWTAndRenewEnabled = (JWT.equalsIgnoreCase(tokenIssuerName) || JWT.equalsIgnoreCase(tokenType))
&& getRenewWithoutRevokingExistingStatus();
boolean isGrantTypeAllowed = OAuth2ServiceComponentHolder.getJwtRenewWithoutRevokeAllowedGrantTypes()
.contains(tokReqMsgCtx.getOauth2AccessTokenReqDTO().getGrantType());

if (isJWTAndRenewEnabled && isGrantTypeAllowed) {
/*
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 @@ -209,6 +243,13 @@ public OAuth2AccessTokenRespDTO issue(OAuthTokenReqMessageContext tokReqMsgCtx)
}
}

private boolean getRenewWithoutRevokingExistingStatus() {

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

}

private void setDetailsToMessageContext(OAuthTokenReqMessageContext tokReqMsgCtx, AccessTokenDO existingToken) {

if (existingToken.getIssuedTime() != null) {
Expand Down Expand Up @@ -1176,16 +1217,61 @@ private boolean hasValidationByApplicationScopeValidatorsFailed(OAuthTokenReqMes
*/
protected String getTokenBindingReference(OAuthTokenReqMessageContext tokReqMsgCtx) {

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", ...]
*/
OAuthAppDO oAuthAppDO = (OAuthAppDO) tokReqMsgCtx.getProperty(OAUTH_APP);
String tokenIssuerName = (oAuthAppDO != null) ? oAuthAppDO.getTokenType() : null;
String tokenType = null;
if (tokenIssuerName != null) {
tokenType = OAuthServerConfiguration.getInstance().getSupportedTokenIssuers()
.get(tokenIssuerName).getAccessTokenType();
}

if (JWT.equalsIgnoreCase(tokenIssuerName) || 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
56 changes: 56 additions & 0 deletions ...o2/carbon/identity/oauth2/token/handlers/grant/AbstractAuthorizationGrantHandlerTest.java
View file
Open in desktop
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 @@ -52,6 +54,7 @@
import org.wso2.carbon.identity.oauth2.internal.OAuth2ServiceComponentHolder;
import org.wso2.carbon.identity.oauth2.model.AccessTokenDO;
import org.wso2.carbon.identity.oauth2.token.OAuthTokenReqMessageContext;
import org.wso2.carbon.identity.oauth2.token.bindings.TokenBinding;
import org.wso2.carbon.identity.oauth2.validators.OAuth2ScopeHandler;

import java.util.Collections;
Expand All @@ -63,7 +66,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 @@ -198,6 +203,57 @@ public void testIssue(boolean cacheEnabled, boolean cacheEntryAvailable, long ca
assertNotNull(tokenRespDTO.getAccessToken());
}


@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 = "IssueWithRenewDataProvider")
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"});

oAuthAppDO.setTokenType("JWT");
tokReqMsgCtx.addProperty("OAuthAppDO", oAuthAppDO);

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

try (MockedStatic<IdentityUtil> identityUtil = mockStatic(IdentityUtil.class)) {
identityUtil.when(() -> IdentityUtil.getProperty(anyString()))
.thenReturn(Boolean.TRUE.toString());

// 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);
assertNotNull(tokenRespDTO.getAccessToken());
}
}

@DataProvider(name = "AuthorizeAccessDelegationDataProvider")
public Object[][] buildAuthorizeAccessDelegationDataProvider() {

Expand Down
Loading