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

Reducing the token synchronize weight for existing tokens #2660

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
+185 −28
Open

Reducing the token synchronize weight for existing tokens #2660

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6bae6d9
Reducing the token synchronize weight for exisiting tokens
indeewari Jan 6, 2025
5606e85
Modifying the logic order
indeewari Jan 7, 2025
7c18cd0
Unit test coverage
indeewari Jan 8, 2025
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
133 changes: 105 additions & 28 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 @@ -172,11 +172,48 @@ public OAuth2AccessTokenRespDTO issue(OAuthTokenReqMessageContext tokReqMsgCtx)
"Error while retrieving oauth issuer for the app with clientId: " + consumerKey, e);
}

synchronized ((consumerKey + ":" + authorizedUserId + ":" + scope + ":" + tokenBindingReference).intern()) {
AccessTokenDO existingTokenBean = null;
AccessTokenDO existingTokenBean = null;
OAuthAppDO oAuthAppDO = (OAuthAppDO) tokReqMsgCtx.getProperty(OAUTH_APP);
String tokenType = oauthTokenIssuer.getAccessTokenType();

/*
This segment handles access token requests that neither require generating a new token
nor renewing an existing one. Instead, it returns the existing token if it is still valid.
As a result, no synchronization lock is needed for these operations.
Additionally, this segment should strictly avoid any write or update operations.
It was introduced to minimize traffic at the token issuance lock for certain token requests.
*/
if (!(JWT.equalsIgnoreCase(tokenType) && getRenewWithoutRevokingExistingStatus() &&
Thumimku marked this conversation as resolved.
Show resolved Hide resolved
OAuth2ServiceComponentHolder.getJwtRenewWithoutRevokeAllowedGrantTypes()
.contains(tokReqMsgCtx.getOauth2AccessTokenReqDTO().getGrantType()))) {

if (isHashDisabled) {
existingTokenBean = getExistingToken(tokReqMsgCtx,
getOAuthCacheKey(scope, consumerKey, authorizedUserId, authenticatedIDP,
tokenBindingReference, authorizedOrganization));
}

if (existingTokenBean != null && !accessTokenRenewedPerRequest(oauthTokenIssuer, tokReqMsgCtx)) {

String requestGrantType = tokReqMsgCtx.getOauth2AccessTokenReqDTO().getGrantType();
boolean isConsentRequiredGrant = OIDCClaimUtil.isConsentBasedClaimFilteringApplicable(requestGrantType);

if (!isConsentRequiredGrant) {

OAuthAppDO oAuthAppDO = (OAuthAppDO) tokReqMsgCtx.getProperty(OAUTH_APP);
String tokenType = oauthTokenIssuer.getAccessTokenType();
long expireTime = getAccessTokenExpiryTimeMillis(existingTokenBean);
if (isExistingTokenValid(existingTokenBean, expireTime)) {
if (log.isDebugEnabled()) {
log.debug("Existing token is active for client Id: " + consumerKey + ", user: " +
authorizedUserId + " and scope: " + scope + ". Therefore issuing the same token.");
}
return issueExistingAccessToken(tokReqMsgCtx, scope, expireTime,
existingTokenBean);
}
}
}
}

synchronized ((consumerKey + ":" + authorizedUserId + ":" + scope + ":" + tokenBindingReference).intern()) {

/*
Check if the token type is JWT and renew without revoking existing tokens is enabled.
Expand All @@ -197,32 +234,18 @@ If the application does not have a token binding type (i.e., no specific binding
}
}

if (isHashDisabled) {
existingTokenBean = getExistingToken(tokReqMsgCtx,
getOAuthCacheKey(scope, consumerKey, authorizedUserId, authenticatedIDP,
tokenBindingReference, authorizedOrganization));
}

if (existingTokenBean != null) {
if (log.isDebugEnabled()) {
log.debug("Latest access token is found in the OAuthCache for the app: " + consumerKey);
}
if (accessTokenRenewedPerRequest(oauthTokenIssuer, tokReqMsgCtx)) {
if (log.isDebugEnabled()) {
log.debug("TokenRenewalPerRequest is enabled. " +
"Proceeding to revoke any existing active tokens and issue new token for client Id: " +
consumerKey + ", user: " + authorizedUserId + " and scope: " + scope + ".");
}
return renewAccessToken(tokReqMsgCtx, scope, consumerKey, existingTokenBean, oauthTokenIssuer);
}

long expireTime = getAccessTokenExpiryTimeMillis(existingTokenBean);
if (isExistingTokenValid(existingTokenBean, expireTime)) {
if (log.isDebugEnabled()) {
log.debug("Existing token is active for client Id: " + consumerKey + ", user: " +
authorizedUserId + " and scope: " + scope + ". Therefore issuing the same token.");
}
return issueExistingAccessToken(tokReqMsgCtx, scope, expireTime, existingTokenBean);
return issueExistingAccessTokenWithConsent(tokReqMsgCtx, scope, expireTime, existingTokenBean);
}
}

Expand Down Expand Up @@ -438,26 +461,80 @@ private OAuth2AccessTokenRespDTO renewAccessToken(OAuthTokenReqMessageContext to
oauthTokenIssuer);
}

private OAuth2AccessTokenRespDTO issueExistingAccessToken(OAuthTokenReqMessageContext tokReqMsgCtx, String scope,
long expireTime, AccessTokenDO existingTokenBean)
/**
* Issues an existing access token by preparing the necessary details in the token request context
* and creating a response with the existing token. This method avoids generating a new token
* when a valid token already exists and can be reused.
*
* @param tokReqMsgCtx The OAuth token request message context containing the details of the current token
* request.
* @param scope The scope associated with the token request.
* @param expireTime The expiration time of the existing token.
* @param existingTokenBean The existing access token object to be reused for the current request.
* @return An OAuth2AccessTokenRespDTO object containing the response details of the
* reused token.
* @throws IdentityOAuth2Exception If an error occurs while setting details to the message context
* or creating the token response.
*/
private OAuth2AccessTokenRespDTO issueExistingAccessToken(OAuthTokenReqMessageContext tokReqMsgCtx,
String scope,
long expireTime,
AccessTokenDO existingTokenBean)
throws IdentityOAuth2Exception {

tokReqMsgCtx.addProperty(EXISTING_TOKEN_ISSUED, true);
setDetailsToMessageContext(tokReqMsgCtx, existingTokenBean);
return createResponseWithTokenBean(existingTokenBean, expireTime, scope);
}

/**
* Issues an existing access token while ensuring that it complies with the consent requirements
* of the current grant type. If the existing token was originally issued for a grant type that did
* not require consent but the current grant type does, the token's consent status is updated accordingly.
* This method avoids unnecessary token generation by reusing an existing token whenever possible,
* while updating its properties to meet the requirements of the current request.
*
* @param tokReqMsgCtx The OAuth token request message context containing the details of the current token
* request.
* @param scope The scope associated with the token request.
* @param expireTime The expiration time of the existing token.
* @param existingTokenBean The existing access token object to be evaluated, updated, and reused if valid.
* @return An {@link OAuth2AccessTokenRespDTO} object containing the response details of the
* issued token.
* @throws IdentityOAuth2Exception If an error occurs while handling consent or issuing the existing access token.
*/
private OAuth2AccessTokenRespDTO issueExistingAccessTokenWithConsent(OAuthTokenReqMessageContext tokReqMsgCtx,
String scope,
long expireTime,
AccessTokenDO existingTokenBean)
throws IdentityOAuth2Exception {

handleConsent(tokReqMsgCtx, existingTokenBean);
return issueExistingAccessToken(tokReqMsgCtx, scope, expireTime, existingTokenBean);
}

/**
* Handles consent updates for an existing access token when issuing it for a new grant type.
* If the existing access token was originally issued for a grant type that does not require consent,
* but the current grant type does require consent, this method updates the existing token to
* reflect that it is now consented.
*
* @param tokReqMsgCtx The OAuth token request message context containing the details
* of the current token request.
* @param existingTokenBean The existing access token to be evaluated and updated as needed.
* @throws IdentityOAuth2Exception If an error occurs while updating the consent status of the token.
*/
private void handleConsent(OAuthTokenReqMessageContext tokReqMsgCtx, AccessTokenDO existingTokenBean)
throws IdentityOAuth2Exception {

String requestGrantType = tokReqMsgCtx.getOauth2AccessTokenReqDTO().getGrantType();
/* When issuing the existing access token, that access token may be originated from a different grant
type. The origin grant type can be a consent required one. If the existing token is issued previously
for a consent not required grant and the current grant requires consent, we update the existing token as
a consented token. */
boolean isConsentRequiredGrant = OIDCClaimUtil.
isConsentBasedClaimFilteringApplicable(requestGrantType);
if (isConsentRequiredGrant && !existingTokenBean.isConsentedToken()) {
existingTokenBean.setIsConsentedToken(true);
OAuthTokenPersistenceFactory.getInstance().getAccessTokenDAO().updateTokenIsConsented(
existingTokenBean.getTokenId(), true);
}

setDetailsToMessageContext(tokReqMsgCtx, existingTokenBean);
return createResponseWithTokenBean(existingTokenBean, expireTime, scope);
}

private OAuth2AccessTokenRespDTO generateNewAccessToken(OAuthTokenReqMessageContext tokReqMsgCtx, String scope,
Expand Down
80 changes: 80 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 @@ -40,6 +40,9 @@
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.cache.CacheEntry;
import org.wso2.carbon.identity.oauth.cache.OAuthCache;
import org.wso2.carbon.identity.oauth.cache.OAuthCacheKey;
import org.wso2.carbon.identity.oauth.common.GrantType;
import org.wso2.carbon.identity.oauth.common.OAuthConstants;
import org.wso2.carbon.identity.oauth.config.OAuthCallbackHandlerMetaData;
Expand All @@ -48,7 +51,9 @@
import org.wso2.carbon.identity.oauth.dao.OAuthAppDO;
import org.wso2.carbon.identity.oauth.internal.OAuthComponentServiceHolder;
import org.wso2.carbon.identity.oauth2.IdentityOAuth2Exception;
import org.wso2.carbon.identity.oauth2.OAuth2Constants;
import org.wso2.carbon.identity.oauth2.TestConstants;
import org.wso2.carbon.identity.oauth2.dao.AccessTokenDAO;
import org.wso2.carbon.identity.oauth2.dao.AccessTokenDAOImpl;
import org.wso2.carbon.identity.oauth2.dao.OAuthTokenPersistenceFactory;
import org.wso2.carbon.identity.oauth2.dto.OAuth2AccessTokenReqDTO;
Expand All @@ -58,6 +63,7 @@
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.OauthTokenIssuerImpl;
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;
Expand All @@ -72,6 +78,8 @@
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyMap;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.mockStatic;
import static org.mockito.Mockito.verify;
Expand Down Expand Up @@ -455,6 +463,78 @@ public void testIsAuthorizedClient(Object tokenRequestMsgCtx, boolean debugEnabl
assertEquals(handler.isAuthorizedClient(tokReqMsgCtx), expectedValue);
}

@Test(dataProvider = "IssueExistingAccessTokensWithoutConsent")
public void testIssueExistingAccessTokensWithoutConsent(boolean idpIdColumnEnabled) throws Exception {

OAuth2ServiceComponentHolder.setIDPIdColumnEnabled(idpIdColumnEnabled);

OAuth2AccessTokenReqDTO oAuth2AccessTokenReqDTO = new OAuth2AccessTokenReqDTO();
oAuth2AccessTokenReqDTO.setClientId(clientId);
oAuth2AccessTokenReqDTO.setGrantType("client_credential");

AccessTokenDO existingAccessTokenDO = new AccessTokenDO();
existingAccessTokenDO.setAuthzUser(authenticatedUser);
existingAccessTokenDO.setScope(new String[]{"scope1", "scope2"});
existingAccessTokenDO.setAccessToken("existingAccessToken");
existingAccessTokenDO.setTokenState(TOKEN_STATE_ACTIVE);
existingAccessTokenDO.setConsumerKey(clientId);

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

try (MockedStatic<IdentityUtil> identityUtil = mockStatic(IdentityUtil.class);
MockedStatic<OAuth2Util> oauth2Util = mockStatic(OAuth2Util.class);
MockedStatic<OAuthTokenPersistenceFactory> factoryMock = mockStatic(OAuthTokenPersistenceFactory.class)) {

OAuthComponentServiceHolder.getInstance().setActionExecutorService(mockActionExecutionService);

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

OauthTokenIssuer oauthTokenIssuer = mock(OauthTokenIssuerImpl.class);
when(oauthTokenIssuer.getAccessTokenType()).thenReturn(OAuth2Constants.TokenTypes.OPAQUE);

oauth2Util.when(() -> OAuth2Util.getOAuthTokenIssuerForOAuthApp(clientId)).thenReturn(oauthTokenIssuer);
oauth2Util.when(() -> OAuth2Util.getAppInformationByClientId(clientId)).thenReturn(oAuthAppDO);
oauth2Util.when(() -> OAuth2Util.isOrganizationValidAndActive(anyString())).thenReturn(true);
oauth2Util.when(() -> OAuth2Util.buildScopeString(any())).thenCallRealMethod();
oauth2Util.when(() -> OAuth2Util.getTokenPartitionedSqlByUserStore(anyString(), any()))
.thenCallRealMethod();
oauth2Util.when(() -> OAuth2Util.getTokenExpireTimeMillis(eq(existingAccessTokenDO), eq(false)))
.thenReturn(3600L);

OAuthCache mockOAuthCache = mock(OAuthCache.class);
handler.oauthCache = mockOAuthCache;
doNothing().when(mockOAuthCache).addToCache(any(OAuthCacheKey.class), any(CacheEntry.class));

OAuthTokenPersistenceFactory mockFactory = mock(OAuthTokenPersistenceFactory.class);
AccessTokenDAO mockAccessTokenDAO = mock(AccessTokenDAO.class);
factoryMock.when(OAuthTokenPersistenceFactory::getInstance).thenReturn(mockFactory);
when(mockFactory.getAccessTokenDAO()).thenReturn(mockAccessTokenDAO);
when(mockAccessTokenDAO.getLatestAccessToken(
eq(clientId),
eq(authenticatedUser),
eq(null),
eq("scope1 scope2"),
eq("NONE"),
eq(false))).thenReturn(existingAccessTokenDO);

OAuth2AccessTokenRespDTO tokenRespDTO = handler.issue(tokReqMsgCtx);
assertNotNull(tokenRespDTO.getAccessToken());
assertEquals(tokenRespDTO.getAccessToken(), existingAccessTokenDO.getAccessToken());
}
}

@DataProvider(name = "IssueExistingAccessTokensWithoutConsent")
public Object[][] issueExistingAccessTokensWithoutConsent() {

return new Object[][]{
{true}, {false}
};
}

private static class MockAuthzGrantHandler extends AbstractAuthorizationGrantHandler {

}
Expand Down
Loading