From 118b7c3b06408112e460dcc47e88c6f627b11ea4 Mon Sep 17 00:00:00 2001
From: Evan Purkhiser <evanpurkhiser@gmail.com>
Date: Thu, 23 Apr 2020 20:31:01 -0700
Subject: [PATCH] fix(alerts): Make clear that superuser may edit comments

---
 .../organization_incident_comment_details.py  |  5 +-
 .../app/components/activity/note/header.tsx   | 53 +++++++++++-------
 ...t_organization_incident_comment_details.py | 55 +++++++++++++++++++
 3 files changed, 91 insertions(+), 22 deletions(-)

diff --git a/src/sentry/api/endpoints/organization_incident_comment_details.py b/src/sentry/api/endpoints/organization_incident_comment_details.py
index 355101cae33277..92d1f8dbbde343 100644
--- a/src/sentry/api/endpoints/organization_incident_comment_details.py
+++ b/src/sentry/api/endpoints/organization_incident_comment_details.py
@@ -28,12 +28,15 @@ def convert_args(self, request, activity_id, *args, **kwargs):
         args, kwargs = super(CommentDetailsEndpoint, self).convert_args(request, *args, **kwargs)
 
         try:
+            # Superusers may mutate any comment
+            user_filter = {} if request.user.is_superuser else {"user": request.user}
+
             kwargs["activity"] = IncidentActivity.objects.get(
                 id=activity_id,
-                user=request.user,
                 incident=kwargs["incident"],
                 # Only allow modifying comments
                 type=IncidentActivityType.COMMENT.value,
+                **user_filter
             )
         except IncidentActivity.DoesNotExist:
             raise ResourceDoesNotExist
diff --git a/src/sentry/static/sentry/app/components/activity/note/header.tsx b/src/sentry/static/sentry/app/components/activity/note/header.tsx
index 38c6b2ba1f8028..4f3c2e351b5269 100644
--- a/src/sentry/static/sentry/app/components/activity/note/header.tsx
+++ b/src/sentry/static/sentry/app/components/activity/note/header.tsx
@@ -7,6 +7,7 @@ import ConfigStore from 'app/stores/configStore';
 import LinkWithConfirmation from 'app/components/links/linkWithConfirmation';
 import {User} from 'app/types';
 import {Theme} from 'app/utils/theme';
+import Tooltip from 'app/components/tooltip';
 
 import EditorTools from './editorTools';
 
@@ -17,28 +18,38 @@ type Props = {
   onDelete: () => void;
 };
 
-function canEdit(editingUser: User) {
-  const user = ConfigStore.get('user');
-  return user && (user.isSuperuser || user.id === editingUser.id);
-}
+const NoteHeader = ({authorName, user, onEdit, onDelete}: Props) => {
+  const activeUser = ConfigStore.get('user');
+  const canEdit = activeUser && (activeUser.isSuperuser || user.id === activeUser.id);
 
-const NoteHeader = ({authorName, user, onEdit, onDelete}: Props) => (
-  <div>
-    <ActivityAuthor>{authorName}</ActivityAuthor>
-    {canEdit(user) && (
-      <EditorTools>
-        <Edit onClick={onEdit}>{t('Edit')}</Edit>
-        <LinkWithConfirmation
-          title={t('Remove')}
-          message={t('Are you sure you wish to delete this comment?')}
-          onConfirm={onDelete}
-        >
-          <Remove>{t('Remove')}</Remove>
-        </LinkWithConfirmation>
-      </EditorTools>
-    )}
-  </div>
-);
+  return (
+    <div>
+      <ActivityAuthor>{authorName}</ActivityAuthor>
+      {canEdit && (
+        <EditorTools>
+          <Tooltip
+            title={t('You can edit this comment due to your superuser status')}
+            disabled={!activeUser.isSuperuser}
+          >
+            <Edit onClick={onEdit}>{t('Edit')}</Edit>
+          </Tooltip>
+          <Tooltip
+            title={t('You can delete this comment due to your superuser status')}
+            disabled={!activeUser.isSuperuser}
+          >
+            <LinkWithConfirmation
+              title={t('Remove')}
+              message={t('Are you sure you wish to delete this comment?')}
+              onConfirm={onDelete}
+            >
+              <Remove>{t('Remove')}</Remove>
+            </LinkWithConfirmation>
+          </Tooltip>
+        </EditorTools>
+      )}
+    </div>
+  );
+};
 
 const getActionStyle = (p: {theme: Theme}) => `
   padding: 0 7px;
diff --git a/tests/sentry/api/endpoints/test_organization_incident_comment_details.py b/tests/sentry/api/endpoints/test_organization_incident_comment_details.py
index 849b3f726b0a48..30d4439744f9c4 100644
--- a/tests/sentry/api/endpoints/test_organization_incident_comment_details.py
+++ b/tests/sentry/api/endpoints/test_organization_incident_comment_details.py
@@ -19,6 +19,11 @@ def setUp(self):
             self.incident, user=self.user, type=IncidentActivityType.DETECTED.value
         )
 
+        user2 = self.create_user()
+        self.user2_activity = self.create_incident_comment(
+            incident=self.incident, user=user2, comment="hello from another user"
+        )
+
     @fixture
     def organization(self):
         return self.create_organization()
@@ -76,6 +81,34 @@ def test_simple(self):
         assert activity.user == self.user
         assert activity.comment == comment
 
+    def test_cannot_edit_others_comment(self):
+        with self.feature("organizations:incidents"):
+            self.get_valid_response(
+                self.organization.slug,
+                self.incident.identifier,
+                self.user2_activity.id,
+                comment="edited comment",
+                status_code=404,
+            )
+
+    def test_superuser_can_edit(self):
+        self.user.is_superuser = True
+        self.user.save()
+
+        edited_comment = "this comment has been edited"
+
+        with self.feature("organizations:incidents"):
+            self.get_valid_response(
+                self.organization.slug,
+                self.incident.identifier,
+                self.user2_activity.id,
+                comment=edited_comment,
+                status_code=200,
+            )
+        activity = IncidentActivity.objects.get(id=self.user2_activity.id)
+        assert activity.user != self.user
+        assert activity.comment == edited_comment
+
 
 class OrganizationIncidentCommentDeleteEndpointTest(BaseIncidentCommentDetailsTest, APITestCase):
     method = "delete"
@@ -86,3 +119,25 @@ def test_simple(self):
                 self.organization.slug, self.incident.identifier, self.activity.id, status_code=204
             )
         assert not IncidentActivity.objects.filter(id=self.activity.id).exists()
+
+    def test_cannot_delete_others_comments(self):
+        with self.feature("organizations:incidents"):
+            self.get_valid_response(
+                self.organization.slug,
+                self.incident.identifier,
+                self.user2_activity.id,
+                status_code=404,
+            )
+
+    def test_superuser_can_delete(self):
+        self.user.is_superuser = True
+        self.user.save()
+
+        with self.feature("organizations:incidents"):
+            self.get_valid_response(
+                self.organization.slug,
+                self.incident.identifier,
+                self.user2_activity.id,
+                status_code=204,
+            )
+        assert not IncidentActivity.objects.filter(id=self.user2_activity.id).exists()