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

Improve request object clean up logic to avoid unnecessary delete requests #2623

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

Improve request object clean up logic to avoid unnecessary delete requests #2623

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
f4b2b22
Improve request object clean up logic to avoid unnecessary delete req…
sadilchamishka Nov 11, 2024
a647341
Revert updated unit test
sadilchamishka Nov 17, 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: 1 addition & 1 deletion ...rg.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth/OAuthUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
10 changes: 7 additions & 3 deletions ....identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/dao/AccessTokenDAOImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 2 additions & 0 deletions ...dentity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/dao/AuthorizationCodeDAO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
23 changes: 13 additions & 10 deletions ...ity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/dao/AuthorizationCodeDAOImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand All @@ -322,8 +323,6 @@ 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();
PreparedStatement prepStmt = null;
try {
Expand All @@ -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
Expand Down
6 changes: 4 additions & 2 deletions ...a/org/wso2/carbon/identity/oauth2/token/handlers/grant/AuthorizationCodeGrantHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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");
}
Expand Down
Loading