feat(jans-auth-server): redirect back to RP when session is expired o…

…r if not possible show error page #4449 (#4505) * feat(jans-auth-server): show error with clear message if exception occurs authz check #4449 * feat(jans-auth-server): default errorHandlingMethod to "remote" value and return correct error during handling #4449 * test(jans-auth-server): covered exception case during deny #4449 * doc(jans-auth-server): corrected doc for errorHandlingMethod #4449
JanssenProject · Apr 7, 2023 · 0983e73 · 0983e73
1 parent 5c0b586
commit 0983e73
Show file tree

Hide file tree

Showing 14 changed files with 200 additions and 140 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
@@ -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/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
@@ -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;

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
@@ -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
@@ -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<String, String> 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/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();