From c2c24799d5346d0962fbd320e799baf7a25ff051 Mon Sep 17 00:00:00 2001 From: YuriyZ Date: Wed, 5 Apr 2023 20:03:58 +0300 Subject: [PATCH 1/4] feat(jans-auth-server): show error with clear message if exception occurs authz check #4449 --- .../authorize/ws/rs/AuthorizeAction.java | 39 ++-- .../as/server/service/AuthorizeService.java | 176 +++++++++--------- .../ExternalAuthenticationService.java | 56 ++++-- .../src/main/resources/jans-auth.properties | 2 + .../main/resources/jans-auth_en.properties | 2 + 5 files changed, 157 insertions(+), 118 deletions(-) diff --git a/jans-auth-server/server/src/main/java/io/jans/as/server/authorize/ws/rs/AuthorizeAction.java b/jans-auth-server/server/src/main/java/io/jans/as/server/authorize/ws/rs/AuthorizeAction.java index d4ce1cd8f4b..d7868753230 100644 --- a/jans-auth-server/server/src/main/java/io/jans/as/server/authorize/ws/rs/AuthorizeAction.java +++ b/jans-auth-server/server/src/main/java/io/jans/as/server/authorize/ws/rs/AuthorizeAction.java @@ -8,6 +8,8 @@ import io.jans.as.common.model.common.User; import io.jans.as.common.model.registration.Client; +import io.jans.as.common.model.session.SessionId; +import io.jans.as.common.model.session.SessionIdState; import io.jans.as.model.authorize.AuthorizeErrorResponseType; import io.jans.as.model.common.Prompt; import io.jans.as.model.common.SubjectType; @@ -19,6 +21,7 @@ import io.jans.as.model.util.Base64Util; import io.jans.as.model.util.JwtUtil; import io.jans.as.model.util.Util; +import io.jans.as.persistence.model.ClientAuthorization; import io.jans.as.persistence.model.Scope; import io.jans.as.server.auth.Authenticator; import io.jans.as.server.i18n.LanguageBean; @@ -28,11 +31,8 @@ import io.jans.as.server.model.authorize.ScopeChecker; import io.jans.as.server.model.common.CibaRequestCacheControl; import io.jans.as.server.model.common.DefaultScope; -import io.jans.as.common.model.session.SessionId; -import io.jans.as.common.model.session.SessionIdState; import io.jans.as.server.model.config.Constants; import io.jans.as.server.model.exception.AcrChangedException; -import io.jans.as.persistence.model.ClientAuthorization; import io.jans.as.server.security.Identity; import io.jans.as.server.service.*; import io.jans.as.server.service.ciba.CibaRequestService; @@ -49,14 +49,7 @@ import io.jans.util.OxConstants; import io.jans.util.StringHelper; import io.jans.util.ilocale.LocaleUtil; -import org.apache.commons.lang.BooleanUtils; -import org.apache.commons.lang.StringUtils; -import org.apache.commons.text.StringEscapeUtils; -import org.apache.logging.log4j.util.Strings; -import org.slf4j.Logger; - import jakarta.enterprise.context.RequestScoped; -import jakarta.faces.application.FacesMessage; import jakarta.faces.context.ExternalContext; import jakarta.faces.context.FacesContext; import jakarta.inject.Inject; @@ -67,6 +60,12 @@ import jakarta.ws.rs.client.ClientBuilder; import jakarta.ws.rs.core.MediaType; import jakarta.ws.rs.core.Response; +import org.apache.commons.lang.BooleanUtils; +import org.apache.commons.lang.StringUtils; +import org.apache.commons.text.StringEscapeUtils; +import org.apache.logging.log4j.util.Strings; +import org.slf4j.Logger; + import java.io.IOException; import java.io.UnsupportedEncodingException; import java.net.URI; @@ -229,7 +228,16 @@ public void checkUiLocales() { } } - public void checkPermissionGranted() throws IOException { + public void checkPermissionGranted() { + try { + checkPermissionGrantedInternal(); + } catch (Exception e) { + log.error("Failed to perform checkPermissionGranted()", e); + permissionDenied(); + } + } + + public void checkPermissionGrantedInternal() throws IOException { if ((clientId == null) || clientId.isEmpty()) { log.debug("Permission denied. client_id should be not empty."); permissionDenied(); @@ -813,8 +821,8 @@ public String getSessionId() { return sessionId; } - public void setSessionId(String p_sessionId) { - sessionId = p_sessionId; + public void setSessionId(String sessionId) { + this.sessionId = sessionId; } public void permissionGranted() { @@ -832,11 +840,6 @@ public void permissionDenied() { authorizeService.permissionDenied(session); } - private void authenticationFailedSessionInvalid() { - facesMessages.add(FacesMessage.SEVERITY_ERROR, "login.errorSessionInvalidMessage"); - facesService.redirect("/error.xhtml"); - } - public void invalidRequest() { log.trace("invalidRequest"); StringBuilder sb = new StringBuilder(); diff --git a/jans-auth-server/server/src/main/java/io/jans/as/server/service/AuthorizeService.java b/jans-auth-server/server/src/main/java/io/jans/as/server/service/AuthorizeService.java index 0d019094d56..e4c291cbaf8 100644 --- a/jans-auth-server/server/src/main/java/io/jans/as/server/service/AuthorizeService.java +++ b/jans-auth-server/server/src/main/java/io/jans/as/server/service/AuthorizeService.java @@ -9,6 +9,7 @@ import com.google.common.collect.Sets; import io.jans.as.common.model.common.User; import io.jans.as.common.model.registration.Client; +import io.jans.as.common.model.session.SessionId; import io.jans.as.common.util.RedirectUri; import io.jans.as.model.authorize.AuthorizeErrorResponseType; import io.jans.as.model.authorize.AuthorizeRequestParam; @@ -33,28 +34,28 @@ import io.jans.as.server.model.common.CibaRequestStatus; import io.jans.as.server.model.common.DeviceAuthorizationCacheControl; import io.jans.as.server.model.common.DeviceAuthorizationStatus; -import io.jans.as.common.model.session.SessionId; import io.jans.as.server.security.Identity; import io.jans.as.server.service.ciba.CibaRequestService; import io.jans.jsf2.message.FacesMessages; import io.jans.jsf2.service.FacesService; import io.jans.util.security.StringEncrypter; - -import org.apache.commons.lang.StringUtils; -import org.slf4j.Logger; - import jakarta.enterprise.context.RequestScoped; import jakarta.faces.application.FacesMessage; import jakarta.faces.context.ExternalContext; import jakarta.inject.Inject; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; -import java.io.UnsupportedEncodingException; +import org.apache.commons.lang.StringUtils; +import org.slf4j.Logger; + import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.Set; +import static org.apache.commons.lang.BooleanUtils.isFalse; +import static org.apache.commons.lang.BooleanUtils.isTrue; + /** * @author Yuriy Movchan * @author Javier Rojas Blum @@ -139,12 +140,12 @@ public SessionId getSession(String sessionId) { authenticator.authenticateBySessionId(sessionId); } - SessionId ldapSessionId = sessionIdService.getSessionId(sessionId); - if (ldapSessionId == null) { + SessionId dbSessionId = sessionIdService.getSessionId(sessionId); + if (dbSessionId == null) { identity.logout(); } - return ldapSessionId; + return dbSessionId; } public void permissionGranted(HttpServletRequest httpRequest, final SessionId session) { @@ -178,7 +179,7 @@ public void permissionGranted(HttpServletRequest httpRequest, final SessionId se identity.setSessionId(session); // OXAUTH-297 - set session_id cookie - if (!appConfiguration.getInvalidateSessionCookiesAfterAuthorizationFlow()) { + if (isFalse(appConfiguration.getInvalidateSessionCookiesAfterAuthorizationFlow())) { cookieService.createSessionIdCookie(session, false); } Map sessionAttribute = requestParameterService.getAllowedParameters(session.getSessionAttributes()); @@ -194,87 +195,91 @@ public void permissionGranted(HttpServletRequest httpRequest, final SessionId se String uri = httpRequest.getContextPath() + "/restv1/authorize?" + parametersAsString; log.trace("permissionGranted, redirectTo: {}", uri); - if (invalidateSessionCookiesIfNeeded()) { - if (!uri.contains(AuthorizeRequestParam.SESSION_ID) && appConfiguration.getSessionIdRequestParameterEnabled()) { - uri += "&session_id=" + session.getId(); - } + final boolean sessionInvalidated = invalidateSessionCookiesIfNeeded(); + if (sessionInvalidated && !uri.contains(AuthorizeRequestParam.SESSION_ID) && isTrue(appConfiguration.getSessionIdRequestParameterEnabled())) { + uri += "&session_id=" + session.getId(); } facesService.redirectToExternalURL(uri); - } catch (UnsupportedEncodingException e) { - log.trace(e.getMessage(), e); + } catch (Exception e) { + log.error("Failed to grant permission", e); + showErrorPage("login.failedToGrantPermission"); } } public void permissionDenied(final SessionId session) { - log.trace("permissionDenied"); - invalidateSessionCookiesIfNeeded(); + try { + log.trace("permissionDenied"); + invalidateSessionCookiesIfNeeded(); - if (session == null) { - authenticationFailedSessionInvalid(); - return; - } - - String baseRedirectUri = session.getSessionAttributes().get(AuthorizeRequestParam.REDIRECT_URI); - String state = session.getSessionAttributes().get(AuthorizeRequestParam.STATE); - ResponseMode responseMode = ResponseMode.fromString(session.getSessionAttributes().get(AuthorizeRequestParam.RESPONSE_MODE)); - List responseType = ResponseType.fromString(session.getSessionAttributes().get(AuthorizeRequestParam.RESPONSE_TYPE), " "); - - RedirectUri redirectUri = new RedirectUri(baseRedirectUri, responseType, responseMode); - redirectUri.parseQueryString(errorResponseFactory.getErrorAsQueryString(AuthorizeErrorResponseType.ACCESS_DENIED, state)); - - // CIBA - Map sessionAttribute = requestParameterService.getAllowedParameters(session.getSessionAttributes()); - if (sessionAttribute.containsKey(AuthorizeRequestParam.AUTH_REQ_ID)) { - String authReqId = sessionAttribute.get(AuthorizeRequestParam.AUTH_REQ_ID); - CibaRequestCacheControl request = cibaRequestService.getCibaRequest(authReqId); - - if (request != null && request.getClient() != null) { - if (request.getStatus() == CibaRequestStatus.PENDING) { - cibaRequestService.removeCibaRequest(authReqId); - } - switch (request.getClient().getBackchannelTokenDeliveryMode()) { - case POLL: - request.setStatus(CibaRequestStatus.DENIED); - request.setTokensDelivered(false); - cibaRequestService.update(request); - break; - case PING: - request.setStatus(CibaRequestStatus.DENIED); - request.setTokensDelivered(false); - cibaRequestService.update(request); - - cibaPingCallbackService.pingCallback( - request.getAuthReqId(), - request.getClient().getBackchannelClientNotificationEndpoint(), - request.getClientNotificationToken() - ); - break; - case PUSH: - cibaPushErrorService.pushError( - request.getAuthReqId(), - request.getClient().getBackchannelClientNotificationEndpoint(), - request.getClientNotificationToken(), - PushErrorResponseType.ACCESS_DENIED, - "The end-user denied the authorization request."); - break; + if (session == null) { + authenticationFailedSessionInvalid(); + return; + } + + String baseRedirectUri = session.getSessionAttributes().get(AuthorizeRequestParam.REDIRECT_URI); + String state = session.getSessionAttributes().get(AuthorizeRequestParam.STATE); + ResponseMode responseMode = ResponseMode.fromString(session.getSessionAttributes().get(AuthorizeRequestParam.RESPONSE_MODE)); + List responseType = ResponseType.fromString(session.getSessionAttributes().get(AuthorizeRequestParam.RESPONSE_TYPE), " "); + + RedirectUri redirectUri = new RedirectUri(baseRedirectUri, responseType, responseMode); + redirectUri.parseQueryString(errorResponseFactory.getErrorAsQueryString(AuthorizeErrorResponseType.ACCESS_DENIED, state)); + + // CIBA + Map sessionAttribute = requestParameterService.getAllowedParameters(session.getSessionAttributes()); + if (sessionAttribute.containsKey(AuthorizeRequestParam.AUTH_REQ_ID)) { + String authReqId = sessionAttribute.get(AuthorizeRequestParam.AUTH_REQ_ID); + CibaRequestCacheControl request = cibaRequestService.getCibaRequest(authReqId); + + if (request != null && request.getClient() != null) { + if (request.getStatus() == CibaRequestStatus.PENDING) { + cibaRequestService.removeCibaRequest(authReqId); + } + switch (request.getClient().getBackchannelTokenDeliveryMode()) { + case POLL: + request.setStatus(CibaRequestStatus.DENIED); + request.setTokensDelivered(false); + cibaRequestService.update(request); + break; + case PING: + request.setStatus(CibaRequestStatus.DENIED); + request.setTokensDelivered(false); + cibaRequestService.update(request); + + cibaPingCallbackService.pingCallback( + request.getAuthReqId(), + request.getClient().getBackchannelClientNotificationEndpoint(), + request.getClientNotificationToken() + ); + break; + case PUSH: + cibaPushErrorService.pushError( + request.getAuthReqId(), + request.getClient().getBackchannelClientNotificationEndpoint(), + request.getClientNotificationToken(), + PushErrorResponseType.ACCESS_DENIED, + "The end-user denied the authorization request."); + break; + } } } - } - if (sessionAttribute.containsKey(DeviceAuthorizationService.SESSION_USER_CODE)) { - processDeviceAuthDeniedResponse(sessionAttribute); - } + if (sessionAttribute.containsKey(DeviceAuthorizationService.SESSION_USER_CODE)) { + processDeviceAuthDeniedResponse(sessionAttribute); + } - if (responseMode == ResponseMode.JWT) { - String clientId = session.getSessionAttributes().get(AuthorizeRequestParam.CLIENT_ID); - Client client = clientService.getClient(clientId); - facesService.redirectToExternalURL(createJarmRedirectUri(redirectUri, client, session)); - } - else - facesService.redirectToExternalURL(redirectUri.toString()); + if (responseMode == ResponseMode.JWT) { + String clientId = session.getSessionAttributes().get(AuthorizeRequestParam.CLIENT_ID); + Client client = clientService.getClient(clientId); + facesService.redirectToExternalURL(createJarmRedirectUri(redirectUri, client)); + } else + facesService.redirectToExternalURL(redirectUri.toString()); + + } catch (Exception e) { + log.error("Unable to perform permission deny", e); + showErrorPage("login.failedToDeny"); + } } - private String createJarmRedirectUri(RedirectUri redirectUri, Client client, final SessionId session) - { + private String createJarmRedirectUri(RedirectUri redirectUri, Client client) { String jarmRedirectUri = redirectUri.toString(); SignatureAlgorithm signatureAlgorithm = SignatureAlgorithm .fromString(client.getAttributes().getAuthorizationSignedResponseAlg()); @@ -298,14 +303,19 @@ private String createJarmRedirectUri(RedirectUri redirectUri, Client client, fin redirectUri.setKeyId(keyId); String jarmQueryString = redirectUri.getQueryString(); - log.info("The JARM Query Response:" + jarmQueryString); + log.info("The JARM Query Response: {}", jarmQueryString); jarmRedirectUri = jarmRedirectUri + jarmQueryString; return jarmRedirectUri; } private void authenticationFailedSessionInvalid() { - facesMessages.add(FacesMessage.SEVERITY_ERROR, "login.errorSessionInvalidMessage"); + showErrorPage("login.errorSessionInvalidMessage"); + } + + private void showErrorPage(String errorCode) { + log.debug("Redirect to /error.xhtml page with {} error code.", errorCode); + facesMessages.add(FacesMessage.SEVERITY_ERROR, errorCode); facesService.redirect("/error.xhtml"); } @@ -317,7 +327,7 @@ public List getScopes() { } public List getScopes(String scopes) { - List result = new ArrayList(); + List result = new ArrayList<>(); if (scopes != null && !scopes.isEmpty()) { String[] scopesName = scopes.split(" "); @@ -333,7 +343,7 @@ public List getScopes(String scopes) { } private boolean invalidateSessionCookiesIfNeeded() { - if (appConfiguration.getInvalidateSessionCookiesAfterAuthorizationFlow()) { + if (isTrue(appConfiguration.getInvalidateSessionCookiesAfterAuthorizationFlow())) { return invalidateSessionCookies(); } return false; diff --git a/jans-auth-server/server/src/main/java/io/jans/as/server/service/external/ExternalAuthenticationService.java b/jans-auth-server/server/src/main/java/io/jans/as/server/service/external/ExternalAuthenticationService.java index d82dc6d9f48..d6494295a8d 100644 --- a/jans-auth-server/server/src/main/java/io/jans/as/server/service/external/ExternalAuthenticationService.java +++ b/jans-auth-server/server/src/main/java/io/jans/as/server/service/external/ExternalAuthenticationService.java @@ -114,9 +114,9 @@ private HashMap buildScriptAliases() { @Override protected void addExternalConfigurations(List newCustomScriptConfigurations) { - if ((ldapAuthConfigs == null) || (ldapAuthConfigs.size() == 0)) { + if ((ldapAuthConfigs == null) || (ldapAuthConfigs.isEmpty())) { // Add internal type only if there is no enabled scripts and external authentication configurations - if (newCustomScriptConfigurations.size() == 0) { + if (newCustomScriptConfigurations.isEmpty()) { newCustomScriptConfigurations.add(getInternalCustomScriptConfiguration()); } } else { @@ -127,7 +127,7 @@ protected void addExternalConfigurations(List newCust } private Map> groupCustomScriptConfigurationsMapByUsageType(Map customScriptConfigurationsMap) { - Map> newCustomScriptConfigurationsMapByUsageType = new HashMap>(); + Map> newCustomScriptConfigurationsMapByUsageType = new HashMap<>(); for (AuthenticationScriptUsageType usageType : AuthenticationScriptUsageType.values()) { List currCustomScriptConfigurationsMapByUsageType = new ArrayList(); @@ -146,7 +146,7 @@ private Map> grou } private Map determineDefaultCustomScriptConfigurationsMap(Map customScriptConfigurationsMap) { - Map newDefaultCustomScriptConfigurationsMap = new HashMap(); + Map newDefaultCustomScriptConfigurationsMap = new HashMap<>(); for (AuthenticationScriptUsageType usageType : AuthenticationScriptUsageType.values()) { CustomScriptConfiguration defaultExternalAuthenticator = null; @@ -169,7 +169,9 @@ private boolean executeExternalIsValidAuthenticationMethod(AuthenticationScriptU log.debug("Executing python 'isValidAuthenticationMethod' authenticator method"); PersonAuthenticationType externalAuthenticator = (PersonAuthenticationType) customScriptConfiguration.getExternalType(); Map configurationAttributes = customScriptConfiguration.getConfigurationAttributes(); - return externalAuthenticator.isValidAuthenticationMethod(usageType, configurationAttributes); + final boolean result = externalAuthenticator.isValidAuthenticationMethod(usageType, configurationAttributes); + log.debug("Executed python 'isValidAuthenticationMethod' authenticator method, result: {}", result); + return result; } catch (Exception ex) { log.error(ex.getMessage(), ex); saveScriptError(customScriptConfiguration.getCustomScript(), ex); @@ -183,7 +185,9 @@ private String executeExternalGetAlternativeAuthenticationMethod(AuthenticationS log.trace("Executing python 'getAlternativeAuthenticationMethod' authenticator method"); PersonAuthenticationType externalAuthenticator = (PersonAuthenticationType) customScriptConfiguration.getExternalType(); Map configurationAttributes = customScriptConfiguration.getConfigurationAttributes(); - return externalAuthenticator.getAlternativeAuthenticationMethod(usageType, configurationAttributes); + final String result = externalAuthenticator.getAlternativeAuthenticationMethod(usageType, configurationAttributes); + log.trace("Executed python 'getAlternativeAuthenticationMethod' authenticator method, result: {}", result); + return result; } catch (Exception ex) { log.error(ex.getMessage(), ex); saveScriptError(customScriptConfiguration.getCustomScript(), ex); @@ -197,7 +201,9 @@ public int executeExternalGetCountAuthenticationSteps(CustomScriptConfiguration log.trace("Executing python 'getCountAuthenticationSteps' authenticator method"); PersonAuthenticationType externalAuthenticator = (PersonAuthenticationType) customScriptConfiguration.getExternalType(); Map configurationAttributes = customScriptConfiguration.getConfigurationAttributes(); - return externalAuthenticator.getCountAuthenticationSteps(configurationAttributes); + final int result = externalAuthenticator.getCountAuthenticationSteps(configurationAttributes); + log.trace("Executed python 'getCountAuthenticationSteps' authenticator method, result: {}", result); + return result; } catch (Exception ex) { log.error(ex.getMessage(), ex); saveScriptError(customScriptConfiguration.getCustomScript(), ex); @@ -211,7 +217,9 @@ public boolean executeExternalAuthenticate(CustomScriptConfiguration customScrip log.trace("Executing python 'authenticate' authenticator method"); PersonAuthenticationType externalAuthenticator = (PersonAuthenticationType) customScriptConfiguration.getExternalType(); Map configurationAttributes = customScriptConfiguration.getConfigurationAttributes(); - return externalAuthenticator.authenticate(configurationAttributes, requestParameters, step); + final boolean result = externalAuthenticator.authenticate(configurationAttributes, requestParameters, step); + log.trace("Executed python 'authenticate' authenticator method, result: {}", result); + return result; } catch (Exception ex) { log.error(ex.getMessage(), ex); saveScriptError(customScriptConfiguration.getCustomScript(), ex); @@ -225,7 +233,9 @@ public int getNextStep(CustomScriptConfiguration customScriptConfiguration, Map< log.trace("Executing python 'getNextStep' authenticator method"); PersonAuthenticationType externalAuthenticator = (PersonAuthenticationType) customScriptConfiguration.getExternalType(); Map configurationAttributes = customScriptConfiguration.getConfigurationAttributes(); - return externalAuthenticator.getNextStep(configurationAttributes, requestParameters, step); + final int result = externalAuthenticator.getNextStep(configurationAttributes, requestParameters, step); + log.trace("Executed python 'getNextStep' authenticator method, result: {}", result); + return result; } catch (Exception ex) { log.error(ex.getMessage(), ex); saveScriptError(customScriptConfiguration.getCustomScript(), ex); @@ -239,7 +249,9 @@ public boolean executeExternalLogout(CustomScriptConfiguration customScriptConfi log.trace("Executing python 'logout' authenticator method"); PersonAuthenticationType externalAuthenticator = (PersonAuthenticationType) customScriptConfiguration.getExternalType(); Map configurationAttributes = customScriptConfiguration.getConfigurationAttributes(); - return externalAuthenticator.logout(configurationAttributes, requestParameters); + final boolean result = externalAuthenticator.logout(configurationAttributes, requestParameters); + log.trace("Executed python 'logout' authenticator method, result: {}", result); + return result; } catch (Exception ex) { log.error(ex.getMessage(), ex); saveScriptError(customScriptConfiguration.getCustomScript(), ex); @@ -253,7 +265,9 @@ public String getLogoutExternalUrl(CustomScriptConfiguration customScriptConfigu log.trace("Executing python 'getLogouExternalUrl' authenticator method"); PersonAuthenticationType externalAuthenticator = (PersonAuthenticationType) customScriptConfiguration.getExternalType(); Map configurationAttributes = customScriptConfiguration.getConfigurationAttributes(); - return externalAuthenticator.getLogoutExternalUrl(configurationAttributes, requestParameters); + final String result = externalAuthenticator.getLogoutExternalUrl(configurationAttributes, requestParameters); + log.trace("Executed python 'getLogouExternalUrl' authenticator method, result: {}", result); + return result; } catch (Exception ex) { log.error(ex.getMessage(), ex); saveScriptError(customScriptConfiguration.getCustomScript(), ex); @@ -264,10 +278,12 @@ public String getLogoutExternalUrl(CustomScriptConfiguration customScriptConfigu public boolean executeExternalPrepareForStep(CustomScriptConfiguration customScriptConfiguration, Map requestParameters, int step) { try { - log.trace("Executing python 'prepareForStep' authenticator method"); + log.trace("Executing python 'prepareForStep' authn"); PersonAuthenticationType externalAuthenticator = (PersonAuthenticationType) customScriptConfiguration.getExternalType(); Map configurationAttributes = customScriptConfiguration.getConfigurationAttributes(); - return externalAuthenticator.prepareForStep(configurationAttributes, requestParameters, step); + final boolean result = externalAuthenticator.prepareForStep(configurationAttributes, requestParameters, step); + log.trace("Executed python 'prepareForStep' authn, result: {}", result); + return result; } catch (Exception ex) { log.error(ex.getMessage(), ex); saveScriptError(customScriptConfiguration.getCustomScript(), ex); @@ -278,10 +294,12 @@ public boolean executeExternalPrepareForStep(CustomScriptConfiguration customScr public List executeExternalGetExtraParametersForStep(CustomScriptConfiguration customScriptConfiguration, int step) { try { - log.trace("Executing python 'getExtraParametersForStep' authenticator method"); + log.trace("Executing python 'getExtraParametersForStep' authn"); PersonAuthenticationType externalAuthenticator = (PersonAuthenticationType) customScriptConfiguration.getExternalType(); Map configurationAttributes = customScriptConfiguration.getConfigurationAttributes(); - return externalAuthenticator.getExtraParametersForStep(configurationAttributes, step); + final List result = externalAuthenticator.getExtraParametersForStep(configurationAttributes, step); + log.trace("Executed python 'getExtraParametersForStep' authn, result: {}", result); + return result; } catch (Exception ex) { log.error(ex.getMessage(), ex); saveScriptError(customScriptConfiguration.getCustomScript(), ex); @@ -295,7 +313,9 @@ public String executeExternalGetPageForStep(CustomScriptConfiguration customScri log.trace("Executing python 'getPageForStep' authenticator method"); PersonAuthenticationType externalAuthenticator = (PersonAuthenticationType) customScriptConfiguration.getExternalType(); Map configurationAttributes = customScriptConfiguration.getConfigurationAttributes(); - return externalAuthenticator.getPageForStep(configurationAttributes, step); + final String result = externalAuthenticator.getPageForStep(configurationAttributes, step); + log.trace("Executed python 'getPageForStep' authenticator method, result: {}", result); + return result; } catch (Exception ex) { log.error(ex.getMessage(), ex); saveScriptError(customScriptConfiguration.getCustomScript(), ex); @@ -308,7 +328,9 @@ public int executeExternalGetApiVersion(CustomScriptConfiguration customScriptCo try { log.trace("Executing python 'getApiVersion' authenticator method"); PersonAuthenticationType externalAuthenticator = (PersonAuthenticationType) customScriptConfiguration.getExternalType(); - return externalAuthenticator.getApiVersion(); + final int result = externalAuthenticator.getApiVersion(); + log.trace("Executed python 'getApiVersion' authenticator method, result: {}", result); + return result; } catch (Exception ex) { log.error(ex.getMessage(), ex); saveScriptError(customScriptConfiguration.getCustomScript(), ex); diff --git a/jans-auth-server/server/src/main/resources/jans-auth.properties b/jans-auth-server/server/src/main/resources/jans-auth.properties index fa86cdd7387..d832bfa4e0d 100644 --- a/jans-auth-server/server/src/main/resources/jans-auth.properties +++ b/jans-auth-server/server/src/main/resources/jans-auth.properties @@ -18,6 +18,8 @@ login.password=Password login.rememberMe=Remember me login.errorMessage=Incorrect username or password. login.errorSessionInvalidMessage=Failed to authenticate. Authentication session has expired. Please navigate back to the original page and try again. +login.failedToGrantPermission=Failed to grant permission. +login.failedToDeny=Failed to deny authorization. login.failedToAuthenticate=Failed to authenticate. login.userAlreadyAuthenticated=User is already authenticated. Re-send authorization request (must be handled by RP). login.youDontHavePermission=You don't have permissions. diff --git a/jans-auth-server/server/src/main/resources/jans-auth_en.properties b/jans-auth-server/server/src/main/resources/jans-auth_en.properties index 71eebc00ca1..1ef6fe264e0 100644 --- a/jans-auth-server/server/src/main/resources/jans-auth_en.properties +++ b/jans-auth-server/server/src/main/resources/jans-auth_en.properties @@ -18,6 +18,8 @@ login.password=Password login.rememberMe=Remember me login.errorMessage=Incorrect username or password. login.errorSessionInvalidMessage=Failed to authenticate. Authentication session has expired. Please navigate back to the original page and try again. +login.failedToGrantPermission=Failed to grant permission. +login.failedToDeny=Failed to deny authorization. login.failedToAuthenticate=Failed to authenticate. login.userAlreadyAuthenticated=User is already authenticated. Re-send authorization request (must be handled by RP). login.youDontHavePermission=You don't have permissions. From 641c4662a744fc719107647d326023a51a52b14d Mon Sep 17 00:00:00 2001 From: YuriyZ Date: Thu, 6 Apr 2023 12:02:25 +0300 Subject: [PATCH 2/4] feat(jans-auth-server): default errorHandlingMethod to "remote" value and return correct error during handling #4449 --- .../jans-cli/cli-jans-authorization-server.md | 2 +- .../as/model/error/ErrorHandlingMethod.java | 2 +- .../io/jans/as/server/auth/Authenticator.java | 23 +++++++++++-------- .../server/service/ErrorHandlerService.java | 6 ++--- .../templates/jans-auth/jans-auth-config.json | 2 +- .../templates/jans-auth/jans-auth-config.json | 2 +- 6 files changed, 20 insertions(+), 17 deletions(-) diff --git a/docs/admin/config-guide/jans-cli/cli-jans-authorization-server.md b/docs/admin/config-guide/jans-cli/cli-jans-authorization-server.md index 2d23f59c0bc..21b9135e69d 100644 --- a/docs/admin/config-guide/jans-cli/cli-jans-authorization-server.md +++ b/docs/admin/config-guide/jans-cli/cli-jans-authorization-server.md @@ -519,7 +519,7 @@ It returns all the information of the Jans Authorization server. "delayTime": 2, "bruteForceProtectionEnabled": false }, - "errorHandlingMethod": "internal", + "errorHandlingMethod": "remote", "keepAuthenticatorAttributesOnAcrChange": false, "deviceAuthzRequestExpiresIn": 1800, "deviceAuthzTokenPollInterval": 5, diff --git a/jans-auth-server/model/src/main/java/io/jans/as/model/error/ErrorHandlingMethod.java b/jans-auth-server/model/src/main/java/io/jans/as/model/error/ErrorHandlingMethod.java index c13bdd8c811..caf1731a042 100644 --- a/jans-auth-server/model/src/main/java/io/jans/as/model/error/ErrorHandlingMethod.java +++ b/jans-auth-server/model/src/main/java/io/jans/as/model/error/ErrorHandlingMethod.java @@ -72,7 +72,7 @@ public String getValue() { public static ErrorHandlingMethod fromString(String param) { if (param != null) { for (ErrorHandlingMethod hm : ErrorHandlingMethod.values()) { - if (param.equals(hm.value)) { + if (hm.value.equalsIgnoreCase(param)) { return hm; } } diff --git a/jans-auth-server/server/src/main/java/io/jans/as/server/auth/Authenticator.java b/jans-auth-server/server/src/main/java/io/jans/as/server/auth/Authenticator.java index 7cf1b98cbdf..48bee7b18d5 100644 --- a/jans-auth-server/server/src/main/java/io/jans/as/server/auth/Authenticator.java +++ b/jans-auth-server/server/src/main/java/io/jans/as/server/auth/Authenticator.java @@ -137,11 +137,12 @@ public boolean authenticate() { } lastResult = authenticateImpl(servletRequest, true, false, false); + logger.debug("authenticate resultCode: {}", lastResult); if (Constants.RESULT_SUCCESS.equals(lastResult)) { return true; } else if (Constants.RESULT_FAILURE.equals(lastResult)) { - authenticationFailed(); + authenticationFailed(sessionId); } else if (Constants.RESULT_NO_PERMISSIONS.equals(lastResult)) { handlePermissionsError(); } else if (Constants.RESULT_EXPIRED.equals(lastResult)) { @@ -167,7 +168,7 @@ public String authenticateWithOutcome() { if (Constants.RESULT_SUCCESS.equals(lastResult)) { return lastResult; } else if (Constants.RESULT_FAILURE.equals(lastResult)) { - authenticationFailed(); + authenticationFailed(sessionIdService.getSessionId()); } else if (Constants.RESULT_NO_PERMISSIONS.equals(lastResult)) { handlePermissionsError(); } else if (Constants.RESULT_EXPIRED.equals(lastResult)) { @@ -483,12 +484,16 @@ protected void handleSessionInvalid() { "Create authorization request to start new authentication session."); } - protected void handleScriptError() { - handleScriptError(AUTHENTICATION_ERROR_MESSAGE); + protected void handleScriptError(SessionId sessionId) { + handleScriptError(sessionId, AUTHENTICATION_ERROR_MESSAGE); } - protected void handleScriptError(String facesMessageId) { - errorHandlerService.handleError(facesMessageId, AuthorizeErrorResponseType.INVALID_AUTHENTICATION_METHOD, + protected void handleScriptError(SessionId sessionId, String facesMessageId) { + final AuthorizeErrorResponseType errorType = sessionId == null ? + AuthorizeErrorResponseType.AUTHENTICATION_SESSION_INVALID : + AuthorizeErrorResponseType.INVALID_AUTHENTICATION_METHOD; + + errorHandlerService.handleError(facesMessageId, errorType, "Contact administrator to fix specific ACR method issue."); } @@ -580,7 +585,7 @@ public String prepareAuthenticationForStep() { if (Constants.RESULT_SUCCESS.equals(lastResult)) { return lastResult; } else if (Constants.RESULT_FAILURE.equals(lastResult)) { - handleScriptError(); + handleScriptError(sessionId); } else if (Constants.RESULT_NO_PERMISSIONS.equals(lastResult)) { handlePermissionsError(); } else if (Constants.RESULT_EXPIRED.equals(lastResult)) { @@ -759,9 +764,9 @@ private void initCustomAuthenticatorVariables(Map sessionIdAttri this.authAcr = sessionIdAttributes.get(JwtClaimName.AUTHENTICATION_CONTEXT_CLASS_REFERENCE); } - private boolean authenticationFailed() { + private boolean authenticationFailed(SessionId sessionId) { addMessage(FacesMessage.SEVERITY_ERROR, "login.errorMessage"); - handleScriptError(null); + handleScriptError(sessionId); return false; } diff --git a/jans-auth-server/server/src/main/java/io/jans/as/server/service/ErrorHandlerService.java b/jans-auth-server/server/src/main/java/io/jans/as/server/service/ErrorHandlerService.java index 73eda5668c1..e36ce7d5509 100644 --- a/jans-auth-server/server/src/main/java/io/jans/as/server/service/ErrorHandlerService.java +++ b/jans-auth-server/server/src/main/java/io/jans/as/server/service/ErrorHandlerService.java @@ -14,14 +14,12 @@ import io.jans.jsf2.message.FacesMessages; import io.jans.jsf2.service.FacesService; import io.jans.util.StringHelper; -import org.python.jline.internal.Log; -import org.slf4j.Logger; - import jakarta.enterprise.context.ApplicationScoped; import jakarta.faces.application.FacesMessage; import jakarta.faces.application.FacesMessage.Severity; import jakarta.inject.Inject; import jakarta.inject.Named; +import org.slf4j.Logger; /** * Helper service to generate either error response or local error based on application settings @@ -76,7 +74,7 @@ private void handleRemoteError(String facesMessageId, IErrorType errorType, Stri String redirectUri = cookieService.getRpOriginIdCookie(); if (StringHelper.isEmpty(redirectUri)) { - Log.error("Failed to get redirect_uri from cookie"); + log.error("Failed to get redirect_uri from cookie"); handleLocalError(facesMessageId); return; } diff --git a/jans-linux-setup/jans_setup/openbanking/templates/jans-auth/jans-auth-config.json b/jans-linux-setup/jans_setup/openbanking/templates/jans-auth/jans-auth-config.json index 4588e944425..2ec925769ca 100644 --- a/jans-linux-setup/jans_setup/openbanking/templates/jans-auth/jans-auth-config.json +++ b/jans-linux-setup/jans_setup/openbanking/templates/jans-auth/jans-auth-config.json @@ -350,7 +350,7 @@ }, "loggingLevel": "TRACE", "loggingLayout": "text", - "errorHandlingMethod":"internal", + "errorHandlingMethod":"remote", "useLocalCache":true, "backchannelTokenDeliveryModesSupported": [ "poll", diff --git a/jans-linux-setup/jans_setup/templates/jans-auth/jans-auth-config.json b/jans-linux-setup/jans_setup/templates/jans-auth/jans-auth-config.json index bfb3f692b57..ad87564c910 100644 --- a/jans-linux-setup/jans_setup/templates/jans-auth/jans-auth-config.json +++ b/jans-linux-setup/jans_setup/templates/jans-auth/jans-auth-config.json @@ -440,7 +440,7 @@ }, "loggingLevel": "INFO", "loggingLayout": "text", - "errorHandlingMethod":"internal", + "errorHandlingMethod":"remote", "useLocalCache":true, "backchannelTokenDeliveryModesSupported": [ "poll", From 10dc00e9070e1f999ced195b1947fc0d712f13c2 Mon Sep 17 00:00:00 2001 From: YuriyZ Date: Fri, 7 Apr 2023 12:26:21 +0300 Subject: [PATCH 3/4] test(jans-auth-server): covered exception case during deny #4449 --- .../authorize/ws/rs/AuthorizeActionTest.java | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/jans-auth-server/server/src/test/java/io/jans/as/server/authorize/ws/rs/AuthorizeActionTest.java b/jans-auth-server/server/src/test/java/io/jans/as/server/authorize/ws/rs/AuthorizeActionTest.java index d3c00453b6e..e955aee20e3 100644 --- a/jans-auth-server/server/src/test/java/io/jans/as/server/authorize/ws/rs/AuthorizeActionTest.java +++ b/jans-auth-server/server/src/test/java/io/jans/as/server/authorize/ws/rs/AuthorizeActionTest.java @@ -22,11 +22,14 @@ import jakarta.faces.context.FacesContext; import org.mockito.InjectMocks; import org.mockito.Mock; +import org.mockito.Spy; import org.mockito.testng.MockitoTestNGListener; import org.slf4j.Logger; import org.testng.annotations.Listeners; import org.testng.annotations.Test; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertTrue; @@ -37,6 +40,7 @@ @Listeners(MockitoTestNGListener.class) public class AuthorizeActionTest { + @Spy @InjectMocks private AuthorizeAction authorizeAction; @@ -127,6 +131,20 @@ public class AuthorizeActionTest { @Mock private AuthorizeRestWebServiceValidator authorizeRestWebServiceValidator; + @Test + public void checkPermissionGranted_whenExceptionThrown_shouldDeny() { + authorizeAction.setClientId("testId"); + + // force throw exception + final RuntimeException exception = new RuntimeException(); + when(clientService.getClient(anyString())).thenThrow(exception); + + authorizeAction.checkPermissionGranted(); + + verify(log).error("Failed to perform checkPermissionGranted()", exception); + verify(authorizeAction).permissionDenied(); + } + @Test public void shouldSkipScript_forExplicitDefaultPasswordAuth_shouldReturnTrue() { final boolean skip = authorizeAction.shouldSkipScript(Lists.newArrayList(OxConstants.SCRIPT_TYPE_INTERNAL_RESERVED_NAME)); From 5469d80ff82c608258cfc60f7ffee8361adcdf46 Mon Sep 17 00:00:00 2001 From: YuriyZ Date: Fri, 7 Apr 2023 12:30:24 +0300 Subject: [PATCH 4/4] doc(jans-auth-server): corrected doc for errorHandlingMethod #4449 --- .../json/properties/janssenauthserver-properties.md | 6 +++--- .../io/jans/as/model/configuration/AppConfiguration.java | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/admin/reference/json/properties/janssenauthserver-properties.md b/docs/admin/reference/json/properties/janssenauthserver-properties.md index 6c1c815cf8c..ce566c15c79 100644 --- a/docs/admin/reference/json/properties/janssenauthserver-properties.md +++ b/docs/admin/reference/json/properties/janssenauthserver-properties.md @@ -110,7 +110,7 @@ tags: | enabledOAuthAuditLogging | enable OAuth Audit Logging | [Details](#enabledoauthauditlogging) | | endSessionEndpoint | URL at the OP to which an RP can perform a redirect to request that the end user be logged out at the OP | [Details](#endsessionendpoint) | | endSessionWithAccessToken | Choose whether to accept access tokens to call end_session endpoint | [Details](#endsessionwithaccesstoken) | -| errorHandlingMethod | A list of possible error handling methods | [Details](#errorhandlingmethod) | +| errorHandlingMethod | A list of possible error handling methods. Possible values: remote (send error back to RP), internal (show error page). | [Details](#errorhandlingmethod) | | errorReasonEnabled | Boolean value specifying whether to return detailed reason of the error from AS. Default value is false | [Details](#errorreasonenabled) | | expirationNotificatorEnabled | Boolean value specifying whether expiration notificator is enabled (used to identify expiration for persistence that support TTL, like Couchbase) | [Details](#expirationnotificatorenabled) | | expirationNotificatorIntervalInSeconds | The expiration notificator interval in second | [Details](#expirationnotificatorintervalinseconds) | @@ -1174,11 +1174,11 @@ tags: ### errorHandlingMethod -- Description: A list of possible error handling methods +- Description: A list of possible error handling methods. Possible values: remote (send error back to RP), internal (show error page) - Required: No -- Default value: None +- Default value: Remote ### errorReasonEnabled diff --git a/jans-auth-server/model/src/main/java/io/jans/as/model/configuration/AppConfiguration.java b/jans-auth-server/model/src/main/java/io/jans/as/model/configuration/AppConfiguration.java index c8f0db76544..e3f6759397d 100644 --- a/jans-auth-server/model/src/main/java/io/jans/as/model/configuration/AppConfiguration.java +++ b/jans-auth-server/model/src/main/java/io/jans/as/model/configuration/AppConfiguration.java @@ -706,8 +706,8 @@ public class AppConfiguration implements Configuration { @DocProperty(description = "Authentication Brute Force Protection Configuration") private AuthenticationProtectionConfiguration authenticationProtectionConfiguration; - @DocProperty(description = "A list of possible error handling methods") - private ErrorHandlingMethod errorHandlingMethod = ErrorHandlingMethod.INTERNAL; + @DocProperty(description = "A list of possible error handling methods. Possible values: remote (send error back to RP), internal (show error page)", defaultValue = "remote") + private ErrorHandlingMethod errorHandlingMethod = ErrorHandlingMethod.REMOTE; @DocProperty(description = "Boolean value specifying whether to disable authentication when max_age=0", defaultValue = "false") private Boolean disableAuthnForMaxAgeZero;