From e802eaebacac23256bf1ec2bae3f7e930761d974 Mon Sep 17 00:00:00 2001 From: Taesu <166604494+bytaesu@users.noreply.github.com> Date: Mon, 23 Feb 2026 02:15:50 +0900 Subject: [PATCH] fix(stripe): use CHECKOUT_SESSION_ID for success_url and remove cancel callback (#8095) --- packages/stripe/src/index.ts | 2 - packages/stripe/src/routes.ts | 159 +++--------- .../stripe/test/stripe-organization.test.ts | 131 +--------- packages/stripe/test/stripe.test.ts | 245 +++++++++++++++++- 4 files changed, 284 insertions(+), 253 deletions(-) diff --git a/packages/stripe/src/index.ts b/packages/stripe/src/index.ts index 9457ec89c6..4b03a8f408 100644 --- a/packages/stripe/src/index.ts +++ b/packages/stripe/src/index.ts @@ -7,7 +7,6 @@ import { STRIPE_ERROR_CODES } from "./error-codes"; import { customerMetadata } from "./metadata"; import { cancelSubscription, - cancelSubscriptionCallback, createBillingPortal, listActiveSubscriptions, restoreSubscription, @@ -37,7 +36,6 @@ export const stripe = (options: O) => { const subscriptionEndpoints = { upgradeSubscription: upgradeSubscription(options), - cancelSubscriptionCallback: cancelSubscriptionCallback(options), cancelSubscription: cancelSubscription(options), restoreSubscription: restoreSubscription(options), listActiveSubscriptions: listActiveSubscriptions(options), diff --git a/packages/stripe/src/routes.ts b/packages/stripe/src/routes.ts index efaed9a7c6..9bdbee0493 100644 --- a/packages/stripe/src/routes.ts +++ b/packages/stripe/src/routes.ts @@ -31,7 +31,6 @@ import { getPlans, isActiveOrTrialing, isPendingCancel, - isStripePendingCancel, resolvePlanItem, resolveQuantity, } from "./utils"; @@ -1065,7 +1064,10 @@ export const upgradeSubscription = (options: StripeOptions) => { ctx.context.baseURL }/subscription/success?callbackURL=${encodeURIComponent( ctx.body.successUrl, - )}&subscriptionId=${encodeURIComponent(subscription.id)}`, + // {CHECKOUT_SESSION_ID} is a string literal; do not change it! + // the actual Session ID is returned in the query parameter when your customer + // is redirected to the success page. + )}&checkoutSessionId={CHECKOUT_SESSION_ID}`, ), cancel_url: getUrl(ctx, ctx.body.cancelUrl), line_items: [ @@ -1127,111 +1129,6 @@ export const upgradeSubscription = (options: StripeOptions) => { ); }; -const cancelSubscriptionCallbackQuerySchema = z - .record(z.string(), z.any()) - .optional(); - -export const cancelSubscriptionCallback = (options: StripeOptions) => { - const client = options.stripeClient; - const subscriptionOptions = options.subscription as SubscriptionOptions; - return createAuthEndpoint( - "/subscription/cancel/callback", - { - method: "GET", - query: cancelSubscriptionCallbackQuerySchema, - metadata: { - openapi: { - operationId: "cancelSubscriptionCallback", - }, - }, - use: [originCheck((ctx) => ctx.query.callbackURL)], - }, - async (ctx) => { - if (!ctx.query || !ctx.query.callbackURL || !ctx.query.subscriptionId) { - throw ctx.redirect(getUrl(ctx, ctx.query?.callbackURL || "/")); - } - const session = await getSessionFromCtx(ctx); - if (!session) { - throw ctx.redirect(getUrl(ctx, ctx.query?.callbackURL || "/")); - } - const { callbackURL, subscriptionId } = ctx.query; - - try { - const subscription = await ctx.context.adapter.findOne({ - model: "subscription", - where: [ - { - field: "id", - value: subscriptionId, - }, - ], - }); - if ( - !subscription || - subscription.status === "canceled" || - isPendingCancel(subscription) - ) { - throw ctx.redirect(getUrl(ctx, callbackURL)); - } - - // Use the subscription's own stripeCustomerId so this works - // for both user and organization subscriptions. - const customerId = subscription.stripeCustomerId; - if (!customerId) { - throw ctx.redirect(getUrl(ctx, callbackURL)); - } - - const stripeSubscription = await client.subscriptions.list({ - customer: customerId, - status: "active", - }); - const currentSubscription = stripeSubscription.data.find( - (sub) => sub.id === subscription.stripeSubscriptionId, - ); - - const isNewCancellation = - currentSubscription && - isStripePendingCancel(currentSubscription) && - !isPendingCancel(subscription); - if (isNewCancellation) { - await ctx.context.adapter.update({ - model: "subscription", - update: { - status: currentSubscription?.status, - cancelAtPeriodEnd: - currentSubscription?.cancel_at_period_end || false, - cancelAt: currentSubscription?.cancel_at - ? new Date(currentSubscription.cancel_at * 1000) - : null, - canceledAt: currentSubscription?.canceled_at - ? new Date(currentSubscription.canceled_at * 1000) - : null, - }, - where: [ - { - field: "id", - value: subscription.id, - }, - ], - }); - await subscriptionOptions.onSubscriptionCancel?.({ - subscription, - cancellationDetails: currentSubscription.cancellation_details, - stripeSubscription: currentSubscription, - event: undefined, - }); - } - } catch (error) { - ctx.context.logger.error( - "Error checking subscription status from Stripe", - error, - ); - } - throw ctx.redirect(getUrl(ctx, callbackURL)); - }, - ); -}; - const cancelSubscriptionBodySchema = z.object({ referenceId: z .string() @@ -1380,14 +1277,7 @@ export const cancelSubscription = (options: StripeOptions) => { const { url } = await client.billingPortal.sessions .create({ customer: subscription.stripeCustomerId, - return_url: getUrl( - ctx, - `${ - ctx.context.baseURL - }/subscription/cancel/callback?callbackURL=${encodeURIComponent( - ctx.body?.returnUrl || "/", - )}&subscriptionId=${encodeURIComponent(subscription.id)}`, - ), + return_url: getUrl(ctx, ctx.body?.returnUrl || "/"), flow_data: { type: "subscription_cancel", subscription_cancel: { @@ -1750,14 +1640,43 @@ export const subscriptionSuccess = (options: StripeOptions) => { use: [originCheck((ctx) => ctx.query.callbackURL)], }, async (ctx) => { - if (!ctx.query || !ctx.query.callbackURL || !ctx.query.subscriptionId) { - throw ctx.redirect(getUrl(ctx, ctx.query?.callbackURL || "/")); - } - const { callbackURL, subscriptionId } = ctx.query; + const callbackURL = ctx.query?.callbackURL || "/"; const session = await getSessionFromCtx(ctx); if (!session) { - throw ctx.redirect(getUrl(ctx, ctx.query?.callbackURL || "/")); + throw ctx.redirect(getUrl(ctx, callbackURL)); + } + + // checkoutSessionId is substituted by Stripe from the {CHECKOUT_SESSION_ID} + // template variable in success_url when redirecting after checkout. + if (!ctx.query?.checkoutSessionId) { + throw ctx.redirect(getUrl(ctx, callbackURL)); + } + + // Resolve subscriptionId from Stripe checkout session metadata. + // The metadata is set server-side when creating the checkout session, + // so it cannot be tampered with — no ownership check needed. + const checkoutSession = await client.checkout.sessions + .retrieve(ctx.query.checkoutSessionId) + .catch((error) => { + ctx.context.logger.error( + "Error retrieving checkout session from Stripe", + error, + ); + return null; + }); + if (!checkoutSession) { + throw ctx.redirect(getUrl(ctx, callbackURL)); + } + + const { subscriptionId } = subscriptionMetadata.get( + checkoutSession.metadata, + ); + if (!subscriptionId) { + ctx.context.logger.warn( + `No subscriptionId in checkout session metadata: ${checkoutSession.id}`, + ); + throw ctx.redirect(getUrl(ctx, callbackURL)); } const subscription = await ctx.context.adapter.findOne({ diff --git a/packages/stripe/test/stripe-organization.test.ts b/packages/stripe/test/stripe-organization.test.ts index 0791174665..e1d7fe4b2e 100644 --- a/packages/stripe/test/stripe-organization.test.ts +++ b/packages/stripe/test/stripe-organization.test.ts @@ -2,7 +2,7 @@ import { organizationClient } from "better-auth/client/plugins"; import { organization } from "better-auth/plugins/organization"; import { getTestInstance } from "better-auth/test"; import type Stripe from "stripe"; -import { beforeEach, describe, expect, it, onTestFinished, vi } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; import { stripe } from "../src"; import { stripeClient } from "../src/client"; import type { StripeOptions, Subscription } from "../src/types"; @@ -457,135 +457,6 @@ describe("stripe - organization customer", () => { ); }); - it("should update subscription on cancel callback using subscription's stripeCustomerId", async () => { - const cancelAt = Math.floor(Date.now() / 1000) + 30 * 24 * 60 * 60; - const canceledAt = Math.floor(Date.now() / 1000); - - // Mock Stripe to return the subscription as pending cancel - mockStripeOrg.subscriptions.list.mockResolvedValueOnce({ - data: [ - { - id: "sub_org_cb_123", - status: "active", - cancel_at_period_end: true, - cancel_at: cancelAt, - canceled_at: canceledAt, - cancellation_details: { - reason: "cancellation_requested", - }, - }, - ], - }); - - // Clean up mocks even if assertions fail to prevent leaking into subsequent tests - onTestFinished(() => { - mockStripeOrg.subscriptions.list.mockReset(); - mockStripeOrg.subscriptions.list.mockResolvedValue({ data: [] }); - mockStripeOrg.subscriptions.update.mockReset(); - }); - - const onSubscriptionCancel = vi.fn(); - const { client, auth, sessionSetter } = await getTestInstance( - { - plugins: [ - organization(), - stripe({ - ...baseOrgStripeOptions, - subscription: { - ...baseOrgStripeOptions.subscription, - onSubscriptionCancel, - }, - }), - ], - }, - { - disableTestUser: true, - clientOptions: { - plugins: [organizationClient(), stripeClient({ subscription: true })], - }, - }, - ); - const ctx = await auth.$context; - - await client.signUp.email( - { ...testUser, email: "org-cancel-cb-test@email.com" }, - { throw: true }, - ); - - const headers = new Headers(); - await client.signIn.email( - { ...testUser, email: "org-cancel-cb-test@email.com" }, - { throw: true, onSuccess: sessionSetter(headers) }, - ); - - const org = await client.organization.create({ - name: "Cancel Callback Org", - slug: "cancel-cb-org", - fetchOptions: { headers }, - }); - const orgId = org.data?.id as string; - - // Organization has stripeCustomerId, user does NOT - await ctx.adapter.update({ - model: "organization", - update: { stripeCustomerId: "cus_cb_org_123" }, - where: [{ field: "id", value: orgId }], - }); - - const sub = await ctx.adapter.create({ - model: "subscription", - data: { - referenceId: orgId, - stripeCustomerId: "cus_cb_org_123", - stripeSubscriptionId: "sub_org_cb_123", - status: "active", - plan: "starter", - }, - }); - - // Call the cancel callback endpoint directly (simulating redirect from Stripe Portal) - const callbackUrl = new URL( - "http://localhost:3000/api/auth/subscription/cancel/callback", - ); - callbackUrl.searchParams.set("callbackURL", "/dashboard"); - callbackUrl.searchParams.set("subscriptionId", sub.id); - - const response = await auth.handler( - new Request(callbackUrl.toString(), { - method: "GET", - headers, - }), - ); - - // Should redirect to the callbackURL - expect(response.status).toBe(302); - - // Verify DB was updated with cancellation info - const updatedSub = await ctx.adapter.findOne({ - model: "subscription", - where: [{ field: "id", value: sub.id }], - }); - expect(updatedSub?.cancelAtPeriodEnd).toBe(true); - expect(updatedSub?.cancelAt).toBeDefined(); - expect(updatedSub?.canceledAt).toBeDefined(); - - // Verify the Stripe API was called with the subscription's customer ID, not the user's - expect(mockStripeOrg.subscriptions.list).toHaveBeenCalledWith( - expect.objectContaining({ - customer: "cus_cb_org_123", - status: "active", - }), - ); - - // Verify onSubscriptionCancel callback was invoked - expect(onSubscriptionCancel).toHaveBeenCalledWith( - expect.objectContaining({ - subscription: expect.objectContaining({ id: sub.id }), - event: undefined, - }), - ); - }); - it("should restore subscription for organization", async () => { mockStripeOrg.subscriptions.list.mockResolvedValueOnce({ data: [ diff --git a/packages/stripe/test/stripe.test.ts b/packages/stripe/test/stripe.test.ts index 72c56b544f..2089eff761 100644 --- a/packages/stripe/test/stripe.test.ts +++ b/packages/stripe/test/stripe.test.ts @@ -53,7 +53,6 @@ describe("stripe type", () => { expectTypeOf().toBeFunction(); expectTypeOf().toBeFunction(); expectTypeOf().toBeFunction(); - expectTypeOf().toBeFunction(); expectTypeOf().toBeFunction(); expectTypeOf().toBeFunction(); expectTypeOf().toBeFunction(); @@ -7300,4 +7299,248 @@ describe("stripe", () => { expect(securityItems).toHaveLength(1); }); }); + + describe("subscriptionSuccess - checkoutSessionId flow", () => { + it("should update subscription via checkoutSessionId and redirect", async () => { + const testSubscriptionId = "sub_success_test"; + const testCheckoutSessionId = "cs_test_123"; + const testCustomerId = "cus_success_test"; + + const stripeForTest = { + ...stripeOptions.stripeClient, + checkout: { + sessions: { + ...stripeOptions.stripeClient.checkout.sessions, + retrieve: vi.fn(), + }, + }, + subscriptions: { + ...stripeOptions.stripeClient.subscriptions, + list: vi.fn().mockResolvedValue({ + data: [ + { + id: testSubscriptionId, + status: "active", + cancel_at_period_end: false, + cancel_at: null, + canceled_at: null, + trial_start: null, + trial_end: null, + items: { + data: [ + { + price: { + id: process.env.STRIPE_PRICE_ID_1, + recurring: { interval: "month" }, + }, + quantity: 1, + current_period_start: Math.floor(Date.now() / 1000), + current_period_end: + Math.floor(Date.now() / 1000) + 30 * 86400, + }, + ], + }, + }, + ], + }), + }, + }; + + const testOptions = { + ...stripeOptions, + stripeClient: stripeForTest as unknown as Stripe, + }; + + const { + client, + auth: testAuth, + sessionSetter, + } = await getTestInstance( + { + database: memory, + plugins: [stripe(testOptions)], + }, + { + disableTestUser: true, + clientOptions: { + plugins: [stripeClient({ subscription: true })], + }, + }, + ); + const testCtx = await testAuth.$context; + + const headers = new Headers(); + const userRes = await client.signUp.email( + { + email: "success-flow@test.com", + password: "password", + name: "Success Test", + }, + { throw: true }, + ); + await client.signIn.email( + { email: "success-flow@test.com", password: "password" }, + { throw: true, onSuccess: sessionSetter(headers) }, + ); + + // Create incomplete subscription in DB + const sub = await testCtx.adapter.create({ + model: "subscription", + data: { + referenceId: userRes.user.id, + stripeCustomerId: testCustomerId, + status: "incomplete", + plan: "starter", + }, + }); + + // Mock checkout session to return correct subscriptionId + (stripeForTest.checkout.sessions.retrieve as any).mockResolvedValue({ + id: testCheckoutSessionId, + metadata: { + userId: userRes.user.id, + subscriptionId: sub.id, + referenceId: userRes.user.id, + }, + }); + + const callbackURL = "/dashboard"; + const url = `http://localhost:3000/api/auth/subscription/success?callbackURL=${encodeURIComponent(callbackURL)}&checkoutSessionId=${testCheckoutSessionId}`; + const response = await testAuth.handler( + new Request(url, { + method: "GET", + headers, + redirect: "manual", + }), + ); + + // Should redirect + expect(response.status).toBe(302); + expect(response.headers.get("location")).toContain(callbackURL); + + // Verify checkout session was retrieved + expect(stripeForTest.checkout.sessions.retrieve).toHaveBeenCalledWith( + testCheckoutSessionId, + ); + + // Verify subscription was updated in DB + const updated = await testCtx.adapter.findOne({ + model: "subscription", + where: [{ field: "id", value: sub.id }], + }); + expect(updated?.status).toBe("active"); + expect(updated?.stripeSubscriptionId).toBe(testSubscriptionId); + expect(updated?.periodStart).toBeInstanceOf(Date); + expect(updated?.periodEnd).toBeInstanceOf(Date); + }); + + it("should redirect without update when checkoutSessionId is missing", async () => { + const { + client, + auth: testAuth, + sessionSetter, + } = await getTestInstance( + { + database: memory, + plugins: [stripe(stripeOptions)], + }, + { + disableTestUser: true, + clientOptions: { + plugins: [stripeClient({ subscription: true })], + }, + }, + ); + + const headers = new Headers(); + await client.signUp.email( + { + email: "no-session-id@test.com", + password: "password", + name: "No Session", + }, + { throw: true }, + ); + await client.signIn.email( + { email: "no-session-id@test.com", password: "password" }, + { throw: true, onSuccess: sessionSetter(headers) }, + ); + + const callbackURL = "/dashboard"; + const url = `http://localhost:3000/api/auth/subscription/success?callbackURL=${encodeURIComponent(callbackURL)}`; + const response = await testAuth.handler( + new Request(url, { + method: "GET", + headers, + redirect: "manual", + }), + ); + + // Should redirect without any Stripe calls + expect(response.status).toBe(302); + expect(response.headers.get("location")).toContain(callbackURL); + }); + + it("should redirect when checkout session retrieval fails", async () => { + const stripeForTest = { + ...stripeOptions.stripeClient, + checkout: { + sessions: { + ...stripeOptions.stripeClient.checkout.sessions, + retrieve: vi.fn().mockRejectedValue(new Error("Invalid session")), + }, + }, + }; + + const testOptions = { + ...stripeOptions, + stripeClient: stripeForTest as unknown as Stripe, + }; + + const { + client, + auth: testAuth, + sessionSetter, + } = await getTestInstance( + { + database: memory, + plugins: [stripe(testOptions)], + }, + { + disableTestUser: true, + clientOptions: { + plugins: [stripeClient({ subscription: true })], + }, + }, + ); + + const headers = new Headers(); + await client.signUp.email( + { + email: "bad-session@test.com", + password: "password", + name: "Bad Session", + }, + { throw: true }, + ); + await client.signIn.email( + { email: "bad-session@test.com", password: "password" }, + { throw: true, onSuccess: sessionSetter(headers) }, + ); + + const callbackURL = "/dashboard"; + const url = `http://localhost:3000/api/auth/subscription/success?callbackURL=${encodeURIComponent(callbackURL)}&checkoutSessionId=cs_invalid`; + const response = await testAuth.handler( + new Request(url, { + method: "GET", + headers, + redirect: "manual", + }), + ); + + // Should redirect gracefully + expect(response.status).toBe(302); + expect(response.headers.get("location")).toContain(callbackURL); + }); + }); });