Revert "ref(mediators): Make validator into a dataclass (#79116)"

This reverts commit e2df3f1.

Co-authored-by: Christinarlong <60594860+Christinarlong@users.noreply.github.com>

src/sentry/mediators/__init__.py

@@ -1,2 +1,3 @@
 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

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

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

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

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,14 +31,15 @@ class GrantExchanger:
 
     def run(self):
         with transaction.atomic(using=router.db_for_write(ApiToken)):
-            if self._validate():
-                token = self._create_token()
+            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(
@@ -47,15 +48,14 @@ def record_analytics(self) -> None:
             exchange_type="authorization",
         )
 
-    def _validate(self) -> bool:
-        is_valid = Validator(install=self.install, client_id=self.client_id, user=self.user).run()
+    def _validate(self) -> None:
+        Validator.run(install=self.install, client_id=self.client_id, user=self.user)
 
         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)):
-            if self._validate():
-                self.token.delete()
+            self._validate()
+            self.token.delete()
 
         self.record_analytics()
         return self._create_new_token()
@@ -41,12 +41,11 @@ def record_analytics(self) -> None:
             exchange_type="refresh",
         )
 
-    def _validate(self) -> bool:
-        is_valid = Validator(install=self.install, client_id=self.client_id, user=self.user).run()
+    def _validate(self) -> None:
+        Validator.run(install=self.install, client_id=self.client_id, user=self.user)
 
         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/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,11 +157,9 @@ 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/token_exchange/test_validator.py → tests/sentry/mediators/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.run()
+        assert self.validator.call()
 
     def test_request_must_be_made_by_sentry_app(self):
         self.validator.user = self.create_user()
 
         with pytest.raises(APIUnauthorized):
-            self.validator.run()
+            self.validator.call()
 
     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.run()
+            self.validator.call()
 
     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.run()
+            self.validator.call()
 
     @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.run()
+            self.validator.call()
 
     def test_raises_with_invalid_client_id(self):
         self.validator.client_id = "123"
 
         with pytest.raises(APIUnauthorized):
-            self.validator.run()
+            self.validator.call()

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,6 +35,14 @@ 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_grant_must_belong_to_installations(self):
         other_install = self.create_sentry_app_installation(prevent_token_exchange=True)
         self.grant_exchanger.code = other_install.api_grant.code

tests/sentry/sentry_apps/token_exchange/test_refresher.py

@@ -42,6 +42,14 @@ 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_token_belongs_to_sentry_app(self):
         refresh_token = ApiToken.objects.create(
             user=self.user,