Merge branch 'main' into rvinnakota/add-update-repo-mutation

api/internal/tests/views/test_account_viewset.py

@@ -8,7 +8,7 @@
 from rest_framework import status
 from rest_framework.reverse import reverse
 from rest_framework.test import APITestCase
-from stripe.error import StripeError
+from stripe import StripeError
 
 from api.internal.tests.test_utils import GetAdminProviderAdapter
 from codecov_auth.models import Service

api/internal/tests/views/test_invoice_viewset.py

@@ -5,7 +5,7 @@
 from rest_framework import status
 from rest_framework.reverse import reverse
 from rest_framework.test import APITestCase
-from stripe.error import InvalidRequestError, StripeError
+from stripe import InvalidRequestError, StripeError
 
 from api.internal.tests.test_utils import GetAdminProviderAdapter
 from codecov_auth.tests.factories import OwnerFactory

api/public/v2/compare/views.py

@@ -1,6 +1,6 @@
-from distutils.util import strtobool
 from inspect import Parameter
 
+from distutils.util import strtobool
 from drf_spectacular.types import OpenApiTypes
 from drf_spectacular.utils import OpenApiParameter, extend_schema
 from rest_framework import mixins

api/shared/permissions.py

@@ -1,8 +1,9 @@
 import logging
+from typing import Any, Tuple
 
 from asgiref.sync import async_to_sync
 from django.conf import settings
-from django.http import Http404
+from django.http import Http404, HttpRequest
 from rest_framework.permissions import (
     SAFE_METHODS,  # ['GET', 'HEAD', 'OPTIONS']
     BasePermission,
@@ -11,6 +12,8 @@
 import services.self_hosted as self_hosted
 from api.shared.mixins import InternalPermissionsMixin, SuperPermissionsMixin
 from api.shared.repo.repository_accessors import RepoAccessors
+from codecov_auth.models import Owner
+from core.models import Repository
 from services.activation import try_auto_activate
 from services.decorators import torngit_safe
 from services.repo_providers import get_generic_adapter_params, get_provider
@@ -20,7 +23,9 @@
 
 class RepositoryPermissionsService:
     @torngit_safe
-    def _fetch_provider_permissions(self, owner, repo):
+    def _fetch_provider_permissions(
+        self, owner: Owner, repo: Repository
+    ) -> Tuple[bool, bool]:
         can_view, can_edit = RepoAccessors().get_repo_permissions(owner, repo)
 
         if can_view:
@@ -30,7 +35,7 @@ def _fetch_provider_permissions(self, owner, repo):
 
         return can_view, can_edit
 
-    def has_read_permissions(self, owner, repo):
+    def has_read_permissions(self, owner: Owner, repo: Repository) -> bool:
         return not repo.private or (
             owner is not None
             and (
@@ -41,13 +46,13 @@ def has_read_permissions(self, owner, repo):
             )
         )
 
-    def has_write_permissions(self, user, repo):
+    def has_write_permissions(self, user: Owner, repo: Repository) -> bool:
         return user.is_authenticated and (
             repo.author.ownerid == user.ownerid
             or self._fetch_provider_permissions(user, repo)[1]
         )
 
-    def user_is_activated(self, current_owner, owner):
+    def user_is_activated(self, current_owner: Owner, owner: Owner) -> bool:
         if not current_owner or not owner:
             return False
         if current_owner.ownerid == owner.ownerid:
@@ -82,7 +87,7 @@ class RepositoryArtifactPermissions(BasePermission):
         "trying to view a private repo but is not activated."
     )
 
-    def has_permission(self, request, view):
+    def has_permission(self, request: HttpRequest, view: Any) -> bool:
         if view.repo.private:
             user_activated_permissions = (
                 request.user.is_authenticated
@@ -107,19 +112,19 @@ def has_permission(self, request, view):
 
 
 class SuperTokenPermissions(BasePermission, SuperPermissionsMixin):
-    def has_permission(self, request, view):
+    def has_permission(self, request: HttpRequest, view: Any) -> bool:
         return self.has_super_token_permissions(request)
 
 
 class InternalTokenPermissions(BasePermission, InternalPermissionsMixin):
-    def has_permission(self, request, view):
+    def has_permission(self, request: HttpRequest, view: Any) -> bool:
         return self.has_internal_token_permissions(request)
 
 
 class ChartPermissions(BasePermission):
     permissions_service = RepositoryPermissionsService()
 
-    def has_permission(self, request, view):
+    def has_permission(self, request: HttpRequest, view: Any) -> bool:
         log.info(
             f"Coverage chart has repositories {view.repositories}",
             extra=dict(user=request.current_owner),
@@ -142,7 +147,7 @@ class UserIsAdminPermissions(BasePermission):
     returns this owner.
     """
 
-    def has_permission(self, request, view):
+    def has_permission(self, request: HttpRequest, view: Any) -> bool:
         if settings.IS_ENTERPRISE:
             return request.user.is_authenticated and self_hosted.is_admin_owner(
                 request.current_owner
@@ -158,7 +163,7 @@ def has_permission(self, request, view):
             )
 
     @torngit_safe
-    def _is_admin_on_provider(self, user, owner):
+    def _is_admin_on_provider(self, user: Owner, owner: Owner) -> bool:
         torngit_provider_adapter = get_provider(
             owner.service,
             {
@@ -183,7 +188,7 @@ class MemberOfOrgPermissions(BasePermission):
     Requires that the view has a '.owner' property that returns this owner.
     """
 
-    def has_permission(self, request, view):
+    def has_permission(self, request: HttpRequest, view: Any) -> bool:
         if not request.user.is_authenticated:
             return False
 

billing/tests/test_views.py

@@ -5,6 +5,7 @@
 import stripe
 from django.conf import settings
 from freezegun import freeze_time
+from pytest import raises
 from rest_framework import status
 from rest_framework.reverse import reverse
 from rest_framework.test import APIRequestFactory, APITestCase
@@ -18,18 +19,12 @@
 
 class MockSubscriptionPlan(object):
     def __init__(self, params):
-        self.name = params["new_plan"]
-
-
-class MockOboOrg(object):
-    def __init__(self, owner):
-        self.obo_organization = owner.ownerid
-        self.obo = 15
+        self.id = params["new_plan"]
 
 
 class MockSubscription(object):
     def __init__(self, owner, params):
-        self.metadata = MockOboOrg(owner)
+        self.metadata = {"obo_organization": owner.ownerid, "obo": 15}
         self.plan = MockSubscriptionPlan(params)
         self.quantity = params["new_quantity"]
         self.customer = "cus_123"
@@ -38,7 +33,7 @@ def __init__(self, owner, params):
             "data": [
                 {
                     "quantity": params["new_quantity"],
-                    "plan": {"name": params["new_plan"]},
+                    "plan": {"id": params["new_plan"]},
                 }
             ]
         }
@@ -53,7 +48,7 @@ def setUp(self):
             stripe_customer_id="20f0", stripe_subscription_id="3p00"
         )
 
