ref(mediators): Make validator into a dataclass

src/sentry/mediators/__init__.py

@@ -1,3 +1,2 @@
 from .mediator import Mediator  # NOQA
 from .param import Param  # NOQA
-from .token_exchange.util import AUTHORIZATION, REFRESH, GrantTypes  # noqa: F401

src/sentry/mediators/token_exchange/__init__.py

@@ -1,2 +0,0 @@
-from .util import AUTHORIZATION, REFRESH, GrantTypes, token_expiration  # NOQA
-from .validator import Validator  # NOQA

src/sentry/sentry_apps/api/endpoints/sentry_app_authorizations.py

@@ -10,10 +10,10 @@
 from sentry.api.serializers.models.apitoken import ApiTokenSerializer
 from sentry.auth.services.auth.impl import promote_request_api_user
 from sentry.coreapi import APIUnauthorized
-from sentry.mediators.token_exchange.util import GrantTypes
 from sentry.sentry_apps.api.bases.sentryapps import SentryAppAuthorizationsBaseEndpoint
 from sentry.sentry_apps.token_exchange.grant_exchanger import GrantExchanger
 from sentry.sentry_apps.token_exchange.refresher import Refresher
+from sentry.sentry_apps.token_exchange.util import GrantTypes
 
 logger = logging.getLogger(__name__)
 

src/sentry/sentry_apps/token_exchange/grant_exchanger.py

@@ -6,14 +6,14 @@
 
 from sentry import analytics
 from sentry.coreapi import APIUnauthorized
-from sentry.mediators.token_exchange.util import token_expiration
-from sentry.mediators.token_exchange.validator import Validator
 from sentry.models.apiapplication import ApiApplication
 from sentry.models.apigrant import ApiGrant
 from sentry.models.apitoken import ApiToken
 from sentry.sentry_apps.models.sentry_app import SentryApp
 from sentry.sentry_apps.models.sentry_app_installation import SentryAppInstallation
 from sentry.sentry_apps.services.app import RpcSentryAppInstallation
+from sentry.sentry_apps.token_exchange.util import token_expiration
+from sentry.sentry_apps.token_exchange.validator import Validator
 from sentry.silo.safety import unguarded_write
 from sentry.users.models.user import User
 
@@ -31,15 +31,14 @@ class GrantExchanger:
 
     def run(self):
         with transaction.atomic(using=router.db_for_write(ApiToken)):
-            self._validate()
-            token = self._create_token()
+            if self._validate():
+                token = self._create_token()
 
-            # Once it's exchanged it's no longer valid and should not be
-            # exchangeable, so we delete it.
-            self._delete_grant()
-            self.record_analytics()
-
-            return token
+                # Once it's exchanged it's no longer valid and should not be
+                # exchangeable, so we delete it.
+                self._delete_grant()
+                self.record_analytics()
+                return token
 
     def record_analytics(self) -> None:
         analytics.record(
@@ -48,14 +47,15 @@ def record_analytics(self) -> None:
             exchange_type="authorization",
         )
 
-    def _validate(self) -> None:
-        Validator.run(install=self.install, client_id=self.client_id, user=self.user)
+    def _validate(self) -> bool:
+        is_valid = Validator(install=self.install, client_id=self.client_id, user=self.user).run()
 
         if not self._grant_belongs_to_install() or not self._sentry_app_user_owns_grant():
             raise APIUnauthorized("Forbidden grant")
 
         if not self._grant_is_active():
             raise APIUnauthorized("Grant has already expired.")
+        return is_valid
 
     def _grant_belongs_to_install(self) -> bool:
         return self.grant.sentry_app_installation.id == self.install.id

src/sentry/sentry_apps/token_exchange/refresher.py

@@ -5,13 +5,13 @@
 
 from sentry import analytics
 from sentry.coreapi import APIUnauthorized
-from sentry.mediators.token_exchange.util import token_expiration
-from sentry.mediators.token_exchange.validator import Validator
 from sentry.models.apiapplication import ApiApplication
 from sentry.models.apitoken import ApiToken
 from sentry.sentry_apps.models.sentry_app import SentryApp
 from sentry.sentry_apps.models.sentry_app_installation import SentryAppInstallation
 from sentry.sentry_apps.services.app import RpcSentryAppInstallation
+from sentry.sentry_apps.token_exchange.util import token_expiration
+from sentry.sentry_apps.token_exchange.validator import Validator
 from sentry.users.models.user import User
 
 
