diff --git a/server/config/settings/base.py b/server/config/settings/base.py index 94b1dcb4..a36bf213 100644 --- a/server/config/settings/base.py +++ b/server/config/settings/base.py @@ -663,10 +663,10 @@ NOTIFICATIONS_NOTIFICATION_MODEL = "notify.Notification" SENDGRID_API_KEY = env("IT_SENDGRID_API_KEY", default="") # Datatrans (payment) -# See https://admin.sandbox.datatrans.com/MerchSecurAdmin.jsp) +# See https://admin.sandbox.datatrans.com/MerchSecurAdmin.jsp DATATRANS_HMAC_KEY = env("DATATRANS_HMAC_KEY", default="") -# See https://admin.sandbox.datatrans.com/MenuDispatch.jsp?main=1&sub=4) +# See https://admin.sandbox.datatrans.com/MenuDispatch.jsp?main=1&sub=4 # => echo -n "Username:Password" | base64 DATATRANS_BASIC_AUTH_KEY = env("DATATRANS_BASIC_AUTH_KEY", default="") diff --git a/server/vbv_lernwelt/shop/README.md b/server/vbv_lernwelt/shop/README.md index e5949c04..fc423a65 100644 --- a/server/vbv_lernwelt/shop/README.md +++ b/server/vbv_lernwelt/shop/README.md @@ -2,8 +2,7 @@ ## Shop Product -In Django Shop App, create new products (Products model) that should be available in the shop. -Products: +In the Django shop app, create new products that should be available in the shop: - `vv-de` Price 30000 (300_00 -> 300.00 CHF), name & description can be anything. - ONLY if `COURSE_VERSICHERUNGSVERMITTLERIN_ID` exists! @@ -12,7 +11,7 @@ Products: - `vv-it` Price 30000 (300_00 -> 300.00 CHF), name & description can be anything. - ONLY if `COURSE_VERSICHERUNGSVERMITTLERIN_ID_IT` exists! -## Datatrans +## Datatrans (Payment Provider) - Set `DATATRANS_BASIC_AUTH_KEY`: - https://admin.sandbox.datatrans.com/MenuDispatch.jsp?main=1&sub=4 @@ -21,27 +20,14 @@ Products: - Set `DATATRANS_HMAC_KEY`: - https://admin.sandbox.datatrans.com/MerchSecurAdmin.jsp -- Ensure that the webhook is set up correctly by Datatrans: - - Be default transitions from `initialized` to `failed` do not trigger the webhook. - - Edgecase: When user starts a datatrans payment and then closes the browser, the payment will be - in `initialized` - state forever. -> That's why we need the webhook for `initialized` -> `failed` transitions. - - This can and needs to be enabled by datatrans (according to Mario from datatrans). - - Livio 21.11.23: Mario promised to enable it, - - Livio 27.11.23. Not yet enabled for the sandbox. -> Followed up! - - Livio: TODO still not enabled. Follow up again! +For Production: -### Production / "going live" - -For Production: We use the proper production datatrans endpoint! - -1. Coordinate with datatrans to get production account. +1. Coordinate with datatrans to get production account. -> TBD! 2. Set `DATATRANS_BASIC_AUTH_KEY` and `DATATRANS_HMAC_KEY` to the production values (see above). -3. Ensure that the webhook is set up correctly by Datatrans (see above). ## OAUTH -Make sure that the following env vars are set: +For Production: Make sure that the following env vars are set: ### Azure B2C @@ -49,26 +35,30 @@ Make sure that the following env vars are set: - Set `OAUTH_SIGNUP_CLIENT_SECRET` - Set `OAUTH_SIGNUP_SERVER_METADATA_URL` (.well-known/openid-configuration) - Set `OAUTH_SIGNUP_TENANT_ID` +- Set `OAUTH_SIGNUP_REDIRECT_URI` (`.../sso/login` e.g. `https://myvbv-stage.iterativ.ch/sso/login`) ### Keycloak - Set `OAUTH_SIGNIN_CLIENT_ID` - Set `OAUTH_SIGNIN_CLIENT_SECRET` - Set `OAUTH_SIGNIN_SERVER_METADATA_URL` (.well-known/openid-configuration) - -### Redirect URIs - -- Set `OAUTH_SIGNUP_REDIRECT_URI` (`.../sso/login` e.g. `https://myvbv-stage.iterativ.ch/sso/login`) - Set `OAUTH_SIGNIN_REDIRECT_URI` (`.../sso/callback` e.g. `https://myvbv-stage.iterativ.ch/sso/callback`) -### Frontend: +### Caprover (VITEx) -- Update `VITE_OAUTH_API_BASE_URL` in `caprover_deploy.sh` for production. - - Should be the SSO Prod one from Lernnetz. +- Set `VITE_OAUTH_API_BASE_URL` in `caprover_deploy.sh` for `prod` environment. + - `OAUTH_SIGNIN_SERVER_METADATA_URL` should help to find the correct value. + - Should be the SSO Prod one from Lernnetz. -> TBD! + +## Testing Payment Flow + +- To get user into state for testing (e.g. test-student1@example.com so that he can buy the course): + - Remove all existing course session users for the user. + - Remove all existing checkout information for the user. ### Cleanup -After everything runs fine, we should be able to remove the following env vars: +After everything runs fine, we should be able to remove the following deprecated env vars: 1. `IT_OAUTH_TENANT_ID` 2. `IT_OAUTH_CLIENT_NAME` @@ -79,3 +69,4 @@ After everything runs fine, we should be able to remove the following env vars: 7. `IT_OAUTH_SERVER_METADATA_URL` 8. `IT_OAUTH_SCOPE` + diff --git a/server/vbv_lernwelt/shop/migrations/0011_alter_checkoutinformation_state.py b/server/vbv_lernwelt/shop/migrations/0011_alter_checkoutinformation_state.py new file mode 100644 index 00000000..bb2dedd9 --- /dev/null +++ b/server/vbv_lernwelt/shop/migrations/0011_alter_checkoutinformation_state.py @@ -0,0 +1,25 @@ +# Generated by Django 3.2.20 on 2023-12-05 13:27 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("shop", "0010_alter_checkoutinformation_state"), + ] + + operations = [ + migrations.AlterField( + model_name="checkoutinformation", + name="state", + field=models.CharField( + choices=[ + ("ongoing", "Ongoing"), + ("paid", "Paid"), + ("canceled", "Canceled"), + ("failed", "Failed"), + ], + max_length=50, + ), + ), + ] diff --git a/server/vbv_lernwelt/shop/models.py b/server/vbv_lernwelt/shop/models.py index f3587830..e2f728b0 100644 --- a/server/vbv_lernwelt/shop/models.py +++ b/server/vbv_lernwelt/shop/models.py @@ -57,19 +57,19 @@ class CheckoutState(models.TextChoices): The state of a checkout process transaction. PAID: Datatrans transaction settled/transmitted. - Rest of the states are self-explanatory, same as in Datatrans docs. + ONGOING: Any state that is not final (e.g. initialized, challenge_ongoing, etc.) - 1) We use the `autoSettle` feature of DataTrans! Therefore, there are less possible states: + 1) We use the `autoSettle` feature of DataTrans! -> https://docs.datatrans.ch/docs/after-the-payment -> https://api-reference.datatrans.ch/#tag/v1transactions/operation/status 2) Difference between `settled` and `transmitted`: - https://www.datatrans.ch/en/know-how/faq/#what-does-the-status-transaction-settled-or-settledtransmitted-mean - 3) Related relevant code:init_transaction and get_transaction_state in shop/services.py + 3) Related code: init_transaction and get_transaction_state in shop/services.py """ - INITIALIZED = "initialized" + ONGOING = "ongoing" PAID = "paid" CANCELED = "canceled" FAILED = "failed" diff --git a/server/vbv_lernwelt/shop/services.py b/server/vbv_lernwelt/shop/services.py index b7c47587..9a2d1faa 100644 --- a/server/vbv_lernwelt/shop/services.py +++ b/server/vbv_lernwelt/shop/services.py @@ -124,9 +124,10 @@ def get_payment_url(transaction_id: str): return f"{settings.DATATRANS_PAY_URL}/v1/start/{transaction_id}" -def datatrans_state_to_checkout_state( - datatrans_transaction_state: str, -) -> CheckoutState: +def datatrans_state_to_checkout_state(datatrans_transaction_state) -> CheckoutState: + """ + https://api-reference.datatrans.ch/#tag/v1transactions/operation/status + """ if datatrans_transaction_state in ["settled", "transmitted"]: return CheckoutState.PAID elif datatrans_transaction_state == "failed": @@ -136,4 +137,4 @@ def datatrans_state_to_checkout_state( else: # An intermediate state such as "initialized", "challenge_ongoing", etc. # -> we don't care about those states, we only care about final states here. - return CheckoutState.INITIALIZED + return CheckoutState.ONGOING diff --git a/server/vbv_lernwelt/shop/tests/test_checkout_api.py b/server/vbv_lernwelt/shop/tests/test_checkout_api.py index 83d04f08..270b3ba5 100644 --- a/server/vbv_lernwelt/shop/tests/test_checkout_api.py +++ b/server/vbv_lernwelt/shop/tests/test_checkout_api.py @@ -77,7 +77,7 @@ class CheckoutAPITestCase(APITestCase): CheckoutInformation.objects.filter( user=self.user, product_sku=VV_DE_PRODUCT_SKU, - state=CheckoutState.INITIALIZED.value, + state=CheckoutState.ONGOING, ).exists() ) @@ -153,7 +153,7 @@ class CheckoutAPITestCase(APITestCase): user=self.user, product_sku=VV_DE_PRODUCT_SKU, product_price=0, - state=CheckoutState.PAID.value, + state=CheckoutState.PAID, ) # WHEN @@ -174,18 +174,25 @@ class CheckoutAPITestCase(APITestCase): response.json()["next_step_url"], ) - def test_checkout_double_checkout(self): + @patch("vbv_lernwelt.shop.views.init_transaction") + def test_checkout_double_checkout(self, mock_init_transaction): + """Advise by Datatrans: Just create a new transaction.""" # GIVEN - transaction_id = "1234567890" + # existing checkout + transaction_id_previous = "1234567890" CheckoutInformation.objects.create( user=self.user, product_sku=VV_DE_PRODUCT_SKU, product_price=0, - state=CheckoutState.INITIALIZED.value, - transaction_id=transaction_id, + state=CheckoutState.ONGOING, + transaction_id=transaction_id_previous, ) + # new checkout / transaction + transaction_id_next = "9999999999" + mock_init_transaction.return_value = transaction_id_next + # WHEN response = self.client.post( path=reverse("checkout-vv"), @@ -200,14 +207,41 @@ class CheckoutAPITestCase(APITestCase): # THEN self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual( - f"https://pay.sandbox.datatrans.com/v1/start/{transaction_id}", + f"https://pay.sandbox.datatrans.com/v1/start/{transaction_id_next}", response.json()["next_step_url"], ) + # check that we have two checkouts + # (one previous and one new) + self.assertEqual( + 2, + CheckoutInformation.objects.count(), + ) + + # previous checkout + self.assertTrue( + CheckoutInformation.objects.filter( + user=self.user, + product_sku=VV_DE_PRODUCT_SKU, + state=CheckoutState.ONGOING, + transaction_id=transaction_id_previous, + ).exists() + ) + + # new checkout + self.assertTrue( + CheckoutInformation.objects.filter( + user=self.user, + product_sku=VV_DE_PRODUCT_SKU, + state=CheckoutState.ONGOING, + transaction_id=transaction_id_next, + ).exists() + ) + @patch("vbv_lernwelt.shop.views.init_transaction") def test_checkout_failed_creates_new(self, mock_init_transaction): # GIVEN - state = CheckoutState.FAILED.value + state = CheckoutState.FAILED transaction_id = "1234567890" mock_init_transaction.return_value = transaction_id @@ -240,7 +274,7 @@ class CheckoutAPITestCase(APITestCase): @patch("vbv_lernwelt.shop.views.init_transaction") def test_checkout_cancelled_creates_new(self, mock_init_transaction): # GIVEN - state = CheckoutState.CANCELED.value + state = CheckoutState.CANCELED transaction_id = "1234567899" mock_init_transaction.return_value = transaction_id diff --git a/server/vbv_lernwelt/shop/tests/test_datatrans_webhook.py b/server/vbv_lernwelt/shop/tests/test_datatrans_webhook.py index 5f75c671..0bc1a5b4 100644 --- a/server/vbv_lernwelt/shop/tests/test_datatrans_webhook.py +++ b/server/vbv_lernwelt/shop/tests/test_datatrans_webhook.py @@ -82,7 +82,7 @@ class DatatransWebhookTestCase(APITestCase): create_checkout_information( user=self.user, transaction_id=transaction_id, - state=CheckoutState.INITIALIZED, + state=CheckoutState.ONGOING, ) mock_is_signature_valid.return_value = True @@ -143,7 +143,7 @@ class DatatransWebhookTestCase(APITestCase): create_checkout_information( user=self.user, transaction_id=transaction_id, - state=CheckoutState.INITIALIZED, + state=CheckoutState.ONGOING, ) mock_is_signature_valid.return_value = True @@ -203,7 +203,7 @@ class DatatransWebhookTestCase(APITestCase): create_checkout_information( user=self.user, transaction_id=transaction_id, - state=CheckoutState.INITIALIZED, + state=CheckoutState.ONGOING, ) mock_is_signature_valid.return_value = True @@ -223,7 +223,7 @@ class DatatransWebhookTestCase(APITestCase): self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual( CheckoutInformation.objects.get(transaction_id=transaction_id).state, - CheckoutState.FAILED.value, + CheckoutState.FAILED, ) @patch("vbv_lernwelt.shop.views.is_signature_valid") @@ -235,7 +235,7 @@ class DatatransWebhookTestCase(APITestCase): create_checkout_information( # noqa user=self.user, transaction_id=transaction_id, - state=CheckoutState.INITIALIZED, + state=CheckoutState.ONGOING, ) mock_is_signature_valid.return_value = True @@ -255,5 +255,5 @@ class DatatransWebhookTestCase(APITestCase): self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual( CheckoutInformation.objects.get(transaction_id=transaction_id).state, - CheckoutState.CANCELED.value, + CheckoutState.CANCELED, ) diff --git a/server/vbv_lernwelt/shop/views.py b/server/vbv_lernwelt/shop/views.py index 981f658e..5e22cc8e 100644 --- a/server/vbv_lernwelt/shop/views.py +++ b/server/vbv_lernwelt/shop/views.py @@ -1,9 +1,11 @@ import structlog +from django.conf import settings from django.http import JsonResponse from rest_framework import status from rest_framework.decorators import api_view, permission_classes from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response +from sentry_sdk import capture_exception from vbv_lernwelt.course.consts import ( COURSE_VERSICHERUNGSVERMITTLERIN_FR_ID, @@ -75,13 +77,15 @@ def update_billing_address(request): @api_view(["POST"]) def transaction_webhook(request): + """IMPORTANT: This is not called for timed out transactions!""" + logger.info("Webhook: Datatrans called transaction webhook", body=request.body) if not is_signature_valid( signature=request.headers.get("Datatrans-Signature", ""), payload=request.body, ): - logger.warning("Invalid signature") + logger.warning("Datatrans Transaction Webhook: Invalid Signature -> Ignored") return JsonResponse({"status": "invalid signature"}, status=400) transaction = request.data @@ -107,7 +111,18 @@ def transaction_webhook(request): @permission_classes([IsAuthenticated]) def checkout_vv(request): """ - Checkout for the Versicherungsvermittler products (vv-de, vv-fr, vv-it) + Check-out for the Versicherungsvermittler products (vv-de, vv-fr, vv-it) + + IMPORTANT: Even if we have an already ONGOING checkout, + we create a new one! This might seem a bit unintuitive, + but it's the advised way to handle it by Datatrans: + + "Fehlverhalten des User können fast gar nicht abgefangen werden, + wichtig wäre aus eurer Sicht das ihr immer einen neuen INIT + schickt, wenn der User im Checkout ist und zum Beispiel + auf «Bezahlen» klickt. Um zum Beispiel White-screens + bei Browser Back redirections zu vermeiden." + """ sku = request.data["product"] base_redirect_url = request.data["redirect_url"] @@ -129,16 +144,11 @@ def checkout_vv(request): product_sku=sku, ) - # already paid (successfully)-> redirect to home - if checkouts.filter(state__in=[CheckoutState.PAID]).exists(): + # already paid successfully -> redirect to home + # any other case create a new checkout (see doc above) + if checkouts.filter(state=CheckoutState.PAID).exists(): return next_step_response(url="/") - # already initialized -> redirect to payment page again - if checkout := checkouts.filter(state=CheckoutState.INITIALIZED).first(): - return next_step_response(url=get_payment_url(checkout.transaction_id)) - - # not yet initialized at all, or canceled/failed - # -> create new transaction and checkout try: transaction_id = init_transaction( user=request.user, @@ -149,6 +159,8 @@ def checkout_vv(request): webhook_url=webhook_url(base_redirect_url), ) except InitTransactionException as e: + if not settings.DEBUG: + capture_exception(e) return next_step_response( url=checkout_error_url( base_url=base_redirect_url, @@ -157,7 +169,7 @@ def checkout_vv(request): CheckoutInformation.objects.create( user=request.user, - state="initialized", + state=CheckoutState.ONGOING, transaction_id=transaction_id, # product product_sku=sku,