From 38c3f3dd8ea5ed412f42af0691a8ebc3d37eda65 Mon Sep 17 00:00:00 2001 From: Shenali Date: Fri, 25 Oct 2024 22:19:17 +0530 Subject: [PATCH] Handle action execution error status and failed status --- .../endpoint/token/OAuth2TokenEndpoint.java | 1 - .../PreIssueAccessTokenResponseProcessor.java | 71 +++++++++++++++++-- .../AbstractAuthorizationGrantHandler.java | 32 +++++++-- .../handlers/grant/RefreshGrantHandler.java | 40 ++++++++--- ...AbstractAuthorizationGrantHandlerTest.java | 2 +- 5 files changed, 127 insertions(+), 19 deletions(-) diff --git a/components/org.wso2.carbon.identity.oauth.endpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/token/OAuth2TokenEndpoint.java b/components/org.wso2.carbon.identity.oauth.endpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/token/OAuth2TokenEndpoint.java index acff55c6e87..75cc97305f2 100644 --- a/components/org.wso2.carbon.identity.oauth.endpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/token/OAuth2TokenEndpoint.java +++ b/components/org.wso2.carbon.identity.oauth.endpoint/src/main/java/org/wso2/carbon/identity/oauth/endpoint/token/OAuth2TokenEndpoint.java @@ -366,7 +366,6 @@ private Response handleServerError() throws OAuthSystemException { EndpointUtil.getRealmInfo()).entity(response.getBody()).build(); } - private Response handleSQLError() throws OAuthSystemException { OAuthResponse response = OAuthASResponse.errorResponse(HttpServletResponse.SC_BAD_GATEWAY). diff --git a/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth/action/PreIssueAccessTokenResponseProcessor.java b/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth/action/PreIssueAccessTokenResponseProcessor.java index f56fa00b26b..9d4cccea942 100644 --- a/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth/action/PreIssueAccessTokenResponseProcessor.java +++ b/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth/action/PreIssueAccessTokenResponseProcessor.java @@ -28,15 +28,20 @@ import org.wso2.carbon.identity.action.execution.exception.ActionExecutionResponseProcessorException; import org.wso2.carbon.identity.action.execution.model.ActionExecutionStatus; import org.wso2.carbon.identity.action.execution.model.ActionInvocationErrorResponse; +import org.wso2.carbon.identity.action.execution.model.ActionInvocationFailureResponse; import org.wso2.carbon.identity.action.execution.model.ActionInvocationSuccessResponse; import org.wso2.carbon.identity.action.execution.model.ActionType; +import org.wso2.carbon.identity.action.execution.model.ErrorResponse; import org.wso2.carbon.identity.action.execution.model.Event; +import org.wso2.carbon.identity.action.execution.model.FailedResponse; import org.wso2.carbon.identity.action.execution.model.PerformableOperation; +import org.wso2.carbon.identity.action.execution.model.Response; import org.wso2.carbon.identity.central.log.mgt.utils.LoggerUtils; import org.wso2.carbon.identity.oauth.action.model.AccessToken; import org.wso2.carbon.identity.oauth.action.model.ClaimPathInfo; import org.wso2.carbon.identity.oauth.action.model.OperationExecutionResult; import org.wso2.carbon.identity.oauth.action.model.PreIssueAccessTokenEvent; +import org.wso2.carbon.identity.oauth.common.OAuth2ErrorCodes; import org.wso2.carbon.identity.oauth2.token.OAuthTokenReqMessageContext; import org.wso2.carbon.utils.DiagnosticLog; @@ -63,7 +68,6 @@ public class PreIssueAccessTokenResponseProcessor implements ActionExecutionResp Pattern.compile("^([a-zA-Z][a-zA-Z0-9+.-]*://[^\\s/$.?#].\\S*)|(^[a-zA-Z0-9.-]+$)"); private static final String LAST_ELEMENT_CHARACTER = "-"; private static final char PATH_SEPARATOR = '/'; - @Override public ActionType getSupportedActionType() { @@ -153,15 +157,59 @@ private void logOperationExecutionResults(ActionType actionType, } } + @Override + public ActionExecutionStatus processFailureResponse(Map eventContext, + Event actionEvent, + ActionInvocationFailureResponse failureResponse) throws + ActionExecutionResponseProcessorException { + + processServerError(failureResponse.getFailureReason()); + Response failedResponse = new FailedResponse(failureResponse.getFailureReason(), + failureResponse.getFailureDescription()); + return new ActionExecutionStatus(ActionExecutionStatus.Status.FAILED, failedResponse); + } + + private void processServerError(String errorCode) + throws ActionExecutionResponseProcessorException { + + if (isServerError(errorCode)) { + if (LoggerUtils.isDiagnosticLogsEnabled()) { + DiagnosticLog.DiagnosticLogBuilder diagLogBuilder = new DiagnosticLog.DiagnosticLogBuilder( + ActionExecutionLogConstants.ACTION_EXECUTION_COMPONENT_ID, + ActionExecutionLogConstants.ActionIDs.EXECUTE_ACTION_OPERATIONS); + diagLogBuilder + .resultMessage("An exception was thrown for action. FAILED status should not be used to " + + "process server errors.") + .logDetailLevel(DiagnosticLog.LogDetailLevel.APPLICATION) + .resultStatus(DiagnosticLog.ResultStatus.FAILED) + .build(); + LoggerUtils.triggerDiagnosticLogEvent(diagLogBuilder); + } + throw new ActionExecutionResponseProcessorException("FAILED status should not be used to process" + + " server errors."); + } + } + + private boolean isServerError(String errorCode) { + + return (errorCode.equalsIgnoreCase(ServerErrorCode.INTERNAL_SERVER_ERROR.code.trim()) || + errorCode.equalsIgnoreCase(ServerErrorCode.SERVER_ERROR.code.trim()) || + errorCode.equalsIgnoreCase(ServerErrorCode.ERROR_500.toString())); + } + @Override public ActionExecutionStatus processErrorResponse(Map map, Event event, ActionInvocationErrorResponse actionInvocationErrorResponse) throws ActionExecutionResponseProcessorException { - //todo: need to implement to process the error so that if a processable error is received - // it is communicated to the client. - // we will look into this as we go along with other extension types validating the way to model this. - return null; + // Client and server errors that occur when calling the service implementing the extension are reported + // as Internal_Server_Error. + // The error description could be utilized to offer additional context by passing along the + // original error returned by the service implementing the extension. + // However, currently this value is not propagated by the endpoint to comply with OAuth specification. + Response errorResponse = new ErrorResponse(OAuth2ErrorCodes.SERVER_ERROR, + actionInvocationErrorResponse.getErrorDescription()); + return new ActionExecutionStatus(ActionExecutionStatus.Status.ERROR, errorResponse); } private void updateTokenMessageContext(OAuthTokenReqMessageContext tokenMessageContext, @@ -743,4 +791,17 @@ private boolean isValidStringOrURI(String input) { Matcher matcher = STRING_OR_URI_PATTERN.matcher(input); return matcher.matches(); } + + private enum ServerErrorCode { + SERVER_ERROR("server_error"), + INTERNAL_SERVER_ERROR("internal_server_error"), + ERROR_500("500"); + + private final String code; + + ServerErrorCode(String code) { + + this.code = code; + } + } } diff --git a/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/token/handlers/grant/AbstractAuthorizationGrantHandler.java b/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/token/handlers/grant/AbstractAuthorizationGrantHandler.java index c7bfab496b9..1f9c9a4653b 100644 --- a/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/token/handlers/grant/AbstractAuthorizationGrantHandler.java +++ b/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/token/handlers/grant/AbstractAuthorizationGrantHandler.java @@ -29,6 +29,8 @@ import org.wso2.carbon.identity.action.execution.exception.ActionExecutionException; import org.wso2.carbon.identity.action.execution.model.ActionExecutionStatus; import org.wso2.carbon.identity.action.execution.model.ActionType; +import org.wso2.carbon.identity.action.execution.model.ErrorResponse; +import org.wso2.carbon.identity.action.execution.model.FailedResponse; import org.wso2.carbon.identity.application.authentication.framework.exception.UserIdNotFoundException; import org.wso2.carbon.identity.application.authentication.framework.model.AuthenticatedUser; import org.wso2.carbon.identity.base.IdentityConstants; @@ -440,7 +442,11 @@ private OAuth2AccessTokenRespDTO generateNewAccessToken(OAuthTokenReqMessageCont Timestamp timestamp = new Timestamp(new Date().getTime()); updateMessageContextToCreateNewToken(tokReqMsgCtx, consumerKey, existingTokenBean, timestamp); - executePreIssueAccessTokenActions(tokReqMsgCtx); + ActionExecutionStatus executionStatus = executePreIssueAccessTokenActions(tokReqMsgCtx); + if (executionStatus != null && (executionStatus.getStatus() == ActionExecutionStatus.Status.FAILED || + executionStatus.getStatus() == ActionExecutionStatus.Status.ERROR)) { + return getFailureOrErrorResponseDTO(executionStatus); + } AccessTokenDO newTokenBean = createNewTokenBean(tokReqMsgCtx, existingTokenBean, oauthTokenIssuer); /* Check whether the existing token needs to be expired and send the corresponding parameters to the @@ -461,9 +467,27 @@ private OAuth2AccessTokenRespDTO generateNewAccessToken(OAuthTokenReqMessageCont return createResponseWithTokenBean(newTokenBean, newTokenBean.getValidityPeriodInMillis(), scope); } - private void executePreIssueAccessTokenActions(OAuthTokenReqMessageContext tokenReqMessageContext) + private OAuth2AccessTokenRespDTO getFailureOrErrorResponseDTO(ActionExecutionStatus executionStatus) { + + OAuth2AccessTokenRespDTO accessTokenResponse = new OAuth2AccessTokenRespDTO(); + if (executionStatus.getStatus() == ActionExecutionStatus.Status.FAILED) { + accessTokenResponse.setError(true); + FailedResponse response = (FailedResponse) executionStatus.getResponse(); + accessTokenResponse.setErrorCode(response.getReason()); + accessTokenResponse.setErrorMsg(response.getDescription()); + } else if (executionStatus.getStatus() == ActionExecutionStatus.Status.ERROR) { + accessTokenResponse.setError(true); + ErrorResponse response = (ErrorResponse) executionStatus.getResponse(); + accessTokenResponse.setErrorCode(response.getError()); + accessTokenResponse.setErrorMsg(response.getDescription()); + } + return accessTokenResponse; + } + + private ActionExecutionStatus executePreIssueAccessTokenActions(OAuthTokenReqMessageContext tokenReqMessageContext) throws IdentityOAuth2Exception { + ActionExecutionStatus executionStatus = null; if (checkExecutePreIssueAccessTokensActions(tokenReqMessageContext)) { Map additionalProperties = new HashMap<>(); @@ -472,8 +496,7 @@ private void executePreIssueAccessTokenActions(OAuthTokenReqMessageContext token mapInitializer.accept(additionalProperties); try { - ActionExecutionStatus executionStatus = - OAuthComponentServiceHolder.getInstance().getActionExecutorService() + executionStatus = OAuthComponentServiceHolder.getInstance().getActionExecutorService() .execute(ActionType.PRE_ISSUE_ACCESS_TOKEN, additionalProperties, IdentityTenantUtil.getTenantDomain(IdentityTenantUtil.getLoginTenantId())); if (log.isDebugEnabled()) { @@ -488,6 +511,7 @@ private void executePreIssueAccessTokenActions(OAuthTokenReqMessageContext token log.error("Error while executing pre issue access token action", e); } } + return executionStatus; } private boolean checkExecutePreIssueAccessTokensActions(OAuthTokenReqMessageContext tokenReqMessageContext) diff --git a/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/token/handlers/grant/RefreshGrantHandler.java b/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/token/handlers/grant/RefreshGrantHandler.java index 6e8c6896849..f48b6c357c3 100644 --- a/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/token/handlers/grant/RefreshGrantHandler.java +++ b/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/token/handlers/grant/RefreshGrantHandler.java @@ -27,6 +27,8 @@ import org.wso2.carbon.identity.action.execution.exception.ActionExecutionException; import org.wso2.carbon.identity.action.execution.model.ActionExecutionStatus; import org.wso2.carbon.identity.action.execution.model.ActionType; +import org.wso2.carbon.identity.action.execution.model.ErrorResponse; +import org.wso2.carbon.identity.action.execution.model.FailedResponse; import org.wso2.carbon.identity.application.authentication.framework.exception.FrameworkException; import org.wso2.carbon.identity.application.authentication.framework.exception.UserIdNotFoundException; import org.wso2.carbon.identity.application.authentication.framework.inbound.FrameworkClientException; @@ -142,7 +144,11 @@ public OAuth2AccessTokenRespDTO issue(OAuthTokenReqMessageContext tokReqMsgCtx) tokReqMsgCtx.setValidityPeriod(validationBean.getAccessTokenValidityInMillis()); - executePreIssueAccessTokenActions(validationBean, tokReqMsgCtx); + ActionExecutionStatus executionStatus = executePreIssueAccessTokenActions(validationBean, tokReqMsgCtx); + if (executionStatus != null && (executionStatus.getStatus() == ActionExecutionStatus.Status.FAILED || + executionStatus.getStatus() == ActionExecutionStatus.Status.ERROR)) { + return getFailureOrErrorResponseDTO(executionStatus); + } AccessTokenDO accessTokenBean = getRefreshTokenGrantProcessor() .createAccessTokenBean(tokReqMsgCtx, tokenReq, validationBean, getTokenType(tokReqMsgCtx)); @@ -178,6 +184,23 @@ public OAuth2AccessTokenRespDTO issue(OAuthTokenReqMessageContext tokReqMsgCtx) return buildTokenResponse(tokReqMsgCtx, accessTokenBean); } + private OAuth2AccessTokenRespDTO getFailureOrErrorResponseDTO(ActionExecutionStatus executionStatus) { + + OAuth2AccessTokenRespDTO accessTokenResponse = new OAuth2AccessTokenRespDTO(); + if (executionStatus.getStatus() == ActionExecutionStatus.Status.FAILED) { + accessTokenResponse.setError(true); + FailedResponse response = (FailedResponse) executionStatus.getResponse(); + accessTokenResponse.setErrorCode(response.getReason()); + accessTokenResponse.setErrorMsg(response.getDescription()); + } else if (executionStatus.getStatus() == ActionExecutionStatus.Status.ERROR) { + accessTokenResponse.setError(true); + ErrorResponse response = (ErrorResponse) executionStatus.getResponse(); + accessTokenResponse.setErrorCode(response.getError()); + accessTokenResponse.setErrorMsg(response.getDescription()); + } + return accessTokenResponse; + } + @Override public boolean validateScope(OAuthTokenReqMessageContext tokReqMsgCtx) throws IdentityOAuth2Exception { @@ -763,10 +786,11 @@ private static void addUserAttributesToCache(AccessTokenDO accessTokenBean, } } - private void executePreIssueAccessTokenActions(RefreshTokenValidationDataDO refreshTokenValidationDataDO, - OAuthTokenReqMessageContext tokenReqMessageContext) - throws IdentityOAuth2Exception { + private ActionExecutionStatus executePreIssueAccessTokenActions( + RefreshTokenValidationDataDO refreshTokenValidationDataDO, + OAuthTokenReqMessageContext tokenReqMessageContext) throws IdentityOAuth2Exception { + ActionExecutionStatus executionStatus = null; if (checkExecutePreIssueAccessTokensActions(refreshTokenValidationDataDO, tokenReqMessageContext)) { setCustomizedAccessTokenAttributesToMessageContext(refreshTokenValidationDataDO, tokenReqMessageContext); @@ -777,10 +801,9 @@ private void executePreIssueAccessTokenActions(RefreshTokenValidationDataDO refr mapInitializer.accept(additionalProperties); try { - ActionExecutionStatus executionStatus = - OAuthComponentServiceHolder.getInstance().getActionExecutorService() - .execute(ActionType.PRE_ISSUE_ACCESS_TOKEN, additionalProperties, - IdentityTenantUtil.getTenantDomain(IdentityTenantUtil.getLoginTenantId())); + executionStatus = OAuthComponentServiceHolder.getInstance().getActionExecutorService() + .execute(ActionType.PRE_ISSUE_ACCESS_TOKEN, additionalProperties, + IdentityTenantUtil.getTenantDomain(IdentityTenantUtil.getLoginTenantId())); if (log.isDebugEnabled()) { log.debug(String.format( "Invoked pre issue access token action for clientID: %s grant types: %s. Status: %s", @@ -793,6 +816,7 @@ private void executePreIssueAccessTokenActions(RefreshTokenValidationDataDO refr log.error("Error while executing pre issue access token action", e); } } + return executionStatus; } private boolean checkExecutePreIssueAccessTokensActions(RefreshTokenValidationDataDO refreshTokenValidationDataDO, diff --git a/components/org.wso2.carbon.identity.oauth/src/test/java/org/wso2/carbon/identity/oauth2/token/handlers/grant/AbstractAuthorizationGrantHandlerTest.java b/components/org.wso2.carbon.identity.oauth/src/test/java/org/wso2/carbon/identity/oauth2/token/handlers/grant/AbstractAuthorizationGrantHandlerTest.java index 7e9dbf6268a..cef6abff7fe 100644 --- a/components/org.wso2.carbon.identity.oauth/src/test/java/org/wso2/carbon/identity/oauth2/token/handlers/grant/AbstractAuthorizationGrantHandlerTest.java +++ b/components/org.wso2.carbon.identity.oauth/src/test/java/org/wso2/carbon/identity/oauth2/token/handlers/grant/AbstractAuthorizationGrantHandlerTest.java @@ -105,7 +105,7 @@ public void setUp() throws IdentityOAuth2Exception, IdentityOAuthAdminException, OAuthComponentServiceHolder.getInstance().setActionExecutorService(mockActionExecutionService); MockitoAnnotations.initMocks(this); when(mockActionExecutionService.execute(any(ActionType.class), anyMap(), any())).thenReturn( - new ActionExecutionStatus(ActionExecutionStatus.Status.SUCCESS, null)); + new ActionExecutionStatus(ActionExecutionStatus.Status.SUCCESS)); authenticatedUser.setUserName("randomUser"); authenticatedUser.setTenantDomain("Homeless");