Skip to content

fix(publishing): enforce permission on retry and restrict getunsendbundles to owner (#36414, #36415) - #36446

Merged
hassandotcms merged 2 commits into
mainfrom
issue-36414-publishing-retry-and-bundle-endpoint-authorization
Jul 8, 2026
Merged

fix(publishing): enforce permission on retry and restrict getunsendbundles to owner (#36414, #36415)#36446
hassandotcms merged 2 commits into
mainfrom
issue-36414-publishing-retry-and-bundle-endpoint-authorization

Conversation

@hassandotcms

@hassandotcms hassandotcms commented Jul 7, 2026

Copy link
Copy Markdown
Member

Hardens two Push Publishing endpoints surfaced during the Publishing Queue Angular migration.

#36414 : retry authorization. POST /api/v1/publishing/retry only required a backend user, so a user with no permission on a bundle's environments could re-fire the push. Retry now verifies PERMISSION_USE on every environment the bundle targets (the same check push already does); admins bypass.

#36415 : bundle draft GET /api/bundle/getunsendbundles/userid/{userId} took the userId from the URL with no owner check, letting any backend user list another user's draft bundles; and passing an admin's id returned all bundles. Now restricted to the caller's own id; admins may query anyone.

Compatibility: No drift for existing clients; all shipped callers (Angular Add-to-Bundle, dot-rules, legacy Dojo dialog, legacy queue JSP) use the current user's own id; legacy retry (RemotePublishAjaxAction) is a separate, untouched endpoint. No GraphQL/schema/DB changes. Rollback-safe.

Tests: Integration coverage for reject + allowed paths in PublishingResourceIntegrationTest and BundleResourceTest

Closes #36414
Closes #36415

This PR fixes: #36414

…rict getunsendbundles to owner (#36414, #36415)

#36414: PublishingRetryHelper now verifies the caller holds PERMISSION_USE on every
environment the bundle targets before re-queuing, mirroring the check push already
performs via PublishingJobsHelper. Retry previously required only a backend user, so a
user with no permission on a bundle's environments could re-fire the push. Admins bypass.

#36415: BundleResource.getUnsendBundles now rejects (403) a caller requesting another
user's draft bundles via the path userId; admins may query any user. The path userId was
client-supplied with no owner check, letting any backend user enumerate others' drafts
(and, when passing an admin id, all bundles via the factory's admin branch).

Adds integration coverage in PublishingResourceIntegrationTest and BundleResourceTest for
the rejection and allowed paths, with user cleanup in @afterclass.
@claude

claude Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Claude finished @hassandotcms's task in 3m 31s —— View job


Code Review — PR #36446

  • Gather context (diff vs parent 005e4078, both source files + tests)
  • Verify reviewer feedback (jcastro-dotcms message wording)
  • Review authorization logic, edge cases, and test coverage

Reviewer feedback status

jcastro-dotcms's requested message change is already applied (commit 18ccb6ee). PublishingRetryHelper.validateRetryEnvironmentPermissions now throws:

User '%s' cannot push bundle '%s' because it does not have permission to use environment '%s'

This matches the requested wording exactly.

New Issues

No blocking issues found. The changes are sound:

  • Retry authz (PublishingRetryHelper.java:184-197): checks PERMISSION_USE on every environment via permissionAPI.doesUserHavePermission, rejecting if the user lacks it on any one. Admins bypass through PermissionAPI. The reject-on-any semantics (vs. push's filter-to-subset) is deliberate and documented — for retry the caller doesn't get to pick a subset, so requiring all is the correct, stricter choice.
  • Bundle owner check (BundleResource.java:203-208): correctly rejects with ForbiddenException (→ 403) when a non-admin requests another user's id. The UtilMethods.isSet(userId) guard is safe — a null/empty userid falls through to BundleFactoryImpl.findUnsendBundles, which returns an empty list for an unset user, so no disclosure.
  • Exceptions propagate correctly: DotPublisherException from the retry check is caught per-bundle in the retryBundles loop and surfaced as a per-bundle failure result (confirmed by the new test asserting success()==false + message).

Minor observations (non-blocking)

  • 🟡 Medium: PublishingRetryHelper.java:186 — the denial message says "cannot push bundle" on a retry path. It's exactly what the reviewer asked for, so leave it, but "push" reads slightly oddly for a retry action. Purely cosmetic.
  • 🟡 Medium: PublishingRetryHelper.java:184 — if findEnvironmentsByBundleId returns an empty list, the loop performs no check and retry proceeds. This isn't a security gap (a bundle with zero environments pushes nowhere), but worth being aware of. No action needed.

Test coverage

Good — both the reject and allowed paths are covered for each endpoint:

  • BundleResourceTest: other-user (403), own-id (200), admin-any-user (200), with proper @AfterClass user cleanup.
  • PublishingResourceIntegrationTest: test_retryBundles_userWithoutEnvironmentUsePermission_isRejected asserts failure + message content, plus the new mockAuthenticatedRequest(User) overload for non-admin scenarios.

Verdict: LGTM. The single review comment is already resolved; no blocking issues remain.

issue-36414-publishing-retry-and-bundle-endpoint-authorization

@hassandotcms hassandotcms changed the title fix(publishing): enforce environment USE permission on retry and restrict getunsendbundles to owner (#36414, #36415) fix(publishing): enforce permission on retry and restrict getunsendbundles to owner (#36414, #36415) Jul 7, 2026
Comment thread dotCMS/src/main/java/com/dotcms/rest/api/v1/publishing/PublishingRetryHelper.java Outdated
@hassandotcms
hassandotcms marked this pull request as ready for review July 8, 2026 12:23
Address PR review: name the USE permission explicitly in the retry authorization
failure message. No logic change.
@mergify

mergify Bot commented Jul 8, 2026

Copy link
Copy Markdown

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

  • Queue this pull request

@hassandotcms
hassandotcms added this pull request to the merge queue Jul 8, 2026
@hassandotcms hassandotcms moved this from QA to In Progress in dotCMS - Product Planning Jul 8, 2026
Merged via the queue into main with commit bce3f77 Jul 8, 2026
60 checks passed
@hassandotcms
hassandotcms deleted the issue-36414-publishing-retry-and-bundle-endpoint-authorization branch July 8, 2026 14:51
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: In Progress

2 participants