From 3e9e7605e610573634b90d226c6bb75c26ebc8c8 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Tue, 28 Nov 2023 15:00:57 -0500 Subject: [PATCH 1/9] WIP --- engine/apps/grafana_plugin/helpers/client.py | 12 +- .../tests/test_mobile_app_gateway.py | 312 ++++++++++++++++++ engine/apps/mobile_app/urls.py | 19 +- engine/apps/mobile_app/views.py | 131 +++++++- ...ganization_grafana_incident_backend_url.py | 18 + .../user_management/models/organization.py | 2 + engine/apps/user_management/sync.py | 8 +- engine/settings/base.py | 2 + 8 files changed, 495 insertions(+), 9 deletions(-) create mode 100644 engine/apps/mobile_app/tests/test_mobile_app_gateway.py create mode 100644 engine/apps/user_management/migrations/0018_organization_grafana_incident_backend_url.py diff --git a/engine/apps/grafana_plugin/helpers/client.py b/engine/apps/grafana_plugin/helpers/client.py index d9e7faaabe..4544f5b9b8 100644 --- a/engine/apps/grafana_plugin/helpers/client.py +++ b/engine/apps/grafana_plugin/helpers/client.py @@ -160,6 +160,9 @@ def request_headers(self) -> APIRequestHeaders: class GrafanaAPIClient(APIClient): + GRAFANA_INCIDENT_PLUGIN = "grafana-incident-app" + GRAFANA_INCIDENT_PLUGIN_BACKEND_URL_KEY = "backendUrl" + USER_PERMISSION_ENDPOINT = f"api/access-control/users/permissions/search?actionPrefix={ACTION_PREFIX}" class Types: @@ -191,6 +194,10 @@ class GrafanaServiceAccountToken(typing.TypedDict): name: str key: str + class PluginSettings(typing.TypedDict): + enabled: bool + jsonData: typing.Dict[str, str] + class TeamsResponse(_BaseGrafanaAPIResponse): teams: typing.List["GrafanaAPIClient.Types.GrafanaTeam"] @@ -289,9 +296,12 @@ def update_alerting_config(self, recipient, config) -> APIClientResponse: def get_alerting_notifiers(self): return self.api_get("api/alert-notifiers") - def get_grafana_plugin_settings(self, recipient: str) -> APIClientResponse: + def get_grafana_plugin_settings(self, recipient: str) -> APIClientResponse["GrafanaAPIClient.Types.PluginSettings"]: return self.api_get(f"api/plugins/{recipient}/settings") + def get_grafana_incident_plugin_settings(self) -> APIClientResponse["GrafanaAPIClient.Types.PluginSettings"]: + return self.get_grafana_plugin_settings(self.GRAFANA_INCIDENT_PLUGIN) + def get_service_account(self, login: str) -> APIClientResponse["GrafanaAPIClient.Types.ServiceAccountResponse"]: return self.api_get(f"api/serviceaccounts/search?query={login}") diff --git a/engine/apps/mobile_app/tests/test_mobile_app_gateway.py b/engine/apps/mobile_app/tests/test_mobile_app_gateway.py new file mode 100644 index 0000000000..c3e5200e91 --- /dev/null +++ b/engine/apps/mobile_app/tests/test_mobile_app_gateway.py @@ -0,0 +1,312 @@ +import json +from unittest.mock import patch + +import pytest +from django.urls import reverse +from rest_framework import status +from rest_framework.test import APIClient +from rest_framework.views import APIView + +DOWNSTREAM_BACKEND = "incident" +MOCK_DOWNSTREAM_INCIDENT_API_URL = "https://mockdownstreamincidentapi.com" +MOCK_DOWNSTREAM_HEADERS = {"Authorization": "Bearer mock_jwt"} + +MOCK_DOWNSTREAM_RESPONSE_DATA = {"foo": "bar"} + + +class MockResponse: + def __init__(self, status_code=status.HTTP_200_OK, data=MOCK_DOWNSTREAM_RESPONSE_DATA): + self.status_code = status_code + self.data = data + + def json(self): + return self.data + + +@pytest.mark.django_db +@patch("apps.mobile_app.views.requests") +@patch( + "apps.mobile_app.views.MobileAppGatewayView._determine_grafana_incident_api_url", + return_value=MOCK_DOWNSTREAM_INCIDENT_API_URL, +) +@patch("apps.mobile_app.views.MobileAppGatewayView._get_downstream_headers", return_value=MOCK_DOWNSTREAM_HEADERS) +@pytest.mark.parametrize("path", ["", "thing", "thing/123", "thing/123/otherthing", "thing/123/otherthing/456"]) +def test_mobile_app_gateway_properly_proxies_paths( + _mock_get_downstream_headers, + _mock_determine_grafana_incident_api_url, + mock_requests, + make_organization_and_user_with_mobile_app_auth_token, + path, +): + mock_requests.post.return_value = MockResponse() + + _, _, auth_token = make_organization_and_user_with_mobile_app_auth_token() + + client = APIClient() + url = reverse("mobile_app:gateway", kwargs={"downstream_backend": DOWNSTREAM_BACKEND, "downstream_path": path}) + + response = client.post(url, HTTP_AUTHORIZATION=auth_token) + assert response.status_code == status.HTTP_200_OK + assert response.json() == MOCK_DOWNSTREAM_RESPONSE_DATA + + mock_requests.post.assert_called_once_with( + f"{MOCK_DOWNSTREAM_INCIDENT_API_URL}/{path}", + data={}, + params={}, + headers=MOCK_DOWNSTREAM_HEADERS, + ) + + +@pytest.mark.django_db +@patch("apps.mobile_app.views.requests") +@patch( + "apps.mobile_app.views.MobileAppGatewayView._determine_grafana_incident_api_url", + return_value=MOCK_DOWNSTREAM_INCIDENT_API_URL, +) +@patch("apps.mobile_app.views.MobileAppGatewayView._get_downstream_headers", return_value=MOCK_DOWNSTREAM_HEADERS) +@pytest.mark.parametrize("method", APIView.http_method_names) +def test_mobile_app_gateway_supports_all_methods( + _mock_get_downstream_headers, + _mock_determine_grafana_incident_api_url, + mock_requests, + make_organization_and_user_with_mobile_app_auth_token, + method, +): + mock_http_verb_method = getattr(mock_requests, method.lower()) + mock_http_verb_method.return_value = MockResponse() + + _, _, auth_token = make_organization_and_user_with_mobile_app_auth_token() + + client = APIClient() + url = reverse("mobile_app:gateway", kwargs={"downstream_backend": DOWNSTREAM_BACKEND, "downstream_path": "test"}) + + response = client.generic(method.upper(), url, HTTP_AUTHORIZATION=auth_token) + assert response.status_code == status.HTTP_200_OK + + mock_http_verb_method.assert_called_once() + + +@pytest.mark.django_db +@patch("apps.mobile_app.views.requests") +@patch( + "apps.mobile_app.views.MobileAppGatewayView._determine_grafana_incident_api_url", + return_value=MOCK_DOWNSTREAM_INCIDENT_API_URL, +) +@patch("apps.mobile_app.views.MobileAppGatewayView._get_downstream_headers", return_value=MOCK_DOWNSTREAM_HEADERS) +def test_mobile_app_gateway_proxies_query_params( + _mock_get_downstream_headers, + _mock_determine_grafana_incident_api_url, + mock_requests, + make_organization_and_user_with_mobile_app_auth_token, +): + path = "test/123" + mock_requests.post.return_value = MockResponse() + + _, _, auth_token = make_organization_and_user_with_mobile_app_auth_token() + + client = APIClient() + url = reverse("mobile_app:gateway", kwargs={"downstream_backend": DOWNSTREAM_BACKEND, "downstream_path": path}) + + response = client.post(f"{url}?foo=bar&baz=hello", HTTP_AUTHORIZATION=auth_token) + assert response.status_code == status.HTTP_200_OK + + mock_requests.post.assert_called_once_with( + f"{MOCK_DOWNSTREAM_INCIDENT_API_URL}/{path}", + data={}, + params={"foo": "bar", "baz": "hello"}, + headers=MOCK_DOWNSTREAM_HEADERS, + ) + + +@pytest.mark.django_db +@patch("apps.mobile_app.views.requests") +@patch( + "apps.mobile_app.views.MobileAppGatewayView._determine_grafana_incident_api_url", + return_value=MOCK_DOWNSTREAM_INCIDENT_API_URL, +) +@patch("apps.mobile_app.views.MobileAppGatewayView._get_downstream_headers", return_value=MOCK_DOWNSTREAM_HEADERS) +@pytest.mark.parametrize( + "upstream_request_body", + [ + None, + {}, + {"vegetable": "potato", "fruit": "apple"}, + ], +) +def test_mobile_app_gateway_properly_proxies_request_body( + _mock_get_downstream_headers, + _mock_determine_grafana_incident_api_url, + mock_requests, + make_organization_and_user_with_mobile_app_auth_token, + upstream_request_body, +): + path = "test/123" + mock_requests.post.return_value = MockResponse() + + _, _, auth_token = make_organization_and_user_with_mobile_app_auth_token() + + client = APIClient() + url = reverse("mobile_app:gateway", kwargs={"downstream_backend": DOWNSTREAM_BACKEND, "downstream_path": path}) + + response = client.post( + url, + data=json.dumps(upstream_request_body), + content_type="application/json", + HTTP_AUTHORIZATION=auth_token, + ) + assert response.status_code == status.HTTP_200_OK + + mock_requests.post.assert_called_once_with( + f"{MOCK_DOWNSTREAM_INCIDENT_API_URL}/{path}", + data=upstream_request_body, + params={}, + headers=MOCK_DOWNSTREAM_HEADERS, + ) + + +@pytest.mark.django_db +@patch("apps.mobile_app.views.requests") +@patch( + "apps.mobile_app.views.MobileAppGatewayView._determine_grafana_incident_api_url", + return_value=MOCK_DOWNSTREAM_INCIDENT_API_URL, +) +@patch("apps.mobile_app.views.MobileAppGatewayView._get_downstream_headers", return_value=MOCK_DOWNSTREAM_HEADERS) +@pytest.mark.parametrize( + "downstream_backend,expected_status", + [ + ("incident", status.HTTP_200_OK), + ("foo", status.HTTP_404_NOT_FOUND), + ], +) +def test_mobile_app_gateway_supported_downstream_backends( + _mock_get_downstream_headers, + _mock_determine_grafana_incident_api_url, + mock_requests, + make_organization_and_user_with_mobile_app_auth_token, + downstream_backend, + expected_status, +): + mock_requests.post.return_value = MockResponse() + + _, _, auth_token = make_organization_and_user_with_mobile_app_auth_token() + + client = APIClient() + url = reverse( + "mobile_app:gateway", kwargs={"downstream_backend": downstream_backend, "downstream_path": "test/123"} + ) + + response = client.post(url, HTTP_AUTHORIZATION=auth_token) + assert response.status_code == expected_status + + +@pytest.mark.django_db +@patch("apps.mobile_app.views.requests") +@patch( + "apps.mobile_app.views.MobileAppGatewayView._determine_grafana_incident_api_url", + return_value=MOCK_DOWNSTREAM_INCIDENT_API_URL, +) +@patch("apps.mobile_app.views.MobileAppGatewayView._get_downstream_headers", return_value=MOCK_DOWNSTREAM_HEADERS) +def test_mobile_app_gateway_mobile_app_auth_token( + _mock_get_downstream_headers, + _mock_determine_grafana_incident_api_url, + mock_requests, + make_organization_and_user_with_mobile_app_auth_token, +): + mock_requests.post.return_value = MockResponse() + + _, _, auth_token = make_organization_and_user_with_mobile_app_auth_token() + + client = APIClient() + url = reverse( + "mobile_app:gateway", kwargs={"downstream_backend": DOWNSTREAM_BACKEND, "downstream_path": "test/123"} + ) + + response = client.post(url, HTTP_AUTHORIZATION="potato") + assert response.status_code == status.HTTP_403_FORBIDDEN + + response = client.post(url, HTTP_AUTHORIZATION=auth_token) + assert response.status_code == status.HTTP_200_OK + + +@pytest.mark.django_db +@patch("apps.mobile_app.views.requests") +@patch("apps.mobile_app.views.MobileAppGatewayView._get_downstream_headers", return_value=MOCK_DOWNSTREAM_HEADERS) +def test_mobile_app_gateway_incident_api_url( + _mock_get_downstream_headers, + mock_requests, + make_organization_and_user_with_mobile_app_auth_token, +): + mock_incident_backend_url = "https://mockincidentbackend.com" + mock_requests.post.return_value = MockResponse() + + client = APIClient() + url = reverse( + "mobile_app:gateway", kwargs={"downstream_backend": DOWNSTREAM_BACKEND, "downstream_path": "test/123"} + ) + + # Grafana API returns None for the incident plugin settings + with patch( + "apps.mobile_app.views.GrafanaAPIClient.get_grafana_incident_plugin_settings" + ) as mock_get_grafana_incident_plugin_settings: + organization, _, auth_token = make_organization_and_user_with_mobile_app_auth_token() + assert organization.grafana_incident_backend_url is None + + mock_get_grafana_incident_plugin_settings.return_value = (None, None) + + response = client.post(url, HTTP_AUTHORIZATION=auth_token) + assert response.status_code == status.HTTP_400_BAD_REQUEST + + # Incident plugin settings jsonData doesn't contain the backend URL + with patch( + "apps.mobile_app.views.GrafanaAPIClient.get_grafana_incident_plugin_settings" + ) as mock_get_grafana_incident_plugin_settings: + organization, _, auth_token = make_organization_and_user_with_mobile_app_auth_token() + assert organization.grafana_incident_backend_url is None + + mock_get_grafana_incident_plugin_settings.return_value = ({"jsonData": {}}, None) + + response = client.post(url, HTTP_AUTHORIZATION=auth_token) + assert response.status_code == status.HTTP_400_BAD_REQUEST + + # Organization already has incident backend URL saved + with patch( + "apps.mobile_app.views.GrafanaAPIClient.get_grafana_incident_plugin_settings" + ) as mock_get_grafana_incident_plugin_settings: + organization, _, auth_token = make_organization_and_user_with_mobile_app_auth_token() + organization.grafana_incident_backend_url = mock_incident_backend_url + organization.save() + + response = client.post(url, HTTP_AUTHORIZATION=auth_token) + assert response.status_code == status.HTTP_200_OK + mock_get_grafana_incident_plugin_settings.assert_not_called() + + # Incident plugin settings jsonData contains the backend URL + # check that it gets saved to the organization + with patch( + "apps.mobile_app.views.GrafanaAPIClient.get_grafana_incident_plugin_settings" + ) as mock_get_grafana_incident_plugin_settings: + organization, _, auth_token = make_organization_and_user_with_mobile_app_auth_token() + assert organization.grafana_incident_backend_url is None + + mock_get_grafana_incident_plugin_settings.return_value = ( + {"jsonData": {"backendUrl": mock_incident_backend_url}}, + None, + ) + + response = client.post(url, HTTP_AUTHORIZATION=auth_token) + assert response.status_code == status.HTTP_200_OK + + organization.refresh_from_db() + assert organization.grafana_incident_backend_url == mock_incident_backend_url + + +# # TODO: +# @pytest.mark.django_db +# def test_mobile_app_gateway_downstream_jwt_auth(make_organization_and_user_with_mobile_app_auth_token): +# _, _, auth_token = make_organization_and_user_with_mobile_app_auth_token() + +# client = APIClient() +# url = reverse("mobile_app:gateway", kwargs={"downstream_backend": DOWNSTREAM_BACKEND, "downstream_path": "test"}) + +# response = client.post(url, HTTP_AUTHORIZATION=auth_token) +# assert response.status_code == status.HTTP_403_FORBIDDEN diff --git a/engine/apps/mobile_app/urls.py b/engine/apps/mobile_app/urls.py index ed36874cf9..3eb5f0b03a 100644 --- a/engine/apps/mobile_app/urls.py +++ b/engine/apps/mobile_app/urls.py @@ -1,5 +1,13 @@ +from django.conf import settings +from django.urls import re_path + from apps.mobile_app.fcm_relay import FCMRelayView -from apps.mobile_app.views import FCMDeviceAuthorizedViewSet, MobileAppAuthTokenAPIView, MobileAppUserSettingsViewSet +from apps.mobile_app.views import ( + FCMDeviceAuthorizedViewSet, + MobileAppAuthTokenAPIView, + MobileAppGatewayView, + MobileAppUserSettingsViewSet, +) from common.api_helpers.optional_slash_router import OptionalSlashRouter, optional_slash_path app_name = "mobile_app" @@ -25,3 +33,12 @@ urlpatterns += [ optional_slash_path("fcm_relay", FCMRelayView.as_view(), name="fcm_relay"), ] + +if settings.MOBILE_APP_GATEWAY_ENABLED: + urlpatterns += [ + re_path( + r"^gateway/(?P\w*)/(?P.*)$", + MobileAppGatewayView.as_view(), + name="gateway", + ), + ] diff --git a/engine/apps/mobile_app/views.py b/engine/apps/mobile_app/views.py index 8a4fe55886..f8492658a5 100644 --- a/engine/apps/mobile_app/views.py +++ b/engine/apps/mobile_app/views.py @@ -1,14 +1,27 @@ +import json +import logging +import typing + +import requests from fcm_django.api.rest_framework import FCMDeviceAuthorizedViewSet as BaseFCMDeviceAuthorizedViewSet from rest_framework import mixins, status, viewsets -from rest_framework.exceptions import NotFound +from rest_framework.exceptions import NotFound, ParseError from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response from rest_framework.views import APIView +from apps.grafana_plugin.helpers.client import GrafanaAPIClient from apps.mobile_app.auth import MobileAppAuthTokenAuthentication, MobileAppVerificationTokenAuthentication from apps.mobile_app.models import MobileAppAuthToken, MobileAppUserSettings from apps.mobile_app.serializers import MobileAppUserSettingsSerializer +if typing.TYPE_CHECKING: + from apps.user_management.models import Organization, User + + +logger = logging.getLogger(__name__) +logger.setLevel(logging.DEBUG) + class FCMDeviceAuthorizedViewSet(BaseFCMDeviceAuthorizedViewSet): authentication_classes = (MobileAppAuthTokenAuthentication,) @@ -72,3 +85,119 @@ def notification_timing_options(self, request): {"value": item[0], "display_name": item[1]} for item in MobileAppUserSettings.NOTIFICATION_TIMING_CHOICES ] return Response(choices) + + +class MobileAppGatewayView(APIView): + authentication_classes = (MobileAppAuthTokenAuthentication,) + permission_classes = (IsAuthenticated,) + + class SupportedDownstreamBackends: + INCIDENT = "incident" + + ALL_SUPPORTED_DOWNSTREAM_BACKENDS = [ + SupportedDownstreamBackends.INCIDENT, + ] + + def _determine_grafana_incident_api_url(self, organization: "Organization") -> typing.Optional[str]: + """ + If the organization already has the Grafana Incident backend URL saved, use that. + Otherwise, ask the Grafana API for the URL from the Incident plugin's settings, then persist it. + """ + if organization.grafana_incident_backend_url: + return organization.grafana_incident_backend_url + + org_pk = organization.pk + + grafana_api_client = GrafanaAPIClient(api_url=organization.grafana_url, api_token=organization.api_token) + grafana_incident_settings, _ = grafana_api_client.get_grafana_incident_plugin_settings() + if grafana_incident_settings is None: + logger.debug(f"Grafana Incident plugin settings not found for organization {org_pk}") + return None + + grafana_incident_backend_url = grafana_incident_settings["jsonData"].get( + GrafanaAPIClient.GRAFANA_INCIDENT_PLUGIN_BACKEND_URL_KEY + ) + if grafana_incident_backend_url is None: + logger.debug(f"Grafana Incident plugin settings do not contain backend URL for organization {org_pk}") + return None + + logger.debug( + f"Found Grafana Incident plugin backend URL {grafana_incident_backend_url} for organization {org_pk}. Persisting..." + ) + + organization.grafana_incident_backend_url = grafana_incident_backend_url + organization.save(update_fields=["grafana_incident_backend_url"]) + + return grafana_incident_backend_url + + def _construct_jwt(self, user: "User") -> str: + # TODO: + return "" + + def _get_downstream_headers(self, user: "User") -> typing.Dict[str, str]: + return { + "Authorization": f"Bearer {self._construct_jwt(user)}", + } + + def _get_downstream_url(self, organization: "Organization", downstream_backend: str, downstream_path: str) -> str: + downstream_url_fetcher = { + self.SupportedDownstreamBackends.INCIDENT: self._determine_grafana_incident_api_url, + }[downstream_backend] + + downstream_url = downstream_url_fetcher(organization) + + if downstream_url is None: + raise ParseError( + f"Downstream URL not found for backend {downstream_backend} for organization {organization.pk}" + ) + + return f"{downstream_url}/{downstream_path}" + + def _proxy_request(self, request, *args, **kwargs): + downstream_backend = kwargs["downstream_backend"] + downstream_path = kwargs["downstream_path"] + method = request.method + user = request.user + + if downstream_backend not in self.ALL_SUPPORTED_DOWNSTREAM_BACKENDS: + raise NotFound(f"Downstream backend {downstream_backend} not supported") + + downstream_url = self._get_downstream_url(user.organization, downstream_backend, downstream_path) + downstream_request_handler = getattr(requests, method.lower()) + + try: + # TODO: figure out how to properly proxy request body + downstream_response = downstream_request_handler( + downstream_url, + data=request.data, + params=request.query_params.dict(), + headers=self._get_downstream_headers(user), + ) + + return Response(status=downstream_response.status_code, data=downstream_response.json()) + except ( + requests.exceptions.ConnectionError, + requests.exceptions.HTTPError, + requests.exceptions.TooManyRedirects, + requests.exceptions.Timeout, + json.JSONDecodeError, + ): + logger.error( + ( + f"MobileAppGatewayView: error while proxying request\n" + f"method={method}\n" + f"downstream_backend={downstream_backend}\n" + f"downstream_path={downstream_path}\n" + f"downstream_url={downstream_url}" + ), + exc_info=True, + ) + return Response(status=status.HTTP_502_BAD_GATEWAY) + + +""" +See the default `APIView.dispatch` for more info. Basically this just routes all requests for +ALL HTTP verbs to the `MobileAppGatewayView._proxy_request` method. +""" +for method in APIView.http_method_names: + setattr(MobileAppGatewayView, method.lower(), MobileAppGatewayView._proxy_request) diff --git a/engine/apps/user_management/migrations/0018_organization_grafana_incident_backend_url.py b/engine/apps/user_management/migrations/0018_organization_grafana_incident_backend_url.py new file mode 100644 index 0000000000..dbce044391 --- /dev/null +++ b/engine/apps/user_management/migrations/0018_organization_grafana_incident_backend_url.py @@ -0,0 +1,18 @@ +# Generated by Django 4.2.7 on 2023-11-28 19:23 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('user_management', '0017_alter_organization_maintenance_author'), + ] + + operations = [ + migrations.AddField( + model_name='organization', + name='grafana_incident_backend_url', + field=models.CharField(default=None, max_length=300, null=True), + ), + ] diff --git a/engine/apps/user_management/models/organization.py b/engine/apps/user_management/models/organization.py index 70fcf8e002..4aef77d8af 100644 --- a/engine/apps/user_management/models/organization.py +++ b/engine/apps/user_management/models/organization.py @@ -248,6 +248,8 @@ def _get_subscription_strategy(self): is_rbac_permissions_enabled = models.BooleanField(default=False) is_grafana_incident_enabled = models.BooleanField(default=False) + grafana_incident_backend_url = models.CharField(max_length=300, null=True, default=None) + class Meta: unique_together = ("stack_id", "org_id") diff --git a/engine/apps/user_management/sync.py b/engine/apps/user_management/sync.py index ebbcf6fd91..a4c1662caa 100644 --- a/engine/apps/user_management/sync.py +++ b/engine/apps/user_management/sync.py @@ -115,12 +115,8 @@ def sync_users_for_teams(client: GrafanaAPIClient, organization: Organization, * def check_grafana_incident_is_enabled(client: GrafanaAPIClient) -> bool: - GRAFANA_INCIDENT_PLUGIN = "grafana-incident-app" - grafana_incident_settings, _ = client.get_grafana_plugin_settings(GRAFANA_INCIDENT_PLUGIN) - is_grafana_incident_enabled = False - if isinstance(grafana_incident_settings, dict) and grafana_incident_settings.get("enabled"): - is_grafana_incident_enabled = True - return is_grafana_incident_enabled + grafana_incident_settings, _ = client.get_grafana_incident_plugin_settings() + return False if grafana_incident_settings is None else grafana_incident_settings["enabled"] def delete_organization_if_needed(organization: Organization) -> bool: diff --git a/engine/settings/base.py b/engine/settings/base.py index 50eee03ddd..4bcc727ffb 100644 --- a/engine/settings/base.py +++ b/engine/settings/base.py @@ -711,6 +711,8 @@ class BrokerTypes: "UPDATE_ON_DUPLICATE_REG_ID": True, "USER_MODEL": "user_management.User", } +# TODO: change default to False here when done testing +MOBILE_APP_GATEWAY_ENABLED = getenv_boolean("MOBILE_APP_GATEWAY_ENABLED", default=True) SELF_HOSTED_SETTINGS = { "STACK_ID": 5, From 3eede8baa24192a9fd277b872c832b463d351d52 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Tue, 28 Nov 2023 15:05:12 -0500 Subject: [PATCH 2/9] disable MOBILE_APP_GATEWAY_ENABLED by default --- engine/settings/base.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/engine/settings/base.py b/engine/settings/base.py index 4bcc727ffb..f542b5f24b 100644 --- a/engine/settings/base.py +++ b/engine/settings/base.py @@ -711,8 +711,7 @@ class BrokerTypes: "UPDATE_ON_DUPLICATE_REG_ID": True, "USER_MODEL": "user_management.User", } -# TODO: change default to False here when done testing -MOBILE_APP_GATEWAY_ENABLED = getenv_boolean("MOBILE_APP_GATEWAY_ENABLED", default=True) +MOBILE_APP_GATEWAY_ENABLED = getenv_boolean("MOBILE_APP_GATEWAY_ENABLED", default=False) SELF_HOSTED_SETTINGS = { "STACK_ID": 5, From 2b937cd706663a0155c1ad31127cc8c87ad122d9 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Tue, 28 Nov 2023 16:07:31 -0500 Subject: [PATCH 3/9] WIP --- .../tests/test_mobile_app_gateway.py | 93 +++++++++++++++++-- engine/apps/mobile_app/views.py | 45 +++++++-- engine/requirements.txt | 1 + engine/settings/base.py | 4 + 4 files changed, 126 insertions(+), 17 deletions(-) diff --git a/engine/apps/mobile_app/tests/test_mobile_app_gateway.py b/engine/apps/mobile_app/tests/test_mobile_app_gateway.py index c3e5200e91..260553d66d 100644 --- a/engine/apps/mobile_app/tests/test_mobile_app_gateway.py +++ b/engine/apps/mobile_app/tests/test_mobile_app_gateway.py @@ -3,16 +3,30 @@ import pytest from django.urls import reverse +from django.utils import timezone from rest_framework import status from rest_framework.test import APIClient from rest_framework.views import APIView +from apps.mobile_app.views import MobileAppGatewayView + DOWNSTREAM_BACKEND = "incident" MOCK_DOWNSTREAM_INCIDENT_API_URL = "https://mockdownstreamincidentapi.com" MOCK_DOWNSTREAM_HEADERS = {"Authorization": "Bearer mock_jwt"} - MOCK_DOWNSTREAM_RESPONSE_DATA = {"foo": "bar"} +MOCK_TIMEZONE_NOW = timezone.datetime(2021, 1, 1, 3, 4, 5, tzinfo=timezone.utc) +MOCK_JWT = "mncn,zxcnv,mznxcv" +MOCK_JWT_PRIVATE_KEY = "asd,mzcxn,vmnzxcv,mnzx,cvmnzaslkdjflaksjdf" + + +@pytest.fixture(autouse=True) +def enable_urls(settings, reload_urls): + settings.MOBILE_APP_GATEWAY_ENABLED = True + settings.MOBILE_APP_GATEWAY_RSA_PRIVATE_KEY = MOCK_JWT_PRIVATE_KEY + + reload_urls() + class MockResponse: def __init__(self, status_code=status.HTTP_200_OK, data=MOCK_DOWNSTREAM_RESPONSE_DATA): @@ -300,13 +314,74 @@ def test_mobile_app_gateway_incident_api_url( assert organization.grafana_incident_backend_url == mock_incident_backend_url -# # TODO: -# @pytest.mark.django_db -# def test_mobile_app_gateway_downstream_jwt_auth(make_organization_and_user_with_mobile_app_auth_token): -# _, _, auth_token = make_organization_and_user_with_mobile_app_auth_token() +@pytest.mark.django_db +@patch("apps.mobile_app.views.requests") +@patch( + "apps.mobile_app.views.MobileAppGatewayView._construct_jwt", + return_value=MOCK_JWT, +) +@patch( + "apps.mobile_app.views.MobileAppGatewayView._determine_grafana_incident_api_url", + return_value=MOCK_DOWNSTREAM_INCIDENT_API_URL, +) +def test_mobile_app_gateway_jwt_header( + _mock_determine_grafana_incident_api_url, + _mock_construct_jwt, + mock_requests, + make_organization_and_user_with_mobile_app_auth_token, +): + path = "test/123" + mock_requests.post.return_value = MockResponse() + + _, _, auth_token = make_organization_and_user_with_mobile_app_auth_token() + + client = APIClient() + url = reverse("mobile_app:gateway", kwargs={"downstream_backend": DOWNSTREAM_BACKEND, "downstream_path": path}) -# client = APIClient() -# url = reverse("mobile_app:gateway", kwargs={"downstream_backend": DOWNSTREAM_BACKEND, "downstream_path": "test"}) + response = client.post(url, HTTP_AUTHORIZATION=auth_token) + assert response.status_code == status.HTTP_200_OK -# response = client.post(url, HTTP_AUTHORIZATION=auth_token) -# assert response.status_code == status.HTTP_403_FORBIDDEN + mock_requests.post.assert_called_once_with( + f"{MOCK_DOWNSTREAM_INCIDENT_API_URL}/{path}", + data={}, + params={}, + headers={"Authorization": f"Bearer {MOCK_JWT}"}, + ) + + +@pytest.mark.django_db +@patch("apps.mobile_app.views.jwt.encode", return_value=MOCK_JWT) +@patch("apps.mobile_app.views.timezone.now", return_value=MOCK_TIMEZONE_NOW) +def test_mobile_app_gateway_properly_generates_a_jwt( + _mock_timezone_now, + mock_jwt_encode, + make_organization, + make_user_for_organization, +): + user_id = 90095905 + stack_id = 895 + organization_id = 8905 + stack_slug = "mvcmnvcmnvc" + org_slug = "raintank" + + organization = make_organization( + stack_id=stack_id, org_id=organization_id, stack_slug=stack_slug, org_slug=org_slug + ) + user = make_user_for_organization(organization, user_id=user_id) + + encoded_jwt = MobileAppGatewayView._construct_jwt(user) + + assert encoded_jwt == MOCK_JWT + mock_jwt_encode.assert_called_once_with( + { + "iat": MOCK_TIMEZONE_NOW, + "exp": MOCK_TIMEZONE_NOW + timezone.timedelta(minutes=1), + "user_id": user.user_id, # grafana user ID + "stack_id": organization.stack_id, + "organization_id": organization.org_id, # grafana org ID + "stack_slug": organization.stack_slug, + "org_slug": organization.org_slug, + }, + MOCK_JWT_PRIVATE_KEY, + algorithm="RS256", + ) diff --git a/engine/apps/mobile_app/views.py b/engine/apps/mobile_app/views.py index f8492658a5..9d3da3d5b6 100644 --- a/engine/apps/mobile_app/views.py +++ b/engine/apps/mobile_app/views.py @@ -2,7 +2,10 @@ import logging import typing +import jwt import requests +from django.conf import settings +from django.utils import timezone from fcm_django.api.rest_framework import FCMDeviceAuthorizedViewSet as BaseFCMDeviceAuthorizedViewSet from rest_framework import mixins, status, viewsets from rest_framework.exceptions import NotFound, ParseError @@ -98,7 +101,8 @@ class SupportedDownstreamBackends: SupportedDownstreamBackends.INCIDENT, ] - def _determine_grafana_incident_api_url(self, organization: "Organization") -> typing.Optional[str]: + @classmethod + def _determine_grafana_incident_api_url(cls, organization: "Organization") -> typing.Optional[str]: """ If the organization already has the Grafana Incident backend URL saved, use that. Otherwise, ask the Grafana API for the URL from the Incident plugin's settings, then persist it. @@ -130,18 +134,43 @@ def _determine_grafana_incident_api_url(self, organization: "Organization") -> t return grafana_incident_backend_url - def _construct_jwt(self, user: "User") -> str: - # TODO: - return "" + @classmethod + def _construct_jwt_payload(cls, user: "User") -> typing.Dict[str, typing.Any]: + organization = user.organization + now = timezone.now() - def _get_downstream_headers(self, user: "User") -> typing.Dict[str, str]: return { - "Authorization": f"Bearer {self._construct_jwt(user)}", + # registered claim names + "iat": now, + "exp": now + timezone.timedelta(minutes=1), # jwt is short lived. expires in 1 minute. + # custom data + "user_id": user.user_id, # grafana user ID + "stack_id": organization.stack_id, + "organization_id": organization.org_id, # grafana org ID + "stack_slug": organization.stack_slug, + "org_slug": organization.org_slug, } - def _get_downstream_url(self, organization: "Organization", downstream_backend: str, downstream_path: str) -> str: + @classmethod + def _construct_jwt(cls, user: "User") -> str: + """ + RS256 = asymmetric = public/private key pair + HS256 = symmetric = shared secret (don't use this) + """ + return jwt.encode( + cls._construct_jwt_payload(user), settings.MOBILE_APP_GATEWAY_RSA_PRIVATE_KEY, algorithm="RS256" + ) + + @classmethod + def _get_downstream_headers(cls, user: "User") -> typing.Dict[str, str]: + return { + "Authorization": f"Bearer {cls._construct_jwt(user)}", + } + + @classmethod + def _get_downstream_url(cls, organization: "Organization", downstream_backend: str, downstream_path: str) -> str: downstream_url_fetcher = { - self.SupportedDownstreamBackends.INCIDENT: self._determine_grafana_incident_api_url, + cls.SupportedDownstreamBackends.INCIDENT: cls._determine_grafana_incident_api_url, }[downstream_backend] downstream_url = downstream_url_fetcher(organization) diff --git a/engine/requirements.txt b/engine/requirements.txt index e72530481a..e03f183629 100644 --- a/engine/requirements.txt +++ b/engine/requirements.txt @@ -54,3 +54,4 @@ lxml==4.9.2 babel==2.12.1 drf-spectacular==0.26.5 grpcio==1.57.0 +PyJWT==2.8.0 diff --git a/engine/settings/base.py b/engine/settings/base.py index f542b5f24b..87939cca30 100644 --- a/engine/settings/base.py +++ b/engine/settings/base.py @@ -711,7 +711,11 @@ class BrokerTypes: "UPDATE_ON_DUPLICATE_REG_ID": True, "USER_MODEL": "user_management.User", } + MOBILE_APP_GATEWAY_ENABLED = getenv_boolean("MOBILE_APP_GATEWAY_ENABLED", default=False) +MOBILE_APP_GATEWAY_RSA_PRIVATE_KEY = os.environ.get("MOBILE_APP_GATEWAY_RSA_PRIVATE_KEY", None) +if MOBILE_APP_GATEWAY_ENABLED and not MOBILE_APP_GATEWAY_RSA_PRIVATE_KEY: + raise RuntimeError("MOBILE_APP_GATEWAY_RSA_PRIVATE_KEY is required when MOBILE_APP_GATEWAY_ENABLED is True") SELF_HOSTED_SETTINGS = { "STACK_ID": 5, From 72f53519cf84e5087fbdf23ef34d496aad566930 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Tue, 28 Nov 2023 16:33:48 -0500 Subject: [PATCH 4/9] WIP --- .../tests/test_mobile_app_gateway.py | 140 +++++------------- engine/apps/mobile_app/views.py | 40 +---- engine/apps/user_management/sync.py | 14 +- .../apps/user_management/tests/test_sync.py | 43 +++--- 4 files changed, 67 insertions(+), 170 deletions(-) diff --git a/engine/apps/mobile_app/tests/test_mobile_app_gateway.py b/engine/apps/mobile_app/tests/test_mobile_app_gateway.py index 260553d66d..cfc7e05e44 100644 --- a/engine/apps/mobile_app/tests/test_mobile_app_gateway.py +++ b/engine/apps/mobile_app/tests/test_mobile_app_gateway.py @@ -11,6 +11,7 @@ from apps.mobile_app.views import MobileAppGatewayView DOWNSTREAM_BACKEND = "incident" +MOCK_DOWNSTREAM_URL = "https://mockdownstream.com" MOCK_DOWNSTREAM_INCIDENT_API_URL = "https://mockdownstreamincidentapi.com" MOCK_DOWNSTREAM_HEADERS = {"Authorization": "Bearer mock_jwt"} MOCK_DOWNSTREAM_RESPONSE_DATA = {"foo": "bar"} @@ -39,22 +40,19 @@ def json(self): @pytest.mark.django_db @patch("apps.mobile_app.views.requests") -@patch( - "apps.mobile_app.views.MobileAppGatewayView._determine_grafana_incident_api_url", - return_value=MOCK_DOWNSTREAM_INCIDENT_API_URL, -) @patch("apps.mobile_app.views.MobileAppGatewayView._get_downstream_headers", return_value=MOCK_DOWNSTREAM_HEADERS) @pytest.mark.parametrize("path", ["", "thing", "thing/123", "thing/123/otherthing", "thing/123/otherthing/456"]) def test_mobile_app_gateway_properly_proxies_paths( _mock_get_downstream_headers, - _mock_determine_grafana_incident_api_url, mock_requests, make_organization_and_user_with_mobile_app_auth_token, path, ): mock_requests.post.return_value = MockResponse() - _, _, auth_token = make_organization_and_user_with_mobile_app_auth_token() + org, _, auth_token = make_organization_and_user_with_mobile_app_auth_token() + org.grafana_incident_backend_url = MOCK_DOWNSTREAM_INCIDENT_API_URL + org.save() client = APIClient() url = reverse("mobile_app:gateway", kwargs={"downstream_backend": DOWNSTREAM_BACKEND, "downstream_path": path}) @@ -73,15 +71,12 @@ def test_mobile_app_gateway_properly_proxies_paths( @pytest.mark.django_db @patch("apps.mobile_app.views.requests") -@patch( - "apps.mobile_app.views.MobileAppGatewayView._determine_grafana_incident_api_url", - return_value=MOCK_DOWNSTREAM_INCIDENT_API_URL, -) +@patch("apps.mobile_app.views.MobileAppGatewayView._get_downstream_url", return_value=MOCK_DOWNSTREAM_URL) @patch("apps.mobile_app.views.MobileAppGatewayView._get_downstream_headers", return_value=MOCK_DOWNSTREAM_HEADERS) @pytest.mark.parametrize("method", APIView.http_method_names) def test_mobile_app_gateway_supports_all_methods( _mock_get_downstream_headers, - _mock_determine_grafana_incident_api_url, + _mock_get_downstream_url, mock_requests, make_organization_and_user_with_mobile_app_auth_token, method, @@ -102,30 +97,26 @@ def test_mobile_app_gateway_supports_all_methods( @pytest.mark.django_db @patch("apps.mobile_app.views.requests") -@patch( - "apps.mobile_app.views.MobileAppGatewayView._determine_grafana_incident_api_url", - return_value=MOCK_DOWNSTREAM_INCIDENT_API_URL, -) +@patch("apps.mobile_app.views.MobileAppGatewayView._get_downstream_url", return_value=MOCK_DOWNSTREAM_URL) @patch("apps.mobile_app.views.MobileAppGatewayView._get_downstream_headers", return_value=MOCK_DOWNSTREAM_HEADERS) def test_mobile_app_gateway_proxies_query_params( _mock_get_downstream_headers, - _mock_determine_grafana_incident_api_url, + _mock_get_downstream_url, mock_requests, make_organization_and_user_with_mobile_app_auth_token, ): - path = "test/123" mock_requests.post.return_value = MockResponse() _, _, auth_token = make_organization_and_user_with_mobile_app_auth_token() client = APIClient() - url = reverse("mobile_app:gateway", kwargs={"downstream_backend": DOWNSTREAM_BACKEND, "downstream_path": path}) + url = reverse("mobile_app:gateway", kwargs={"downstream_backend": DOWNSTREAM_BACKEND, "downstream_path": "test"}) response = client.post(f"{url}?foo=bar&baz=hello", HTTP_AUTHORIZATION=auth_token) assert response.status_code == status.HTTP_200_OK mock_requests.post.assert_called_once_with( - f"{MOCK_DOWNSTREAM_INCIDENT_API_URL}/{path}", + MOCK_DOWNSTREAM_URL, data={}, params={"foo": "bar", "baz": "hello"}, headers=MOCK_DOWNSTREAM_HEADERS, @@ -134,10 +125,7 @@ def test_mobile_app_gateway_proxies_query_params( @pytest.mark.django_db @patch("apps.mobile_app.views.requests") -@patch( - "apps.mobile_app.views.MobileAppGatewayView._determine_grafana_incident_api_url", - return_value=MOCK_DOWNSTREAM_INCIDENT_API_URL, -) +@patch("apps.mobile_app.views.MobileAppGatewayView._get_downstream_url", return_value=MOCK_DOWNSTREAM_URL) @patch("apps.mobile_app.views.MobileAppGatewayView._get_downstream_headers", return_value=MOCK_DOWNSTREAM_HEADERS) @pytest.mark.parametrize( "upstream_request_body", @@ -149,18 +137,17 @@ def test_mobile_app_gateway_proxies_query_params( ) def test_mobile_app_gateway_properly_proxies_request_body( _mock_get_downstream_headers, - _mock_determine_grafana_incident_api_url, + _mock_get_downstream_url, mock_requests, make_organization_and_user_with_mobile_app_auth_token, upstream_request_body, ): - path = "test/123" mock_requests.post.return_value = MockResponse() _, _, auth_token = make_organization_and_user_with_mobile_app_auth_token() client = APIClient() - url = reverse("mobile_app:gateway", kwargs={"downstream_backend": DOWNSTREAM_BACKEND, "downstream_path": path}) + url = reverse("mobile_app:gateway", kwargs={"downstream_backend": DOWNSTREAM_BACKEND, "downstream_path": "test"}) response = client.post( url, @@ -171,7 +158,7 @@ def test_mobile_app_gateway_properly_proxies_request_body( assert response.status_code == status.HTTP_200_OK mock_requests.post.assert_called_once_with( - f"{MOCK_DOWNSTREAM_INCIDENT_API_URL}/{path}", + MOCK_DOWNSTREAM_URL, data=upstream_request_body, params={}, headers=MOCK_DOWNSTREAM_HEADERS, @@ -180,10 +167,7 @@ def test_mobile_app_gateway_properly_proxies_request_body( @pytest.mark.django_db @patch("apps.mobile_app.views.requests") -@patch( - "apps.mobile_app.views.MobileAppGatewayView._determine_grafana_incident_api_url", - return_value=MOCK_DOWNSTREAM_INCIDENT_API_URL, -) +@patch("apps.mobile_app.views.MobileAppGatewayView._get_downstream_url", return_value=MOCK_DOWNSTREAM_URL) @patch("apps.mobile_app.views.MobileAppGatewayView._get_downstream_headers", return_value=MOCK_DOWNSTREAM_HEADERS) @pytest.mark.parametrize( "downstream_backend,expected_status", @@ -194,7 +178,7 @@ def test_mobile_app_gateway_properly_proxies_request_body( ) def test_mobile_app_gateway_supported_downstream_backends( _mock_get_downstream_headers, - _mock_determine_grafana_incident_api_url, + _mock_get_downstream_url, mock_requests, make_organization_and_user_with_mobile_app_auth_token, downstream_backend, @@ -215,14 +199,11 @@ def test_mobile_app_gateway_supported_downstream_backends( @pytest.mark.django_db @patch("apps.mobile_app.views.requests") -@patch( - "apps.mobile_app.views.MobileAppGatewayView._determine_grafana_incident_api_url", - return_value=MOCK_DOWNSTREAM_INCIDENT_API_URL, -) +@patch("apps.mobile_app.views.MobileAppGatewayView._get_downstream_url", return_value=MOCK_DOWNSTREAM_URL) @patch("apps.mobile_app.views.MobileAppGatewayView._get_downstream_headers", return_value=MOCK_DOWNSTREAM_HEADERS) def test_mobile_app_gateway_mobile_app_auth_token( _mock_get_downstream_headers, - _mock_determine_grafana_incident_api_url, + _mock_get_downstream_url, mock_requests, make_organization_and_user_with_mobile_app_auth_token, ): @@ -231,9 +212,7 @@ def test_mobile_app_gateway_mobile_app_auth_token( _, _, auth_token = make_organization_and_user_with_mobile_app_auth_token() client = APIClient() - url = reverse( - "mobile_app:gateway", kwargs={"downstream_backend": DOWNSTREAM_BACKEND, "downstream_path": "test/123"} - ) + url = reverse("mobile_app:gateway", kwargs={"downstream_backend": DOWNSTREAM_BACKEND, "downstream_path": "test"}) response = client.post(url, HTTP_AUTHORIZATION="potato") assert response.status_code == status.HTTP_403_FORBIDDEN @@ -254,95 +233,46 @@ def test_mobile_app_gateway_incident_api_url( mock_requests.post.return_value = MockResponse() client = APIClient() - url = reverse( - "mobile_app:gateway", kwargs={"downstream_backend": DOWNSTREAM_BACKEND, "downstream_path": "test/123"} - ) - - # Grafana API returns None for the incident plugin settings - with patch( - "apps.mobile_app.views.GrafanaAPIClient.get_grafana_incident_plugin_settings" - ) as mock_get_grafana_incident_plugin_settings: - organization, _, auth_token = make_organization_and_user_with_mobile_app_auth_token() - assert organization.grafana_incident_backend_url is None - - mock_get_grafana_incident_plugin_settings.return_value = (None, None) - - response = client.post(url, HTTP_AUTHORIZATION=auth_token) - assert response.status_code == status.HTTP_400_BAD_REQUEST - - # Incident plugin settings jsonData doesn't contain the backend URL - with patch( - "apps.mobile_app.views.GrafanaAPIClient.get_grafana_incident_plugin_settings" - ) as mock_get_grafana_incident_plugin_settings: - organization, _, auth_token = make_organization_and_user_with_mobile_app_auth_token() - assert organization.grafana_incident_backend_url is None + url = reverse("mobile_app:gateway", kwargs={"downstream_backend": DOWNSTREAM_BACKEND, "downstream_path": "test"}) - mock_get_grafana_incident_plugin_settings.return_value = ({"jsonData": {}}, None) + # Organization has no incident backend URL saved + organization, _, auth_token = make_organization_and_user_with_mobile_app_auth_token() + assert organization.grafana_incident_backend_url is None - response = client.post(url, HTTP_AUTHORIZATION=auth_token) - assert response.status_code == status.HTTP_400_BAD_REQUEST + response = client.post(url, HTTP_AUTHORIZATION=auth_token) + assert response.status_code == status.HTTP_400_BAD_REQUEST # Organization already has incident backend URL saved - with patch( - "apps.mobile_app.views.GrafanaAPIClient.get_grafana_incident_plugin_settings" - ) as mock_get_grafana_incident_plugin_settings: - organization, _, auth_token = make_organization_and_user_with_mobile_app_auth_token() - organization.grafana_incident_backend_url = mock_incident_backend_url - organization.save() - - response = client.post(url, HTTP_AUTHORIZATION=auth_token) - assert response.status_code == status.HTTP_200_OK - mock_get_grafana_incident_plugin_settings.assert_not_called() - - # Incident plugin settings jsonData contains the backend URL - # check that it gets saved to the organization - with patch( - "apps.mobile_app.views.GrafanaAPIClient.get_grafana_incident_plugin_settings" - ) as mock_get_grafana_incident_plugin_settings: - organization, _, auth_token = make_organization_and_user_with_mobile_app_auth_token() - assert organization.grafana_incident_backend_url is None - - mock_get_grafana_incident_plugin_settings.return_value = ( - {"jsonData": {"backendUrl": mock_incident_backend_url}}, - None, - ) + organization, _, auth_token = make_organization_and_user_with_mobile_app_auth_token() + organization.grafana_incident_backend_url = mock_incident_backend_url + organization.save() - response = client.post(url, HTTP_AUTHORIZATION=auth_token) - assert response.status_code == status.HTTP_200_OK - - organization.refresh_from_db() - assert organization.grafana_incident_backend_url == mock_incident_backend_url + response = client.post(url, HTTP_AUTHORIZATION=auth_token) + assert response.status_code == status.HTTP_200_OK @pytest.mark.django_db @patch("apps.mobile_app.views.requests") -@patch( - "apps.mobile_app.views.MobileAppGatewayView._construct_jwt", - return_value=MOCK_JWT, -) -@patch( - "apps.mobile_app.views.MobileAppGatewayView._determine_grafana_incident_api_url", - return_value=MOCK_DOWNSTREAM_INCIDENT_API_URL, -) +@patch("apps.mobile_app.views.MobileAppGatewayView._construct_jwt", return_value=MOCK_JWT) +@patch("apps.mobile_app.views.MobileAppGatewayView._get_downstream_url", return_value=MOCK_DOWNSTREAM_URL) def test_mobile_app_gateway_jwt_header( - _mock_determine_grafana_incident_api_url, + _mock_get_downstream_url, _mock_construct_jwt, mock_requests, make_organization_and_user_with_mobile_app_auth_token, ): - path = "test/123" mock_requests.post.return_value = MockResponse() _, _, auth_token = make_organization_and_user_with_mobile_app_auth_token() client = APIClient() - url = reverse("mobile_app:gateway", kwargs={"downstream_backend": DOWNSTREAM_BACKEND, "downstream_path": path}) + url = reverse("mobile_app:gateway", kwargs={"downstream_backend": DOWNSTREAM_BACKEND, "downstream_path": "test"}) response = client.post(url, HTTP_AUTHORIZATION=auth_token) assert response.status_code == status.HTTP_200_OK mock_requests.post.assert_called_once_with( - f"{MOCK_DOWNSTREAM_INCIDENT_API_URL}/{path}", + MOCK_DOWNSTREAM_URL, data={}, params={}, headers={"Authorization": f"Bearer {MOCK_JWT}"}, diff --git a/engine/apps/mobile_app/views.py b/engine/apps/mobile_app/views.py index 9d3da3d5b6..26fd486633 100644 --- a/engine/apps/mobile_app/views.py +++ b/engine/apps/mobile_app/views.py @@ -13,7 +13,6 @@ from rest_framework.response import Response from rest_framework.views import APIView -from apps.grafana_plugin.helpers.client import GrafanaAPIClient from apps.mobile_app.auth import MobileAppAuthTokenAuthentication, MobileAppVerificationTokenAuthentication from apps.mobile_app.models import MobileAppAuthToken, MobileAppUserSettings from apps.mobile_app.serializers import MobileAppUserSettingsSerializer @@ -101,39 +100,6 @@ class SupportedDownstreamBackends: SupportedDownstreamBackends.INCIDENT, ] - @classmethod - def _determine_grafana_incident_api_url(cls, organization: "Organization") -> typing.Optional[str]: - """ - If the organization already has the Grafana Incident backend URL saved, use that. - Otherwise, ask the Grafana API for the URL from the Incident plugin's settings, then persist it. - """ - if organization.grafana_incident_backend_url: - return organization.grafana_incident_backend_url - - org_pk = organization.pk - - grafana_api_client = GrafanaAPIClient(api_url=organization.grafana_url, api_token=organization.api_token) - grafana_incident_settings, _ = grafana_api_client.get_grafana_incident_plugin_settings() - if grafana_incident_settings is None: - logger.debug(f"Grafana Incident plugin settings not found for organization {org_pk}") - return None - - grafana_incident_backend_url = grafana_incident_settings["jsonData"].get( - GrafanaAPIClient.GRAFANA_INCIDENT_PLUGIN_BACKEND_URL_KEY - ) - if grafana_incident_backend_url is None: - logger.debug(f"Grafana Incident plugin settings do not contain backend URL for organization {org_pk}") - return None - - logger.debug( - f"Found Grafana Incident plugin backend URL {grafana_incident_backend_url} for organization {org_pk}. Persisting..." - ) - - organization.grafana_incident_backend_url = grafana_incident_backend_url - organization.save(update_fields=["grafana_incident_backend_url"]) - - return grafana_incident_backend_url - @classmethod def _construct_jwt_payload(cls, user: "User") -> typing.Dict[str, typing.Any]: organization = user.organization @@ -169,12 +135,10 @@ def _get_downstream_headers(cls, user: "User") -> typing.Dict[str, str]: @classmethod def _get_downstream_url(cls, organization: "Organization", downstream_backend: str, downstream_path: str) -> str: - downstream_url_fetcher = { - cls.SupportedDownstreamBackends.INCIDENT: cls._determine_grafana_incident_api_url, + downstream_url = { + cls.SupportedDownstreamBackends.INCIDENT: organization.grafana_incident_backend_url, }[downstream_backend] - downstream_url = downstream_url_fetcher(organization) - if downstream_url is None: raise ParseError( f"Downstream URL not found for backend {downstream_backend} for organization {organization.pk}" diff --git a/engine/apps/user_management/sync.py b/engine/apps/user_management/sync.py index a4c1662caa..f28a3a756f 100644 --- a/engine/apps/user_management/sync.py +++ b/engine/apps/user_management/sync.py @@ -36,7 +36,13 @@ def sync_organization(organization: Organization) -> None: organization.api_token_status = Organization.API_TOKEN_STATUS_OK sync_users_and_teams(grafana_api_client, organization) organization.last_time_synced = timezone.now() - organization.is_grafana_incident_enabled = check_grafana_incident_is_enabled(grafana_api_client) + + grafana_incident_settings, _ = grafana_api_client.get_grafana_incident_plugin_settings() + if grafana_incident_settings is not None: + organization.is_grafana_incident_enabled = grafana_incident_settings["enabled"] + organization.grafana_incident_backend_url = grafana_incident_settings["jsonData"].get( + GrafanaAPIClient.GRAFANA_INCIDENT_PLUGIN_BACKEND_URL_KEY + ) else: organization.api_token_status = Organization.API_TOKEN_STATUS_FAILED @@ -53,6 +59,7 @@ def sync_organization(organization: Organization) -> None: "gcom_token_org_last_time_synced", "is_rbac_permissions_enabled", "is_grafana_incident_enabled", + "grafana_incident_backend_url", ] ) @@ -114,11 +121,6 @@ def sync_users_for_teams(client: GrafanaAPIClient, organization: Organization, * Team.objects.sync_for_organization(organization=organization, api_teams=api_teams) -def check_grafana_incident_is_enabled(client: GrafanaAPIClient) -> bool: - grafana_incident_settings, _ = client.get_grafana_incident_plugin_settings() - return False if grafana_incident_settings is None else grafana_incident_settings["enabled"] - - def delete_organization_if_needed(organization: Organization) -> bool: # Organization has a manually set API token, it will not be found within GCOM # and would need to be deleted manually. diff --git a/engine/apps/user_management/tests/test_sync.py b/engine/apps/user_management/tests/test_sync.py index 7153aa5cc5..e5627ad189 100644 --- a/engine/apps/user_management/tests/test_sync.py +++ b/engine/apps/user_management/tests/test_sync.py @@ -8,7 +8,9 @@ from apps.api.permissions import LegacyAccessControlRole from apps.grafana_plugin.helpers.client import GcomAPIClient, GrafanaAPIClient from apps.user_management.models import Team, User -from apps.user_management.sync import check_grafana_incident_is_enabled, cleanup_organization, sync_organization +from apps.user_management.sync import cleanup_organization, sync_organization + +MOCK_GRAFANA_INCIDENT_BACKEND_URL = "https://grafana-incident.test" @pytest.mark.django_db @@ -200,11 +202,15 @@ def test_sync_users_for_team(make_organization, make_user_for_organization, make ), ) @patch.object(GrafanaAPIClient, "check_token", return_value=(None, {"connected": True})) -@patch.object(GrafanaAPIClient, "get_grafana_plugin_settings", return_value=({"enabled": True}, None)) +@patch.object( + GrafanaAPIClient, + "get_grafana_incident_plugin_settings", + return_value=({"enabled": True, "jsonData": {"backendUrl": MOCK_GRAFANA_INCIDENT_BACKEND_URL}}, None), +) @patch("apps.user_management.sync.org_sync_signal") def test_sync_organization( mocked_org_sync_signal, - _mock_get_grafana_plugin_settings, + _mock_get_grafana_incident_plugin_settings, _mock_check_token, _mock_get_teams, _mock_get_users, @@ -243,6 +249,7 @@ def test_sync_organization( # check that is_grafana_incident_enabled flag is set assert organization.is_grafana_incident_enabled is True + assert organization.grafana_incident_backend_url == MOCK_GRAFANA_INCIDENT_BACKEND_URL mocked_org_sync_signal.send.assert_called_once_with(sender=None, organization=organization) @@ -294,7 +301,12 @@ def test_sync_organization_is_rbac_permissions_enabled_open_source(make_organiza GrafanaAPIClient, "check_token", return_value=(None, api_check_token_call_status) ): with patch.object( - GrafanaAPIClient, "get_grafana_plugin_settings", return_value=({"enabled": True}, None) + GrafanaAPIClient, + "get_grafana_incident_plugin_settings", + return_value=( + {"enabled": True, "jsonData": {"backendUrl": MOCK_GRAFANA_INCIDENT_BACKEND_URL}}, + None, + ), ): sync_organization(organization) @@ -352,7 +364,12 @@ def test_sync_organization_is_rbac_permissions_enabled_cloud(mocked_gcom_client, with patch.object(GrafanaAPIClient, "get_teams", return_value=(api_teams_response, None)): with patch.object(GrafanaAPIClient, "get_team_members", return_value=(api_members_response, None)): with patch.object( - GrafanaAPIClient, "get_grafana_plugin_settings", return_value=({"enabled": True}, None) + GrafanaAPIClient, + "get_grafana_incident_plugin_settings", + return_value=( + {"enabled": True, "jsonData": {"backendUrl": MOCK_GRAFANA_INCIDENT_BACKEND_URL}}, + None, + ), ): sync_organization(organization) @@ -406,19 +423,3 @@ def test_cleanup_organization_deleted(make_organization): organization.refresh_from_db() assert organization.deleted_at is not None - - -@pytest.mark.django_db -@pytest.mark.parametrize( - "response,expected_result", - [ - (({"enabled": True}, {}), True), - (({"enabled": False}, {}), False), - ((None, {}), False), - ], -) -def test_check_grafana_incident_is_enabled(response, expected_result): - client = GrafanaAPIClient("", "") - with patch.object(GrafanaAPIClient, "get_grafana_plugin_settings", return_value=response): - result = check_grafana_incident_is_enabled(client) - assert result == expected_result From 120046d57dd12de6ec922f645a73de531e21fd34 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Tue, 28 Nov 2023 16:38:17 -0500 Subject: [PATCH 5/9] remove outdated todo comment --- engine/apps/mobile_app/views.py | 1 - 1 file changed, 1 deletion(-) diff --git a/engine/apps/mobile_app/views.py b/engine/apps/mobile_app/views.py index 26fd486633..378ac3bd59 100644 --- a/engine/apps/mobile_app/views.py +++ b/engine/apps/mobile_app/views.py @@ -159,7 +159,6 @@ def _proxy_request(self, request, *args, **kwargs): downstream_request_handler = getattr(requests, method.lower()) try: - # TODO: figure out how to properly proxy request body downstream_response = downstream_request_handler( downstream_url, data=request.data, From bd303e003340ec5322ab072ccb18a3d8a5e42b8e Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Tue, 28 Nov 2023 17:11:10 -0500 Subject: [PATCH 6/9] update changelog --- CHANGELOG.md | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d123b651a9..90bced35a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +### Added + +- Add proxying capabilities for the OnCall mobile API by @joeyorlando ([#3449](https://github.com/grafana/oncall/pull/3449)) + ### Fixed - User profile UI tweaks ([#3443](https://github.com/grafana/oncall/pull/3443)) @@ -22,9 +26,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Add ability to use Grafana Service Account Tokens for OnCall API (This is only enabled for resolution_notes -endpoint currently) @mderynck ([#3189](https://github.com/grafana/oncall/pull/3189)) + endpoint currently) @mderynck ([#3189](https://github.com/grafana/oncall/pull/3189)) - Add ability for webhook presets to mask sensitive headers @mderynck -([#3189](https://github.com/grafana/oncall/pull/3189)) + ([#3189](https://github.com/grafana/oncall/pull/3189)) ### Changed @@ -33,7 +37,7 @@ endpoint currently) @mderynck ([#3189](https://github.com/grafana/oncall/pull/31 ### Fixed - Fixed issue that blocked saving webhooks with presets if the preset is controlling the URL @mderynck -([#3189](https://github.com/grafana/oncall/pull/3189)) + ([#3189](https://github.com/grafana/oncall/pull/3189)) - User filter doesn't display current value on Alert Groups page ([1714](https://github.com/grafana/oncall/issues/1714)) - Remove displaying rotation modal for Terraform/API based schedules - Filters polishing ([3183](https://github.com/grafana/oncall/issues/3183)) From efe7f114f54c6752d4260466c4f09c915f0585c0 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Wed, 29 Nov 2023 08:10:43 -0500 Subject: [PATCH 7/9] address PR comment --- .../tests/test_mobile_app_gateway.py | 38 +++++++++++++++++++ engine/apps/mobile_app/views.py | 20 +++++----- 2 files changed, 49 insertions(+), 9 deletions(-) diff --git a/engine/apps/mobile_app/tests/test_mobile_app_gateway.py b/engine/apps/mobile_app/tests/test_mobile_app_gateway.py index cfc7e05e44..a6d3139cec 100644 --- a/engine/apps/mobile_app/tests/test_mobile_app_gateway.py +++ b/engine/apps/mobile_app/tests/test_mobile_app_gateway.py @@ -2,6 +2,7 @@ from unittest.mock import patch import pytest +import requests from django.urls import reverse from django.utils import timezone from rest_framework import status @@ -197,6 +198,43 @@ def test_mobile_app_gateway_supported_downstream_backends( assert response.status_code == expected_status +@pytest.mark.django_db +@patch("apps.mobile_app.views.requests.post") +@patch("apps.mobile_app.views.MobileAppGatewayView._get_downstream_url", return_value=MOCK_DOWNSTREAM_URL) +@patch("apps.mobile_app.views.MobileAppGatewayView._get_downstream_headers", return_value=MOCK_DOWNSTREAM_HEADERS) +@pytest.mark.parametrize( + "ExceptionClass,exception_args,expected_status", + [ + (requests.exceptions.ConnectionError, (), status.HTTP_502_BAD_GATEWAY), + (requests.exceptions.HTTPError, (), status.HTTP_502_BAD_GATEWAY), + (requests.exceptions.TooManyRedirects, (), status.HTTP_502_BAD_GATEWAY), + (requests.exceptions.Timeout, (), status.HTTP_502_BAD_GATEWAY), + (requests.exceptions.JSONDecodeError, ("", "", 5), status.HTTP_400_BAD_REQUEST), + ], +) +def test_mobile_app_gateway_catches_errors_from_downstream_server( + _mock_get_downstream_headers, + _mock_get_downstream_url, + mock_requests_post, + make_organization_and_user_with_mobile_app_auth_token, + ExceptionClass, + exception_args, + expected_status, +): + def _raise_exception(*args, **kwargs): + raise ExceptionClass(*exception_args) + + mock_requests_post.side_effect = _raise_exception + + _, _, auth_token = make_organization_and_user_with_mobile_app_auth_token() + + client = APIClient() + url = reverse("mobile_app:gateway", kwargs={"downstream_backend": DOWNSTREAM_BACKEND, "downstream_path": "test"}) + + response = client.post(url, HTTP_AUTHORIZATION=auth_token) + assert response.status_code == expected_status + + @pytest.mark.django_db @patch("apps.mobile_app.views.requests") @patch("apps.mobile_app.views.MobileAppGatewayView._get_downstream_url", return_value=MOCK_DOWNSTREAM_URL) diff --git a/engine/apps/mobile_app/views.py b/engine/apps/mobile_app/views.py index 378ac3bd59..5b2d333beb 100644 --- a/engine/apps/mobile_app/views.py +++ b/engine/apps/mobile_app/views.py @@ -1,4 +1,3 @@ -import json import logging import typing @@ -168,23 +167,26 @@ def _proxy_request(self, request, *args, **kwargs): return Response(status=downstream_response.status_code, data=downstream_response.json()) except ( - requests.exceptions.ConnectionError, - requests.exceptions.HTTPError, - requests.exceptions.TooManyRedirects, - requests.exceptions.Timeout, - json.JSONDecodeError, - ): + requests.exceptions.RequestException, + requests.exceptions.JSONDecodeError, + ) as e: + if isinstance(e, requests.exceptions.JSONDecodeError): + final_status = status.HTTP_400_BAD_REQUEST + else: + final_status = status.HTTP_502_BAD_GATEWAY + logger.error( ( f"MobileAppGatewayView: error while proxying request\n" f"method={method}\n" f"downstream_backend={downstream_backend}\n" f"downstream_path={downstream_path}\n" - f"downstream_url={downstream_url}" + f"downstream_url={downstream_url}\n" + f"final_status={final_status}" ), exc_info=True, ) - return Response(status=status.HTTP_502_BAD_GATEWAY) + return Response(status=final_status) """ From 98569264cfa0c80a49333d584a5a72bb1feb3437 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Wed, 29 Nov 2023 10:44:45 -0500 Subject: [PATCH 8/9] update migration file --- ...l.py => 0019_organization_grafana_incident_backend_url.py} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename engine/apps/user_management/migrations/{0018_organization_grafana_incident_backend_url.py => 0019_organization_grafana_incident_backend_url.py} (73%) diff --git a/engine/apps/user_management/migrations/0018_organization_grafana_incident_backend_url.py b/engine/apps/user_management/migrations/0019_organization_grafana_incident_backend_url.py similarity index 73% rename from engine/apps/user_management/migrations/0018_organization_grafana_incident_backend_url.py rename to engine/apps/user_management/migrations/0019_organization_grafana_incident_backend_url.py index dbce044391..6175a0156f 100644 --- a/engine/apps/user_management/migrations/0018_organization_grafana_incident_backend_url.py +++ b/engine/apps/user_management/migrations/0019_organization_grafana_incident_backend_url.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.7 on 2023-11-28 19:23 +# Generated by Django 4.2.7 on 2023-11-29 15:44 from django.db import migrations, models @@ -6,7 +6,7 @@ class Migration(migrations.Migration): dependencies = [ - ('user_management', '0017_alter_organization_maintenance_author'), + ('user_management', '0018_auto_20231115_1206'), ] operations = [ From 4ffdff3df4d384c9c23e81bdf59adb11bd922daa Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Wed, 29 Nov 2023 11:28:33 -0500 Subject: [PATCH 9/9] update tests --- .../tests/test_mobile_app_gateway.py | 4 ++-- engine/conftest.py | 12 +++++++++--- engine/settings/ci-test.py | 18 ++++++++++++++++++ 3 files changed, 29 insertions(+), 5 deletions(-) diff --git a/engine/apps/mobile_app/tests/test_mobile_app_gateway.py b/engine/apps/mobile_app/tests/test_mobile_app_gateway.py index a6d3139cec..b0bbcd0f04 100644 --- a/engine/apps/mobile_app/tests/test_mobile_app_gateway.py +++ b/engine/apps/mobile_app/tests/test_mobile_app_gateway.py @@ -23,11 +23,11 @@ @pytest.fixture(autouse=True) -def enable_urls(settings, reload_urls): +def setup_urls(settings, reload_urls): settings.MOBILE_APP_GATEWAY_ENABLED = True settings.MOBILE_APP_GATEWAY_RSA_PRIVATE_KEY = MOCK_JWT_PRIVATE_KEY - reload_urls() + reload_urls("apps.mobile_app.urls") class MockResponse: diff --git a/engine/conftest.py b/engine/conftest.py index 1698befec8..d25de418a4 100644 --- a/engine/conftest.py +++ b/engine/conftest.py @@ -870,13 +870,19 @@ def reload_urls(settings): Reloads Django URLs, especially useful when testing conditionally registered URLs """ - def _reload_urls(): + def _reload_urls(app_url_file_to_reload: str = None): clear_url_caches() + + # this can be useful when testing conditionally registered URLs + # for example when a django app's urls.py file has conditional logic that is being + # overriden/tested, you will need to reload that urls.py file before relaoding the ROOT_URLCONF file + if app_url_file_to_reload is not None: + reload(import_module(app_url_file_to_reload)) + urlconf = settings.ROOT_URLCONF if urlconf in sys.modules: reload(sys.modules[urlconf]) - else: - import_module(urlconf) + import_module(urlconf) return _reload_urls diff --git a/engine/settings/ci-test.py b/engine/settings/ci-test.py index 9751c49d09..eba6152511 100644 --- a/engine/settings/ci-test.py +++ b/engine/settings/ci-test.py @@ -40,3 +40,21 @@ TWILIO_AUTH_TOKEN = "dummy_twilio_auth_token" EXTRA_MESSAGING_BACKENDS = [("apps.base.tests.messaging_backend.TestOnlyBackend", 42)] + +# if you have django-silk enabled when running the tests it can lead to some weird errors like: +# RuntimeError: Database access not allowed, use the "django_db" mark, or the "db" or "transactional_db" +# fixtures to enable it. +# +# ERROR django.request:log.py:241 Internal Server Error: /startupprobe/ +# Traceback (most recent call last): +# File "/usr/local/lib/python3.11/site-packages/django/core/handlers/exception.py", line 55, in inner +# response = get_response(request) +# ^^^^^^^^^^^^^^^^^^^^^ +# File "/usr/local/lib/python3.11/site-packages/silk/middleware.py", line 70, in __call__ +# self.process_request(request) +# File "/usr/local/lib/python3.11/site-packages/silk/middleware.py", line 120, in process_request +# request_model = RequestModelFactory(request).construct_request_model() +# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +# File "/usr/local/lib/python3.11/site-packages/silk/model_factory.py", line 243, in construct_request_model +# request_model = models.Request.objects.create( +SILK_PROFILER_ENABLED = False