Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,9 @@ public void logoutPost(@Parameter(description = "Identity Provider configuration
SamlName.DOT_SAML_LOGOUT_SERVICE_ENDPOINT_URL,
()-> "/dotAdmin/#/public/logout");
RedirectUtil.sendRedirectHTML(httpServletResponse, logoutPath);
// The redirect commits the response; return so we don't fall through to the
// unconditional DoesNotExistException throw below (which would write a second response).
return;


}
Expand Down Expand Up @@ -485,6 +488,9 @@ public void logoutGet(@Parameter(description = "Identity Provider configuration


RedirectUtil.sendRedirectHTML(httpServletResponse, logoutPath);
// The redirect commits the response; return so we don't fall through to the
// unconditional DoesNotExistException throw below (which would write a second response).
return;

}
} finally {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,45 @@ public Result intercept(final HttpServletRequest request,
// if the auto login couldn't log in the user, then send it to the IdP login page (if it is not already logged in).
if (null == session || !autoLoginResult.isAutoLogin() || this.samlWebUtils.isNotLogged(request)) {

// Loop-guard: if auto-login actually resolved a SAML user (isAutoLogin)
// yet the request is STILL not logged in for this destination, re-authenticating
// will resolve the very same user and produce the same result — that is
// an infinite IdP redirect loop (e.g. a front-end-only SAML user sent
// to a back-end URL like /dotAdmin). Break it
// with a clean 403 instead of bouncing to the IdP again.
//
// Note: this is self-healing for a transient auto-login failure (e.g. a
// one-off doCookieLogin error rather than a genuine realm mismatch). getUser()
// consumes (removes) the SAML_USER_ID from the session, so isAutoLogin can only
// be true for the single request following an IdP round-trip: a transient failure
// yields one 403, and the next request has no SAML_USER_ID (isAutoLogin=false) and
// re-runs a full IdP authentication. So a temporary glitch cannot lock the user out.
if (autoLoginResult.isAutoLogin() && this.samlWebUtils.isNotLogged(request)) {

Logger.warn(this, ()-> "SAML auth redirect loop detected for URI '"
+ request.getRequestURI() + "': the user is authenticated but not "
+ "authorized for this destination. Returning 403 instead of "
+ "redirecting to the IdP again.");
SecurityLogger.logInfo(this.getClass(), "SAML auth redirect loop broken for URI: "
+ request.getRequestURI());
// Mirror SecurityUtils.sendPermissionDenied's authenticated-403 path: clear any
// stale REDIRECT_AFTER_LOGIN so it cannot resurface as an unwanted redirect on the
// next login. We clear it HERE (not by relying on the JSP 403 branch) because for
// /api/* destinations custom-error-page.jsp returns early before that branch runs.
// Cleared inline rather than via sendPermissionDenied because the resolved User is
// not in scope here and this path must never fall to the 401 branch (which would
// re-set REDIRECT_AFTER_LOGIN and re-enter the loop).
final HttpSession loopSession = request.getSession(false);
if (null != loopSession) {
loopSession.removeAttribute(WebKeys.REDIRECT_AFTER_LOGIN);
}
// isCommitted() guard avoids IllegalStateException / a double response.
if (!response.isCommitted()) {
response.sendError(HttpServletResponse.SC_FORBIDDEN);
}
return Result.SKIP_NO_CHAIN;
}

Logger.debug(this, ()-> "User is not log in, doing SAML Authentication request");
this.doAuthentication(request, response, session, identityProviderConfiguration, host.getIdentifier());
return Result.SKIP_NO_CHAIN;
Expand Down
55 changes: 55 additions & 0 deletions dotCMS/src/main/java/com/dotcms/util/SecurityUtils.java
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
package com.dotcms.util;

import java.io.IOException;
import java.net.URL;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.TimeUnit;

import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import javax.servlet.http.HttpSession;

import com.dotcms.security.multipart.IllegalFileExtensionsValidator;
import com.dotcms.security.multipart.IllegalTraversalFilePathValidator;
Expand All @@ -16,8 +19,10 @@
import com.dotmarketing.util.Logger;
import com.dotmarketing.util.SecurityLogger;
import com.dotmarketing.util.UtilMethods;
import com.dotmarketing.util.WebKeys;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
import com.liferay.portal.model.User;
import com.liferay.util.Xss;

import io.vavr.control.Try;
Expand Down Expand Up @@ -83,6 +88,56 @@ public static String stripReferer(HttpServletRequest request, String referer) th
return ref;
}

/**
* Centralizes the front-end "permission denied" HTTP response so authentication and
* authorization are cleanly separated across every servlet/filter that handles a
* {@code DotSecurityException}. This mirrors the Spring Security split between an
* {@code AuthenticationEntryPoint} (401, "who are you?") and an {@code AccessDeniedHandler}
* (403, "I know who you are, but you cannot have this"):
* <ul>
* <li><b>Authenticated (non-anonymous) user</b> &rarr; {@code 403 Forbidden}. The
* {@link WebKeys#REDIRECT_AFTER_LOGIN} intent is cleared so the container error page does
* <b>not</b> bounce the browser back through the login/SSO flow &mdash; re-authenticating
* can never grant a missing permission, which is exactly what produces the infinite SAML
* redirect loop (see issue #36541).</li>
* <li><b>Anonymous / not-logged-in user</b> &rarr; {@code 401 Unauthorized} and
* {@link WebKeys#REDIRECT_AFTER_LOGIN} is set to {@code originalUri} so the legitimate
* login flow can return the user to the resource afterwards.</li>
* </ul>
* The user is considered anonymous when it is {@code null} or {@link User#isAnonymousUser()}
* is {@code true}.
*
* @param user the resolved user (may be {@code null} or the anonymous user)
* @param originalUri the URI to return to after login (used only for the anonymous 401 path)
* @param request the current request
* @param response the current response
* @throws IOException if the error response cannot be written
*/
public static void sendPermissionDenied(final User user, final String originalUri,
final HttpServletRequest request, final HttpServletResponse response) throws IOException {

if (response.isCommitted()) {
return;
}

if (user != null && !user.isAnonymousUser()) {
// Authenticated but not authorized: clean 403, never re-trigger authentication.
final HttpSession session = request.getSession(false);
if (session != null) {
session.removeAttribute(WebKeys.REDIRECT_AFTER_LOGIN);
}
response.sendError(HttpServletResponse.SC_FORBIDDEN);
} else {
// Anonymous: remember where the user was headed and require login.
final HttpSession session = request.getSession();
if (session != null && UtilMethods.isSet(originalUri)) {
Logger.debug(SecurityUtils.class, () -> "Setting redirect after login to requested uri: " + originalUri);
session.setAttribute(WebKeys.REDIRECT_AFTER_LOGIN, originalUri);
}
response.sendError(HttpServletResponse.SC_UNAUTHORIZED);
}
}

/**
* This is a useful mechanism for dealing with DoS and similar security attacks in which it might be
* adequate to temporarily pause a request for a specific time before letting it continue. This is
Expand Down
10 changes: 6 additions & 4 deletions dotCMS/src/main/java/com/dotmarketing/filters/CMSUrlUtil.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.dotmarketing.filters;

import com.dotcms.contenttype.model.type.BaseContentType;
import com.dotcms.util.SecurityUtils;
import com.dotmarketing.beans.Host;
import com.dotmarketing.beans.Identifier;
import com.dotmarketing.business.APILocator;
Expand Down Expand Up @@ -561,15 +562,15 @@ public boolean isUnauthorizedAndHandleError(final Permissionable permissionable,
// Check if the page is visible by a CMS Anonymous role
if (!permissionAPI.doesUserHavePermission(permissionable, PERMISSION_READ, user, mode.respectAnonPerms)) {

if (null == user) {// Not logged in user
if (null == user || user.isAnonymousUser()) {// Not logged in / anonymous user

Logger.debug(this.getClass(),
"CHECKING PERMISSION: Page doesn't have anonymous access [" + requestedURIForLogging + "]");
Logger.debug(this.getClass(), "401 URI = " + requestedURIForLogging);
Logger.debug(this.getClass(), "Unauthorized URI = " + requestedURIForLogging);

request.getSession().setAttribute(com.dotmarketing.util.WebKeys.REDIRECT_AFTER_LOGIN, requestedURIForLogging);
response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "The requested page/file is unauthorized");
// Centralized auth/authz split: anonymous -> 401 + REDIRECT_AFTER_LOGIN.
SecurityUtils.sendPermissionDenied(user, requestedURIForLogging, request, response);
return true;
} else if (!permissionAPI.getRolesWithPermission(permissionable, PERMISSION_READ)
.contains(APILocator.getRoleAPI().loadLoggedinSiteRole())) {
Expand All @@ -579,7 +580,8 @@ public boolean isUnauthorizedAndHandleError(final Permissionable permissionable,
// go to unauthorized page
Logger.warn(this.getClass(),
"CHECKING PERMISSION: Page doesn't have any access for this user [" + requestedURIForLogging + "]");
response.sendError(HttpServletResponse.SC_FORBIDDEN, "The requested page/file is forbidden");
// Centralized auth/authz split: authenticated -> clean 403, no login redirect.
SecurityUtils.sendPermissionDenied(user, requestedURIForLogging, request, response);
return true;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -675,19 +675,10 @@ public void doGet(HttpServletRequest req, HttpServletResponse resp) throws Servl
}
} catch (DotSecurityException e) {
try {
if(req.getSession()!=null){
// any authenticated (non-anonymous) user lacking READ gets a clean 403.
// Only anonymous/not-logged-in users are redirected to login with a 401.
if(user != null && !user.isAnonymousUser()){
req.getSession().removeAttribute(com.dotmarketing.util.WebKeys.REDIRECT_AFTER_LOGIN);
resp.sendError(HttpServletResponse.SC_FORBIDDEN);
}else{
final String requestUri = (String) req.getAttribute("javax.servlet.forward.request_uri");
Logger.debug(this, "Setting redirect after login to requested uri: " + requestUri);
req.getSession().setAttribute(com.dotmarketing.util.WebKeys.REDIRECT_AFTER_LOGIN, requestUri);
resp.sendError(HttpServletResponse.SC_UNAUTHORIZED);
}
}
// Centralized auth/authz split: authenticated (non-anonymous) user lacking READ
// gets a clean 403; only anonymous/not-logged-in users are redirected to login with a 401.
final String requestUri = (String) req.getAttribute("javax.servlet.forward.request_uri");
SecurityUtils.sendPermissionDenied(user, requestUri, req, resp);
} catch (Exception e1) {
Logger.error(BinaryExporterServlet.class, "An error occurred when accessing '" + uri + "': " + e1.getMessage(), e1);
if(!resp.isCommitted()){
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
402-title=Payment Required (402 error)
403-body1=You do not have permissions to view the Page or file you were looking for.<br/>(If you are logged in, please contact your administrator for access).
403-body2=If the problem persists, please return to the <a href="/">Home Page</a>.
403-logout-sso=Sign out of your identity provider and sign in with a different account
403-copywright=dotCMS LLC
403-image-title=dotCMS Content Management System
403-page-title=dotCMS: 403 Forbidden
Expand Down
28 changes: 28 additions & 0 deletions dotCMS/src/main/webapp/html/error/custom-error-page.jsp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ if(PageMode.get(request).isAdmin && Config.getBooleanProperty("SIMPLE_ERROR_PAGE
final int status = response.getStatus();
final String title = LanguageUtil.get(pageContext, status + "-page-title");
final String body = LanguageUtil.get(pageContext, status + "-body1");
// Extra HTML rendered below the body (e.g. the Access Denied sign-out link on a 403).
String accessDeniedExtraHtml = "";
try {
final long languageId = WebAPILocator.getLanguageWebAPI().getLanguage(request).getId();
final boolean isAPICall = pageContext.getErrorData().getRequestURI().startsWith("/api/");
Expand Down Expand Up @@ -91,6 +93,30 @@ if(PageMode.get(request).isAdmin && Config.getBooleanProperty("SIMPLE_ERROR_PAGE
}
request.getRequestDispatcher("/dotCMS/login").forward(request, response);
}
} else if (status == 403) {
// Access Denied: the user is authenticated but not authorized for this resource.
// Clear any stale redirect intent and offer a sign-out
// link so the user can terminate the SSO session and retry with a different account.
session.removeAttribute(WebKeys.REDIRECT_AFTER_LOGIN);

String logoutUrl = "/dotCMS/logout";
String logoutLabel = LanguageUtil.get(pageContext, "Logout");
try {
final com.dotcms.saml.DotSamlProxyFactory samlProxy = com.dotcms.saml.DotSamlProxyFactory.getInstance();
if (null != site && samlProxy.isAnyHostConfiguredAsSAML()) {
final com.dotcms.saml.IdentityProviderConfiguration idpConfig =
samlProxy.identityProviderConfigurationFactory().findIdentityProviderConfigurationById(site.getIdentifier());
if (null != idpConfig && idpConfig.isEnabled()) {
// dotCMS performs SLO against whichever IdP the host is configured for (Entra ID, Okta, ...).
logoutUrl = "/api/v1/dotsaml/logout/" + site.getIdentifier();
logoutLabel = LanguageUtil.get(pageContext, "403-logout-sso");
}
}
} catch (Exception e) {
Logger.debug(this.getClass(), "Unable to resolve SAML logout link for 403 page: " + e.getMessage());
}

accessDeniedExtraHtml = "<p><a href=\"" + logoutUrl + "\">" + logoutLabel + "</a></p>";
} else if (status == 404) {
ClickstreamFactory.add404Request(request, response, site);
} else if (status == 500) {
Expand Down Expand Up @@ -149,6 +175,8 @@ h1 {

<p><%= body %></p>

<%= accessDeniedExtraHtml %>

</div>
</div>
</body>
Expand Down
Loading
Loading