Skip to content

fix(security): prevent origin hostname leak via SAML and add anonymous access warning - #34999

Open
mbiuki wants to merge 4 commits into
mainfrom
fix/content-api-origin-exposure-34997
Open

fix(security): prevent origin hostname leak via SAML and add anonymous access warning#34999
mbiuki wants to merge 4 commits into
mainfrom
fix/content-api-origin-exposure-34997

Conversation

@mbiuki

@mbiuki mbiuki commented Mar 16, 2026

Copy link
Copy Markdown
Member

Summary

Fixes #34997

Three defensive code changes to reduce the risk of origin server exposure in CDN-fronted deployments. The real architectural fix remains at the infrastructure layer (restrict origin to CloudFront IP ranges), but these changes address what code can do:

  • Stop the SAML origin leak — buildBaseUrlFromRequest() now prefers sPEndpointHostname config over request.getServerName(), so the SAML logout redirect never exposes the raw origin hostname
  • Startup security warning — AnonymousAccess.systemSetting() logs a WARN when CONTENT_APIS_ALLOW_ANONYMOUS=READ/WRITE, alerting operators to ensure the origin is network-restricted
  • Improved config documentation — dotmarketing-config.properties comment now includes CDN/origin security guidance and a reference to the com.dotcms.userproxy plugin

What this PR does NOT change

  • The CONTENT_APIS_ALLOW_ANONYMOUS=READ default is unchanged — changing it would break public-facing pages that serve content via the API
  • No network-level restrictions (infrastructure, not code)
  • No changes to the com.dotcms.userproxy plugin (separate community repo)

Files Changed

File Change
DotSamlResource.java buildBaseUrlFromRequest() prefers sPEndpointHostname config when set
AnonymousAccess.java Startup Logger.warn() when anonymous content access is enabled
dotmarketing-config.properties Expanded comment with CDN/origin security note and UserProxy reference

Test Plan

  • Set sPEndpointHostname=https://public.cdn.example.com in SAML app settings → verify SAML logout redirect uses CDN hostname, not origin
  • Leave sPEndpointHostname unset → verify SAML logout redirect falls back to request.getServerName() (no regression)
  • Start dotCMS with default config → verify SECURITY: CONTENT_APIS_ALLOW_ANONYMOUS=READ warning in logs
  • Set CONTENT_APIS_ALLOW_ANONYMOUS=NONE → verify no warning logged
  • Review dotmarketing-config.properties in build artifact — confirm updated comment

🤖 Generated with Claude Code

@mbiuki @claude
fix(security): prevent origin hostname leak via SAML and add anonymou…
d1a6c35 
…s access warning

Fixes #34997

Three defensive changes to reduce the risk of origin server exposure in
CDN-fronted deployments:

1. DotSamlResource.buildBaseUrlFromRequest() now prefers the configured
   sPEndpointHostname (DOT_SAML_SERVICE_PROVIDER_HOST_NAME) over
   request.getServerName(). This lets operators set the CDN/public hostname
   so the SAML logout redirect never exposes the origin server name.

2. AnonymousAccess.systemSetting() logs a SECURITY warning at startup
   when CONTENT_APIS_ALLOW_ANONYMOUS=READ or WRITE, alerting operators
   to ensure the origin is network-restricted to trusted proxies.

3. dotmarketing-config.properties expands the CONTENT_APIS_ALLOW_ANONYMOUS
   comment with a CDN/origin security note and a reference to the
   com.dotcms.userproxy plugin for selective per-path auth.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions github-actions Bot mentioned this pull request Mar 16, 2026
Open
5 tasks
@github-actions github-actions Bot added the Area : Backend PR changes Java/Maven backend code label Mar 16, 2026
mbiuki and others added 3 commits March 17, 2026 00:06
@mbiuki
Merge branch 'main' into fix/content-api-origin-exposure-34997
2e06073
@mbiuki @claude
test: add unit tests for AnonymousAccess.systemSetting() and DotSamlR…
d4e12d4 
…esource.buildBaseUrlFromRequest()

- AnonymousAccessTest: 6 tests covering default/READ/WRITE/NONE/invalid/case-insensitive
  resolution of CONTENT_APIS_ALLOW_ANONYMOUS config property, and verifying that
  READ and WRITE (the warning-triggering values) are correctly identified as non-NONE

- DotSamlResourceBuildBaseUrlTest: 5 tests verifying that buildBaseUrlFromRequest()
  prefers sPEndpointHostname config over request.getServerName(), falls back when
  config is absent or empty, always appends /dotAdmin/show-logout, and does not
  leak the origin hostname when the CDN hostname is configured

