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

dotCMS/src/main/java/com/dotcms/rest/BundleResource.java

@@ -28,6 +28,7 @@
 import org.apache.commons.io.IOUtils;
 import com.dotcms.rest.annotation.NoCache;
 import com.dotcms.rest.exception.BadRequestException;
+import com.dotcms.rest.exception.ForbiddenException;
 import com.dotcms.rest.exception.NotFoundException;
 import com.dotcms.rest.exception.mapper.ExceptionMapperUtil;
 import com.dotcms.rest.param.ISODateParam;
@@ -197,6 +198,17 @@ public Response getUnsendBundles (@Context HttpServletRequest request, @Context
 
         //Reading the parameters
         String userId = initData.getParamsMap().get( "userid" );
+
+        // Authorization: a caller may only list their own unsent bundles. The path userId is
+        // client-supplied, so without this check any backend user could enumerate another user's
+        // draft bundles by editing the URL. CMS Administrators may query any user's bundles.
+        final User currentUser = initData.getUser();
+        if ( UtilMethods.isSet( userId ) && !userId.equals( currentUser.getUserId() ) && !currentUser.isAdmin() ) {
+            throw new ForbiddenException( String.format(
+                    "User '%s' is not allowed to list bundles owned by user '%s'",
+                    currentUser.getUserId(), userId ) );
+        }
+
         String bundleName = request.getParameter( "name" );
         String startParam = request.getParameter( "start" );
         String countParam = request.getParameter( "count" );

dotCMS/src/main/java/com/dotcms/rest/api/v1/publishing/PublishingRetryHelper.java

@@ -27,6 +27,7 @@
 import com.dotcms.publishing.manifest.ManifestReaderFactory;
 import com.dotcms.publishing.manifest.ManifestReason;
 import com.dotmarketing.business.APILocator;
+import com.dotmarketing.business.PermissionAPI;
 import com.dotmarketing.exception.DotDataException;
 import com.dotmarketing.util.ConfigUtils;
 import com.dotmarketing.util.Logger;
@@ -62,6 +63,7 @@ public class PublishingRetryHelper {
     private final BundleAPI bundleAPI;
     private final EnvironmentAPI environmentAPI;
     private final PublishingEndPointAPI publishingEndPointAPI;
+    private final PermissionAPI permissionAPI;
 
     /**
      * Default constructor using APILocator for dependencies.
@@ -71,7 +73,8 @@ public PublishingRetryHelper() {
              PublishAuditAPI.getInstance(),
              APILocator.getBundleAPI(),
              APILocator.getEnvironmentAPI(),
-             APILocator.getPublisherEndPointAPI());
+             APILocator.getPublisherEndPointAPI(),
+             APILocator.getPermissionAPI());
     }
 
     /**
@@ -82,12 +85,14 @@ public PublishingRetryHelper(final PublisherAPI publisherAPI,
                                   final PublishAuditAPI publishAuditAPI,
                                   final BundleAPI bundleAPI,
                                   final EnvironmentAPI environmentAPI,
-                                  final PublishingEndPointAPI publishingEndPointAPI) {
+                                  final PublishingEndPointAPI publishingEndPointAPI,
+                                  final PermissionAPI permissionAPI) {
         this.publisherAPI = publisherAPI;
         this.publishAuditAPI = publishAuditAPI;
         this.bundleAPI = bundleAPI;
         this.environmentAPI = environmentAPI;
         this.publishingEndPointAPI = publishingEndPointAPI;
+        this.permissionAPI = permissionAPI;
     }
 
     /**
@@ -132,6 +137,11 @@ public RetryResultDTO retryBundle(
                             status.getStatus().name()));
         }
 
+        // Authorization: retry re-sends the bundle to every environment it targets, so the caller
+        // must hold USE permission on each of them (push enforces the same check via
+        // PublishingJobsHelper#validateEnvironmentPermissions). Admins pass automatically.
+        validateRetryEnvironmentPermissions(trimmedBundleId, user);
+
         // Check if bundle is already in queue
         final List<PublishQueueElement> foundBundles =
                 publisherAPI.getQueueElementsByBundleId(trimmedBundleId);
@@ -160,6 +170,32 @@ public RetryResultDTO retryBundle(
         }
     }
 
+    /**
+     * Verifies the caller is allowed to re-send the given bundle.
+     *
+     * <p>Unlike {@code push} (where the caller selects a permitted subset of environments), retry
+     * re-fires the bundle to <b>all</b> of its already-configured environments at once. The caller
+     * must therefore hold {@link PermissionAPI#PERMISSION_USE} on every one of them; lacking it on
+     * any single environment rejects the whole retry. CMS Administrators pass automatically via the
+     * {@link PermissionAPI}.</p>
+     *
+     * @param bundleId the bundle being retried
+     * @param user     the user requesting the retry
+     * @throws DotPublisherException if the user lacks USE permission on any targeted environment
+     * @throws DotDataException      if a data access error occurs while resolving permissions
+     */
+    private void validateRetryEnvironmentPermissions(final String bundleId, final User user)
+            throws DotDataException, DotPublisherException {
+        final List<Environment> environments = environmentAPI.findEnvironmentsByBundleId(bundleId);
+        for (final Environment environment : environments) {
+            if (!permissionAPI.doesUserHavePermission(environment, PermissionAPI.PERMISSION_USE, user)) {
+                throw new DotPublisherException(String.format(
+                        "User '%s' cannot push bundle '%s' because it does not have permission to use environment '%s'",
+                        user.getUserId(), bundleId, environment.getName()));
+            }
+        }
+    }
+
     /**
      * Retry a static publishing bundle (AWS S3 or static file system).
      */

dotcms-integration/src/test/java/com/dotcms/rest/BundleResourceTest.java

@@ -13,8 +13,10 @@
 import com.dotcms.publisher.business.PublishAuditHistory;
 import com.dotcms.publisher.business.PublishAuditStatus;
 import com.dotcms.publisher.pusher.PushPublisherConfig;
+import com.dotcms.datagen.UserDataGen;
 import com.dotcms.publishing.FilterDescriptor;
 import com.dotcms.publishing.PublisherAPIImpl;
+import com.dotcms.rest.exception.ForbiddenException;
 import com.dotcms.util.IntegrationTestInitService;
 import com.dotmarketing.business.APILocator;
 import com.dotmarketing.business.CacheLocator;
@@ -31,6 +33,7 @@
 import org.glassfish.jersey.media.multipart.BodyPart;
 import org.glassfish.jersey.media.multipart.ContentDisposition;
 import org.glassfish.jersey.media.multipart.FormDataMultiPart;
+import org.junit.AfterClass;
 import org.junit.Assert;
 import org.junit.BeforeClass;
 import org.junit.Test;
@@ -58,6 +61,7 @@ public class BundleResourceTest {
     private static BundleResource bundleResource;
     private static User adminUser;
     static HttpServletResponse response;
+    private static final List<User> createdUsers = new ArrayList<>();
 
     @BeforeClass
     public static void prepare() throws Exception {
@@ -72,6 +76,14 @@ public static void prepare() throws Exception {
         response = new MockHttpResponse();
     }
 
+    @AfterClass
+    public static void cleanup() {
+        for (final User user : createdUsers) {
+            UserDataGen.remove(user, Boolean.TRUE);
+        }
+        createdUsers.clear();
+    }
+
     /**
      * Method to Test: {@link BundleResource#uploadBundleSync(HttpServletRequest, HttpServletResponse, FormDataMultiPart)}
      *                  and {@link com.dotcms.enterprise.publishing.remote.handler.ContentHandler#handle(File, Boolean)}
@@ -276,4 +288,68 @@ private void insertPublishAuditStatus(final PublishAuditStatus.Status status, fi
         publishAuditStatus.setStatus(status);
         APILocator.getPublishAuditAPI().insertPublishAuditStatus(publishAuditStatus);
     }
+
+    private static HttpServletRequest mockRequestFor(final User user) {
+        final HttpServletRequest request = mock(HttpServletRequest.class);
+        when(request.getAttribute(WebKeys.USER)).thenReturn(user);
+        return request;
+    }
+
+    private static User newBackendUser() throws Exception {
+        final User user = new UserDataGen()
+                .roles(APILocator.getRoleAPI().loadBackEndUserRole())
+                .nextPersisted();
+        createdUsers.add(user);
+        return user;
+    }
+
+    /**
+     * Reproduces the horizontal information-disclosure gap in issue #36415.
+     *
+     * Method to Test: {@link BundleResource#getUnsendBundles(HttpServletRequest, HttpServletResponse, String)}
+     * Given: a non-admin backend user
+     * When:  they request the unsent bundles of ANOTHER user via the path userId
+     * Should: be rejected with 403 Forbidden (a user may only list their own drafts).
+     *         Before the fix this returned 200 with the other user's bundles.
+     */
+    @Test(expected = ForbiddenException.class)
+    public void test_getUnsendBundles_otherUsersId_isForbidden() throws Exception {
+        final User userA = newBackendUser();
+        final User userB = newBackendUser();
+
+        bundleResource.getUnsendBundles(
+                mockRequestFor(userA), response, "userid/" + userB.getUserId());
+    }
+
+    /**
+     * Method to Test: {@link BundleResource#getUnsendBundles(HttpServletRequest, HttpServletResponse, String)}
+     * Given: a non-admin backend user
+     * When:  they request their OWN unsent bundles
+     * Should: succeed (200) - the shipped Add-to-Bundle flow always queries the caller's own id.
+     */
+    @Test
+    public void test_getUnsendBundles_ownId_isAllowed() throws Exception {
+        final User userA = newBackendUser();
+
+        final Response resp = bundleResource.getUnsendBundles(
+                mockRequestFor(userA), response, "userid/" + userA.getUserId());
+
+        assertEquals(200, resp.getStatus());
+    }
+
+    /**
+     * Method to Test: {@link BundleResource#getUnsendBundles(HttpServletRequest, HttpServletResponse, String)}
+     * Given: a CMS Administrator
+     * When:  they request another user's unsent bundles
+     * Should: succeed (200) - admins may query any user's drafts.
+     */
+    @Test
+    public void test_getUnsendBundles_adminCanQueryAnyUser() throws Exception {
+        final User userB = newBackendUser();
+
+        final Response resp = bundleResource.getUnsendBundles(
+                mockRequestFor(APILocator.systemUser()), response, "userid/" + userB.getUserId());
+
+        assertEquals(200, resp.getStatus());
+    }
 }

dotcms-integration/src/test/java/com/dotcms/rest/api/v1/publishing/PublishingResourceIntegrationTest.java

@@ -8,6 +8,7 @@
 import com.dotcms.datagen.FolderDataGen;
 import com.dotcms.datagen.SiteDataGen;
 import com.dotcms.datagen.TestUserUtils;
+import com.dotcms.datagen.UserDataGen;
 import com.dotcms.mock.request.MockAttributeRequest;
 import com.dotcms.mock.request.MockHeaderRequest;
 import com.dotcms.mock.request.MockHttpRequestIntegrationTest;
@@ -85,6 +86,7 @@ public class PublishingResourceIntegrationTest {
     private static List<String> createdBundleIds;
     private static List<String> createdEnvironmentIds;
     private static List<String> createdFilterKeys;
+    private static List<User> createdUsers;
     private static PublishingResource publishingResource;
     private static HttpServletResponse response;
 
@@ -101,6 +103,7 @@ public static void prepare() throws Exception {
         createdBundleIds = new ArrayList<>();
         createdEnvironmentIds = new ArrayList<>();
         createdFilterKeys = new ArrayList<>();
+        createdUsers = new ArrayList<>();
         publishingResource = new PublishingResource();
         response = new MockHttpResponse();
     }
@@ -133,6 +136,13 @@ public static void cleanup() {
             }
         }
 
+        // Cleanup users created by tests
+        if (createdUsers != null) {
+            for (final User user : createdUsers) {
+                UserDataGen.remove(user, Boolean.TRUE);
+            }
+        }
+
         // Note: Filters are stored in memory and will be cleared on restart
     }
 
@@ -1192,6 +1202,10 @@ private String createBundleWithFailedEndpoint(final String bundleName)
     }
 
     private HttpServletRequest mockAuthenticatedRequest() {
+        return mockAuthenticatedRequest(adminUser);
+    }
+
+    private HttpServletRequest mockAuthenticatedRequest(final User user) {
         final MockHeaderRequest request = new MockHeaderRequest(
                 new MockSessionRequest(
                         new MockAttributeRequest(
@@ -1200,7 +1214,7 @@ private HttpServletRequest mockAuthenticatedRequest() {
                                 .request())
                         .request());
 
-        request.setAttribute(WebKeys.USER, adminUser);
+        request.setAttribute(WebKeys.USER, user);
         return request;
     }
 
@@ -1448,6 +1462,61 @@ public void test_retryBundles_singleBundleReturnsResult() throws Exception {
         assertEquals("DeliveryStrategy should match", DeliveryStrategy.ALL_ENDPOINTS, bundleResult.deliveryStrategy());
     }
 
+    /**
+     * Reproduces the authorization gap in issue #36414.
+     *
+     * <p>Retry re-sends a bundle to every environment it targets. Before the fix, the endpoint
+     * only required a backend user, so a user with no USE permission on the bundle's environment
+     * could still re-fire the push. Push already guards this via
+     * {@code PublishingJobsHelper#validateEnvironmentPermissions}; retry did not.</p>
+     *
+     * Given: a retryable bundle linked to an environment the caller cannot USE
+     * When:  a non-admin backend user (no USE permission) retries it
+     * Then:  the bundle result is a failure that names the permission denial (no push occurs)
+     */
+    @Test
+    public void test_retryBundles_userWithoutEnvironmentUsePermission_isRejected() throws Exception {
+        // Environment with no USE permission granted to our limited user
+        final Environment environment = createEnvironmentWithPermission("retry-authz-env");
+
+        // A retryable bundle linked to that environment
+        final Bundle bundle = PublisherTestUtil.createBundle(
+                "retry-authz-bundle_" + System.currentTimeMillis(), adminUser, environment);
+        final String bundleId = bundle.getId();
+        createdBundleIds.add(bundleId);
+
+        final PublishAuditStatus auditStatus = new PublishAuditStatus(bundleId);
+        auditStatus.setStatus(Status.FAILED_TO_PUBLISH);
+        auditStatus.setStatusUpdated(new Date());
+        final PublishAuditHistory history = new PublishAuditHistory();
+        auditStatus.setStatusPojo(history);
+        publishAuditAPI.insertPublishAuditStatus(auditStatus);
+        publishAuditAPI.updatePublishAuditStatus(bundleId, Status.FAILED_TO_PUBLISH, history);
+
+        // A non-admin backend user with no permission on the environment
+        final User limitedUser = new UserDataGen()
+                .roles(APILocator.getRoleAPI().loadBackEndUserRole())
+                .nextPersisted();
+        createdUsers.add(limitedUser);
+
+        final RetryBundlesForm form = RetryBundlesForm.builder()
+                .bundleIds(List.of(bundleId))
+                .forcePush(false)
+                .deliveryStrategy(DeliveryStrategy.ALL_ENDPOINTS)
+                .build();
+
+        final ResponseEntityRetryBundlesView result = publishingResource.retryBundles(
+                mockAuthenticatedRequest(limitedUser), response, form);
+
+        assertEquals("Should have one result", 1, result.getEntity().size());
+        final RetryBundleResultView bundleResult = result.getEntity().get(0);
+        assertFalse("Retry must be rejected for a user lacking USE permission on the environment",
+                bundleResult.success());
+        assertNotNull("Failure message should not be null", bundleResult.message());
+        assertTrue("Failure message should cite the permission denial, was: " + bundleResult.message(),
+                bundleResult.message().toLowerCase().contains("does not have permission"));
+    }
+
     /**
      * Given: Multiple bundles with different statuses exist
      * When: Retry request with multiple bundleIds