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 api based auth error handling #5288

Merged
merged 1 commit into from
Dec 11, 2023
Merged

Improve api based auth error handling #5288

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
72 changes: 52 additions & 20 deletions .../org/wso2/carbon/identity/application/authentication/framework/AuthenticationService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,33 +181,52 @@ private void handleFailedConcludedAuthResponse(AuthServiceRequestWrapper request
if (authenticationResult != null) {
internalErrorCode = (String) authenticationResult.getProperty(FrameworkConstants.AUTH_ERROR_CODE);
internalErrorMessage = (String) authenticationResult.getProperty(FrameworkConstants.AUTH_ERROR_MSG);
} else if (request.isSentToRetry()) {
// Check for retry status to resolve mapped error.
String retryStatus = (String) request.getAttribute(FrameworkConstants.REQ_ATTR_RETRY_STATUS);
if (StringUtils.isNotBlank(retryStatus)) {
internalErrorCode = retryStatus;
}
}

AuthServiceConstants.ErrorMessage mappedError = getMappedError(internalErrorCode);
if (mappedError != null) {
errorCode = mappedError.code();
errorMessage = mappedError.message();
errorDescription = mappedError.description();
} else {
String builtErrorMessage = buildFailedConcludedErrorMessage(internalErrorCode, internalErrorMessage);
if (StringUtils.isNotBlank(builtErrorMessage)) {
errorMessage = builtErrorMessage;
}
}

AuthServiceErrorInfo errorInfo = new AuthServiceErrorInfo(errorCode, errorMessage, errorDescription);
authServiceResponse.setErrorInfo(errorInfo);
}

