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.

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

Refactor the access token flow to terminate for ERROR or FAILED action execution status #2620

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -21,22 +21,31 @@
import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.apache.commons.httpclient.HttpStatus;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.wso2.carbon.identity.action.execution.ActionExecutionLogConstants;
import org.wso2.carbon.identity.action.execution.ActionExecutionResponseProcessor;
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.Error;
import org.wso2.carbon.identity.action.execution.model.ErrorStatus;
import org.wso2.carbon.identity.action.execution.model.Event;
import org.wso2.carbon.identity.action.execution.model.FailedStatus;
import org.wso2.carbon.identity.action.execution.model.Failure;
import org.wso2.carbon.identity.action.execution.model.PerformableOperation;
import org.wso2.carbon.identity.action.execution.model.Success;
import org.wso2.carbon.identity.action.execution.model.SuccessStatus;
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;

Expand Down Expand Up @@ -71,7 +80,7 @@ public ActionType getSupportedActionType() {
}

@Override
public ActionExecutionStatus processSuccessResponse(Map<String, Object> eventContext, Event event,
public ActionExecutionStatus<Success> processSuccessResponse(Map<String, Object> eventContext, Event event,
ActionInvocationSuccessResponse actionInvocationSuccessResponse)
throws ActionExecutionResponseProcessorException {

Expand Down Expand Up @@ -110,7 +119,7 @@ public ActionExecutionStatus processSuccessResponse(Map<String, Object> eventCon
AccessToken responseAccessToken = responseAccessTokenBuilder.build();
updateTokenMessageContext(tokenMessageContext, responseAccessToken);

return new ActionExecutionStatus(ActionExecutionStatus.Status.SUCCESS, eventContext);
return new SuccessStatus.Builder().setResponseContext(eventContext).build();
}

private void logOperationExecutionResults(ActionType actionType,
Expand Down Expand Up @@ -154,14 +163,64 @@ private void logOperationExecutionResults(ActionType actionType,
}

@Override
public ActionExecutionStatus processErrorResponse(Map<String, Object> map, Event event,
ActionInvocationErrorResponse actionInvocationErrorResponse)
public ActionExecutionStatus<Failure> processFailureResponse(Map<String, Object> eventContext,
Event actionEvent,
ActionInvocationFailureResponse failureResponse) throws
ActionExecutionResponseProcessorException {

handleInvalidErrorCodes(failureResponse.getFailureReason());
return new FailedStatus(new Failure(failureResponse.getFailureReason(),
failureResponse.getFailureDescription()));
}

/**
* This method validates the failedReason attribute in the FAILED status.
* @param errorCode
* @throws ActionExecutionResponseProcessorException
*/
private void handleInvalidErrorCodes(String errorCode) throws ActionExecutionResponseProcessorException {

// According to the current API contract server_error is considered as an invalid value for the failureReason
// attribute.
if (isServerError(errorCode)) {
if (LoggerUtils.isDiagnosticLogsEnabled()) {
DiagnosticLog.DiagnosticLogBuilder diagLogBuilder = new DiagnosticLog.DiagnosticLogBuilder(
ActionExecutionLogConstants.ACTION_EXECUTION_COMPONENT_ID,
ActionExecutionLogConstants.ActionIDs.VALIDATE_ACTION_RESPONSE);
diagLogBuilder
.resultMessage("Invalid value for failedReason attribute at FAILED state.")
.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("internal_server_error") ||
errorCode.equalsIgnoreCase("server_error") ||
errorCode.equalsIgnoreCase(String.valueOf(HttpStatus.SC_INTERNAL_SERVER_ERROR)));
}

@Override
public ActionExecutionStatus<Error> processErrorResponse(Map<String, Object> 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.
*/
return new ErrorStatus(new Error(OAuth2ErrorCodes.SERVER_ERROR,
actionInvocationErrorResponse.getErrorDescription()));
}

private void updateTokenMessageContext(OAuthTokenReqMessageContext tokenMessageContext,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.Error;
import org.wso2.carbon.identity.action.execution.model.Failure;
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;
Expand Down Expand Up @@ -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
Expand All @@ -461,9 +467,26 @@ private OAuth2AccessTokenRespDTO generateNewAccessToken(OAuthTokenReqMessageCont
return createResponseWithTokenBean(newTokenBean, newTokenBean.getValidityPeriodInMillis(), scope);
}

private void executePreIssueAccessTokenActions(OAuthTokenReqMessageContext tokenReqMessageContext)
throws IdentityOAuth2Exception {
private OAuth2AccessTokenRespDTO getFailureOrErrorResponseDTO(ActionExecutionStatus<?> executionStatus) {

OAuth2AccessTokenRespDTO accessTokenResponse = new OAuth2AccessTokenRespDTO();
accessTokenResponse.setError(true);
if (executionStatus.getStatus() == ActionExecutionStatus.Status.FAILED) {
Failure failureResponse = (Failure) executionStatus.getResponse();
accessTokenResponse.setErrorCode(failureResponse.getReason());
accessTokenResponse.setErrorMsg(failureResponse.getDescription());
} else if (executionStatus.getStatus() == ActionExecutionStatus.Status.ERROR) {
Error errorResponse = (Error) executionStatus.getResponse();
accessTokenResponse.setErrorCode(errorResponse.getErrorMessage());
accessTokenResponse.setErrorMsg(errorResponse.getErrorDescription());
}
return accessTokenResponse;
}

private ActionExecutionStatus<?> executePreIssueAccessTokenActions(
OAuthTokenReqMessageContext tokenReqMessageContext) throws IdentityOAuth2Exception {

ActionExecutionStatus<?> executionStatus = null;
if (checkExecutePreIssueAccessTokensActions(tokenReqMessageContext)) {

Map<String, Object> additionalProperties = new HashMap<>();
Expand All @@ -472,8 +495,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()) {
Expand All @@ -488,6 +510,7 @@ private void executePreIssueAccessTokenActions(OAuthTokenReqMessageContext token
log.error("Error while executing pre issue access token action", e);
}
}
return executionStatus;
}

private boolean checkExecutePreIssueAccessTokensActions(OAuthTokenReqMessageContext tokenReqMessageContext)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.Error;
import org.wso2.carbon.identity.action.execution.model.Failure;
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;
Expand Down Expand Up @@ -142,7 +144,12 @@ 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));

Expand Down Expand Up @@ -178,6 +185,22 @@ public OAuth2AccessTokenRespDTO issue(OAuthTokenReqMessageContext tokReqMsgCtx)
return buildTokenResponse(tokReqMsgCtx, accessTokenBean);
}

private OAuth2AccessTokenRespDTO getFailureOrErrorResponseDTO(ActionExecutionStatus<?> executionStatus) {

OAuth2AccessTokenRespDTO accessTokenResponse = new OAuth2AccessTokenRespDTO();
accessTokenResponse.setError(true);
if (executionStatus.getStatus() == ActionExecutionStatus.Status.FAILED) {
Failure failureResponse = (Failure) executionStatus.getResponse();
accessTokenResponse.setErrorCode(failureResponse.getReason());
accessTokenResponse.setErrorMsg(failureResponse.getDescription());
} else if (executionStatus.getStatus() == ActionExecutionStatus.Status.ERROR) {
Error errorResponse = (Error) executionStatus.getResponse();
accessTokenResponse.setErrorCode(errorResponse.getErrorMessage());
accessTokenResponse.setErrorMsg(errorResponse.getErrorDescription());
}
return accessTokenResponse;
}

@Override
public boolean validateScope(OAuthTokenReqMessageContext tokReqMsgCtx)
throws IdentityOAuth2Exception {
Expand Down Expand Up @@ -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);
Expand All @@ -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",
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
Loading