Improved: previous request information lost at login

framework/base/src/main/java/org/apache/ofbiz/base/util/UtilHttp.java

@@ -411,6 +411,18 @@ public static Map<String, Object> getPathInfoOnlyParameterMap(String path, Predi
                         UtilHttp::canonicalizeParameterMap));
     }
 
+    /**
+     * Return pathInfo without parameters passed as query
+     * @param request
+     */
+    public static String getPathInfoWithoutQuery(HttpServletRequest request) {
+        String pathInfo = request.getPathInfo();
+        pathInfo = pathInfo.indexOf('?') > -1
+                ? pathInfo.substring(0, pathInfo.indexOf('?'))
+                : pathInfo;
+        return pathInfo.replaceAll("[^\\p{Alnum}/\\-_]", ""); // TODO find better place
+    }
+
     public static Map<String, Object> getUrlOnlyParameterMap(HttpServletRequest request) {
         // NOTE: these have already been through canonicalizeParameterMap, so not doing it again here
         Map<String, Object> paramMap = getQueryStringOnlyParameterMap(request.getQueryString());
@@ -1820,4 +1832,22 @@ private static List<String> getAllowedProtocols() {
         return allowedProtocolList;
     }
 
+    /**
+     * Return true if the request is identified as AjaxRequest
+     * @param request
+     */
+    public static boolean isAjaxCall(HttpServletRequest request) {
+        return "XMLHttpRequest".equalsIgnoreCase(request.getHeader("X-Requested-With"));
+    }
+
+    /**
+     * Return true if the request match logins uri present en security properties
+     * @param request
+     */
+    public static boolean isLoginRequest(HttpServletRequest request) {
+        List<String> loginUris = StringUtil.split(EntityUtilProperties.getPropertyValue("security", "login.uris",
+                (Delegator) request.getAttribute("delegator")), ",");
+        return loginUris.stream()
+                .anyMatch(uri -> UtilValidate.isUriEquals(getPathInfoWithoutQuery(request), uri));
+    }
 }

framework/base/src/main/java/org/apache/ofbiz/base/util/UtilValidate.java

@@ -1314,4 +1314,20 @@ public static boolean isValidPhoneNumber(String phoneNumber, String geoId, Deleg
         }
         return isValid;
     }
+
+    /**
+     * return true if the two uri are equals
+     *  /uri == uri
+     *  Uri == uri
+     * @param firstUri
+     * @param secondUri
+     */
+    public static boolean isUriEquals(String firstUri, String secondUri) {
+        String firstUriClean = firstUri.trim();
+        firstUriClean = firstUriClean.startsWith("/") ? firstUriClean.substring(1) : firstUriClean;
+
+        String secondUriClean = secondUri.trim();
+        secondUriClean = secondUriClean.startsWith("/") ? secondUriClean.substring(1) : secondUriClean;
+        return firstUriClean.equalsIgnoreCase(secondUriClean);
+    }
 }

framework/security/config/security.properties

@@ -388,8 +388,8 @@ allowZUGFeRDnotSecure=false
 afterlogin.lastvisit.show=
 
 #-- uri used for login (cf jira OFBIZ-12047)
-#-- it's a list, each uri should be separated by comma, without space
-login.uris=login
+#-- it's a list a login or related to login url, each uri should be separated by comma, without space
+login.uris=logout,login,checkLogin,checkLogin/login
 
 #-- If you need to use localhost or 127.0.0.1 in textareas URLs then you can uncomment the allowedProtocols property, here given as an example
 #-- You may also put other protocols you want to use, instead or with those

framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/LoginWorker.java