-    def _send_event(self, payload):
+    def _send_event(self, payload, errorSig=None):
         timestamp = time.time_ns()
 
         request = APIRequestFactory().post(
@@ -63,7 +58,8 @@ def _send_event(self, payload):
         return self.client.post(
             reverse("stripe-webhook"),
             **{
-                StripeHTTPHeaders.SIGNATURE: "t={},v1={}".format(
+                StripeHTTPHeaders.SIGNATURE: errorSig
+                or "t={},v1={}".format(
                     timestamp,
                     stripe.WebhookSignature._compute_signature(
                         "{}.{}".format(timestamp, request.body.decode("utf-8")),
@@ -75,6 +71,16 @@ def _send_event(self, payload):
             format="json",
         )
 
+    def test_invalid_event_signature(self):
+        response = self._send_event(
+            payload={
+                "type": "blah",
+                "data": {},
+            },
+            errorSig="lol",
+        )
+        assert response.status_code == status.HTTP_400_BAD_REQUEST
+
     def test_invoice_payment_succeeded_sets_owner_delinquent_false(self):
         self.owner.deliquent = True
         self.owner.save()
@@ -178,14 +184,14 @@ def test_customer_subscription_created_does_nothing_if_no_plan_id(self):
         self.owner.stripe_customer_id = None
         self.owner.save()
 
-        response = self._send_event(
+        self._send_event(
             payload={
                 "type": "customer.subscription.created",
                 "data": {
                     "object": {
                         "id": "FOEKDCDEQ",
                         "customer": "sdo050493",
-                        "plan": {"id": None, "name": "users-inappy"},
+                        "plan": {"id": None},
                         "metadata": {"obo_organization": self.owner.ownerid},
                         "quantity": 20,
                     }
@@ -211,7 +217,7 @@ def test_customer_subscription_created_does_nothing_if_plan_not_paid_user_plan(
                     "object": {
                         "id": "FOEKDCDEQ",
                         "customer": "sdo050493",
-                        "plan": {"id": "fieown4", "name": "users-free"},
+                        "plan": {"id": "?"},
                         "metadata": {"obo_organization": self.owner.ownerid},
                         "quantity": 20,
                     }
@@ -240,7 +246,7 @@ def test_customer_subscription_created_sets_plan_info(self):
                     "object": {
                         "id": stripe_subscription_id,
                         "customer": stripe_customer_id,
-                        "plan": {"id": "fieown4", "name": plan_name},
+                        "plan": {"id": "plan_H6P16wij3lUuxg"},
                         "metadata": {"obo_organization": self.owner.ownerid},
                         "quantity": quantity,
                         "status": "active",
@@ -274,7 +280,7 @@ def test_customer_subscription_created_can_trigger_trial_expiration(
                     "object": {
                         "id": stripe_subscription_id,
                         "customer": stripe_customer_id,
-                        "plan": {"id": "fieown4", "name": plan_name},
+                        "plan": {"id": "plan_H6P16wij3lUuxg"},
                         "metadata": {"obo_organization": self.owner.ownerid},
                         "quantity": quantity,
                         "default_payment_method": "blabla",
@@ -301,7 +307,7 @@ def test_customer_subscription_updated_does_not_change_subscription_if_not_paid_
                     "object": {
                         "id": self.owner.stripe_subscription_id,
                         "customer": self.owner.stripe_customer_id,
-                        "plan": {"id": "fieown4", "name": "users-free"},
+                        "plan": {"id": "?"},
                         "metadata": {"obo_organization": self.owner.ownerid},
                         "quantity": 20,
                         "status": "active",
@@ -334,7 +340,7 @@ def test_customer_subscription_updated_does_not_change_subscription_if_there_is_
                     "object": {
                         "id": self.owner.stripe_subscription_id,
                         "customer": self.owner.stripe_customer_id,
-                        "plan": {"id": "fieown4", "name": "users-free"},
+                        "plan": {"id": "?"},
                         "metadata": {"obo_organization": self.owner.ownerid},
                         "quantity": 20,
                         "status": "active",
@@ -372,7 +378,9 @@ def test_customer_subscription_updated_sets_free_and_deactivates_all_repos_if_in
                     "object": {
                         "id": self.owner.stripe_subscription_id,
                         "customer": self.owner.stripe_customer_id,
-                        "plan": {"id": "fieown4", "name": "users-pr-inappy"},
+                        "plan": {
+                            "id": "plan_H6P16wij3lUuxg",
+                        },
                         "metadata": {"obo_organization": self.owner.ownerid},
                         "quantity": 20,
                         "status": "incomplete_expired",
@@ -402,14 +410,14 @@ def test_customer_subscription_updated_sets_fields_on_success(self, upm_mock):
         plan_name = "users-pr-inappy"
         quantity = 20
 
-        response = self._send_event(
+        self._send_event(
             payload={
                 "type": "customer.subscription.updated",
                 "data": {
                     "object": {
                         "id": self.owner.stripe_subscription_id,
                         "customer": self.owner.stripe_customer_id,
-                        "plan": {"id": "fieown4", "name": plan_name},
+                        "plan": {"id": "plan_H6P16wij3lUuxg"},
                         "metadata": {"obo_organization": self.owner.ownerid},
                         "quantity": quantity,
                         "status": "active",
@@ -435,7 +443,8 @@ def test_subscription_schedule_released_updates_owner_with_existing_subscription
         self.owner.save()
 
         self.new_params = {
-            "new_plan": "users-pr-inappm",
+            "new_plan": "plan_H6P3KZXwmAbqPS",
+            "new_id": "plan_H6P3KZXwmAbqPS",
             "new_quantity": 7,
             "subscription_id": None,
         }
@@ -456,7 +465,7 @@ def test_subscription_schedule_released_updates_owner_with_existing_subscription
         )
 
         self.owner.refresh_from_db()
-        assert self.owner.plan == self.new_params["new_plan"]
+        assert self.owner.plan == settings.STRIPE_PLAN_VALS[self.new_params["new_plan"]]
         assert self.owner.plan_user_count == self.new_params["new_quantity"]
 
     @patch("services.billing.stripe.Subscription.retrieve")
@@ -471,7 +480,7 @@ def test_subscription_schedule_created_logs_a_new_schedule(
         self.owner.save()
 
         self.params = {
-            "new_plan": "users-pr-inappm",
+            "new_plan": "plan_H6P3KZXwmAbqPS",
             "new_quantity": 7,
             "subscription_id": subscription_id,
         }