From edbf94e2d49a2d7dab1346fea9ed1b4b50283ddf Mon Sep 17 00:00:00 2001
From: sadilchamishka <sandilchamishka@gmail.com>
Date: Mon, 11 Nov 2024 07:47:28 +0530
Subject: [PATCH] Improve request object clean up logic to avoid unnessary
delete requests
---
.../wso2/carbon/identity/oauth/OAuthUtil.java | 2 +-
.../oauth2/dao/AccessTokenDAOImpl.java | 10 +++++---
.../oauth2/dao/AuthorizationCodeDAO.java | 2 ++
.../oauth2/dao/AuthorizationCodeDAOImpl.java | 25 +++++++++++--------
.../grant/AuthorizationCodeGrantHandler.java | 6 +++--
.../dao/AuthorizationCodeDAOImplTest.java | 2 +-
6 files changed, 29 insertions(+), 18 deletions(-)
diff --git a/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth/OAuthUtil.java b/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth/OAuthUtil.java
index 4cf88072dcf..991170280e5 100755
--- a/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth/OAuthUtil.java
+++ b/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth/OAuthUtil.java
@@ -689,7 +689,7 @@ public static boolean revokeAuthzCodes(String username, UserStoreManager userSto
authorizationCode.getAuthorizationCode())));
OAuthTokenPersistenceFactory.getInstance().getAuthorizationCodeDAO()
.updateAuthorizationCodeState(authorizationCode.getAuthorizationCode(),
- OAuthConstants.AuthorizationCodeState.REVOKED);
+ authorizationCode.getAuthzCodeId(), OAuthConstants.AuthorizationCodeState.REVOKED);
}
} catch (IdentityOAuth2Exception e) {
String errorMsg = "Error occurred while revoking authorization codes for user: " + username;
diff --git a/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/dao/AccessTokenDAOImpl.java b/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/dao/AccessTokenDAOImpl.java
index bc9cbdd406d..ac350c0fcdd 100644
--- a/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/dao/AccessTokenDAOImpl.java
+++ b/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/dao/AccessTokenDAOImpl.java
@@ -1494,12 +1494,16 @@ public void revokeAccessTokensInBatch(String[] tokens, boolean isHashedToken) th
}
ps.executeUpdate();
- // To revoke request objects which have persisted against the access token.
- OAuth2TokenUtil.postUpdateAccessTokens(Arrays.asList(tokens), OAuthConstants.TokenStates.
- TOKEN_STATE_REVOKED);
+
if (isTokenCleanupFeatureEnabled) {
oldTokenCleanupObject.cleanupTokenByTokenValue(
getHashingPersistenceProcessor().getProcessedAccessTokenIdentifier(tokens[0]), connection);
+ /* When token is deleted, the request objects get on delete cascade except for the SQL server.
+ Hence, invoke the event listener to revoke the request objects.*/
+ if (connection.getMetaData().getDriverName().contains("Microsoft")) {
+ OAuth2TokenUtil.postUpdateAccessTokens(Arrays.asList(tokens), OAuthConstants.TokenStates.
+ TOKEN_STATE_REVOKED);
+ }
}
} catch (SQLException e) {
// IdentityDatabaseUtil.rollbackTransaction(connection);
diff --git a/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/dao/AuthorizationCodeDAO.java b/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/dao/AuthorizationCodeDAO.java
index 8307b01ecfa..8bdc09b2728 100644
--- a/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/dao/AuthorizationCodeDAO.java
+++ b/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/dao/AuthorizationCodeDAO.java
@@ -66,6 +66,8 @@ void insertAuthorizationCode(String authzCode, String consumerKey, String appTen
AuthorizationCodeValidationResult validateAuthorizationCode(String consumerKey, String authorizationKey)
throws IdentityOAuth2Exception;
+ void updateAuthorizationCodeState(String authzCode, String codeId, String newState) throws IdentityOAuth2Exception;
+
void updateAuthorizationCodeState(String authzCode, String newState) throws IdentityOAuth2Exception;
void deactivateAuthorizationCode(AuthzCodeDO authzCodeDO) throws
diff --git a/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/dao/AuthorizationCodeDAOImpl.java b/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/dao/AuthorizationCodeDAOImpl.java
index 2cb2e33ed36..9043f4ae8cb 100644
--- a/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/dao/AuthorizationCodeDAOImpl.java
+++ b/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/dao/AuthorizationCodeDAOImpl.java
@@ -312,7 +312,8 @@ private String getTokenBindingReference(Connection connection, String tokenId, i
}
@Override
- public void updateAuthorizationCodeState(String authzCode, String newState) throws IdentityOAuth2Exception {
+ public void updateAuthorizationCodeState(String authzCode, String codeId, String newState)
+ throws IdentityOAuth2Exception {
if (log.isDebugEnabled()) {
if (IdentityUtil.isTokenLoggable(IdentityConstants.IdentityTokens.AUTHORIZATION_CODE)) {
@@ -322,9 +323,7 @@ public void updateAuthorizationCodeState(String authzCode, String newState) thro
log.debug("Changing state of authorization code to: " + newState);
}
}
- boolean tokenUpdateSuccessful;
- String authCodeStoreTable = OAuthConstants.AUTHORIZATION_CODE_STORE_TABLE;
- Connection connection = IdentityDatabaseUtil.getDBConnection();
+ Connection connection = IdentityDatabaseUtil.getDBConnection(true);
PreparedStatement prepStmt = null;
try {
prepStmt = connection.prepareStatement(SQLQueries.UPDATE_AUTHORIZATION_CODE_STATE);
@@ -332,19 +331,23 @@ public void updateAuthorizationCodeState(String authzCode, String newState) thro
prepStmt.setString(2, getHashingPersistenceProcessor().getProcessedAuthzCode(authzCode));
prepStmt.execute();
IdentityDatabaseUtil.commitTransaction(connection);
- tokenUpdateSuccessful = true;
} catch (SQLException e) {
IdentityDatabaseUtil.rollbackTransaction(connection);
throw new IdentityOAuth2Exception("Error occurred while updating the state of Authorization Code : " +
- authzCode.toString(), e);
+ authzCode, e);
} finally {
IdentityDatabaseUtil.closeAllConnections(connection, null, prepStmt);
}
- if (tokenUpdateSuccessful) {
- //If the code state is updated to inactive or expired request object which is persisted against the code
- // should be updated/removed.
- OAuth2TokenUtil.postRevokeCode(authzCode, newState, null, null);
- }
+ //If the code state is updated to inactive or expired request object which is persisted against the code
+ // should be updated/removed.
+ OAuth2TokenUtil.postRevokeCode(codeId, newState, null, authzCode);
+ }
+
+
+ @Override
+ public void updateAuthorizationCodeState(String authzCode, String newState) throws IdentityOAuth2Exception {
+
+ updateAuthorizationCodeState(authzCode, null, newState);
}
@Override
diff --git a/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/token/handlers/grant/AuthorizationCodeGrantHandler.java b/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/token/handlers/grant/AuthorizationCodeGrantHandler.java
index 21b3ae46d03..0c58c9a1d5c 100644
--- a/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/token/handlers/grant/AuthorizationCodeGrantHandler.java
+++ b/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/token/handlers/grant/AuthorizationCodeGrantHandler.java
@@ -550,7 +550,7 @@ private boolean isAuthzCodeExpired(AuthzCodeDO authzCodeBean)
private void markAsExpired(AuthzCodeDO authzCodeBean) throws IdentityOAuth2Exception {
OAuthTokenPersistenceFactory.getInstance().getAuthorizationCodeDAO()
- .updateAuthorizationCodeState(authzCodeBean.getAuthorizationCode(),
+ .updateAuthorizationCodeState(authzCodeBean.getAuthorizationCode(), authzCodeBean.getAuthzCodeId(),
OAuthConstants.AuthorizationCodeState.EXPIRED);
if (log.isDebugEnabled()) {
log.debug("Changed state of authorization code : " + authzCodeBean.getAuthorizationCode() + " to expired");
@@ -594,8 +594,10 @@ private boolean validatePKCECode(AuthzCodeDO authzCodeBean, String verificationC
}
private void revokeAuthorizationCode(AuthzCodeDO authzCodeBean) throws IdentityOAuth2Exception {
+
OAuthTokenPersistenceFactory.getInstance().getAuthorizationCodeDAO().updateAuthorizationCodeState(
- authzCodeBean.getAuthorizationCode(), OAuthConstants.AuthorizationCodeState.REVOKED);
+ authzCodeBean.getAuthorizationCode(), authzCodeBean.getAuthzCodeId(),
+ OAuthConstants.AuthorizationCodeState.REVOKED);
if (log.isDebugEnabled()) {
log.debug("Changed state of authorization code : " + authzCodeBean.getAuthorizationCode() + " to revoked");
}
diff --git a/components/org.wso2.carbon.identity.oauth/src/test/java/org/wso2/carbon/identity/oauth2/dao/AuthorizationCodeDAOImplTest.java b/components/org.wso2.carbon.identity.oauth/src/test/java/org/wso2/carbon/identity/oauth2/dao/AuthorizationCodeDAOImplTest.java
index 660dd49bd6b..9f6615f1418 100644
--- a/components/org.wso2.carbon.identity.oauth/src/test/java/org/wso2/carbon/identity/oauth2/dao/AuthorizationCodeDAOImplTest.java
+++ b/components/org.wso2.carbon.identity.oauth/src/test/java/org/wso2/carbon/identity/oauth2/dao/AuthorizationCodeDAOImplTest.java
@@ -282,7 +282,7 @@ public void testGetActiveAuthorizationCodesByConsumerKey() throws Exception {
.thenAnswer(
(Answer<Void>) invocation -> null);
authorizationCodeDAO.updateAuthorizationCodeState(authzCodeDO1.getAuthorizationCode(),
- OAuthConstants.AuthorizationCodeState.REVOKED);
+ authzCodeDO1.getAuthzCodeId(), OAuthConstants.AuthorizationCodeState.REVOKED);
Set<String> availableAuthzCodes = new HashSet<>();
availableAuthzCodes.add(authzCode2);