Also makes buildBaseUrlFromRequest() package-private with @VisibleForTesting
to enable direct unit testing.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@mbiuki @claude
fix: use getPropertyName() instead of propName() on SamlName enum
0a3b0ec 
SamlName enum exposes getPropertyName(), not propName(). Fix compilation
error in DotSamlResource.buildBaseUrlFromRequest() and the corresponding
unit test.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@mbiuki

mbiuki commented Mar 16, 2026

Copy link
Copy Markdown
Member Author

✅ Unit Test Results — Local Run

Tests were run locally against JDK 21 after installing placeholder build artifacts.

T E S T S
─────────────────────────────────────────────────────────────────────────
Running com.dotcms.auth.providers.saml.v1.DotSamlResourceBuildBaseUrlTest
Tests run: 5, Failures: 0, Errors: 0, Skipped: 0  (4.889s)

Running com.dotcms.rest.AnonymousAccessTest
Tests run: 7, Failures: 0, Errors: 0, Skipped: 0  (0.004s)

Results: Tests run: 12, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS

DotSamlResourceBuildBaseUrlTest — 5 tests

Test Scenario Result
testBuildBaseUrl_usesConfiguredHostname_whenSet sPEndpointHostname=https://cdn.example.com configured ✅ CDN URL used
testBuildBaseUrl_fallsBackToRequestHostname_whenNotConfigured No config set ✅ Falls back to scheme://requestHost:port
testBuildBaseUrl_fallsBackToRequestHostname_whenConfiguredHostIsEmpty sPEndpointHostname="" ✅ Falls back to request hostname
testBuildBaseUrl_alwaysEndsWithLogoutPath Any config ✅ Always ends with /dotAdmin/show-logout
testBuildBaseUrl_doesNotLeakOriginHostname_whenConfigured CDN hostname configured ✅ Origin hostname absent from URL

AnonymousAccessTest — 7 tests

Test Scenario Result
testSystemSetting_defaultsToRead_whenPropertyNotSet No property set ✅ Returns READ
testSystemSetting_returnsRead_whenSetToRead CONTENT_APIS_ALLOW_ANONYMOUS=READ ✅ Returns READ
testSystemSetting_returnsNone_whenSetToNone =NONE ✅ Returns NONE
testSystemSetting_returnsWrite_whenSetToWrite =WRITE ✅ Returns WRITE
testSystemSetting_returnsNone_whenValueIsInvalid =INVALID_VALUE ✅ Returns NONE (safe fallback)
testSystemSetting_isCaseInsensitive =read (lowercase) ✅ Returns READ
testSystemSetting_readAndWrite_areNotNone_warningBranchReached READ and WRITE values ✅ Both confirmed ≠ NONE (warning branch reachable)

Note: One compile error was caught during test execution — SamlName.propName() does not exist; corrected to SamlName.getPropertyName() in both production code and test (commit 0a3b0ec).

@mbiuki mbiuki moved this to In Review in dotCMS - Product Planning Mar 16, 2026
@mbiuki mbiuki added OKR : Security & Privacy Owned by Mehdi Team : Security Issues related to security and privacy dotCMS : Security labels Mar 16, 2026
@rsh1k rsh1k self-assigned this Apr 1, 2026
jsanca
jsanca reviewed Apr 23, 2026
View reviewed changes
Comment thread dotCMS/src/main/java/com/dotcms/auth/providers/saml/v1/DotSamlResource.java

final String uri = httpServletRequest.getScheme() + "://" + httpServletRequest.getServerName() + ":"
+ httpServletRequest.getServerPort() + "/dotAdmin/show-logout";
final String configuredHost = Config.getStringProperty(

@jsanca jsanca Apr 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be probably taken from the SAML Config per site, in addition o a global fallback

@github-actions

github-actions Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Security Review

No high-confidence security findings on the changes in this PR.

1 similar comment
@github-actions

github-actions Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Security Review

No high-confidence security findings on the changes in this PR.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wezell wezell Awaiting requested review from wezell
@dsilvam dsilvam Awaiting requested review from dsilvam
@dsolistorres dsolistorres Awaiting requested review from dsolistorres
@nollymar nollymar Awaiting requested review from nollymar
@dcolina dcolina Awaiting requested review from dcolina
@nathan-hildebrandt nathan-hildebrandt Awaiting requested review from nathan-hildebrandt
@spbolton spbolton Awaiting requested review from spbolton
1 more reviewer
@jsanca jsanca jsanca left review comments
Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

@rsh1k rsh1k

Labels

Area : Backend PR changes Java/Maven backend code dotCMS : Security OKR : Security & Privacy Owned by Mehdi Team : Security Issues related to security and privacy

Projects

dotCMS - Product Planning
Status: In Review

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

security: CONTENT_APIS_ALLOW_ANONYMOUS defaults to READ, exposing content API to unauthenticated access

3 participants

@mbiuki @jsanca @rsh1k