diff --git a/CHANGELOG.md b/CHANGELOG.md index d8285a6194..0e1811a520 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Add models and framework to use different services (Phone, SMS, Verify) in Twilio depending on -the destination country code by @mderynck ([#1976](https://github.com/grafana/oncall/pull/1976)) + the destination country code by @mderynck ([#1976](https://github.com/grafana/oncall/pull/1976)) - Prometheus exporter backend for alert groups related metrics - Much expanded/improved docs for mobile app ([2026](https://github.com/grafana/oncall/pull/2026>)) - Enable by-day selection when defining monthly and hourly rotations ([2037](https://github.com/grafana/oncall/pull/2037)) @@ -19,6 +19,8 @@ the destination country code by @mderynck ([#1976](https://github.com/grafana/on - Fix error when updating closed modal window in Slack by @vadimkerr ([#2019](https://github.com/grafana/oncall/pull/2019)) - Fix final schedule export failing to update when ical imported events set start/end as date ([#2025](https://github.com/grafana/oncall/pull/2025)) +- Properly address `Organization.DoesNotExist` exceptions thrown which result in HTTP 500 for the Slack `interactive_api_endpoint` + endpoint by @joeyorlando ([#2040](https://github.com/grafana/oncall/pull/2040)) ### Changed diff --git a/engine/apps/slack/tests/test_interactive_api_endpoint.py b/engine/apps/slack/tests/test_interactive_api_endpoint.py new file mode 100644 index 0000000000..99a6acdd0b --- /dev/null +++ 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 index 6c0a4d16c1..7572c05bad 100644 --- 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: