fix(permissions): access-denied 403 page + auth/authz split + SAML loop-guard (#32365)

[dsolistorres]

Problem

On SAML-protected sites, a SAML user without a back-end role (front-end-only) who lands on a back-end URL (e.g. /dotAdmin/) — or any authenticated front-end user requesting a resource they lack READ on — was bounced back to the IdP repeatedly. Because the IdP re-authenticates the already-signed-in user but that never grants the missing access, the result was an infinite redirect loop (ERR_TOO_MANY_REDIRECTS) with no default "Access Denied" landing. This is the counterpart to the file-asset status-code fix in #36541.

What this PR does

Cleanly separates authentication (401 → start login) from authorization (403 → Access Denied, never re-authenticate), and gives those users a real destination:

• Centralized 401-vs-403 decision — new SecurityUtils.sendPermissionDenied(user, uri, req, resp): authenticated (non-anonymous) → 403 and clears REDIRECT_AFTER_LOGIN; anonymous/null → 401 + sets it. Adopted by BinaryExporterServlet, SpeedyAssetServlet, and CMSUrlUtil (which previously keyed only on user == null).
• Default 403 "Access Denied" page in custom-error-page.jsp with a provider-agnostic sign-out link: SAML SLO (/api/v1/dotsaml/logout/{host}) when the host has SAML enabled (Entra ID, Okta, …), else /dotCMS/logout. Honors the existing per-site /cms403Page vanity override.
• Deterministic SAML loop-guard in SamlWebInterceptor: if auto-login resolved a SAML user yet the request is still not logged in for its destination (isAutoLogin() && isNotLogged()), return a clean 403 instead of redirecting to the IdP again. This trips on the first futile cycle and is immune to the per-callback session churn caused by SameSite cookies.

Anonymous (not-logged-in) users still get the legitimate 401 → login flow; back-end/admin users still get 403.

Fixes / Refs

• Fixes Add Default "Access Denied" Page for SAML Users Without Backend Role #32365
• Refs BinaryExporterServlet returns 401 (redirect-to-login) instead of 403 for authenticated front-end users lacking READ on a protected file asset #36541

Testing

• Unit: SecurityUtilsTest (401-vs-403 resolver) — 19/19 green.
• Manual (native login and real Entra ID): authenticated-but-unauthorized front-end user → clean 403 on pages and file assets (no REDIRECT_AFTER_LOGIN); anonymous → 401 → login; SAML front-end user hitting a back-end URL now gets a 403 Access Denied with a sign-out link instead of an infinite loop.

Follow-up (tracked separately)

• Preserve the original front-end URL through the SAML round-trip (default RelayState → original request, with open-redirect validation) so users return to the exact page they requested.
• Fix DotSamlResource.logoutGet/logoutPost missing return after redirect (spurious DoesNotExistException).

🤖 Generated with Claude Code

This PR fixes: #32365

[claude]

Claude finished @dsolistorres's task in 7m 3s —— View job

---

🔍 dotCMS Backend Review

• Load dotCMS conventions (CLAUDE.md, Java/DB/REST/Security docs)
• Get Java diff (de0fe190..8e2a308e)
• Security review — NO_FINDINGS
• Database review — NO_FINDINGS
• Java standards review — NO_FINDINGS
• REST API review — NO_FINDINGS
• Consolidate findings
• Upsert sticky comment
• Submit formal review (✅ Approved)

All four review passes came back clean on the latest commit (8e2a308e). The four findings from the earlier review round were verified resolved:

• SamlWebInterceptor's loop-guard now clears WebKeys.REDIRECT_AFTER_LOGIN inline before returning 403, covering /api/* destinations.
• SpeedyAssetServlet reverted to sessionless sendError(SC_UNAUTHORIZED) — no session allocation on this high-traffic endpoint.
• DotSamlResource.logoutGet/logoutPost now return; after the redirect, consistent with sibling processLogin.
• CMSUrlUtil now imports and calls SecurityUtils unqualified, matching BinaryExporterServlet.

The consolidated findings were posted to the sticky backend-review comment (updated in place, now showing "no issues found"), and a formal ✅ approving GitHub review was submitted, superseding the earlier request-changes review.

[claude]

✅ dotCMS Backend Review: no issues found.

All four review passes (security, database, Java standards, REST API) came back clean on the latest commit (8e2a308e). The four findings from the previous review round (loop-guard not clearing REDIRECT_AFTER_LOGIN, SpeedyAssetServlet session allocation, DotSamlResource logout fall-through, FQN-inline imports) have all been verified as resolved:

• SamlWebInterceptor's loop-guard now clears WebKeys.REDIRECT_AFTER_LOGIN inline before returning 403, covering /api/* destinations where custom-error-page.jsp would otherwise skip the cleanup.
• SpeedyAssetServlet was reverted to its original sessionless sendError(SC_UNAUTHORIZED) behavior — no new session allocation on this high-traffic endpoint.
• DotSamlResource.logoutGet/logoutPost now return; after RedirectUtil.sendRedirectHTML(...), consistently with the sibling processLogin method — no more fall-through into the unconditional DoesNotExistException over a committed response.
• CMSUrlUtil now imports SecurityUtils and calls it unqualified, matching BinaryExporterServlet.

---

Next steps

• Ready to merge from a backend-review standpoint.

[claude]

🚫 Critical or High severity findings must be resolved before merging. See the review comment above for details.

[dsolistorres]

Thanks for the review — addressed all four items in 8e2a308ebf:

🟠 High — loop-guard skipped clearing REDIRECT_AFTER_LOGIN (SamlWebInterceptor)
The authenticated-403 guard now clears WebKeys.REDIRECT_AFTER_LOGIN before returning the 403, so a stale redirect can't survive the session — including for /api/* destinations where custom-error-page.jsp returns before its 403 branch would clear it. I cleared it inline rather than routing through SecurityUtils.sendPermissionDenied on purpose: the resolved User isn't in scope at that point, and this path must never fall to the helper's 401 branch (which would re-set REDIRECT_AFTER_LOGIN and re-enter the very loop we're breaking). Added a test assertion that the attribute is cleared.

🟡 Medium — logoutGet/logoutPost fall-through (DotSamlResource)
Good call that the new sign-out link makes this user-facing on every click — pulled the one-line fix into this PR instead of deferring it: return; after RedirectUtil.sendRedirectHTML(...) in both methods, so they no longer fall through to the unconditional DoesNotExistException throw over the committed response. (Removed from the follow-up ticket #36591.)

🟡 Medium — SpeedyAssetServlet session allocation (SpeedyAssetServlet)
Reverted this file entirely to the original sessionless sendError(SC_UNAUTHORIZED) on the auth-time SecurityException — no new HttpSession / REDIRECT_AFTER_LOGIN write on this high-traffic static-asset endpoint. Asset authorization is still enforced downstream by BinaryExporterServlet via the /contentAsset forward, so nothing is lost.

🟡 Medium — FQN-inline call in CMSUrlUtil
Added import com.dotcms.util.SecurityUtils; and call it unqualified at both sites, matching BinaryExporterServlet.

The two SpeedyAssetServlet notes from the earlier review are moot now that the file is reverted. Unit tests green (SamlWebInterceptorLoopGuardTest 2/2 incl. the new clear-redirect assertion, SecurityUtilsTest 19/19).

[claude]

✅ No Critical or High severity issues found.

[mergify]

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

• Queue this pull request