diff --git a/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilHttp.java b/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilHttp.java index 469eb29b753..27be77d82f7 100644 --- a/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilHttp.java +++ b/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilHttp.java @@ -411,6 +411,18 @@ public static Map 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 getUrlOnlyParameterMap(HttpServletRequest request) { // NOTE: these have already been through canonicalizeParameterMap, so not doing it again here Map paramMap = getQueryStringOnlyParameterMap(request.getQueryString()); @@ -1820,4 +1832,22 @@ private static List 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 loginUris = StringUtil.split(EntityUtilProperties.getPropertyValue("security", "login.uris", + (Delegator) request.getAttribute("delegator")), ","); + return loginUris.stream() + .anyMatch(uri -> UtilValidate.isUriEquals(getPathInfoWithoutQuery(request), uri)); + } } diff --git a/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilValidate.java b/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilValidate.java index d0f86e86981..1a77931ed42 100644 --- a/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilValidate.java +++ b/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); + } } diff --git a/framework/security/config/security.properties b/framework/security/config/security.properties index 088a4399b4c..ff1179c4fe3 100644 --- a/framework/security/config/security.properties +++ b/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 diff --git a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/LoginWorker.java b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/LoginWorker.java index 7e92561aade..979e9765305 100644 --- a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/LoginWorker.java +++ b/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 urlParams = UtilHttp.getUrlOnlyParameterMap(request); - if (UtilValidate.isNotEmpty(urlParams)) { - session.setAttribute("_PREVIOUS_PARAM_MAP_URL_", urlParams); - } - Predicate isUrlParam = urlParams.keySet()::contains; - Map 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 urlParams = UtilHttp.getUrlOnlyParameterMap(request); + if (UtilValidate.isNotEmpty(urlParams)) { + session.setAttribute("_PREVIOUS_PARAM_MAP_URL_", urlParams); + } + Predicate isUrlParam = urlParams.keySet()::contains; + Map formParams = UtilHttp.getParameterMap(request, isUrlParam.negate()); + if (UtilValidate.isNotEmpty(formParams)) { + session.setAttribute("_PREVIOUS_PARAM_MAP_FORM_", formParams); + } } response.setStatus(HttpStatus.SC_UNAUTHORIZED); return "error"; diff --git a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java index 6d54d7f0a5f..04fbfaa3a74 100644 --- a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java +++ b/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 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 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 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 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"); diff --git a/framework/widget/src/main/java/org/apache/ofbiz/widget/WidgetWorker.java b/framework/widget/src/main/java/org/apache/ofbiz/widget/WidgetWorker.java index 43cd182bd99..b7ac80e043c 100644 --- a/framework/widget/src/main/java/org/apache/ofbiz/widget/WidgetWorker.java +++ b/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 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"; }