diff --git a/api/internal/owner/serializers.py b/api/internal/owner/serializers.py index 3d21367a2c..7672f61d64 100644 --- a/api/internal/owner/serializers.py +++ b/api/internal/owner/serializers.py @@ -109,8 +109,14 @@ class StripeCardSerializer(serializers.Serializer): last4 = serializers.CharField() +class StripeUSBankAccountSerializer(serializers.Serializer): + bank_name = serializers.CharField() + last4 = serializers.CharField() + + class StripePaymentMethodSerializer(serializers.Serializer): card = StripeCardSerializer(read_only=True) + us_bank_account = StripeUSBankAccountSerializer(read_only=True) billing_details = serializers.JSONField(read_only=True) diff --git a/api/internal/owner/views.py b/api/internal/owner/views.py index 96fe636844..74900ab2a0 100644 --- a/api/internal/owner/views.py +++ b/api/internal/owner/views.py @@ -80,12 +80,33 @@ def update_payment(self, request, *args, **kwargs): @action(detail=False, methods=["patch"]) @stripe_safe def update_email(self, request, *args, **kwargs): + """ + Update the email address associated with the owner's billing account. + + Args: + request: The HTTP request object containing: + - new_email: The new email address to update to + - apply_to_default_payment_method: Boolean flag to update email on the default payment method (default False) + + Returns: + Response with serialized owner data + + Raises: + ValidationError: If no new_email is provided in the request + """ new_email = request.data.get("new_email") if not new_email: raise ValidationError(detail="No new_email sent") owner = self.get_object() billing = BillingService(requesting_user=request.current_owner) - billing.update_email_address(owner, new_email) + apply_to_default_payment_method = request.data.get( + "apply_to_default_payment_method", False + ) + billing.update_email_address( + owner, + new_email, + apply_to_default_payment_method=apply_to_default_payment_method, + ) return Response(self.get_serializer(owner).data) @action(detail=False, methods=["patch"]) diff --git a/api/internal/tests/views/test_account_viewset.py b/api/internal/tests/views/test_account_viewset.py index 1aabeab654..c183cfef24 100644 --- a/api/internal/tests/views/test_account_viewset.py +++ b/api/internal/tests/views/test_account_viewset.py @@ -1228,6 +1228,46 @@ def test_update_email_address(self, modify_customer_mock, retrieve_mock): self.current_owner.stripe_customer_id, email=new_email ) + @patch("services.billing.stripe.Subscription.retrieve") + @patch("services.billing.stripe.Customer.modify") + @patch("services.billing.stripe.PaymentMethod.modify") + @patch("services.billing.stripe.Customer.retrieve") + def test_update_email_address_with_propagate( + self, + customer_retrieve_mock, + payment_method_mock, + modify_customer_mock, + retrieve_mock, + ): + self.current_owner.stripe_customer_id = "flsoe" + self.current_owner.stripe_subscription_id = "djfos" + self.current_owner.save() + + payment_method_id = "pm_123" + customer_retrieve_mock.return_value = { + "invoice_settings": {"default_payment_method": payment_method_id} + } + + new_email = "test@gmail.com" + kwargs = { + "service": self.current_owner.service, + "owner_username": self.current_owner.username, + } + data = {"new_email": new_email, "apply_to_default_payment_method": True} + url = reverse("account_details-update-email", kwargs=kwargs) + response = self.client.patch(url, data=data, format="json") + assert response.status_code == status.HTTP_200_OK + + modify_customer_mock.assert_called_once_with( + self.current_owner.stripe_customer_id, email=new_email + ) + customer_retrieve_mock.assert_called_once_with( + self.current_owner.stripe_customer_id + ) + payment_method_mock.assert_called_once_with( + payment_method_id, billing_details={"email": new_email} + ) + def test_update_billing_address_without_body(self): kwargs = { "service": self.current_owner.service, diff --git a/codecov/settings_base.py b/codecov/settings_base.py index 6cd27bf275..736d7c678d 100644 --- a/codecov/settings_base.py +++ b/codecov/settings_base.py @@ -429,6 +429,10 @@ SHELTER_PUBSUB_PROJECT_ID = get_config("setup", "shelter", "pubsub_project_id") SHELTER_PUBSUB_SYNC_REPO_TOPIC_ID = get_config("setup", "shelter", "sync_repo_topic_id") +STRIPE_PAYMENT_METHOD_CONFIGURATION_ID = get_config( + "setup", "stripe", "payment_method_configuration_id", default=None +) + # Allows to do migrations from another module MIGRATION_MODULES = { "codecov_auth": "shared.django_apps.codecov_auth.migrations", diff --git a/codecov_auth/commands/owner/interactors/create_stripe_setup_intent.py b/codecov_auth/commands/owner/interactors/create_stripe_setup_intent.py new file mode 100644 index 0000000000..0fc5598a67 --- /dev/null +++ b/codecov_auth/commands/owner/interactors/create_stripe_setup_intent.py @@ -0,0 +1,42 @@ +import logging + +import stripe + +from codecov.commands.base import BaseInteractor +from codecov.commands.exceptions import Unauthenticated, Unauthorized, ValidationError +from codecov.db import sync_to_async +from codecov_auth.helpers import current_user_part_of_org +from codecov_auth.models import Owner +from services.billing import BillingService + +log = logging.getLogger(__name__) + + +class CreateStripeSetupIntentInteractor(BaseInteractor): + def validate(self, owner_obj: Owner) -> None: + if not self.current_user.is_authenticated: + raise Unauthenticated() + if not owner_obj: + raise ValidationError("Owner not found") + if not current_user_part_of_org(self.current_owner, owner_obj): + raise Unauthorized() + + def create_setup_intent(self, owner_obj: Owner) -> stripe.SetupIntent: + try: + billing = BillingService(requesting_user=self.current_owner) + return billing.create_setup_intent(owner_obj) + except Exception as e: + log.error( + "Error getting setup intent", + extra={ + "ownerid": owner_obj.ownerid, + "error": str(e), + }, + ) + raise ValidationError("Unable to create setup intent") + + @sync_to_async + def execute(self, owner: str) -> stripe.SetupIntent: + owner_obj = Owner.objects.filter(username=owner, service=self.service).first() + self.validate(owner_obj) + return self.create_setup_intent(owner_obj) diff --git a/codecov_auth/commands/owner/owner.py b/codecov_auth/commands/owner/owner.py index 2b97f5017a..53b21a9577 100644 --- a/codecov_auth/commands/owner/owner.py +++ b/codecov_auth/commands/owner/owner.py @@ -2,6 +2,7 @@ from .interactors.cancel_trial import CancelTrialInteractor from .interactors.create_api_token import CreateApiTokenInteractor +from .interactors.create_stripe_setup_intent import CreateStripeSetupIntentInteractor from .interactors.create_user_token import CreateUserTokenInteractor from .interactors.delete_session import DeleteSessionInteractor from .interactors.fetch_owner import FetchOwnerInteractor @@ -28,6 +29,9 @@ class OwnerCommands(BaseCommand): def create_api_token(self, name): return self.get_interactor(CreateApiTokenInteractor).execute(name) + def create_stripe_setup_intent(self, owner): + return self.get_interactor(CreateStripeSetupIntentInteractor).execute(owner) + def delete_session(self, sessionid: int): return self.get_interactor(DeleteSessionInteractor).execute(sessionid) diff --git a/graphql_api/tests/mutation/test_create_stripe_setup_intent.py b/graphql_api/tests/mutation/test_create_stripe_setup_intent.py new file mode 100644 index 0000000000..609d84f579 --- /dev/null +++ b/graphql_api/tests/mutation/test_create_stripe_setup_intent.py @@ -0,0 +1,67 @@ +from unittest.mock import patch + +from django.test import TransactionTestCase +from shared.django_apps.core.tests.factories import OwnerFactory + +from graphql_api.tests.helper import GraphQLTestHelper + +query = """ +mutation($input: CreateStripeSetupIntentInput!) { + createStripeSetupIntent(input: $input) { + error { + __typename + } + clientSecret + } +} +""" + + +class CreateStripeSetupIntentTestCase(GraphQLTestHelper, TransactionTestCase): + def setUp(self): + self.owner = OwnerFactory(username="codecov-user") + + def test_when_unauthenticated(self): + data = self.gql_request(query, variables={"input": {"owner": "somename"}}) + assert ( + data["createStripeSetupIntent"]["error"]["__typename"] + == "UnauthenticatedError" + ) + + def test_when_unauthorized(self): + other_owner = OwnerFactory(username="other-user") + data = self.gql_request( + query, + owner=self.owner, + variables={"input": {"owner": other_owner.username}}, + ) + assert ( + data["createStripeSetupIntent"]["error"]["__typename"] + == "UnauthorizedError" + ) + + @patch("services.billing.stripe.SetupIntent.create") + def test_when_validation_error(self, setup_intent_create_mock): + setup_intent_create_mock.side_effect = Exception("Some error") + data = self.gql_request( + query, owner=self.owner, variables={"input": {"owner": self.owner.username}} + ) + assert ( + data["createStripeSetupIntent"]["error"]["__typename"] == "ValidationError" + ) + + def test_when_owner_not_found(self): + data = self.gql_request( + query, owner=self.owner, variables={"input": {"owner": "nonexistent-user"}} + ) + assert ( + data["createStripeSetupIntent"]["error"]["__typename"] == "ValidationError" + ) + + @patch("services.billing.stripe.SetupIntent.create") + def test_success(self, setup_intent_create_mock): + setup_intent_create_mock.return_value = {"client_secret": "test-client-secret"} + data = self.gql_request( + query, owner=self.owner, variables={"input": {"owner": self.owner.username}} + ) + assert data["createStripeSetupIntent"]["clientSecret"] == "test-client-secret" diff --git a/graphql_api/types/inputs/create_stripe_setup_intent.graphql b/graphql_api/types/inputs/create_stripe_setup_intent.graphql new file mode 100644 index 0000000000..5fe147ba5b --- /dev/null +++ b/graphql_api/types/inputs/create_stripe_setup_intent.graphql @@ -0,0 +1,3 @@ +input CreateStripeSetupIntentInput { + owner: String! +} diff --git a/graphql_api/types/invoice/invoice.graphql b/graphql_api/types/invoice/invoice.graphql index 1c6b2ae1ec..ef11d76607 100644 --- a/graphql_api/types/invoice/invoice.graphql +++ b/graphql_api/types/invoice/invoice.graphql @@ -34,6 +34,7 @@ type Period { type PaymentMethod { billingDetails: BillingDetails card: Card + usBankAccount: USBankAccount } type Card { @@ -43,6 +44,11 @@ type Card { last4: String } +type USBankAccount { + bankName: String + last4: String +} + type BillingDetails { address: Address email: String diff --git a/graphql_api/types/mutation/__init__.py b/graphql_api/types/mutation/__init__.py index adc9f3501f..24ddd033ff 100644 --- a/graphql_api/types/mutation/__init__.py +++ b/graphql_api/types/mutation/__init__.py @@ -3,6 +3,7 @@ from .activate_measurements import gql_activate_measurements from .cancel_trial import gql_cancel_trial from .create_api_token import gql_create_api_token +from .create_stripe_setup_intent import gql_create_stripe_setup_intent from .create_user_token import gql_create_user_token from .delete_component_measurements import gql_delete_component_measurements from .delete_flag import gql_delete_flag @@ -31,6 +32,7 @@ mutation = ariadne_load_local_graphql(__file__, "mutation.graphql") mutation = mutation + gql_create_api_token +mutation = mutation + gql_create_stripe_setup_intent mutation = mutation + gql_sync_with_git_provider mutation = mutation + gql_delete_session mutation = mutation + gql_set_yaml_on_owner diff --git a/graphql_api/types/mutation/create_stripe_setup_intent/__init__.py b/graphql_api/types/mutation/create_stripe_setup_intent/__init__.py new file mode 100644 index 0000000000..967ae52579 --- /dev/null +++ b/graphql_api/types/mutation/create_stripe_setup_intent/__init__.py @@ -0,0 +1,12 @@ +from graphql_api.helpers.ariadne import ariadne_load_local_graphql + +from .create_stripe_setup_intent import ( + error_create_stripe_setup_intent, + resolve_create_stripe_setup_intent, +) + +gql_create_stripe_setup_intent = ariadne_load_local_graphql( + __file__, "create_stripe_setup_intent.graphql" +) + +__all__ = ["error_create_stripe_setup_intent", "resolve_create_stripe_setup_intent"] diff --git a/graphql_api/types/mutation/create_stripe_setup_intent/create_stripe_setup_intent.graphql b/graphql_api/types/mutation/create_stripe_setup_intent/create_stripe_setup_intent.graphql new file mode 100644 index 0000000000..9f33c57626 --- /dev/null +++ b/graphql_api/types/mutation/create_stripe_setup_intent/create_stripe_setup_intent.graphql @@ -0,0 +1,6 @@ +union CreateStripeSetupIntentError = UnauthenticatedError | UnauthorizedError | ValidationError + +type CreateStripeSetupIntentPayload { + error: CreateStripeSetupIntentError + clientSecret: String +} diff --git a/graphql_api/types/mutation/create_stripe_setup_intent/create_stripe_setup_intent.py b/graphql_api/types/mutation/create_stripe_setup_intent/create_stripe_setup_intent.py new file mode 100644 index 0000000000..0b1f0590e2 --- /dev/null +++ b/graphql_api/types/mutation/create_stripe_setup_intent/create_stripe_setup_intent.py @@ -0,0 +1,24 @@ +from typing import Any, Dict + +from ariadne import UnionType +from ariadne.types import GraphQLResolveInfo + +from graphql_api.helpers.mutation import ( + resolve_union_error_type, + wrap_error_handling_mutation, +) + + +@wrap_error_handling_mutation +async def resolve_create_stripe_setup_intent( + _: Any, info: GraphQLResolveInfo, input: Dict[str, str] +) -> Dict[str, str]: + command = info.context["executor"].get_command("owner") + resp = await command.create_stripe_setup_intent(input.get("owner")) + return { + "client_secret": resp["client_secret"], + } + + +error_create_stripe_setup_intent = UnionType("CreateStripeSetupIntentError") +error_create_stripe_setup_intent.type_resolver(resolve_union_error_type) diff --git a/graphql_api/types/mutation/mutation.graphql b/graphql_api/types/mutation/mutation.graphql index bad930d06f..0c91bc768c 100644 --- a/graphql_api/types/mutation/mutation.graphql +++ b/graphql_api/types/mutation/mutation.graphql @@ -1,5 +1,6 @@ type Mutation { createApiToken(input: CreateApiTokenInput!): CreateApiTokenPayload + createStripeSetupIntent(input: CreateStripeSetupIntentInput!): CreateStripeSetupIntentPayload createUserToken(input: CreateUserTokenInput!): CreateUserTokenPayload revokeUserToken(input: RevokeUserTokenInput!): RevokeUserTokenPayload setYamlOnOwner(input: SetYamlOnOwnerInput!): SetYamlOnOwnerPayload diff --git a/graphql_api/types/mutation/mutation.py b/graphql_api/types/mutation/mutation.py index d1fd6ea7dd..ecf39f05d2 100644 --- a/graphql_api/types/mutation/mutation.py +++ b/graphql_api/types/mutation/mutation.py @@ -6,6 +6,10 @@ ) from .cancel_trial import error_cancel_trial, resolve_cancel_trial from .create_api_token import error_create_api_token, resolve_create_api_token +from .create_stripe_setup_intent import ( + error_create_stripe_setup_intent, + resolve_create_stripe_setup_intent, +) from .create_user_token import error_create_user_token, resolve_create_user_token from .delete_component_measurements import ( error_delete_component_measurements, @@ -68,6 +72,7 @@ # Here, bind the resolvers from each subfolder to the Mutation type mutation_bindable.field("createApiToken")(resolve_create_api_token) +mutation_bindable.field("createStripeSetupIntent")(resolve_create_stripe_setup_intent) mutation_bindable.field("createUserToken")(resolve_create_user_token) mutation_bindable.field("revokeUserToken")(resolve_revoke_user_token) mutation_bindable.field("setYamlOnOwner")(resolve_set_yaml_on_owner) @@ -108,6 +113,7 @@ mutation_resolvers = [ mutation_bindable, error_create_api_token, + error_create_stripe_setup_intent, error_create_user_token, error_revoke_user_token, error_set_yaml_error, diff --git a/services/billing.py b/services/billing.py index 4568f26d40..825cd7ccab 100644 --- a/services/billing.py +++ b/services/billing.py @@ -83,6 +83,10 @@ def get_schedule(self, owner): def apply_cancellation_discount(self, owner: Owner): pass + @abstractmethod + def create_setup_intent(self, owner): + pass + class StripeService(AbstractPaymentService): def __init__(self, requesting_user): @@ -512,8 +516,8 @@ def create_checkout_session(self, owner: Owner, desired_plan): ) session = stripe.checkout.Session.create( + payment_method_configuration=settings.STRIPE_PAYMENT_METHOD_CONFIGURATION_ID, billing_address_collection="required", - payment_method_types=["card"], payment_method_collection="if_required", client_reference_id=str(owner.ownerid), success_url=success_url, @@ -580,7 +584,12 @@ def update_payment_method(self, owner: Owner, payment_method): ) @_log_stripe_error - def update_email_address(self, owner: Owner, email_address: str): + def update_email_address( + self, + owner: Owner, + email_address: str, + apply_to_default_payment_method: bool = False, + ): if not re.fullmatch(r"[^@]+@[^@]+\.[^@]+", email_address): return None @@ -595,6 +604,35 @@ def update_email_address(self, owner: Owner, email_address: str): f"Stripe successfully updated email address for owner {owner.ownerid} by user #{self.requesting_user.ownerid}" ) + if apply_to_default_payment_method: + try: + default_payment_method = stripe.Customer.retrieve( + owner.stripe_customer_id + )["invoice_settings"]["default_payment_method"] + + stripe.PaymentMethod.modify( + default_payment_method, + billing_details={"email": email_address}, + ) + log.info( + "Stripe successfully updated billing email for payment method", + extra=dict( + payment_method=default_payment_method, + stripe_customer_id=owner.stripe_customer_id, + ownerid=owner.ownerid, + ), + ) + except Exception as e: + log.error( + "Unable to update billing email for payment method", + extra=dict( + payment_method=default_payment_method, + stripe_customer_id=owner.stripe_customer_id, + error=str(e), + ownerid=owner.ownerid, + ), + ) + @_log_stripe_error def update_billing_address(self, owner: Owner, name, billing_address): log.info(f"Stripe update billing address for owner {owner.ownerid}") @@ -664,6 +702,22 @@ def apply_cancellation_discount(self, owner: Owner): coupon=owner.stripe_coupon_id, ) + @_log_stripe_error + def create_setup_intent(self, owner: Owner) -> stripe.SetupIntent: + log.info( + "Stripe create setup intent for owner", + extra=dict( + owner_id=owner.ownerid, + requesting_user_id=self.requesting_user.ownerid, + subscription_id=owner.stripe_subscription_id, + customer_id=owner.stripe_customer_id, + ), + ) + return stripe.SetupIntent.create( + payment_method_configuration=settings.STRIPE_PAYMENT_METHOD_CONFIGURATION_ID, + customer=owner.stripe_customer_id, + ) + class EnterprisePaymentService(AbstractPaymentService): # enterprise has no payments setup so these are all noops @@ -701,6 +755,9 @@ def get_schedule(self, owner): def apply_cancellation_discount(self, owner: Owner): pass + def create_setup_intent(self, owner): + pass + class BillingService: payment_service = None @@ -762,14 +819,21 @@ def update_payment_method(self, owner, payment_method): """ return self.payment_service.update_payment_method(owner, payment_method) - def update_email_address(self, owner: Owner, email_address: str): + def update_email_address( + self, + owner: Owner, + email_address: str, + apply_to_default_payment_method: bool = False, + ): """ Takes an owner and a new email. Email is a string coming directly from the front-end. If the owner has a payment id and if it's a valid email, the payment service will update the email address in the upstream service. Otherwise returns None. """ - return self.payment_service.update_email_address(owner, email_address) + return self.payment_service.update_email_address( + owner, email_address, apply_to_default_payment_method + ) def update_billing_address(self, owner: Owner, name: str, billing_address): """ @@ -781,3 +845,10 @@ def update_billing_address(self, owner: Owner, name: str, billing_address): def apply_cancellation_discount(self, owner: Owner): return self.payment_service.apply_cancellation_discount(owner) + + def create_setup_intent(self, owner: Owner): + """ + Creates a SetupIntent for the given owner to securely collect payment details + See https://docs.stripe.com/api/setup_intents/create + """ + return self.payment_service.create_setup_intent(owner) diff --git a/services/tests/test_billing.py b/services/tests/test_billing.py index 66b3b1b383..a2edd6640a 100644 --- a/services/tests/test_billing.py +++ b/services/tests/test_billing.py @@ -1512,7 +1512,7 @@ def test_create_checkout_session_with_no_stripe_customer_id( create_checkout_session_mock.assert_called_once_with( billing_address_collection="required", - payment_method_types=["card"], + payment_method_configuration=settings.STRIPE_PAYMENT_METHOD_CONFIGURATION_ID, payment_method_collection="if_required", client_reference_id=str(owner.ownerid), customer=None, @@ -1560,7 +1560,7 @@ def test_create_checkout_session_with_stripe_customer_id( create_checkout_session_mock.assert_called_once_with( billing_address_collection="required", - payment_method_types=["card"], + payment_method_configuration=settings.STRIPE_PAYMENT_METHOD_CONFIGURATION_ID, payment_method_collection="if_required", client_reference_id=str(owner.ownerid), customer=owner.stripe_customer_id, @@ -1825,6 +1825,13 @@ def test_apply_cancellation_discount_existing_coupon( assert not customer_modify_mock.called assert not coupon_create_mock.called + @patch("services.billing.stripe.SetupIntent.create") + def test_create_setup_intent(self, setup_intent_create_mock): + owner = OwnerFactory(stripe_customer_id="test-customer-id") + setup_intent_create_mock.return_value = {"client_secret": "test-client-secret"} + resp = self.stripe.create_setup_intent(owner) + assert resp["client_secret"] == "test-client-secret" + class MockPaymentService(AbstractPaymentService): def list_filtered_invoices(self, owner, limit=10): @@ -1860,6 +1867,9 @@ def get_schedule(self, owner): def apply_cancellation_discount(self, owner): pass + def create_setup_intent(self, owner): + pass + class BillingServiceTests(TestCase): def setUp(self): @@ -2014,8 +2024,8 @@ def test_update_payment_method(self, get_subscription_mock): @patch("services.tests.test_billing.MockPaymentService.update_email_address") def test_email_address(self, get_subscription_mock): owner = OwnerFactory() - self.billing_service.update_email_address(owner, "test@gmail.com") - get_subscription_mock.assert_called_once_with(owner, "test@gmail.com") + self.billing_service.update_email_address(owner, "test@gmail.com", False) + get_subscription_mock.assert_called_once_with(owner, "test@gmail.com", False) @patch("services.tests.test_billing.MockPaymentService.get_invoice") def test_get_invoice(self, get_invoice_mock): diff --git a/utils/test_utils.py b/utils/test_utils.py index 6cac27e04e..6deeb8251a 100644 --- a/utils/test_utils.py +++ b/utils/test_utils.py @@ -45,10 +45,10 @@ def app(self) -> str: migrate_to = None def setUp(self) -> None: - assert self.migrate_from and self.migrate_to, ( - "TestCase '{}' must define migrate_from and migrate_to properties".format( - type(self).__name__ - ) + assert ( + self.migrate_from and self.migrate_to + ), "TestCase '{}' must define migrate_from and migrate_to properties".format( + type(self).__name__ ) self.migrate_from = [(self.app, self.migrate_from)] self.migrate_to = [(self.app, self.migrate_to)]