@@ -28,8 +28,8 @@ class Refresher:
 
     def run(self) -> ApiToken:
         with transaction.atomic(router.db_for_write(ApiToken)):
-            self._validate()
-            self.token.delete()
+            if self._validate():
+                self.token.delete()
 
         self.record_analytics()
         return self._create_new_token()
@@ -41,11 +41,12 @@ def record_analytics(self) -> None:
             exchange_type="refresh",
         )
 
-    def _validate(self) -> None:
-        Validator.run(install=self.install, client_id=self.client_id, user=self.user)
+    def _validate(self) -> bool:
+        is_valid = Validator(install=self.install, client_id=self.client_id, user=self.user).run()
 
         if self.token.application != self.application:
             raise APIUnauthorized("Token does not belong to the application")
+        return is_valid
 
     def _create_new_token(self) -> ApiToken:
         token = ApiToken.objects.create(

src/sentry/mediators/token_exchange/validator.py → src/sentry/sentry_apps/token_exchange/validator.py

@@ -1,53 +1,54 @@
-from django.db import router
+from dataclasses import dataclass
+
 from django.utils.functional import cached_property
 
 from sentry.coreapi import APIUnauthorized
-from sentry.mediators.mediator import Mediator
-from sentry.mediators.param import Param
 from sentry.models.apiapplication import ApiApplication
 from sentry.sentry_apps.models.sentry_app import SentryApp
 from sentry.sentry_apps.services.app import RpcSentryAppInstallation
 from sentry.users.models.user import User
 
 
-class Validator(Mediator):
+@dataclass
+class Validator:
     """
     Validates general authorization params for all types of token exchanges.
     """
 
-    install = Param(RpcSentryAppInstallation)
-    client_id = Param(str)
-    user = Param(User)
-    using = router.db_for_write(User)
+    install: RpcSentryAppInstallation
+    client_id: str
+    user: User
 
-    def call(self):
+    def run(self) -> bool:
         self._validate_is_sentry_app_making_request()
         self._validate_app_is_owned_by_user()
         self._validate_installation()
         return True
 
-    def _validate_is_sentry_app_making_request(self):
+    def _validate_is_sentry_app_making_request(self) -> None:
         if not self.user.is_sentry_app:
-            raise APIUnauthorized
+            raise APIUnauthorized("User is not a Sentry App")
 
-    def _validate_app_is_owned_by_user(self):
+    def _validate_app_is_owned_by_user(self) -> None:
         if self.sentry_app.proxy_user != self.user:
-            raise APIUnauthorized
+            raise APIUnauthorized("Sentry App does not belong to given user")
 
-    def _validate_installation(self):
+    def _validate_installation(self) -> None:
         if self.install.sentry_app.id != self.sentry_app.id:
-            raise APIUnauthorized
+            raise APIUnauthorized(
+                f"Sentry App Installation is not for Sentry App: {self.sentry_app.slug}"
+            )
 
     @cached_property
-    def sentry_app(self):
+    def sentry_app(self) -> SentryApp:
         try:
             return self.application.sentry_app
         except SentryApp.DoesNotExist:
-            raise APIUnauthorized
+            raise APIUnauthorized("Sentry App does not exist")
 
     @cached_property
-    def application(self):
+    def application(self) -> ApiApplication:
         try:
             return ApiApplication.objects.get(client_id=self.client_id)
         except ApiApplication.DoesNotExist:
-            raise APIUnauthorized
+            raise APIUnauthorized("Application does not exist")

src/sentry/web/frontend/oauth_token.py

@@ -9,10 +9,10 @@
 from rest_framework.request import Request
 
 from sentry import options
-from sentry.mediators.token_exchange.util import GrantTypes
 from sentry.models.apiapplication import ApiApplication, ApiApplicationStatus
 from sentry.models.apigrant import ApiGrant
 from sentry.models.apitoken import ApiToken
+from sentry.sentry_apps.token_exchange.util import GrantTypes
 from sentry.utils import json, metrics
 from sentry.web.frontend.base import control_silo_view
 from sentry.web.frontend.openidtoken import OpenIDToken
@@ -157,9 +157,11 @@ def process_token_details(
         token_information = {
             "access_token": token.token,
             "refresh_token": token.refresh_token,
-            "expires_in": int((token.expires_at - timezone.now()).total_seconds())
-            if token.expires_at
-            else None,
+            "expires_in": (
+                int((token.expires_at - timezone.now()).total_seconds())
+                if token.expires_at
+                else None
+            ),
             "expires_at": token.expires_at,
             "token_type": "bearer",
             "scope": " ".join(token.get_scopes()),

tests/sentry/sentry_apps/api/endpoints/test_sentry_app_authorizations.py

@@ -3,9 +3,9 @@
 from django.urls import reverse
 from django.utils import timezone
 
-from sentry.mediators.token_exchange.util import GrantTypes
 from sentry.models.apiapplication import ApiApplication
 from sentry.models.apitoken import ApiToken
+from sentry.sentry_apps.token_exchange.util import GrantTypes
 from sentry.silo.base import SiloMode
 from sentry.testutils.cases import APITestCase
 from sentry.testutils.silo import assume_test_silo_mode, control_silo_test

tests/sentry/sentry_apps/token_exchange/test_grant_exchanger.py

@@ -35,13 +35,8 @@ def test_adds_token_to_installation(self):
         token = self.grant_exchanger.run()
         assert SentryAppInstallation.objects.get(id=self.install.id).api_token == token
 
-    @patch("sentry.mediators.token_exchange.Validator.run")
-    def test_validate_generic_token_exchange_requirements(self, validator):
-        self.grant_exchanger.run()
-
-        validator.assert_called_once_with(
-            install=self.install, client_id=self.client_id, user=self.user
-        )
+    def test_validate_generic_token_exchange_requirements(self):
+        assert self.grant_exchanger._validate()
 
     def test_grant_must_belong_to_installations(self):
         other_install = self.create_sentry_app_installation(prevent_token_exchange=True)

tests/sentry/sentry_apps/token_exchange/test_refresher.py

@@ -42,13 +42,8 @@ def test_deletes_refreshed_token(self):
         self.refresher.run()
         assert not ApiToken.objects.filter(id=self.token.id).exists()
 
-    @patch("sentry.mediators.token_exchange.Validator.run")
-    def test_validates_generic_token_exchange_requirements(self, validator):
-        self.refresher.run()
-
-        validator.assert_called_once_with(
-            install=self.install, client_id=self.client_id, user=self.user
-        )
+    def test_validates_generic_token_exchange_requirements(self):
+        assert self.refresher._validate()
 
     def test_validates_token_belongs_to_sentry_app(self):
         refresh_token = ApiToken.objects.create(

tests/sentry/mediators/token_exchange/test_validator.py → tests/sentry/sentry_apps/token_exchange/test_validator.py

@@ -3,9 +3,9 @@
 import pytest
 
 from sentry.coreapi import APIUnauthorized
-from sentry.mediators.token_exchange.validator import Validator
 from sentry.sentry_apps.models.sentry_app import SentryApp
 from sentry.sentry_apps.services.app import app_service
+from sentry.sentry_apps.token_exchange.validator import Validator
 from sentry.testutils.cases import TestCase
 from sentry.testutils.silo import control_silo_test
 
@@ -24,35 +24,35 @@ def setUp(self):
         )
 
     def test_happy_path(self):
-        assert self.validator.call()
+        assert self.validator.run()
 
     def test_request_must_be_made_by_sentry_app(self):
         self.validator.user = self.create_user()
 
         with pytest.raises(APIUnauthorized):
-            self.validator.call()
+            self.validator.run()
 
     def test_request_user_must_own_sentry_app(self):
         self.validator.user = self.create_user(is_sentry_app=True)
 
         with pytest.raises(APIUnauthorized):
-            self.validator.call()
+            self.validator.run()
 
     def test_installation_belongs_to_sentry_app_with_client_id(self):
         self.validator.install = self.create_sentry_app_installation()
 
         with pytest.raises(APIUnauthorized):
-            self.validator.call()
+            self.validator.run()
 
     @patch("sentry.models.ApiApplication.sentry_app")
     def test_raises_when_sentry_app_cannot_be_found(self, sentry_app):
         sentry_app.side_effect = SentryApp.DoesNotExist()
 
         with pytest.raises(APIUnauthorized):
-            self.validator.call()
+            self.validator.run()
 
     def test_raises_with_invalid_client_id(self):
         self.validator.client_id = "123"
 
         with pytest.raises(APIUnauthorized):
-            self.validator.call()
+            self.validator.run()