From 6dd32059786ce45d489f6bb7154335ee8a07c5ce Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Mon, 14 Aug 2023 18:31:59 -0600 Subject: [PATCH 1/8] Return API URL as part of status --- engine/apps/grafana_plugin/views/status.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/engine/apps/grafana_plugin/views/status.py b/engine/apps/grafana_plugin/views/status.py index 70a4b6adcf..cc2e538239 100644 --- a/engine/apps/grafana_plugin/views/status.py +++ b/engine/apps/grafana_plugin/views/status.py @@ -14,7 +14,7 @@ class StatusView(GrafanaHeadersMixin, APIView): - permission_classes = (PluginTokenVerified,) + permission_classes = PluginTokenVerified def post(self, request: Request) -> Response: """ @@ -36,11 +36,14 @@ def post(self, request: Request) -> Response: is_installed = False token_ok = False allow_signup = True + api_url = settings.BASE_URL # Check if organization is in OnCall database if organization := Organization.objects.get(stack_id=stack_id, org_id=org_id): is_installed = True token_ok = organization.api_token_status == Organization.API_TOKEN_STATUS_OK + if organization.is_moved: + api_url = organization.migration_destination.oncall_backend_url else: allow_signup = DynamicSetting.objects.get_or_create( name="allow_plugin_organization_signup", defaults={"boolean_value": True} @@ -69,6 +72,7 @@ def post(self, request: Request) -> Response: "version": settings.VERSION, "recaptcha_site_key": settings.RECAPTCHA_V3_SITE_KEY, "currently_undergoing_maintenance_message": settings.CURRENTLY_UNDERGOING_MAINTENANCE_MESSAGE, + "api_url": api_url, } ) From 65c0f3efb6a7f858989f2ede27e0e88e8f6703b6 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Mon, 14 Aug 2023 18:33:15 -0600 Subject: [PATCH 2/8] Revert permission_classes --- engine/apps/grafana_plugin/views/status.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/apps/grafana_plugin/views/status.py b/engine/apps/grafana_plugin/views/status.py index cc2e538239..38bf0085fb 100644 --- a/engine/apps/grafana_plugin/views/status.py +++ b/engine/apps/grafana_plugin/views/status.py @@ -14,7 +14,7 @@ class StatusView(GrafanaHeadersMixin, APIView): - permission_classes = PluginTokenVerified + permission_classes = (PluginTokenVerified,) def post(self, request: Request) -> Response: """ From c132c6d036731546aa8639b4ad7bba0d11eec6c6 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Tue, 15 Aug 2023 12:00:53 -0600 Subject: [PATCH 3/8] Remove different permissions code for grafana_plugin, reuse PluginAuthentication and allow MobileAppAuthTokenAuthentication for status --- engine/apps/grafana_plugin/permissions.py | 28 ------------------- .../apps/grafana_plugin/tests/test_status.py | 24 ++++++++++++++-- engine/apps/grafana_plugin/views/install.py | 4 +-- engine/apps/grafana_plugin/views/status.py | 11 +++++--- engine/apps/grafana_plugin/views/sync.py | 3 +- .../grafana_plugin/views/sync_organization.py | 4 +-- engine/apps/mobile_app/auth.py | 6 ---- 7 files changed, 34 insertions(+), 46 deletions(-) delete mode 100644 engine/apps/grafana_plugin/permissions.py diff --git a/engine/apps/grafana_plugin/permissions.py b/engine/apps/grafana_plugin/permissions.py deleted file mode 100644 index 47bb8bf271..0000000000 --- a/engine/apps/grafana_plugin/permissions.py +++ /dev/null @@ -1,28 +0,0 @@ -import json -import logging - -from django.views import View -from rest_framework import permissions -from rest_framework.authentication import get_authorization_header -from rest_framework.request import Request - -from apps.auth_token.exceptions import InvalidToken -from apps.grafana_plugin.helpers.gcom import check_token - -logger = logging.getLogger(__name__) - - -class PluginTokenVerified(permissions.BasePermission): - # The grafana plugin can either use a token from gcom or one generated internally by oncall - # Tokens from gcom will be prefixed with gcom: otherwise they will be treated as local - def has_permission(self, request: Request, view: View) -> bool: - token_string = get_authorization_header(request).decode() - context = json.loads(request.headers.get("X-Instance-Context")) - try: - auth_token = check_token(token_string, context) - if auth_token: - return True - except InvalidToken: - logger.warning(f"Invalid token used: {context}") - - return False diff --git a/engine/apps/grafana_plugin/tests/test_status.py b/engine/apps/grafana_plugin/tests/test_status.py index 74e74a11c2..2ae68fd679 100644 --- a/engine/apps/grafana_plugin/tests/test_status.py +++ b/engine/apps/grafana_plugin/tests/test_status.py @@ -59,8 +59,28 @@ def test_allow_signup(make_organization_and_user_with_plugin_token, make_user_au ) response = client.get(reverse("grafana-plugin:status"), format="json", **auth_headers) - # if the org doesn't exist this will never return 200 due to - # the PluginTokenVerified permission class.. # should consider removing the DynamicSetting logic because technically this # condition will never be reached in the code... assert response.status_code == status.HTTP_403_FORBIDDEN + + +@pytest.mark.django_db +def test_status_mobile_app_auth_token( + make_organization_and_user_with_mobile_app_auth_token, + make_user_auth_headers, +): + organization, user, auth_token = make_organization_and_user_with_mobile_app_auth_token() + + client = APIClient() + url = reverse("grafana-plugin:status") + + response = client.post(url) + assert response.status_code == status.HTTP_403_FORBIDDEN + + auth_headers = make_user_auth_headers( + user, + auth_token, + ) + + response = client.post(url, format="json", **auth_headers) + assert response.status_code == status.HTTP_200_OK diff --git a/engine/apps/grafana_plugin/views/install.py b/engine/apps/grafana_plugin/views/install.py index f13f2f95b8..b481f24884 100644 --- a/engine/apps/grafana_plugin/views/install.py +++ b/engine/apps/grafana_plugin/views/install.py @@ -3,14 +3,14 @@ from rest_framework.response import Response from rest_framework.views import APIView -from apps.grafana_plugin.permissions import PluginTokenVerified +from apps.auth_token.auth import PluginAuthentication from apps.user_management.models import Organization from apps.user_management.sync import sync_organization from common.api_helpers.mixins import GrafanaHeadersMixin class InstallView(GrafanaHeadersMixin, APIView): - permission_classes = (PluginTokenVerified,) + authentication_classes = (PluginAuthentication,) def post(self, request: Request) -> Response: stack_id = self.instance_context["stack_id"] diff --git a/engine/apps/grafana_plugin/views/status.py b/engine/apps/grafana_plugin/views/status.py index 38bf0085fb..be4bfd8a82 100644 --- a/engine/apps/grafana_plugin/views/status.py +++ b/engine/apps/grafana_plugin/views/status.py @@ -7,14 +7,17 @@ from apps.auth_token.auth import PluginAuthentication from apps.base.models import DynamicSetting from apps.grafana_plugin.helpers import GrafanaAPIClient -from apps.grafana_plugin.permissions import PluginTokenVerified from apps.grafana_plugin.tasks.sync import plugin_sync_organization_async +from apps.mobile_app.auth import MobileAppAuthTokenAuthentication from apps.user_management.models import Organization from common.api_helpers.mixins import GrafanaHeadersMixin class StatusView(GrafanaHeadersMixin, APIView): - permission_classes = (PluginTokenVerified,) + authentication_classes = ( + MobileAppAuthTokenAuthentication, + PluginAuthentication, + ) def post(self, request: Request) -> Response: """ @@ -67,7 +70,7 @@ def post(self, request: Request) -> Response: "is_installed": is_installed, "token_ok": token_ok, "allow_signup": allow_signup, - "is_user_anonymous": self.grafana_context["IsAnonymous"], + "is_user_anonymous": self.grafana_context.get("IsAnonymous", False), "license": settings.LICENSE, "version": settings.VERSION, "recaptcha_site_key": settings.RECAPTCHA_V3_SITE_KEY, @@ -100,7 +103,7 @@ def get(self, _request: Request) -> Response: "is_installed": is_installed, "token_ok": token_ok, "allow_signup": allow_signup, - "is_user_anonymous": self.grafana_context["IsAnonymous"], + "is_user_anonymous": self.grafana_context.get("IsAnonymous", False), "license": settings.LICENSE, "version": settings.VERSION, } diff --git a/engine/apps/grafana_plugin/views/sync.py b/engine/apps/grafana_plugin/views/sync.py index 6c17f352cd..2dd90142bd 100644 --- a/engine/apps/grafana_plugin/views/sync.py +++ b/engine/apps/grafana_plugin/views/sync.py @@ -7,7 +7,6 @@ from rest_framework.views import APIView from apps.auth_token.auth import PluginAuthentication -from apps.grafana_plugin.permissions import PluginTokenVerified from apps.grafana_plugin.tasks.sync import plugin_sync_organization_async from apps.user_management.models import Organization from common.api_helpers.mixins import GrafanaHeadersMixin @@ -16,7 +15,7 @@ class PluginSyncView(GrafanaHeadersMixin, APIView): - permission_classes = (PluginTokenVerified,) + authentication_classes = (PluginAuthentication,) def post(self, request: Request) -> Response: """Deprecated. May be used for the plugins with versions < 1.3.17""" diff --git a/engine/apps/grafana_plugin/views/sync_organization.py b/engine/apps/grafana_plugin/views/sync_organization.py index faf3b5cac2..2f04ae10f5 100644 --- a/engine/apps/grafana_plugin/views/sync_organization.py +++ b/engine/apps/grafana_plugin/views/sync_organization.py @@ -5,14 +5,14 @@ from rest_framework.response import Response from rest_framework.views import APIView -from apps.grafana_plugin.permissions import PluginTokenVerified +from apps.auth_token.auth import PluginAuthentication from apps.user_management.models import Organization from apps.user_management.sync import sync_organization from common.api_helpers.mixins import GrafanaHeadersMixin class SyncOrganizationView(GrafanaHeadersMixin, APIView): - permission_classes = (PluginTokenVerified,) + authentication_classes = (PluginAuthentication,) def post(self, request: Request) -> Response: stack_id = self.instance_context["stack_id"] diff --git a/engine/apps/mobile_app/auth.py b/engine/apps/mobile_app/auth.py index 5b1d249789..72d0646adb 100644 --- a/engine/apps/mobile_app/auth.py +++ b/engine/apps/mobile_app/auth.py @@ -4,7 +4,6 @@ from rest_framework.authentication import BaseAuthentication, get_authorization_header from apps.auth_token.exceptions import InvalidToken -from apps.user_management.exceptions import OrganizationDeletedException, OrganizationMovedException from apps.user_management.models import User from .models import MobileAppAuthToken, MobileAppVerificationToken @@ -43,9 +42,4 @@ def authenticate_credentials(self, token_string: str) -> Tuple[Optional[User], O except InvalidToken: return None, None - if auth_token.organization.is_moved: - raise OrganizationMovedException(auth_token.organization) - if auth_token.organization.deleted_at: - raise OrganizationDeletedException(auth_token.organization) - return auth_token.user, auth_token From bbc0d0cfe07ce61c467fbd42146176cbeab41f52 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Tue, 15 Aug 2023 12:06:53 -0600 Subject: [PATCH 4/8] Update CHANGELOG.md --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a871ecbcce..8f35123d34 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 +### Changed + +- Grafana plugin app to use common PluginAuthentication, allow mobile app to access status endpoint @mderynck ([#2791](https://github.com/grafana/oncall/pull/2791)) + ### Fixed - Ignore ical cancelled events when calculating shifts ([#2776](https://github.com/grafana/oncall/pull/2776)) From 6747210064c9249ee6181081c6071496b7f430c3 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Tue, 15 Aug 2023 16:24:35 -0600 Subject: [PATCH 5/8] Split PluginAuthentication so grafana-app can use a version without checking user --- engine/apps/auth_token/auth.py | 43 +++++++++++++++---- engine/apps/grafana_plugin/views/install.py | 4 +- engine/apps/grafana_plugin/views/status.py | 11 +++-- engine/apps/grafana_plugin/views/sync.py | 6 +-- .../grafana_plugin/views/sync_organization.py | 4 +- 5 files changed, 47 insertions(+), 21 deletions(-) diff --git a/engine/apps/auth_token/auth.py b/engine/apps/auth_token/auth.py index 462315a515..03cec125ee 100644 --- a/engine/apps/auth_token/auth.py +++ b/engine/apps/auth_token/auth.py @@ -54,7 +54,13 @@ def authenticate_credentials(self, token): return auth_token.user, auth_token -class PluginAuthentication(BaseAuthentication): +class BasePluginAuthentication(BaseAuthentication): + """ + Authentication used by grafana-plugin app where we tolerate user not being set yet due to being in + a state of initialization, Only validates that the plugin should be talking to the backend. Outside of + this app PluginAuthentication should be used since it also checks the user. + """ + def authenticate_header(self, request): # Check parent's method comments return "Bearer" @@ -95,10 +101,10 @@ def _get_user(request: Request, organization: Organization) -> User: try: context = dict(json.loads(request.headers.get("X-Grafana-Context"))) except (ValueError, TypeError): - raise exceptions.AuthenticationFailed("Grafana context must be JSON dict.") + return None if "UserId" not in context and "UserID" not in context: - raise exceptions.AuthenticationFailed("Invalid Grafana context.") + return None try: user_id = context["UserId"] @@ -108,18 +114,39 @@ def _get_user(request: Request, organization: Organization) -> User: try: return organization.users.get(user_id=user_id) except User.DoesNotExist: - logger.debug(f"Could not get user from grafana request. Context {context}") - raise exceptions.AuthenticationFailed("Non-existent or anonymous user.") + return None @classmethod - def is_user_from_request_present_in_organization(cls, request: Request, organization: Organization) -> User: + def is_user_from_request_present_in_organization(cls, request: Request, organization: Organization) -> bool: try: - cls._get_user(request, organization) - return True + return cls._get_user(request, organization) is not None except exceptions.AuthenticationFailed: return False +class PluginAuthentication(BasePluginAuthentication): + @staticmethod + def _get_user(request: Request, organization: Organization) -> User: + try: + context = dict(json.loads(request.headers.get("X-Grafana-Context"))) + except (ValueError, TypeError): + raise exceptions.AuthenticationFailed("Grafana context must be JSON dict.") + + if "UserId" not in context and "UserID" not in context: + raise exceptions.AuthenticationFailed("Invalid Grafana context.") + + try: + user_id = context["UserId"] + except KeyError: + user_id = context["UserID"] + + try: + return organization.users.get(user_id=user_id) + except User.DoesNotExist: + logger.debug(f"Could not get user from grafana request. Context {context}") + raise exceptions.AuthenticationFailed("Non-existent or anonymous user.") + + class GrafanaIncidentUser(AnonymousUser): @property def is_authenticated(self): diff --git a/engine/apps/grafana_plugin/views/install.py b/engine/apps/grafana_plugin/views/install.py index b481f24884..44fefaecde 100644 --- a/engine/apps/grafana_plugin/views/install.py +++ b/engine/apps/grafana_plugin/views/install.py @@ -3,14 +3,14 @@ from rest_framework.response import Response from rest_framework.views import APIView -from apps.auth_token.auth import PluginAuthentication +from apps.auth_token.auth import BasePluginAuthentication from apps.user_management.models import Organization from apps.user_management.sync import sync_organization from common.api_helpers.mixins import GrafanaHeadersMixin class InstallView(GrafanaHeadersMixin, APIView): - authentication_classes = (PluginAuthentication,) + authentication_classes = (BasePluginAuthentication,) def post(self, request: Request) -> Response: stack_id = self.instance_context["stack_id"] diff --git a/engine/apps/grafana_plugin/views/status.py b/engine/apps/grafana_plugin/views/status.py index be4bfd8a82..70defa7778 100644 --- a/engine/apps/grafana_plugin/views/status.py +++ b/engine/apps/grafana_plugin/views/status.py @@ -1,10 +1,9 @@ from django.conf import settings -from django.http import JsonResponse from rest_framework.request import Request from rest_framework.response import Response from rest_framework.views import APIView -from apps.auth_token.auth import PluginAuthentication +from apps.auth_token.auth import BasePluginAuthentication from apps.base.models import DynamicSetting from apps.grafana_plugin.helpers import GrafanaAPIClient from apps.grafana_plugin.tasks.sync import plugin_sync_organization_async @@ -16,7 +15,7 @@ class StatusView(GrafanaHeadersMixin, APIView): authentication_classes = ( MobileAppAuthTokenAuthentication, - PluginAuthentication, + BasePluginAuthentication, ) def post(self, request: Request) -> Response: @@ -27,8 +26,8 @@ def post(self, request: Request) -> Response: """ # Check if the plugin is currently undergoing maintenance, and return response without querying db if settings.CURRENTLY_UNDERGOING_MAINTENANCE_MESSAGE: - return JsonResponse( - { + return Response( + data={ "currently_undergoing_maintenance_message": settings.CURRENTLY_UNDERGOING_MAINTENANCE_MESSAGE, } ) @@ -53,7 +52,7 @@ def post(self, request: Request) -> Response: )[0].boolean_value # Check if current user is in OnCall database - user_is_present_in_org = PluginAuthentication.is_user_from_request_present_in_organization( + user_is_present_in_org = BasePluginAuthentication.is_user_from_request_present_in_organization( request, organization ) # If user is not present in OnCall database, set token_ok to False, which will trigger reinstall diff --git a/engine/apps/grafana_plugin/views/sync.py b/engine/apps/grafana_plugin/views/sync.py index 2dd90142bd..1e2ce912cc 100644 --- a/engine/apps/grafana_plugin/views/sync.py +++ b/engine/apps/grafana_plugin/views/sync.py @@ -6,7 +6,7 @@ from rest_framework.response import Response from rest_framework.views import APIView -from apps.auth_token.auth import PluginAuthentication +from apps.auth_token.auth import BasePluginAuthentication from apps.grafana_plugin.tasks.sync import plugin_sync_organization_async from apps.user_management.models import Organization from common.api_helpers.mixins import GrafanaHeadersMixin @@ -15,7 +15,7 @@ class PluginSyncView(GrafanaHeadersMixin, APIView): - authentication_classes = (PluginAuthentication,) + authentication_classes = (BasePluginAuthentication,) def post(self, request: Request) -> Response: """Deprecated. May be used for the plugins with versions < 1.3.17""" @@ -30,7 +30,7 @@ def post(self, request: Request) -> Response: if organization.api_token_status == Organization.API_TOKEN_STATUS_OK: is_installed = True - user_is_present_in_org = PluginAuthentication.is_user_from_request_present_in_organization( + user_is_present_in_org = BasePluginAuthentication.is_user_from_request_present_in_organization( request, organization ) if not user_is_present_in_org: diff --git a/engine/apps/grafana_plugin/views/sync_organization.py b/engine/apps/grafana_plugin/views/sync_organization.py index 2f04ae10f5..6d5d3b6fcd 100644 --- a/engine/apps/grafana_plugin/views/sync_organization.py +++ b/engine/apps/grafana_plugin/views/sync_organization.py @@ -5,14 +5,14 @@ from rest_framework.response import Response from rest_framework.views import APIView -from apps.auth_token.auth import PluginAuthentication +from apps.auth_token.auth import BasePluginAuthentication from apps.user_management.models import Organization from apps.user_management.sync import sync_organization from common.api_helpers.mixins import GrafanaHeadersMixin class SyncOrganizationView(GrafanaHeadersMixin, APIView): - authentication_classes = (PluginAuthentication,) + authentication_classes = (BasePluginAuthentication,) def post(self, request: Request) -> Response: stack_id = self.instance_context["stack_id"] From ebeef7970f9e441f8dd427a35e744577239a4b61 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Tue, 15 Aug 2023 17:33:56 -0600 Subject: [PATCH 6/8] Cleanup tests --- engine/apps/auth_token/auth.py | 7 --- .../apps/grafana_plugin/tests/test_status.py | 55 ++++++++++--------- engine/apps/grafana_plugin/views/status.py | 10 +--- engine/apps/grafana_plugin/views/sync.py | 5 +- 4 files changed, 34 insertions(+), 43 deletions(-) diff --git a/engine/apps/auth_token/auth.py b/engine/apps/auth_token/auth.py index 03cec125ee..aee403f545 100644 --- a/engine/apps/auth_token/auth.py +++ b/engine/apps/auth_token/auth.py @@ -116,13 +116,6 @@ def _get_user(request: Request, organization: Organization) -> User: except User.DoesNotExist: return None - @classmethod - def is_user_from_request_present_in_organization(cls, request: Request, organization: Organization) -> bool: - try: - return cls._get_user(request, organization) is not None - except exceptions.AuthenticationFailed: - return False - class PluginAuthentication(BasePluginAuthentication): @staticmethod diff --git a/engine/apps/grafana_plugin/tests/test_status.py b/engine/apps/grafana_plugin/tests/test_status.py index 2ae68fd679..966d370835 100644 --- a/engine/apps/grafana_plugin/tests/test_status.py +++ b/engine/apps/grafana_plugin/tests/test_status.py @@ -1,48 +1,52 @@ -from unittest.mock import patch - import pytest from django.test import override_settings from django.urls import reverse from rest_framework import status from rest_framework.test import APIClient +from apps.user_management.models import Organization + GRAFANA_TOKEN = "TESTTOKEN" GRAFANA_URL = "hello.com" LICENSE = "asdfasdf" VERSION = "asdfasdfasdf" +BASE_URL = "http://asdasdqweqweqw.com/oncall" GRAFANA_CONTEXT_DATA = {"IsAnonymous": False} -SETTINGS = {"LICENSE": LICENSE, "VERSION": VERSION} +SETTINGS = {"LICENSE": LICENSE, "VERSION": VERSION, "BASE_URL": BASE_URL} + + +def _check_status_response(auth_headers, client): + response = client.post(reverse("grafana-plugin:status"), format="json", **auth_headers) + response_data = response.data + assert response.status_code == status.HTTP_200_OK + assert response_data["token_ok"] is True + assert response_data["is_installed"] is True + assert response_data["allow_signup"] is True + assert response_data["is_user_anonymous"] is False + assert response_data["license"] == LICENSE + assert response_data["version"] == VERSION + assert response_data["api_url"] == BASE_URL @pytest.mark.django_db @override_settings(**SETTINGS) -@patch("apps.grafana_plugin.views.status.GrafanaAPIClient") def test_token_ok_is_based_on_grafana_api_check_token_response( - mocked_grafana_api_client, make_organization_and_user_with_plugin_token, make_user_auth_headers + make_organization_and_user_with_plugin_token, make_user_auth_headers ): - mocked_grafana_api_client.return_value.check_token.return_value = (None, {"connected": True}) - organization, user, token = make_organization_and_user_with_plugin_token() organization.grafana_url = GRAFANA_URL - organization.save(update_fields=["grafana_url"]) + organization.api_token_status = Organization.API_TOKEN_STATUS_OK + organization.save(update_fields=["grafana_url", "api_token_status"]) client = APIClient() + url = reverse("grafana-plugin:status") + response = client.post(url) + assert response.status_code == status.HTTP_403_FORBIDDEN + auth_headers = make_user_auth_headers( user, token, grafana_token=GRAFANA_TOKEN, grafana_context_data=GRAFANA_CONTEXT_DATA ) - response = client.get(reverse("grafana-plugin:status"), format="json", **auth_headers) - response_data = response.data - - assert response.status_code == status.HTTP_200_OK - assert response_data["token_ok"] is True - assert response_data["is_installed"] is True - assert response_data["allow_signup"] is True - assert response_data["is_user_anonymous"] is False - assert response_data["license"] == LICENSE - assert response_data["version"] == VERSION - - assert mocked_grafana_api_client.called_once_with(api_url=GRAFANA_URL, api_token=GRAFANA_TOKEN) - assert mocked_grafana_api_client.return_value.check_token.called_once_with() + _check_status_response(auth_headers, client) @pytest.mark.django_db @@ -65,15 +69,18 @@ def test_allow_signup(make_organization_and_user_with_plugin_token, make_user_au @pytest.mark.django_db +@override_settings(**SETTINGS) def test_status_mobile_app_auth_token( make_organization_and_user_with_mobile_app_auth_token, make_user_auth_headers, ): organization, user, auth_token = make_organization_and_user_with_mobile_app_auth_token() + organization.grafana_url = GRAFANA_URL + organization.api_token_status = Organization.API_TOKEN_STATUS_OK + organization.save(update_fields=["grafana_url", "api_token_status"]) client = APIClient() url = reverse("grafana-plugin:status") - response = client.post(url) assert response.status_code == status.HTTP_403_FORBIDDEN @@ -81,6 +88,4 @@ def test_status_mobile_app_auth_token( user, auth_token, ) - - response = client.post(url, format="json", **auth_headers) - assert response.status_code == status.HTTP_200_OK + _check_status_response(auth_headers, client) diff --git a/engine/apps/grafana_plugin/views/status.py b/engine/apps/grafana_plugin/views/status.py index 70defa7778..480d5474a7 100644 --- a/engine/apps/grafana_plugin/views/status.py +++ b/engine/apps/grafana_plugin/views/status.py @@ -51,12 +51,8 @@ def post(self, request: Request) -> Response: name="allow_plugin_organization_signup", defaults={"boolean_value": True} )[0].boolean_value - # Check if current user is in OnCall database - user_is_present_in_org = BasePluginAuthentication.is_user_from_request_present_in_organization( - request, organization - ) # If user is not present in OnCall database, set token_ok to False, which will trigger reinstall - if not user_is_present_in_org: + if not request.user: token_ok = False organization.api_token_status = Organization.API_TOKEN_STATUS_PENDING organization.save(update_fields=["api_token_status"]) @@ -69,7 +65,7 @@ def post(self, request: Request) -> Response: "is_installed": is_installed, "token_ok": token_ok, "allow_signup": allow_signup, - "is_user_anonymous": self.grafana_context.get("IsAnonymous", False), + "is_user_anonymous": self.grafana_context.get("IsAnonymous", request.user is None), "license": settings.LICENSE, "version": settings.VERSION, "recaptcha_site_key": settings.RECAPTCHA_V3_SITE_KEY, @@ -102,7 +98,7 @@ def get(self, _request: Request) -> Response: "is_installed": is_installed, "token_ok": token_ok, "allow_signup": allow_signup, - "is_user_anonymous": self.grafana_context.get("IsAnonymous", False), + "is_user_anonymous": self.grafana_context.get("IsAnonymous", _request.user is None), "license": settings.LICENSE, "version": settings.VERSION, } diff --git a/engine/apps/grafana_plugin/views/sync.py b/engine/apps/grafana_plugin/views/sync.py index 1e2ce912cc..98df65103a 100644 --- a/engine/apps/grafana_plugin/views/sync.py +++ b/engine/apps/grafana_plugin/views/sync.py @@ -30,10 +30,7 @@ def post(self, request: Request) -> Response: if organization.api_token_status == Organization.API_TOKEN_STATUS_OK: is_installed = True - user_is_present_in_org = BasePluginAuthentication.is_user_from_request_present_in_organization( - request, organization - ) - if not user_is_present_in_org: + if not request.auth.user: organization.api_token_status = Organization.API_TOKEN_STATUS_PENDING organization.save(update_fields=["api_token_status"]) From 057d87f521f88ce47eea3554b7df78d822d70245 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Wed, 16 Aug 2023 07:42:52 -0600 Subject: [PATCH 7/8] Remove deprecated endpoints --- engine/apps/grafana_plugin/urls.py | 9 +-- engine/apps/grafana_plugin/views/__init__.py | 1 - engine/apps/grafana_plugin/views/status.py | 31 -------- engine/apps/grafana_plugin/views/sync.py | 80 -------------------- 4 files changed, 1 insertion(+), 120 deletions(-) delete mode 100644 engine/apps/grafana_plugin/views/sync.py diff --git a/engine/apps/grafana_plugin/urls.py b/engine/apps/grafana_plugin/urls.py index 8cdd4ca596..0b3e76c807 100644 --- a/engine/apps/grafana_plugin/urls.py +++ b/engine/apps/grafana_plugin/urls.py @@ -1,12 +1,6 @@ from django.urls import re_path -from apps.grafana_plugin.views import ( - InstallView, - PluginSyncView, - SelfHostedInstallView, - StatusView, - SyncOrganizationView, -) +from apps.grafana_plugin.views import InstallView, SelfHostedInstallView, StatusView, SyncOrganizationView app_name = "grafana-plugin" @@ -15,5 +9,4 @@ re_path(r"status/?", StatusView().as_view(), name="status"), re_path(r"install/?", InstallView().as_view(), name="install"), re_path(r"sync_organization/?", SyncOrganizationView().as_view(), name="sync-organization"), - re_path(r"sync/?", PluginSyncView().as_view(), name="sync"), ] diff --git a/engine/apps/grafana_plugin/views/__init__.py b/engine/apps/grafana_plugin/views/__init__.py index 3b3f1a3ac4..23e2437334 100644 --- a/engine/apps/grafana_plugin/views/__init__.py +++ b/engine/apps/grafana_plugin/views/__init__.py @@ -1,5 +1,4 @@ from .install import InstallView # noqa: F401 from .self_hosted_install import SelfHostedInstallView # noqa: F401 from .status import StatusView # noqa: F401 -from .sync import PluginSyncView # noqa: F401 from .sync_organization import SyncOrganizationView # noqa: F401 diff --git a/engine/apps/grafana_plugin/views/status.py b/engine/apps/grafana_plugin/views/status.py index 480d5474a7..7e32de3065 100644 --- a/engine/apps/grafana_plugin/views/status.py +++ b/engine/apps/grafana_plugin/views/status.py @@ -5,7 +5,6 @@ from apps.auth_token.auth import BasePluginAuthentication from apps.base.models import DynamicSetting -from apps.grafana_plugin.helpers import GrafanaAPIClient from apps.grafana_plugin.tasks.sync import plugin_sync_organization_async from apps.mobile_app.auth import MobileAppAuthTokenAuthentication from apps.user_management.models import Organization @@ -73,33 +72,3 @@ def post(self, request: Request) -> Response: "api_url": api_url, } ) - - def get(self, _request: Request) -> Response: - """Deprecated. May be used for the plugins with versions < 1.3.17""" - stack_id = self.instance_context["stack_id"] - org_id = self.instance_context["org_id"] - is_installed = False - token_ok = False - allow_signup = True - - if organization := Organization.objects.filter(stack_id=stack_id, org_id=org_id).first(): - is_installed = True - _, resp = GrafanaAPIClient(api_url=organization.grafana_url, api_token=organization.api_token).check_token() - token_ok = resp["connected"] - else: - from apps.base.models import DynamicSetting - - allow_signup = DynamicSetting.objects.get_or_create( - name="allow_plugin_organization_signup", defaults={"boolean_value": True} - )[0].boolean_value - - return Response( - data={ - "is_installed": is_installed, - "token_ok": token_ok, - "allow_signup": allow_signup, - "is_user_anonymous": self.grafana_context.get("IsAnonymous", _request.user is None), - "license": settings.LICENSE, - "version": settings.VERSION, - } - ) diff --git a/engine/apps/grafana_plugin/views/sync.py b/engine/apps/grafana_plugin/views/sync.py deleted file mode 100644 index 98df65103a..0000000000 --- a/engine/apps/grafana_plugin/views/sync.py +++ /dev/null @@ -1,80 +0,0 @@ -import logging - -from django.conf import settings -from rest_framework import status -from rest_framework.request import Request -from rest_framework.response import Response -from rest_framework.views import APIView - -from apps.auth_token.auth import BasePluginAuthentication -from apps.grafana_plugin.tasks.sync import plugin_sync_organization_async -from apps.user_management.models import Organization -from common.api_helpers.mixins import GrafanaHeadersMixin - -logger = logging.getLogger(__name__) - - -class PluginSyncView(GrafanaHeadersMixin, APIView): - authentication_classes = (BasePluginAuthentication,) - - def post(self, request: Request) -> Response: - """Deprecated. May be used for the plugins with versions < 1.3.17""" - stack_id = self.instance_context["stack_id"] - org_id = self.instance_context["org_id"] - is_installed = False - allow_signup = True - - try: - # Check if organization is in OnCall database - organization = Organization.objects.get(stack_id=stack_id, org_id=org_id) - if organization.api_token_status == Organization.API_TOKEN_STATUS_OK: - is_installed = True - - if not request.auth.user: - organization.api_token_status = Organization.API_TOKEN_STATUS_PENDING - organization.save(update_fields=["api_token_status"]) - - if not organization: - from apps.base.models import DynamicSetting - - allow_signup = DynamicSetting.objects.get_or_create( - name="allow_plugin_organization_signup", defaults={"boolean_value": True} - )[0].boolean_value - - plugin_sync_organization_async.apply_async((organization.pk,)) - except Organization.DoesNotExist: - logger.info(f"Organization for stack {stack_id} org {org_id} was not found") - - return Response( - status=status.HTTP_202_ACCEPTED, - data={ - "is_installed": is_installed, - "is_user_anonymous": self.grafana_context["IsAnonymous"], - "allow_signup": allow_signup, - }, - ) - - def get(self, _request: Request) -> Response: - """Deprecated. May be used for the plugins with versions < 1.3.17""" - stack_id = self.instance_context["stack_id"] - org_id = self.instance_context["org_id"] - token_ok = False - - try: - organization = Organization.objects.get(stack_id=stack_id, org_id=org_id) - if organization.api_token_status == Organization.API_TOKEN_STATUS_PENDING: - return Response(status=status.HTTP_202_ACCEPTED) - elif organization.api_token_status == Organization.API_TOKEN_STATUS_OK: - token_ok = True - except Organization.DoesNotExist: - logger.info(f"Organization for stack {stack_id} org {org_id} was not found") - - return Response( - status=status.HTTP_200_OK, - data={ - "token_ok": token_ok, - "license": settings.LICENSE, - "version": settings.VERSION, - "recaptcha_site_key": settings.RECAPTCHA_V3_SITE_KEY, - }, - ) From a74284b929b50a8e7ab6d49e778228258ef29096 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Wed, 16 Aug 2023 07:45:23 -0600 Subject: [PATCH 8/8] Amend changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f35123d34..816e51e852 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed -- Grafana plugin app to use common PluginAuthentication, allow mobile app to access status endpoint @mderynck ([#2791](https://github.com/grafana/oncall/pull/2791)) +- Allow mobile app to access status endpoint @mderynck ([#2791](https://github.com/grafana/oncall/pull/2791)) ### Fixed