imprv: Permissions to operate comment (#4466)

* check author when updating and removing * restrict to show controls when the user is not the author
weseek · Oct 8, 2021 · 863bfd7 · 863bfd7
1 parent 83dcf12
commit 863bfd7
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 35 deletions.
diff --git a/packages/app/src/client/services/CommentContainer.js b/packages/app/src/client/services/CommentContainer.js
@@ -132,11 +132,9 @@ export default class CommentContainer extends Container {
     return this.appContainer.apiPost('/comments.update', {
       commentForm: {
         comment,
-        page_id: pageId,
-        revision_id: revisionId,
         is_markdown: isMarkdown,
+        revision_id: revisionId,
         comment_id: commentId,
-        author,
       },
     })
       .then((res) => {

diff --git a/packages/app/src/components/PageComment/Comment.jsx b/packages/app/src/components/PageComment/Comment.jsx
@@ -73,10 +73,6 @@ class Comment extends React.PureComponent {
     interceptorManager.process('postRenderCommentHtml', this.currentRenderingContext);
   }
 
-  checkPermissionToControlComment() {
-    return this.props.appContainer.isAdmin || this.isCurrentUserEqualsToAuthor();
-  }
-
   isCurrentUserEqualsToAuthor() {
     const { creator } = this.props.comment;
     if (creator == null) {
@@ -210,7 +206,7 @@ class Comment extends React.PureComponent {
                   </UncontrolledTooltip>
                 </span>
               </div>
-              {this.checkPermissionToControlComment() && (
+              {this.isCurrentUserEqualsToAuthor() && (
                 <CommentControl
                   onClickDeleteBtn={this.deleteBtnClickedHandler}
                   onClickEditBtn={() => this.setState({ isReEdit: true })}

diff --git a/packages/app/src/server/models/comment.js b/packages/app/src/server/models/comment.js
@@ -65,16 +65,6 @@ module.exports = function(crowi) {
     }));
   };
 
-  commentSchema.statics.updateCommentsByPageId = function(comment, isMarkdown, commentId) {
-    const Comment = this;
-
-    return Comment.findOneAndUpdate(
-      { _id: commentId },
-      { $set: { comment, isMarkdown } },
-    );
-
-  };
-
   commentSchema.statics.removeCommentsByPageId = function(pageId) {
     const Comment = this;
 

diff --git a/packages/app/src/server/routes/comment.js b/packages/app/src/server/routes/comment.js
@@ -310,10 +310,10 @@ module.exports = function(crowi, app) {
    *                            $ref: '#/components/schemas/Page/properties/_id'
    *                          revision_id:
    *                            $ref: '#/components/schemas/Revision/properties/_id'
+   *                          comment_id:
+   *                            $ref: '#/components/schemas/Comment/properties/_id'
    *                          comment:
    *                            $ref: '#/components/schemas/Comment/properties/comment'
-   *                          comment_position:
-   *                            $ref: '#/components/schemas/Comment/properties/commentPosition'
    *                required:
    *                  - form
    *        responses:
@@ -340,33 +340,41 @@ module.exports = function(crowi, app) {
   api.update = async function(req, res) {
     const { commentForm } = req.body;
 
-    const pageId = commentForm.page_id;
-    const comment = commentForm.comment;
+    const commentStr = commentForm.comment;
     const isMarkdown = commentForm.is_markdown;
     const commentId = commentForm.comment_id;
-    const author = commentForm.author;
+    const revision = commentForm.revision_id;
 
-    if (comment === '') {
+    if (commentStr === '') {
       return res.json(ApiResponse.error('Comment text is required'));
     }
 
     if (commentId == null) {
       return res.json(ApiResponse.error('\'comment_id\' is undefined'));
     }
 
-    if (author !== req.user.username) {
-      return res.json(ApiResponse.error('Only the author can edit'));
-    }
-
-    // check whether accessible
-    const isAccessible = await Page.isAccessiblePageByViewer(pageId, req.user);
-    if (!isAccessible) {
-      return res.json(ApiResponse.error('Current user is not accessible to this page.'));
-    }
-
     let updatedComment;
     try {
-      updatedComment = await Comment.updateCommentsByPageId(comment, isMarkdown, commentId);
+      const comment = await Comment.findById(commentId).exec();
+
+      if (comment == null) {
+        throw new Error('This comment does not exist.');
+      }
+
+      // check whether accessible
+      const pageId = comment.page;
+      const isAccessible = await Page.isAccessiblePageByViewer(pageId, req.user);
+      if (!isAccessible) {
+        throw new Error('Current user is not accessible to this page.');
+      }
+      if (req.user.id !== comment.creator.toString()) {
+        throw new Error('Current user is not operatable to this comment.');
+      }
+
+      updatedComment = await Comment.findOneAndUpdate(
+        { _id: commentId },
+        { $set: { comment: commentStr, isMarkdown, revision } },
+      );
     }
     catch (err) {
       logger.error(err);
@@ -438,6 +446,9 @@ module.exports = function(crowi, app) {
       if (!isAccessible) {
         throw new Error('Current user is not accessible to this page.');
       }
+      if (req.user.id !== comment.creator.toString()) {
+        throw new Error('Current user is not operatable to this comment.');
+      }
 
       await comment.removeWithReplies();
       await Page.updateCommentCount(comment.page);