Properly address Organization.DoesNotExist resulting in HTTP 500 on Slack interactive_api_endpoint (#2040)
# Which issue(s) this PR fixes Closes https://github.com/grafana/oncall-private/issues/1836 ## TODO: - [ ] add unit tests for this scenario ## Checklist - [ ] Unit, integration, and e2e (if applicable) tests updated - [ ] Documentation added (or `pr:no public docs` PR label added if not required) (N/A) - [x] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required)
This commit is contained in:
Joey Orlando
GitHub
parent
29ecfc5df9
commit
4ebc7231c9
3 changed files with 187 additions and 70 deletions
|
|
@ -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
|
||||
|
||||
|
|
|
|||
96
engine/apps/slack/tests/test_interactive_api_endpoint.py
Normal file
96
engine/apps/slack/tests/test_interactive_api_endpoint.py
Normal file
|
|
@ -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])
|
||||
|
|
@ -55,6 +55,7 @@ from apps.slack.scenarios.slack_usergroup import STEPS_ROUTING as SLACK_USERGROU
|
|||
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 @@ class SlackEventApiEndpointView(APIView):
|
|||
# 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 @@ class SlackEventApiEndpointView(APIView):
|
|||
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
|
||||
|
||||
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
|
||||
# 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
|
||||
|
||||
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:
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue