purchasing: make it so stripe checkout payment intents are also recorded

- prevents potential double pay for brand new customers, e.g., students,
  when using async payment methods (e.g., bank transfer)

src/packages/frontend/purchases/stripe-payment.tsx

@@ -459,6 +459,7 @@ function PaymentForm({ style, onFinished, paymentIntent }) {
           success={success}
           isSubmitting={isSubmitting}
           cancellablePaymentIntentId={!finalized ? paymentIntent.id : undefined}
+          cancelText="Close"
           onCancel={() => {
             onFinished?.();
           }}
@@ -484,6 +485,7 @@ export function ConfirmButton({
   onCancel,
   showAddress,
   cancellablePaymentIntentId,
+  cancelText,
 }: {
   disabled?: boolean;
   onClick;
@@ -493,6 +495,7 @@ export function ConfirmButton({
   notPrimary?: boolean;
   onCancel?: Function;
   showAddress?: boolean;
+  cancelText?: string;
   // if given, also include button to cancel the given payment intent
   cancellablePaymentIntentId?: string;
 }) {
@@ -506,7 +509,7 @@ export function ConfirmButton({
               onClick={() => onCancel()}
               style={{ height: "44px" }}
             >
-              Cancel
+              {cancelText ?? "Cancel"}
             </Button>
           )}
           <Button

src/packages/server/purchases/stripe/create-payment-intent.ts

@@ -199,47 +199,7 @@ ${await support()}
     );
   }
 
-  if (purpose == SHOPPING_CART_CHECKOUT) {
-    try {
-      await setShoppingCartPaymentIntent({
-        account_id,
-        payment_intent: paymentIntentId,
-      });
-    } catch (err) {
-      // This is bad -- we couldn't properly mark what is being bought, but
-      // the payment intent exists. This could happen if the database went
-      // down.  In this case, we cancel the payment intent (no money has been taken yet!),
-      // and do NOT start the payment below!
-
-      // In the highly unlikely case this failed, that would be bad because the
-      // payment would be left hanging, but we haven't even tried to charge them,
-      // so I think they might have to go out of their way to pay.  They might NOT
-      // get their items automatically if they pay, but they would get their credit
-      // and could buy them later.  So basically double pay is perhaps still possible,
-      // but a user would have to try really, really hard.
-      await cancelPaymentIntent({
-        id: paymentIntentId,
-        reason: "abandoned",
-      });
-
-      // the user will get back an error message.  This should happen when cocalc
-      // is badly broken.  They can try again, but there's no harm in this case.
-      throw err;
-    }
-    // Now in case of shopping, the items in the cart have been moved to a new state
-    // so they can't be bought again, so it's safe to start trying to get the user
-    // to pay us, which is what happens next below.
-  } else if (purpose == STUDENT_PAY) {
-    await studentPaySetPaymentIntent({
-      project_id: metadata.project_id,
-      paymentIntentId,
-    });
-  } else if (purpose == RESUME_SUBSCRIPTION) {
-    await resumeSubscriptionSetPaymentIntent({
-      subscription_id: parseInt(metadata.subscription_id),
-      paymentIntentId,
-    });
-  }
+  await recordPaymentIntent({ purpose, account_id, paymentIntentId, metadata });
 
   await stripe.paymentIntents.update(paymentIntentId, {
     description,
@@ -352,3 +312,65 @@ export async function getPaymentIntentAccountId(
   const paymentIntent = await stripe.paymentIntents.retrieve(id);
   return paymentIntent.metadata?.account_id;
 }
+
+// When a payment intent is created we change some state in cocalc to
+// indicate this, which is critical to avoid double payments.
+// This is called right after creating and finalizing a payment intent
+// explicitly above, but ALSO a payment intent (with no invoice)
+// gets created implicitly as part of the stripe checkout process
+// so we call this code when handling payment intents that have no
+// invoice.
+export async function recordPaymentIntent({
+  purpose,
+  account_id,
+  paymentIntentId,
+  metadata,
+}) {
+  logger.debug("recordPaymentIntent", {
+    purpose,
+    account_id,
+    paymentIntentId,
+    metadata,
+  });
+  if (purpose == SHOPPING_CART_CHECKOUT) {
+    try {
+      await setShoppingCartPaymentIntent({
+        account_id,
+        payment_intent: paymentIntentId,
+      });
+    } catch (err) {
+      // This is bad -- we couldn't properly mark what is being bought, but
+      // the payment intent exists. This could happen if the database went
+      // down.  In this case, we cancel the payment intent (no money has been taken yet!),
+      // and do NOT start the payment below!
+
+      // In the highly unlikely case this failed, that would be bad because the
+      // payment would be left hanging, but we haven't even tried to charge them,
+      // so I think they might have to go out of their way to pay.  They might NOT
+      // get their items automatically if they pay, but they would get their credit
+      // and could buy them later.  So basically double pay is perhaps still possible,
+      // but a user would have to try really, really hard.
+      await cancelPaymentIntent({
+        id: paymentIntentId,
+        reason: "abandoned",
+      });
+
+      // the user will get back an error message.  This should happen when cocalc
+      // is badly broken.  They can try again, but there's no harm in this case.
+      throw err;
+    }
+    // Now in case of shopping, the items in the cart have been moved to a new state
+    // so they can't be bought again, so it's safe to start trying to get the user
+    // to pay us, which is what happens next below.
+  } else if (purpose == STUDENT_PAY) {
+    await studentPaySetPaymentIntent({
+      project_id: metadata.project_id,
+      paymentIntentId,
+    });
+  } else if (purpose == RESUME_SUBSCRIPTION) {
+    await resumeSubscriptionSetPaymentIntent({
+      subscription_id: parseInt(metadata.subscription_id),
+      paymentIntentId,
+    });
+  }
+}

src/packages/server/purchases/stripe/process-payment-intents.ts

@@ -28,6 +28,7 @@ import { currency, round2down } from "@cocalc/util/misc";
 import { reuseInFlight } from "@cocalc/util/reuse-in-flight";
 import getBalance from "@cocalc/server/purchases/get-balance";
 import getPool from "@cocalc/database/pool";
+import { recordPaymentIntent } from "./create-payment-intent";
 
 const logger = getLogger("purchases:stripe:process-payment-intents");
 
@@ -65,6 +66,10 @@ export default async function processPaymentIntents({
     });
     paymentIntents = recentPaymentIntents.data.concat(olderPaymentIntents.data);
   }
+  logger.debug(
+    `processing ${paymentIntents.length} payment intents`,
+    account_id != null ? `for account_id=${account_id}` : "",
+  );
 
   const seen = new Set<string>();
   const purchase_ids = new Set<number>([]);
@@ -73,6 +78,21 @@ export default async function processPaymentIntents({
       continue;
     }
     seen.add(paymentIntent.id);
+    if (needsToBeRecorded(paymentIntent)) {
+      try {
+        await recordPaymentIntent({
+          paymentIntentId: paymentIntent.id,
+          purpose: paymentIntent.metadata.purpose,
+          account_id: paymentIntent.metadata.account_id,
+          metadata: paymentIntent.metadata,
+        });
+        await setMetadataRecorded(paymentIntent);
+      } catch (err) {
+        logger.debug(
+          `WARNING: issue processing a payment intent ${paymentIntent.id} -- ${err}`,
+        );
+      }
+    }
     if (isReadyToProcess(paymentIntent)) {
       try {
         const id = await processPaymentIntent(paymentIntent);
@@ -104,10 +124,32 @@ export function isReadyToProcess(paymentIntent) {
       paymentIntent.status == "canceled") &&
     paymentIntent.metadata["processed"] != "true" &&
     paymentIntent.metadata["purpose"] &&
+    paymentIntent.metadata["deleted"] != "true" &&
     paymentIntent.invoice
   );
 }
 
+// Is this a payment intent coming from a stripe checkout session that we haven't
+// yet recorded its impacted?   paymentIntent.invoice being null means it's stripe
+// checkout since we make our non-checkout payment intents from an invoice.
+function needsToBeRecorded(paymentIntent) {
+  return (
+    !isReadyToProcess(paymentIntent) &&
+    !paymentIntent.invoice &&
+    paymentIntent.metadata["purpose"] &&
+    paymentIntent.metadata["recorded"] != "true" &&
+    paymentIntent.metadata["deleted"] != "true"
+  );
+}
+
+async function setMetadataRecorded(paymentIntent) {
+  const stripe = await getConn();
+  paymentIntent.metadata.recorded = "true";
+  await stripe.paymentIntents.update(paymentIntent.id, {
+    metadata: paymentIntent.metadata,
+  });
+}
+
 // NOT a critical assumption.  We do NOT assume processPaymentIntent is never run twice at
 // the same time for the same payment, either in the same process or on the cluster.
 // If $n$ attempts to run this happen at once, the createCredit call will succeed for

src/packages/server/purchases/student-pay.ts

@@ -257,7 +257,34 @@ export async function studentPaySetPaymentIntent({
   project_id,
   paymentIntentId,
 }) {
+  logger.debug("studentPaySetPaymentIntent", { project_id, paymentIntentId });
   const pool = getPool();
+  const { rows } = await pool.query(
+    "SELECT course#>>'{payment_intent_id}' as payment_intent_id FROM projects WHERE project_id=$1",
+    [project_id],
+  );
+  const current = rows[0]?.payment_intent_id;
+  if (current) {
+    const stripe = await getConn();
+    const currentIntent = await stripe.paymentIntents.retrieve(current);
+    if (
+      currentIntent.status != "succeeded" &&
+      currentIntent.status != "canceled" &&
+      currentIntent.metadata["deleted"] != "true"
+    ) {
+      logger.debug(
+        "studentPaySetPaymentIntent",
+        {
+          project_id,
+        },
+        "NOT changing, since there is already a payment intent set that is not deleted and is active",
+      );
+      // there is already an unfinished payment intent for this course.  do
+      // not change it.  User has to explicitly cancel or pay existing one.
+      return;
+    }
+  }
+
   // OK, set the payment intent.
   // paymentIntentId only comes directly from stripe, not the user, so no danger of SQL injection,
   // but we are still careful!