Revert "Revert slack org does not exist changes breaking escalate com…

…mand (#2057)" This reverts commit 835d267.
grafana · May 31, 2023 · 03e19b7 · 03e19b7
1 parent 5975b9d
commit 03e19b7
Show file tree

Hide file tree

Showing 3 changed files with 185 additions and 68 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 
 - Fix templates when slack or telegram is disabled ([#2064](https://github.com/grafana/oncall/pull/2064))
+- Fix + revert [#2057](https://github.com/grafana/oncall/pull/2057) which reverted a change which properly handles
+  `Organization.DoesNotExist` exceptions for Slack events by @joeyorlando ([#TBD](https://github.com/grafana/oncall/pull/TBD))
 
 ## v1.2.33 (2023-05-30)
 

diff --git a/engine/apps/slack/tests/test_interactive_api_endpoint.py b/engine/apps/slack/tests/test_interactive_api_endpoint.py
@@ -0,0 +1,96 @@
+import json
+from unittest.mock import call, patch
+
+import pytest
+from rest_framework import status
+from rest_framework.test import APIClient
+
+from apps.slack.scenarios.scenario_step import PAYLOAD_TYPE_BLOCK_ACTIONS
+
+EVENT_TRIGGER_ID = "5333959822612.4122782784722.4734ff484b2ac4d36a185bb242ee9932"
+WARNING_TEXT = (
+    "OnCall is not able to process this action because one of the following scenarios: \n"
+    "1. The Slack chatops integration was disconnected from the instance that the Alert Group belongs "
+    "to, BUT the Slack workspace is still connected to another instance as well. In this case, simply log "
+    "in to the OnCall web interface and re-install the Slack Integration with this workspace again.\n"
+    "2. (Less likely) The Grafana instance belonging to this Alert Group was deleted. In this case the Alert Group is orphaned and cannot be acted upon."
+)
+
+
+def _make_request(payload):
+    return APIClient().post(
+        "/slack/interactive_api_endpoint/",
+        format="json",
+        data=payload,
+        **{
+            "HTTP_X_SLACK_SIGNATURE": "asdfasdf",
+            "HTTP_X_SLACK_REQUEST_TIMESTAMP": "xxcxcvx",
+        },
+    )
+
+
+@patch("apps.slack.views.SlackEventApiEndpointView.verify_signature", return_value=True)
+@patch("apps.slack.views.SlackEventApiEndpointView._open_warning_window_if_needed")
+@pytest.mark.django_db
+def test_organization_not_found_scenario_properly_handled(
+    mock_open_warning_window_if_needed,
+    _mock_verify_signature,
+    make_organization,
+    make_slack_user_identity,
+    make_slack_team_identity,
+):
+    slack_team_id = "T043LP0P2M8"
+    slack_access_token = "asdfasdf"
+    slack_bot_access_token = "cmncvmnvcnm"
+    slack_bot_user_id = "mncvnmvcmnvcmncv,,cx,"
+
+    slack_user_id = "iurtiurituritu"
+
+    def _make_slack_team_identity():
+        return make_slack_team_identity(
+            slack_id=slack_team_id,
+            detected_token_revoked=None,
+            access_token=slack_access_token,
+            bot_access_token=slack_bot_access_token,
+            bot_user_id=slack_bot_user_id,
+        )
+
+    # SCENARIO 1
+    # two orgs connected to same slack workspace, the one belonging to the alert group/slack message
+    # is no longer connected to the slack workspace, but another org still is
+    slack_team_identity = _make_slack_team_identity()
+    make_slack_user_identity(slack_team_identity=slack_team_identity, slack_id=slack_user_id)
+
+    make_organization(slack_team_identity=slack_team_identity)
+    org2 = make_organization()
+    event_payload_actions = [
+        {
+            "value": json.dumps({"organization_id": org2.id}),
+        }
+    ]
+
+    event_payload = {
+        "type": PAYLOAD_TYPE_BLOCK_ACTIONS,
+        "trigger_id": EVENT_TRIGGER_ID,
+        "user": {
+            "id": slack_user_id,
+        },
+        "team": {
+            "id": slack_team_id,
+        },
+        "actions": event_payload_actions,
+    }
+
+    response = _make_request(event_payload)
+    assert response.status_code == status.HTTP_200_OK
+
+    # SCENARIO 2
+    # the org that was associated w/ the alert group, has since been deleted
+    # and the slack message is now orphaned
+    org2.hard_delete()
+
+    response = _make_request(event_payload)
+    assert response.status_code == status.HTTP_200_OK
+
+    mock_call = call(event_payload, slack_team_identity, WARNING_TEXT)
+    mock_open_warning_window_if_needed.assert_has_calls([mock_call, mock_call])
diff --git a/engine/apps/slack/views.py b/engine/apps/slack/views.py
@@ -55,6 +55,7 @@
 from apps.slack.slack_client import SlackClientWithErrorHandling
 from apps.slack.slack_client.exceptions import SlackAPIException, SlackAPITokenException
 from apps.slack.tasks import clean_slack_integration_leftovers, unpopulate_slack_user_identities
+from apps.user_management.models import Organization
 from common.insight_log import ChatOpsEvent, ChatOpsTypePlug, write_chatops_insight_log
 from common.oncall_gateway import delete_slack_connector
 
@@ -285,6 +286,19 @@ def post(self, request):
                 # Open pop-up to inform user why OnCall bot doesn't work if any action was triggered
                 self._open_warning_window_if_needed(payload, slack_team_identity, warning_text)
                 return Response(status=200)
+        elif organization is None:
+            # see this GitHub issue for more context on how this situation can arise
+            # https://github.com/grafana/oncall-private/issues/1836
+            warning_text = (
+                "OnCall is not able to process this action because one of the following scenarios: \n"
+                "1. The Slack chatops integration was disconnected from the instance that the Alert Group belongs "
+                "to, BUT the Slack workspace is still connected to another instance as well. In this case, simply log "
+                "in to the OnCall web interface and re-install the Slack Integration with this workspace again.\n"
+                "2. (Less likely) The Grafana instance belonging to this Alert Group was deleted. In this case the Alert Group is orphaned and cannot be acted upon."
+            )
+            # Open pop-up to inform user why OnCall bot doesn't work if any action was triggered
+            self._open_warning_window_if_needed(payload, slack_team_identity, warning_text)
+            return Response(status=200)
         elif not slack_user_identity.users.exists():
             # Means that slack_user_identity doesn't have any connected user
             # Open pop-up to inform user why OnCall bot doesn't work if any action was triggered
@@ -420,76 +434,81 @@ def _get_organization_from_payload(self, payload, slack_team_identity):
         channel_id = None
         organization = None
 
-        # view submission or actions in view
-        if "view" in payload:
-            organization_id = None
-            private_metadata = payload["view"].get("private_metadata")
-            # steps with private_metadata in which we know organization before open view
-            if private_metadata and "organization_id" in private_metadata:
-                organization_id = json.loads(private_metadata).get("organization_id")
-            # steps with organization selection in view (e.g. slash commands)
-            elif SELECT_ORGANIZATION_AND_ROUTE_BLOCK_ID in payload["view"].get("state", {}).get("values", {}):
-                payload_values = payload["view"]["state"]["values"]
-                selected_value = payload_values[SELECT_ORGANIZATION_AND_ROUTE_BLOCK_ID][
-                    SELECT_ORGANIZATION_AND_ROUTE_BLOCK_ID
-                ]["selected_option"]["value"]
-                organization_id = int(selected_value.split("-")[0])
-            if organization_id:
-                organization = slack_team_identity.organizations.get(pk=organization_id)
-                return organization
-        # buttons and actions
-        elif payload.get("type") in [
-            PAYLOAD_TYPE_BLOCK_ACTIONS,
-            PAYLOAD_TYPE_INTERACTIVE_MESSAGE,
-            PAYLOAD_TYPE_MESSAGE_ACTION,
-        ]:
-            # for cases when we put organization_id into action value (e.g. public suggestion)
-            if (
-                payload.get("actions")
-                and payload["actions"][0].get("value", {})
-                and "organization_id" in payload["actions"][0]["value"]
-            ):
-                organization_id = int(json.loads(payload["actions"][0]["value"])["organization_id"])
-                organization = slack_team_identity.organizations.get(pk=organization_id)
-                return organization
-
-            channel_id = payload["channel"]["id"]
-            if "message" in payload:
-                message_ts = payload["message"].get("thread_ts") or payload["message"]["ts"]
-            # for interactive message
-            elif "message_ts" in payload:
-                message_ts = payload["message_ts"]
-            else:
-                return
-        # events
-        elif payload.get("type") == PAYLOAD_TYPE_EVENT_CALLBACK:
-            if "channel" in payload["event"]:  # events without channel: user_change, events with subteam, etc.
-                channel_id = payload["event"]["channel"]
-
-            if "message" in payload["event"]:
-                message_ts = payload["event"]["message"].get("thread_ts") or payload["event"]["message"]["ts"]
-            elif "thread_ts" in payload["event"]:
-                message_ts = payload["event"]["thread_ts"]
-            else:
-                return
+        try:
+            # view submission or actions in view
+            if "view" in payload:
+                organization_id = None
+                private_metadata = payload["view"].get("private_metadata")
+                # steps with private_metadata in which we know organization before open view
+                if private_metadata and "organization_id" in private_metadata:
+                    organization_id = json.loads(private_metadata).get("organization_id")
+                # steps with organization selection in view (e.g. slash commands)
+                elif SELECT_ORGANIZATION_AND_ROUTE_BLOCK_ID in payload["view"].get("state", {}).get("values", {}):
+                    payload_values = payload["view"]["state"]["values"]
+                    selected_value = payload_values[SELECT_ORGANIZATION_AND_ROUTE_BLOCK_ID][
+                        SELECT_ORGANIZATION_AND_ROUTE_BLOCK_ID
+                    ]["selected_option"]["value"]
+                    organization_id = int(selected_value.split("-")[0])
+                if organization_id:
+                    organization = slack_team_identity.organizations.get(pk=organization_id)
+                    return organization
+            # buttons and actions
+            elif payload.get("type") in [
+                PAYLOAD_TYPE_BLOCK_ACTIONS,
+                PAYLOAD_TYPE_INTERACTIVE_MESSAGE,
+                PAYLOAD_TYPE_MESSAGE_ACTION,
+            ]:
+                # for cases when we put organization_id into action value (e.g. public suggestion)
+                if (
+                    payload.get("actions")
+                    and payload["actions"][0].get("value", {})
+                    and "organization_id" in payload["actions"][0]["value"]
+                ):
+                    organization_id = int(json.loads(payload["actions"][0]["value"])["organization_id"])
+                    organization = slack_team_identity.organizations.get(pk=organization_id)
+                    return organization
+
+                channel_id = payload["channel"]["id"]
+                if "message" in payload:
+                    message_ts = payload["message"].get("thread_ts") or payload["message"]["ts"]
+                # for interactive message
+                elif "message_ts" in payload:
+                    message_ts = payload["message_ts"]
+                else:
+                    return
+            # events
+            elif payload.get("type") == PAYLOAD_TYPE_EVENT_CALLBACK:
+                if "channel" in payload["event"]:  # events without channel: user_change, events with subteam, etc.
+                    channel_id = payload["event"]["channel"]
+
+                if "message" in payload["event"]:
+                    message_ts = payload["event"]["message"].get("thread_ts") or payload["event"]["message"]["ts"]
+                elif "thread_ts" in payload["event"]:
+                    message_ts = payload["event"]["thread_ts"]
+                else:
+                    return
 
-        if not (message_ts and channel_id):
-            return
+            if not (message_ts and channel_id):
+                return
 
-        try:
-            slack_message = SlackMessage.objects.get(
-                slack_id=message_ts,
-                _slack_team_identity=slack_team_identity,
-                channel_id=channel_id,
-            )
-        except SlackMessage.DoesNotExist:
-            pass
-        else:
-            alert_group = slack_message.get_alert_group()
-            if alert_group:
-                organization = alert_group.channel.organization
-                return organization
-        return organization
+            try:
+                slack_message = SlackMessage.objects.get(
+                    slack_id=message_ts,
+                    _slack_team_identity=slack_team_identity,
+                    channel_id=channel_id,
+                )
+            except SlackMessage.DoesNotExist:
+                pass
+            else:
+                alert_group = slack_message.get_alert_group()
+                if alert_group:
+                    organization = alert_group.channel.organization
+                    return organization
+            return organization
+        except Organization.DoesNotExist:
+            # see this GitHub issue for more context on how this situation can arise
+            # https://github.com/grafana/oncall-private/issues/1836
+            return None
 
     def _open_warning_window_if_needed(self, payload, slack_team_identity, warning_text) -> None:
         if payload.get("trigger_id") is not None: