refactor(billing): extract shared Stripe client, relax URL check in dev

Closes #3263, closes #3264

Author: PierreBrisorgueil

modules/billing/controllers/billing.webhook.controller.js

@@ -1,27 +1,10 @@
 /**
  * Module dependencies
  */
-import Stripe from 'stripe';
-
 import config from '../../../config/index.js';
+import getStripe from '../lib/stripe.js';
 import BillingWebhookService from '../services/billing.webhook.service.js';
 
-/**
- * Lazily instantiated Stripe client
- */
-let stripeClient = null;
-
-/**
- * @desc Get or create the Stripe client instance
- * @returns {Object|null} Stripe client or null if not configured
- */
-const getStripe = () => {
-  if (stripeClient) return stripeClient;
-  if (!config.stripe?.secretKey) return null;
-  stripeClient = new Stripe(config.stripe.secretKey);
-  return stripeClient;
-};
-
 /**
  * @desc Endpoint to handle Stripe webhook events
  * @param {Object} req - Express request object

modules/billing/lib/stripe.js

@@ -0,0 +1,24 @@
+/**
+ * Module dependencies
+ */
+import Stripe from 'stripe';
+
+import config from '../../../config/index.js';
+
+/**
+ * Lazily instantiated Stripe client
+ */
+let stripeClient = null;
+
+/**
+ * @desc Get or create the Stripe client instance
+ * @returns {Object|null} Stripe client or null if not configured
+ */
+const getStripe = () => {
+  if (stripeClient) return stripeClient;
+  if (!config.stripe?.secretKey) return null;
+  stripeClient = new Stripe(config.stripe.secretKey);
+  return stripeClient;
+};
+
+export default getStripe;

modules/billing/services/billing.plans.service.js

@@ -1,9 +1,7 @@
 /**
  * Module dependencies
  */
-import Stripe from 'stripe';
-
-import config from '../../../config/index.js';
+import getStripe from '../lib/stripe.js';
 
 /**
  * In-memory cache for plans
@@ -24,22 +22,6 @@ const DEFAULT_FREE_PLAN = {
   stripePriceAnnual: null,
 };
 
-/**
- * Lazily instantiated Stripe client
- */
-let stripeClient = null;
-
-/**
- * @desc Get or create the Stripe client instance
- * @returns {Object|null} Stripe client or null if not configured
- */
-const getStripe = () => {
-  if (stripeClient) return stripeClient;
-  if (!config.stripe?.secretKey) return null;
-  stripeClient = new Stripe(config.stripe.secretKey);
-  return stripeClient;
-};
-
 /**
  * @desc Fetch plans from Stripe and map to normalized format
  * @param {Object} stripe - Stripe client instance

modules/billing/services/billing.service.js

@@ -1,28 +1,11 @@
 /**
  * Module dependencies
  */
-import Stripe from 'stripe';
-
 import config from '../../../config/index.js';
+import getStripe from '../lib/stripe.js';
 import BillingPlansService from './billing.plans.service.js';
 import SubscriptionRepository from '../repositories/billing.subscription.repository.js';
 
-/**
- * Lazily instantiated Stripe client
- */
-let stripeClient = null;
-
-/**
- * @desc Get or create the Stripe client instance
- * @returns {Object|null} Stripe client or null if not configured
- */
-const getStripe = () => {
-  if (stripeClient) return stripeClient;
-  if (!config.stripe?.secretKey) return null;
-  stripeClient = new Stripe(config.stripe.secretKey);
-  return stripeClient;
-};
-
 /**
  * @desc Validate that a URL uses https (or http in dev/test) and belongs to the app domain
  * @param {String} url - URL to validate
@@ -34,7 +17,7 @@ const isAllowedUrl = (url) => {
     const env = process.env.NODE_ENV || 'development';
     const allowHttp = env === 'development' || env === 'test';
     if (!allowHttp && parsed.protocol !== 'https:') return false;
-    if (config.domain) {
+    if (config.domain && !allowHttp) {
       const configHost = new URL(config.domain.startsWith('http') ? config.domain : `https://${config.domain}`).hostname;
       if (parsed.hostname !== configHost) return false;
     }

modules/billing/tests/billing.checkout.unit.tests.js

@@ -105,17 +105,21 @@ describe('Billing service unit tests:', () => {
       ).rejects.toThrow('Invalid redirect URL');
     });
 
-    test('should reject URL with wrong domain when config.domain is set', async () => {
+    test('should allow any domain in dev/test even when config.domain is set', async () => {
       jest.unstable_mockModule('../../../config/index.js', () => ({
         default: { stripe: { secretKey: 'sk_test_domain' }, domain: 'https://myapp.com' },
       }));
 
+      mockSubscriptionRepository.findByOrganization.mockResolvedValue({
+        stripeCustomerId: 'cus_existing',
+      });
+
       const mod = await import('../services/billing.service.js');
       BillingService = mod.default;
 
-      await expect(
-        BillingService.createCheckout(mockOrganization, 'price_starter_m', 'http://evil.com/success', 'http://myapp.com/cancel'),
-      ).rejects.toThrow('Invalid redirect URL');
+      const url = await BillingService.createCheckout(mockOrganization, 'price_starter_m', 'http://evil.com/success', 'http://myapp.com/cancel');
+
+      expect(url).toBe('https://checkout.stripe.com/session_123');
     });
 
     test('should throw when priceId is not in allowed plans', async () => {