Skip to content

fix(content): enforce anonymous access restrictions for content references endpoints#35509

Open
erickgonzalez wants to merge 1 commit intomainfrom
issue-30584-fix-permissions-references
Open

fix(content): enforce anonymous access restrictions for content references endpoints#35509
erickgonzalez wants to merge 1 commit intomainfrom
issue-30584-fix-permissions-references

Conversation

@erickgonzalez
Copy link
Copy Markdown
Member

  • Updated getContentletReferences and getAllContentletReferencesCount to require AnonymousAccess.NONE, ensuring unauthenticated callers are blocked.
  • Added tests to verify that unauthorized (anonymous) requests throw SecurityException as expected, mapping to HTTP 401 responses.
  • Enhanced OpenAPI documentation, adding 403 Forbidden responses to describe cases where users lack contentlet read permissions.
  • Refactored createAnonymousRequest utility for better test coverage.

ref: #30584

…ences endpoints

- Updated `getContentletReferences` and `getAllContentletReferencesCount` to require `AnonymousAccess.NONE`, ensuring unauthenticated callers are blocked.
- Added tests to verify that unauthorized (anonymous) requests throw `SecurityException` as expected, mapping to HTTP 401 responses.
- Enhanced OpenAPI documentation, adding `403 Forbidden` responses to describe cases where users lack contentlet read permissions.
- Refactored `createAnonymousRequest` utility for better test coverage.

ref: #30584
Signed-off-by: erickgonzalez <[email protected]>
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 29, 2026

Claude finished @erickgonzalez's task in 2m 10s —— View job


PR Review

  • Gather context
  • Read changed files
  • Post review

The security change itself is correct and straightforward. A few things worth a second look:

Tests create content that is never reached

ContentResourceIntegrationTest.java:996-999 and 1018-1021 both create a Structure + Contentlet whose identifier is then passed to the endpoint. But WebResource.checkAnonymousPermissions fires at InitBuilder.init() — before any content lookup. The SecurityException would be thrown even with a random UUID like "00000000-0000-0000-0000-000000000000". This wastes database setup time in an integration test that is purely about auth.

// current — unnecessary DB work
final Structure structure = new StructureDataGen().nextPersisted();
final Contentlet content = new ContentletDataGen(structure.getInode())
        .languageId(english.getId()).nextPersisted();
new ContentResource().getContentletReferences(
        createAnonymousRequest(), mock(HttpServletResponse.class),
        content.getIdentifier(), "");

// simpler
new ContentResource().getContentletReferences(
        createAnonymousRequest(), mock(HttpServletResponse.class),
        "non-existent-id", "");

Fix this →


getAllContentletReferencesCount has no 403 annotation — is that intentional?

getContentletReferences adds a 403 response (correct: it calls resolveContentletOrFallBack + getContentletReferences(..., user, mode.respectAnonPerms) which can throw DotSecurityException). The count endpoint at ContentResource.java:460-472 does not, and it calls contentletAPI.getAllContentletReferencesCount(identifier) with no explicit permission gate. This means an authenticated user with no read permission on a contentlet can still learn how many references it has. Pre-existing issue, not introduced here, but the PR is touching this endpoint — worth confirming whether a permission check was intentionally omitted.


AnonymousAccess.NONE vs system config

WebResource.java:368 reads:

if (builder.anonAccess == AnonymousAccess.NONE || AnonymousAccess.systemSetting() == AnonymousAccess.NONE)

So when the endpoint specifies NONE, anonymous users are blocked regardless of the CONTENT_APIS_ALLOW_ANONYMOUS system config. This is a stronger, explicit guarantee — good defensive practice. Just noting it in case someone expects system config to override.


openapi.yaml — only one endpoint updated

The YAML gets a 403 for /{inodeOrIdentifier}/references but not for /{identifier}/references/count. This is consistent with the Java annotations and the actual permission behavior, so it's fine as-is.


Overall: the core security fix (READNONE) is correct and the behavior is verified by WebResource.checkAnonymousPermissions. The main actionable item is trimming the unnecessary content setup in the two new tests.

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

Labels

AI: Safe To Rollback Area : Backend PR changes Java/Maven backend code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Add Page References Dialog for Contentlet Usage in General Tab

2 participants