fix: inconsistent publish permissions (#20668)

* fix: inconsistent publish permissions

* fix: tests

* fix: api tests

* fix: is create should be is nil

* fix: publish and update by using locale

* fix: remove some conditions to the Document actions

---------

Co-authored-by: Simone Taeggi <startae14@gmail.com>

packages/core/content-manager/admin/src/pages/EditView/components/DocumentActions.tsx

@@ -666,8 +666,6 @@ const PublishAction: DocumentActionComponent = ({
      *  - the document is already published & not modified
      *  - the document is being created & not modified
      *  - the user doesn't have the permission to publish
-     *  - the user doesn't have the permission to create a new document
-     *  - the user doesn't have the permission to update the document
      */
     disabled:
       isCloning ||
@@ -676,8 +674,7 @@ const PublishAction: DocumentActionComponent = ({
       activeTab === 'published' ||
       (!modified && isDocumentPublished) ||
       (!modified && !document?.documentId) ||
-      !canPublish ||
+      !canPublish,
-      Boolean((!document?.documentId && !canCreate) || (document?.documentId && !canUpdate)),
     label: formatMessage({
       id: 'app.utils.publish',
       defaultMessage: 'Publish',
@@ -754,14 +751,8 @@ const UpdateAction: DocumentActionComponent = ({
      * - the form is submitting
      * - the document is not modified & we're not cloning (you can save a clone entity straight away)
      * - the active tab is the published tab
-     * - the user doesn't have the permission to create a new document
-     * - the user doesn't have the permission to update the document
      */
-    disabled:
+    disabled: isSubmitting || (!modified && !isCloning) || activeTab === 'published',
-      isSubmitting ||
-      (!modified && !isCloning) ||
-      activeTab === 'published' ||
-      Boolean((!documentId && !canCreate) || (documentId && !canUpdate)),
     label: formatMessage({
       id: 'content-manager.containers.Edit.save',
       defaultMessage: 'Save',

packages/core/content-manager/server/src/controllers/collection-types.ts

@@ -1,5 +1,6 @@
-import { setCreatorFields, async, errors } from '@strapi/utils';
+import { isNil } from 'lodash/fp';
 
+import { setCreatorFields, async, errors } from '@strapi/utils';
 import type { Modules, UID } from '@strapi/types';
 
 import { getService } from '../utils';
@@ -390,17 +391,49 @@ export default {
         .countRelations()
         .build();
 
-      const document = id
+      let document: any;
-        ? await updateDocument(ctx, { populate })
+
-        : await createDocument(ctx, { populate });
+      const { locale } = await getDocumentLocaleAndStatus(body, model);
+
+      /**
+       * Publish can be called on two scenarios:
+       * 1. Create a new document and publish it in one request
+       * 2. Update an existing document and publish it in one request
+       *
+       * Based on user permissions:
+       * 1. User cannot create a document, but can publish
+       *    Action will be forbidden as user cannot create a document
+       * 2. User can update and publish a document
+       *    Action will be allowed, but document will not be updated, only published with the latest draft
+       */
+      const isCreate = isNil(id);
+      if (isCreate) {
+        if (permissionChecker.cannot.create()) {
+          throw new errors.ForbiddenError();
+        }
+
+        document = await createDocument(ctx, { populate });
+      }
+
+      const isUpdate = !isCreate;
+      if (isUpdate) {
+        document = await documentManager.findOne(id!, model, { populate, locale });
+
+        if (!document) {
+          throw new errors.NotFoundError('Document not found');
+        }
+
+        // Only Update if user has update permissions
+        if (permissionChecker.can.update(document)) {
+          await updateDocument(ctx);
+        }
+      }
 
       if (permissionChecker.cannot.publish(document)) {
         throw new errors.ForbiddenError();
       }
 
-      const { locale } = await getDocumentLocaleAndStatus(body, model);
+      const publishResult = await documentManager.publish(document.documentId, model, {
-
-      const publishResult = await documentManager.publish(document!.documentId, model, {
         locale,
         // TODO: Allow setting creator fields on publish
         // data: setCreatorFields({ user, isEdition: true })({}),

tests/api/core/admin/admin-permissions-custom-conditions.test.api.js

@@ -267,6 +267,7 @@ describe('Admin Permissions - Conditions', () => {
     const res = await rq({
       method: 'POST',
       url: `/content-manager/collection-types/api::article.article/${documentId}/actions/publish`,
+      body: { ...localTestData.cheapArticle, category: localTestData.categories[0].documentId },
     });
 
     expect(res.statusCode).toBe(200);