Modify EditComments extension to let anyone use it conditionally and support inline editing

RESOLVED FIXED

Status

()

RESOLVED FIXED
7 months ago
3 months ago

People

(Reporter: kohei, Assigned: kohei)

Tracking

Production
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

In Bug 1484477 Comment 6, :dylan mentioned that the extension allows you to edit anyone’s comment. Currently It’s fine because only admins have the privilege, but it’s gonna be a problem if we allow all editbugs members to edit comments. 

Regular users should only be allowed to edit their own posted comments, while admins should be able to edit anyone’s comment with no configuration.
Assignee: nobody → kohei.yoshino
Status: NEW → ASSIGNED
Are there any cases admins have deleted sensitive information by editing comments? The editcomments.html page has a group membership validation probably for that purpose, so currently only admins can view history:

https://github.com/mozilla-bteam/bmo/blob/master/extensions/EditComments/Extension.pm#L73-L79

For transparency, comment edit history should be visible to anyone just like Facebook or GitHub. Probably admins should be able to delete/hide a specific revision?

See also: https://help.github.com/articles/tracking-changes-in-a-comment/
Flags: needinfo?(dylan)
(In reply to Kohei Yoshino [:kohei] from comment #2)
> WIP: https://github.com/kyoshino/bmo/compare/bug-1484892-editcomments-priv

Updated to allow admins to hide specific revisions if needed. All existing revisions will also be hidden by default, in case any of these contains sensitive info.
(In reply to Kohei Yoshino [:kohei] from comment #1)
> Are there any cases admins have deleted sensitive information by editing
> comments? The editcomments.html page has a group membership validation
> probably for that purpose, so currently only admins can view history:

Yes we have done that many times. Passwords, API keys, etc. are removed from comments. We would want the certain history items to be invisible for that reason.

dkl
Okay, then per-revision visibility control is certainly needed. Here are screenshots of how it will look like:

Admin: https://screenshots.firefox.com/K2QZYsVlfwvc5haS/bmo-web.vm
Non-admin: https://screenshots.firefox.com/kPFKrKKs78X7ek6F/bmo-web.vm
Flags: needinfo?(dylan)
(In reply to Kohei Yoshino [:kohei] from comment #5)
> Okay, then per-revision visibility control is certainly needed. Here are
> screenshots of how it will look like:
> 
> Admin: https://screenshots.firefox.com/K2QZYsVlfwvc5haS/bmo-web.vm
> Non-admin: https://screenshots.firefox.com/kPFKrKKs78X7ek6F/bmo-web.vm

Looks good to me. I would just implement it the same way we do private comments and only allow those in the insider comments group to see the private revision. What we could do is, when editing a current comment, if the user is an insider, display another checkbox above the textarea called "Hidden" or "Hide Revision" or something similar. 

dkl
The users in the insiders group are already privileged enough to see the hidden comment changes so there is no need to just limit to admins only.
My current implementation is adding a new group:

* (existing) edit_comments_group; default: admin
* (new) edit_own_comments_group bit; default: editbugs

But yeah, $user->is_insider should be able to edit any comments.

* (existing) insidergroup; default: admins and others?
* (existing) edit_comments_group; default: editbugs

Actually my initial implementation was like that. Will update again and send a PR.
Added "Hide This Revision" checkbox above the textarea: https://screenshots.firefox.com/E57oyLFaDxQNisS9/bmo-web.vm
Summary: Allow non-admin users to only edit their own comments → Modify EditComments extension to allow anyone to use it conditionally
Summary: Modify EditComments extension to allow anyone to use it conditionally → Modify EditComments extension to let anyone use it conditionally and support inline editing
Kate, do you see any issues here with how we're allowing edits on bugs?
Flags: needinfo?(kmancuso)
Comment hidden (obsolete)
Ugh, sorry, this was the implementation bug. Redirecting folks to admin Bug 1484477...
Flags: needinfo?(kmancuso)
Flags: needinfo?(cneiman)
Comment hidden (obsolete)
Status: ASSIGNED → RESOLVED
Last Resolved: 4 months ago
Resolution: --- → FIXED
Only the schema is going out first, we'll resolve this when the main code lands.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Merged to master, again \o/
Status: REOPENED → RESOLVED
Last Resolved: 4 months ago4 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.