@@ -354,20 +354,24 @@ public static String checkLogin(HttpServletRequest request, HttpServletResponse
                 // return an error
                 request.removeAttribute("_LOGIN_PASSED_");
 
-                // keep the previous request name in the session
-                session.setAttribute("_PREVIOUS_REQUEST_", request.getPathInfo());
-
-                // NOTE: not using the old _PREVIOUS_PARAMS_ attribute at all because it was a security hole as it was used to put data in
-                // the URL (never encrypted) that was originally in a form field that may have been encrypted
-                // keep 2 maps: one for URL parameters and one for form parameters
-                Map<String, Object> urlParams = UtilHttp.getUrlOnlyParameterMap(request);
-                if (UtilValidate.isNotEmpty(urlParams)) {
-                    session.setAttribute("_PREVIOUS_PARAM_MAP_URL_", urlParams);
-                }
-                Predicate<String> isUrlParam = urlParams.keySet()::contains;
-                Map<String, Object> formParams = UtilHttp.getParameterMap(request, isUrlParam.negate());
-                if (UtilValidate.isNotEmpty(formParams)) {
-                    session.setAttribute("_PREVIOUS_PARAM_MAP_FORM_", formParams);
+                // keep the previous request name in the session if the requestUri is auth and without event
+                if (!UtilHttp.isLoginRequest(request) && !UtilHttp.isAjaxCall(request)
+                        && RequestHandler.isSecurityAuthRequest(request)
+                        && !RequestHandler.isRequestWithEvent(request)) {
+                    session.setAttribute("_PREVIOUS_REQUEST_", request.getPathInfo());
+
+                    // NOTE: not using the old _PREVIOUS_PARAMS_ attribute at all because it was a security hole as it was used to put data in
+                    // the URL (never encrypted) that was originally in a form field that may have been encrypted
+                    // keep 2 maps: one for URL parameters and one for form parameters
+                    Map<String, Object> urlParams = UtilHttp.getUrlOnlyParameterMap(request);
+                    if (UtilValidate.isNotEmpty(urlParams)) {
+                        session.setAttribute("_PREVIOUS_PARAM_MAP_URL_", urlParams);
+                    }
+                    Predicate<String> isUrlParam = urlParams.keySet()::contains;
+                    Map<String, Object> formParams = UtilHttp.getParameterMap(request, isUrlParam.negate());
+                    if (UtilValidate.isNotEmpty(formParams)) {
+                        session.setAttribute("_PREVIOUS_PARAM_MAP_FORM_", formParams);
+                    }
                 }
                 response.setStatus(HttpStatus.SC_UNAUTHORIZED);
                 return "error";

framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java

@@ -26,7 +26,6 @@
 import java.net.URL;
 import java.security.cert.X509Certificate;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Enumeration;
@@ -346,6 +345,26 @@ public static RequestHandler from(HttpServletRequest request) {
         return UtilGenerics.cast(request.getServletContext().getAttribute("_REQUEST_HANDLER_"));
     }
 
+    /**
+     * check if HTTP request match requestMaps with a security auth at true.
+     * @param request the HTTP request containing the request handler
+     * @return true if all requestMap linked are security auth at true
+     */
+    public static boolean isSecurityAuthRequest(HttpServletRequest request) {
+        Collection<RequestMap> requestMaps = resolveURI(Objects.requireNonNull(from(request).getControllerConfig()), request);
+        return requestMaps.stream().allMatch(RequestMap::isSecurityAuth);
+    }
+
+    /**
+     * check if HTTP request match requestMaps with a event.
+     * @param request the HTTP request containing the request handler
+     * @return true if all requestMap linked are security auth at true
+     */
+    public static boolean isRequestWithEvent(HttpServletRequest request) {
+        Collection<RequestMap> requestMaps = resolveURI(Objects.requireNonNull(from(request).getControllerConfig()), request);
+        return requestMaps.stream().anyMatch(requestMap -> requestMap.getEvent() != null);
+    }
+
     public ConfigXMLReader.ControllerConfig getControllerConfig() {
         try {
             return ConfigXMLReader.getControllerConfig(this.controllerConfigURL);
@@ -385,8 +404,7 @@ public void doRequest(HttpServletRequest request, HttpServletResponse response,
         // Grab data from request object to process
         String defaultRequestUri = RequestHandler.getRequestUri(request.getPathInfo());
 
-        String requestMissingErrorMessage = "Unknown request ["
-                + defaultRequestUri
+        String requestMissingErrorMessage = "Unknown request [" + UtilHttp.getPathInfoWithoutQuery(request)
                 + "]; this request does not exist or cannot be called directly.";
 
         String path = request.getPathInfo();
@@ -624,17 +642,10 @@ public void doRequest(HttpServletRequest request, HttpServletResponse response,
                 // previous URL already saved by event, so just do as the return says...
                 eventReturn = checkLoginReturnString;
                 // if the request is an ajax request we don't want to return the default login check
-                if (!"XMLHttpRequest".equals(request.getHeader("X-Requested-With"))) {
-                    requestMap = ccfg.getRequestMapMap().get("checkLogin");
-                } else {
-                    requestMap = ccfg.getRequestMapMap().get("ajaxCheckLogin");
-                }
-            }
-        } else if (requestUri != null) {
-            String[] loginUris = EntityUtilProperties.getPropertyValue("security", "login.uris", delegator).split(",");
-            if (Arrays.asList(loginUris).contains(requestUri)) {
-                // Remove previous request attribute on navigation to non-authenticated request
-                request.getSession().removeAttribute("_PREVIOUS_REQUEST_");
+                requestMap = ccfg.getRequestMapMap()
+                        .get(UtilHttp.isAjaxCall(request)
+                                ? "ajaxCheckLogin"
+                                : "checkLogin");
             }
         }
 
@@ -771,38 +782,31 @@ public void doRequest(HttpServletRequest request, HttpServletResponse response,
         }
 
         // if previous request exists, and a login just succeeded, do that now.
-        if (previousRequest != null && loginPass != null && "TRUE".equalsIgnoreCase(loginPass)) {
+        if (previousRequest != null && "TRUE".equalsIgnoreCase(loginPass)) {
             request.getSession().removeAttribute("_PREVIOUS_REQUEST_");
             // special case to avoid login/logout looping: if request was "logout" before the login, change to null for default success view; do
             // the same for "login" to avoid going back to the same page
-            if ("logout".equals(previousRequest) || "/logout".equals(previousRequest) || "login".equals(previousRequest)
-                    || "/login".equals(previousRequest) || "checkLogin".equals(previousRequest) || "/checkLogin".equals(previousRequest)
-                    || "/checkLogin/login".equals(previousRequest)) {
-                Debug.logWarning("Found special _PREVIOUS_REQUEST_ of [" + previousRequest + "], setting to null to avoid problems, not running "
-                        + "request again", MODULE);
-            } else {
-                if (Debug.infoOn()) {
-                    Debug.logInfo("[Doing Previous Request]: " + previousRequest + showSessionId(request), MODULE);
-                }
+            if (Debug.infoOn()) {
+                Debug.logInfo("[Doing Previous Request]: " + previousRequest + showSessionId(request), MODULE);
+            }
 
-                // note that the previous form parameters are not setup (only the URL ones here), they will be found in the session later and
-                // handled when the old request redirect comes back
-                Map<String, Object> previousParamMap = UtilGenerics.checkMap(request.getSession().getAttribute("_PREVIOUS_PARAM_MAP_URL_"),
-                        String.class, Object.class);
-                String queryString = UtilHttp.urlEncodeArgs(previousParamMap, false);
-                String redirectTarget = previousRequest;
-                if (UtilValidate.isNotEmpty(queryString)) {
-                    redirectTarget += "?" + queryString;
-                }
-                String link = makeLink(request, response, redirectTarget);
+            // note that the previous form parameters are not setup (only the URL ones here), they will be found in the session later and
+            // handled when the old request redirect comes back
+            Map<String, Object> previousParamMap = UtilGenerics.checkMap(request.getSession().getAttribute("_PREVIOUS_PARAM_MAP_URL_"),
+                    String.class, Object.class);
+            String queryString = UtilHttp.urlEncodeArgs(previousParamMap, false);
+            String redirectTarget = previousRequest;
+            if (UtilValidate.isNotEmpty(queryString)) {
+                redirectTarget += "?" + queryString;
+            }
+            String link = makeLink(request, response, redirectTarget);
 
-                // add / update csrf token to link when required
-                String tokenValue = CsrfUtil.generateTokenForNonAjax(request, redirectTarget);
-                link = CsrfUtil.addOrUpdateTokenInUrl(link, tokenValue);
+            // add / update csrf token to link when required
+            String tokenValue = CsrfUtil.generateTokenForNonAjax(request, redirectTarget);
+            link = CsrfUtil.addOrUpdateTokenInUrl(link, tokenValue);
 
-                callRedirect(link, response, request, ccfg.getStatusCode());
-                return;
-            }
+            callRedirect(link, response, request, ccfg.getStatusCode());
+            return;
         }
 
         ConfigXMLReader.RequestResponse successResponse = requestMap.getRequestResponseMap().get("success");

framework/widget/src/main/java/org/apache/ofbiz/widget/WidgetWorker.java

@@ -33,7 +33,6 @@
 import org.apache.ofbiz.base.util.UtilValidate;
 import org.apache.ofbiz.entity.Delegator;
 import org.apache.ofbiz.service.LocalDispatcher;
-import org.apache.ofbiz.webapp.control.ConfigXMLReader;
 import org.apache.ofbiz.webapp.control.RequestHandler;
 import org.apache.ofbiz.webapp.taglib.ContentUrlTag;
 import org.apache.ofbiz.widget.model.CommonWidgetModels;
@@ -234,14 +233,8 @@ public static String makeLinkHiddenFormName(Map<String, Object> context, ModelFo
 
     public static String determineAutoLinkType(String linkType, String target, String targetType, HttpServletRequest request) {
         if ("auto".equals(linkType)) {
-            if ("intra-app".equals(targetType)) {
-                String requestUri = (target.indexOf('?') > -1) ? target.substring(0, target.indexOf('?')) : target;
-                ServletContext servletContext = request.getSession().getServletContext();
-                RequestHandler rh = (RequestHandler) servletContext.getAttribute("_REQUEST_HANDLER_");
-                ConfigXMLReader.RequestMap requestMap = rh.getControllerConfig().getRequestMapMap().get(requestUri);
-                if (requestMap != null && requestMap.getEvent() != null) {
-                    return "hidden-form";
-                }
+            if ("intra-app".equals(targetType) && RequestHandler.isRequestWithEvent(request)) {
+                return "hidden-form";
             }
             return "anchor";
         }