private static String buildFailedConcludedErrorMessage(String errorCode, String errorMessage) {

String errorMsgBuilder = StringUtils.EMPTY;
if (StringUtils.isNotBlank(internalErrorCode)) {
errorMsgBuilder = internalErrorCode;
if (StringUtils.isNotBlank(errorCode)) {
errorMsgBuilder = errorCode;
}

if (StringUtils.isNotBlank(internalErrorMessage)) {
if (StringUtils.isNotBlank(errorMessage)) {
if (StringUtils.isNotBlank(errorMsgBuilder)) {
errorMsgBuilder = new StringJoiner(" ")
.add(errorMsgBuilder)
.add(AuthServiceConstants.INTERNAL_ERROR_MSG_SEPARATOR)
.add(internalErrorMessage).toString();
.add(errorMessage).toString();
} else if (StringUtils.isBlank(errorMsgBuilder)) {
errorMsgBuilder = internalErrorMessage;
errorMsgBuilder = errorMessage;
}
}

/* If there is an error message and an error code provided from the authentication framework then the
final error message will be set as "<internal errorCode> - <internal errorMessage>".
This is done to preserve the error details while sending out a standard error response.*/
if (StringUtils.isNotBlank(errorMsgBuilder)) {
errorMessage = errorMsgBuilder;
}

AuthServiceErrorInfo errorInfo = new AuthServiceErrorInfo(errorCode, errorMessage, errorDescription);
authServiceResponse.setErrorInfo(errorInfo);
return errorMsgBuilder;
}

private void handleFailedIncompleteAuthResponse(AuthServiceRequestWrapper request, AuthServiceResponseWrapper
Expand Down Expand Up @@ -317,7 +336,7 @@ private boolean isAuthFlowFailed(AuthServiceRequestWrapper request, AuthServiceR
throws AuthServiceException {

return AuthenticatorFlowStatus.FAIL_COMPLETED == request.getAuthFlowStatus() || response.isErrorResponse() ||
isSentToRetryPageOnMissingContext(request, response);
isSentToRetryPage(request);
}

private boolean isAuthFlowIncomplete(AuthServiceRequestWrapper request) {
Expand All @@ -339,15 +358,12 @@ private AuthenticationResult getAuthenticationResult(AuthServiceRequestWrapper r
return authenticationResult;
}

private boolean isSentToRetryPageOnMissingContext(AuthServiceRequestWrapper request,
AuthServiceResponseWrapper response) throws AuthServiceException {
private boolean isSentToRetryPage(AuthServiceRequestWrapper request) {

// If it's a retry due to context being null there is nothing to retry again the flow should be restarted.
if (AuthenticatorFlowStatus.INCOMPLETE == request.getAuthFlowStatus() &&
Boolean.TRUE.equals(request.getAttribute(FrameworkConstants.IS_SENT_TO_RETRY))) {
Map<String, String> queryParams = AuthServiceUtils.extractQueryParams(response.getRedirectURL());
return StringUtils.equals(queryParams.get(FrameworkConstants.STATUS_PARAM),
FrameworkConstants.ERROR_STATUS_AUTH_CONTEXT_NULL);
if (request.isSentToRetry()) {
// If it's a retry the flow should be restarted.
request.setAuthFlowConcluded(true);
return true;
}
return false;
}
Expand Down Expand Up @@ -498,4 +514,20 @@ private boolean isInitialAuthRequest(AuthServiceRequest authServiceRequest) {
return Boolean.TRUE.equals(authServiceRequest.getRequest().getAttribute(
AuthServiceConstants.REQ_ATTR_IS_INITIAL_API_BASED_AUTH_REQUEST));
}

private AuthServiceConstants.ErrorMessage getMappedError(String errorCode) {

if (errorCode == null) {
return null;
}

switch (errorCode) {
case FrameworkConstants.ERROR_STATUS_AUTH_FLOW_TIMEOUT:
return AuthServiceConstants.ErrorMessage.ERROR_AUTHENTICATION_FLOW_TIMEOUT;
case FrameworkConstants.ERROR_STATUS_AUTH_CONTEXT_NULL:
return AuthServiceConstants.ErrorMessage.ERROR_AUTHENTICATION_CONTEXT_NULL;
default:
return null;
}
}
}
3 changes: 2 additions & 1 deletion .../application/authentication/framework/handler/request/impl/DefaultRequestCoordinator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,8 @@ public void handle(HttpServletRequest request, HttpServletResponse response) thr
(FrameworkUtils.getCurrentStandardNano() > context.getExpiryTime())) {
log.error("Redirecting to retry page as the authentication context has expired.");
FrameworkUtils.sendToRetryPage(request, responseWrapper, context,
"authentication.flow.timeout", "authentication.flow.timeout.description");
FrameworkConstants.ERROR_STATUS_AUTH_FLOW_TIMEOUT,
FrameworkConstants.ERROR_DESCRIPTION_AUTH_FLOW_TIMEOUT);
return;
}

Expand Down
23 changes: 22 additions & 1 deletion ...ty/application/authentication/framework/model/auth/service/AuthServiceRequestWrapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
public class AuthServiceRequestWrapper extends HttpServletRequestWrapper {

private Map<String, String[]> parameters = new HashMap<>();
private boolean isAuthFlowConcluded;

public AuthServiceRequestWrapper(HttpServletRequest request, Map<String, String[]> parameters) {

Expand Down Expand Up @@ -133,7 +134,17 @@ public AuthenticatorFlowStatus getAuthFlowStatus() {
*/
public boolean isAuthFlowConcluded() {

return Boolean.TRUE.equals(getAttribute(FrameworkConstants.IS_AUTH_FLOW_CONCLUDED));
return Boolean.TRUE.equals(getAttribute(FrameworkConstants.IS_AUTH_FLOW_CONCLUDED)) || isAuthFlowConcluded;
}

/**
* Mark whether the flow is concluded.
*
* @param isAuthFlowConcluded set true if the flow is concluded.
*/
public void setAuthFlowConcluded(boolean isAuthFlowConcluded) {

this.isAuthFlowConcluded = isAuthFlowConcluded;
}

private void setSessionDataKey(Map<String, String[]> parameters) {
Expand Down Expand Up @@ -169,4 +180,14 @@ public String getSessionDataKey() {

return null;
}

/**
* Check if the request was sent to retry.
*
* @return True if sent to retry.
*/
public boolean isSentToRetry() {

return Boolean.TRUE.equals(getAttribute(FrameworkConstants.IS_SENT_TO_RETRY));
}
}
3 changes: 3 additions & 0 deletions ...rg/wso2/carbon/identity/application/authentication/framework/util/FrameworkConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,8 +241,11 @@ public abstract class FrameworkConstants {
public static final String IS_USER_RESOLVED = "isUserResolved";
public static final String ERROR_STATUS_AUTH_CONTEXT_NULL = "authentication.context.null";
public static final String ERROR_DESCRIPTION_AUTH_CONTEXT_NULL = "authentication.context.null.description";
public static final String ERROR_STATUS_AUTH_FLOW_TIMEOUT = "authentication.flow.timeout";
public static final String ERROR_DESCRIPTION_AUTH_FLOW_TIMEOUT = "authentication.flow.timeout.description";
public static final String IS_SENT_TO_RETRY = "isSentToRetry";
public static final String CONTEXT_IDENTIFIER = "contextIdentifier";
public static final String REQ_ATTR_RETRY_STATUS = "retryStatus";

private FrameworkConstants() {

Expand Down
3 changes: 3 additions & 0 deletions ...va/org/wso2/carbon/identity/application/authentication/framework/util/FrameworkUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -658,6 +658,9 @@ public static void sendToRetryPage(HttpServletRequest request, HttpServletRespon
}
request.setAttribute(FrameworkConstants.RequestParams.FLOW_STATUS, AuthenticatorFlowStatus.INCOMPLETE);
request.setAttribute(FrameworkConstants.IS_SENT_TO_RETRY, true);
if (status != null) {
request.setAttribute(FrameworkConstants.REQ_ATTR_RETRY_STATUS, status);
}
if (context != null) {
if (IdentityTenantUtil.isTenantedSessionsEnabled()) {
uriBuilder.addParameter(USER_TENANT_DOMAIN_HINT, context.getUserTenantDomain());
Expand Down
6 changes: 6 additions & 0 deletions ...identity/application/authentication/framework/util/auth/service/AuthServiceConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,12 @@ public enum ErrorMessage {
ERROR_API_BASED_AUTH_NOT_ENABLED("60007",
"App native authentication is not enabled for the application.",
"App native authentication is not enabled for this application with id %s"),
ERROR_AUTHENTICATION_FLOW_TIMEOUT("60008",
"Authentication flow time out.",
"Authentication flow has timed out as it took too long to complete."),
ERROR_AUTHENTICATION_CONTEXT_NULL("60009",
"Invalid flow identifier.",
"The provided flowId is invalid."),
// Server Error starting from 650xx.
/* The 65001 ERROR_UNABLE_TO_PROCEED is used as the default server error
therefor be cautious if that is being changed.*/
Expand Down
Loading