Skip to content
This repository has been archived by the owner on May 31, 2024. It is now read-only.

Users setting comments' postId, but not modifying it. #1231

Closed
wichersn opened this issue Jan 8, 2016 · 2 comments
Closed

Users setting comments' postId, but not modifying it. #1231

wichersn opened this issue Jan 8, 2016 · 2 comments

Comments

@wichersn
Copy link

wichersn commented Jan 8, 2016

The postId field of comments has the todo should users be able to set postId, but not modify it? here.

I think this should be fixed, since it allows to move their comment, along with all sub comments to any other post. So it can look like a user commented on a post that they actually didn't comment on.

I'm not sure what the best way to fix this is. If the editableBy: ["member"] permision is taken off, then the check will fail here when the user tries to comment to a post. And that check shouldn't be removed of course since it's necessary for other security purposes.

I would be happy to submit a pull request for this if I have some guidance on what the best way to accomplish this is.

@wichersn
Copy link
Author

wichersn commented Jan 8, 2016

One solution could be to create another field in the comment schema:

// Same as the original postId field  
setPostId: {
    type: String,
    optional: true,
    // regEx: SimpleSchema.RegEx.Id,
    max: 500,
    editableBy: ["member", "admin"],
    autoform: {
      omit: true // never show this
    }
  },

and change the original postId field to

postId: {
    type: String,
    optional: true,
    // regEx: SimpleSchema.RegEx.Id,
    max: 500,
    editableBy: [ "admin"], // Without the member permissions
    autoform: {
      omit: true // never show this
    }
  },

The user would set the setPostId field. In the submitPost method, the server will set the postId to the setPostId field. Then later, the user can change the setPostId but this won't affect the actual postId field.

This will work, but I don't think it's a very good solution, and it's confusing to have 2 fields for the postId.

@SachaG
Copy link
Contributor

SachaG commented Feb 6, 2016

Thanks for pointing this out. I think I found a simpler solution, just treat postId as an edge case.

@SachaG SachaG closed this as completed Feb 6, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants