diff --git a/package.json b/package.json index 1e630a56..1351a846 100644 --- a/package.json +++ b/package.json @@ -66,7 +66,7 @@ "@aws-sdk/client-s3": "^3.1076.0", "@contentrain/mcp": "3.9.0", "@contentrain/query": "7.4.0", - "@contentrain/types": "1.44.0", + "@contentrain/types": "1.45.0", "@gitbeaker/rest": "^43.8.0", "@nuxt/eslint": "1.16.0", "@nuxt/image": "2.0.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9dd49dc6..96294b07 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -29,8 +29,8 @@ importers: specifier: 7.4.0 version: 7.4.0 '@contentrain/types': - specifier: 1.44.0 - version: 1.44.0 + specifier: 1.45.0 + version: 1.45.0 '@gitbeaker/rest': specifier: ^43.8.0 version: 43.8.0 @@ -713,8 +713,8 @@ packages: '@contentrain/types@1.30.0': resolution: {integrity: sha512-inJhFqAY25wIw4NvpqDXOn24PRbVEh43s6GGFQSRyBbbmGiWu+xD67D2UEqaEllaFNLSVQ0j2DpOjDj+P6ezQA==} - '@contentrain/types@1.44.0': - resolution: {integrity: sha512-77d1Sf5rUKd65MxtINltBvs9wkH7z7m6NnUkbM3xH6dhiytfwRtXAWJvozgk4u0qgmU0btPanc6uZKx2OxB2MQ==} + '@contentrain/types@1.45.0': + resolution: {integrity: sha512-VTkKNxXGJxwme1Ln1CTafZo8RwQx5Q1vUur6R1JcfndYgM4ULrJpJt6LJJV72j15xjo8BOVYNyBk08WS4Istzw==} '@conventional-changelog/git-client@3.1.2': resolution: {integrity: sha512-jZqwnJwf7nboIlAcw/mkOjVa6DexCcUOgT2oOQgkoi3z9vR8tGFkcMy2BFcYwjhL9sYcDDXkRQDayiDieCoW7A==} @@ -7938,7 +7938,7 @@ snapshots: '@contentrain/types@1.30.0': {} - '@contentrain/types@1.44.0': {} + '@contentrain/types@1.45.0': {} '@conventional-changelog/git-client@3.1.2(conventional-commits-parser@7.1.2)': dependencies: diff --git a/server/api/billing/webhook/[provider].post.ts b/server/api/billing/webhook/[provider].post.ts index bc2a7871..88128f5b 100644 --- a/server/api/billing/webhook/[provider].post.ts +++ b/server/api/billing/webhook/[provider].post.ts @@ -279,6 +279,10 @@ export default defineEventHandler(async (event) => { case 'subscription.created': { // Fresh subscription — upsert the active account for this workspace. if (!result.workspaceId || !result.customerId) break + // A second payment for a bundle (an old checkout) must not replace the valid subscription as the + // workspace's account: its refund would cancel the plan. Logged as an ALARM; ops refunds it. + if (result.migrateGrantId && result.subscriptionId + && await isDuplicateBundleSubscription(result.migrateGrantId, result.subscriptionId, result.checkoutId ?? null)) break const overageLock = await planOverageLock(db, { workspaceId: result.workspaceId, billableMeters: result.billableMeters, @@ -320,7 +324,7 @@ export default defineEventHandler(async (event) => { // grant up: no second included trial after cancel-and-resubscribe. // Idempotent — whichever of created/updated arrives first marks it. if (result.migrateGrantId) { - await redeemMigrateGrant(provider, result.migrateGrantId, result.subscriptionId ?? null) + await redeemMigrateGrant(provider, result.migrateGrantId, result.subscriptionId ?? null, result.checkoutId ?? null) } // First 'trialing' observation consumes the workspace's one-time // trial, so a later re-checkout (after cancel/expiry) gets a paid @@ -341,6 +345,8 @@ export default defineEventHandler(async (event) => { case 'subscription.updated': { if (!result.workspaceId || !result.customerId) break + if (result.migrateGrantId && result.subscriptionId + && await isDuplicateBundleSubscription(result.migrateGrantId, result.subscriptionId, result.checkoutId ?? null)) break // Read the existing account BEFORE upsert so we can detect the // transitions worth emailing on (trial→active, payment failed or // recovered, cancellation scheduled). @@ -429,7 +435,7 @@ export default defineEventHandler(async (event) => { // grant up: no second included trial after cancel-and-resubscribe. // Idempotent — whichever of created/updated arrives first marks it. if (result.migrateGrantId) { - await redeemMigrateGrant(provider, result.migrateGrantId, result.subscriptionId ?? null) + await redeemMigrateGrant(provider, result.migrateGrantId, result.subscriptionId ?? null, result.checkoutId ?? null) } const workspaceUpdate: Record = {} diff --git a/server/providers/payment/plugins/polar.ts b/server/providers/payment/plugins/polar.ts index e2c7da7c..18811df4 100644 --- a/server/providers/payment/plugins/polar.ts +++ b/server/providers/payment/plugins/polar.ts @@ -100,6 +100,7 @@ interface PolarSubscriptionLike { status: string customerId: string productId: string + checkoutId?: string | null currentPeriodStart: Date | string | null currentPeriodEnd: Date | string | null trialEnd: Date | string | null @@ -154,6 +155,7 @@ function subscriptionToResult( plan: planFromProductId(sub.productId, productMap) ?? planFromMeta, productId: sub.productId, subscriptionId: sub.id, + ...(sub.checkoutId ? { checkoutId: sub.checkoutId } : {}), customerId: sub.customerId, subscriptionStatus: sub.status, currentPeriodStart: isoOrUndefined(sub.currentPeriodStart), diff --git a/server/providers/payment/types.ts b/server/providers/payment/types.ts index ea0d7773..f1ebe262 100644 --- a/server/providers/payment/types.ts +++ b/server/providers/payment/types.ts @@ -89,6 +89,8 @@ export interface WebhookResult { workspaceId?: string plan?: string subscriptionId?: string + /** The checkout that created the subscription (Polar `checkoutId`), when the provider reports it. */ + checkoutId?: string customerId?: string /** Provider-normalised status: trialing, active, past_due, canceled, unpaid, incomplete. */ subscriptionStatus?: string diff --git a/server/providers/supabase-auth.ts b/server/providers/supabase-auth.ts index 60ab2749..5a6c1303 100644 --- a/server/providers/supabase-auth.ts +++ b/server/providers/supabase-auth.ts @@ -1,4 +1,5 @@ import type { AuthProvider, AuthSession, AuthTokens, AuthUser, OAuthRedirectResult, ProviderTokens } from './auth' +import { IdentityConflictError } from './auth' import { createSupabaseAdminClient, createSupabaseAuthFlowClient } from './supabase-client' /** @@ -273,7 +274,17 @@ export function createSupabaseAuthProvider(): AuthProvider { // returned as is, and GoTrue links the GitHub identity at their first GitHub sign-in // (same verified email). A new user is created confirmed; the bootstrap trigger fires. const byEmail = await this.getUserByEmail(input.email) - if (byEmail) return byEmail + if (byEmail) { + // The account behind this email already has a different GitHub identity: linking ours would hand + // it to a stranger (same guard as the managed pair). Read from `auth.identities` (the user's + // `identities`), not from `user_metadata.provider_id`, which only names the last sign-in provider. + const { data: full, error: fullError } = await createSupabaseAdminClient().auth.admin.getUserById(byEmail.id) + if (fullError) throw fullError + const other = full?.user?.identities?.find(identity => identity.provider === input.provider) + const otherId = other ? String(other.identity_data?.provider_id ?? other.identity_data?.sub ?? other.id ?? '') : '' + if (other && otherId !== input.accountId) throw new IdentityConflictError() + return byEmail + } const admin = createSupabaseAdminClient() const { data, error } = await admin.auth.admin.createUser({ email: input.email, diff --git a/server/utils/migrate-bundle-subscription.ts b/server/utils/migrate-bundle-subscription.ts index 6fd16974..0a34eeeb 100644 --- a/server/utils/migrate-bundle-subscription.ts +++ b/server/utils/migrate-bundle-subscription.ts @@ -57,7 +57,13 @@ export async function reconcileMigrateBundles(payment: PaymentProvider, now: Dat const summary: BundleReconcileSummary = { checked: pending.length, applied: 0, stillPending: 0, alarms: 0 } for (const grant of pending) { const subscriptionId = grant.redeemed_subscription_id as string | null - if (!subscriptionId) continue + if (!subscriptionId) { + // Paid (redeemed) but no subscription id was recorded: nothing can be moved, and the renewal would charge the ad-hoc price. + summary.alarms++ + // eslint-disable-next-line no-console -- the alarm: watched by the platform's log alert + console.error(`[migrate-bundle] ALARM grant ${String(grant.id)} is redeemed without a subscription id; its ad-hoc price cannot be moved`) + continue + } const result = await applyBundleListProduct(payment, grant, subscriptionId) if (result === 'applied') { summary.applied++ @@ -82,8 +88,37 @@ export async function reconcileMigrateBundles(payment: PaymentProvider, now: Dat * grant's subscription moves to its list product. Idempotent: whichever of * `subscription.created` / `.updated` arrives first does the work. */ -export async function redeemMigrateGrant(payment: PaymentProvider, grantId: string, subscriptionId: string | null): Promise { +/** + * A subscription that is not the one a bundle grant already has came from another checkout (Polar cannot + * expire the old one): a duplicate payment. It must not become the workspace's active account, or refunding + * it would cancel and drop the valid bundle's plan. The webhook asks first and skips every account write. + */ +export async function isDuplicateBundleSubscription(grantId: string, subscriptionId: string, checkoutId: string | null = null): Promise { + const grant = await useDatabaseProvider().getMigrateGrantById(grantId) + const known = grant?.kind === 'bundle' ? (grant.redeemed_subscription_id as string | null) : null + if (!known || known === subscriptionId) return false + // eslint-disable-next-line no-console -- the alarm: watched by the platform's log alert + console.error(`[migrate-bundle] ALARM duplicate payment: grant ${grantId} already has subscription ${known}, subscription ${subscriptionId} (checkout ${checkoutId ?? 'unknown'}) came from another checkout; refund it`) + return true +} + +export async function redeemMigrateGrant( + payment: PaymentProvider, + grantId: string, + subscriptionId: string | null, + checkoutId: string | null = null, +): Promise { const db = useDatabaseProvider() + const before = await db.getMigrateGrantById(grantId) + // Polar cannot expire a checkout, so an old one can still be paid after a re-quote or beside the current one. + // That is money: say so loudly, and never let a second payment pass as the grant's subscription. + if (before?.kind === 'bundle' && subscriptionId) { + if (await isDuplicateBundleSubscription(grantId, subscriptionId, checkoutId)) return + if (checkoutId && before.checkout_id && before.checkout_id !== checkoutId) { + // eslint-disable-next-line no-console -- the alarm: watched by the platform's log alert + console.error(`[migrate-bundle] ALARM stale checkout paid: grant ${grantId} subscription ${subscriptionId} came from checkout ${checkoutId}, the current one is ${String(before.checkout_id)}; check the amount paid against the quote`) + } + } await db.markMigrateGrantRedeemed(grantId, subscriptionId) if (!subscriptionId) return const grant = await db.getMigrateGrantById(grantId) diff --git a/tests/integration/billing-webhook.integration.test.ts b/tests/integration/billing-webhook.integration.test.ts index 63391cd6..f8d139d2 100644 --- a/tests/integration/billing-webhook.integration.test.ts +++ b/tests/integration/billing-webhook.integration.test.ts @@ -25,6 +25,7 @@ describe('billing webhook integration', () => { // The real util (auto-imported in Nitro) marks the grant, then moves a bundle's subscription; the // move itself is covered in migrate-bundle-subscription.test.ts, here only what the webhook hands it. let redeemMigrateGrant: ReturnType + let isDuplicateBundleSubscription: ReturnType beforeEach(() => { vi.resetModules() @@ -33,6 +34,8 @@ describe('billing webhook integration', () => { await (globalThis as unknown as { useDatabaseProvider: () => { markMigrateGrantRedeemed: (g: string, s: string | null) => Promise } }).useDatabaseProvider().markMigrateGrantRedeemed(grantId, subscriptionId) }) vi.stubGlobal('redeemMigrateGrant', redeemMigrateGrant) + isDuplicateBundleSubscription = vi.fn().mockResolvedValue(false) + vi.stubGlobal('isDuplicateBundleSubscription', isDuplicateBundleSubscription) vi.stubGlobal('defineEventHandler', (handler: unknown) => handler) vi.stubGlobal('createError', createErrorLike) vi.stubGlobal('readRawBody', vi.fn().mockResolvedValue('{}')) @@ -175,6 +178,25 @@ describe('billing webhook integration', () => { })) }) + it('a duplicate bundle payment never becomes the active account, so refunding it leaves the valid plan alone', async () => { + isDuplicateBundleSubscription.mockResolvedValue(true) + const created = { event: 'subscription.created', workspaceId: 'ws-1', plan: 'pro', customerId: 'cus_123', subscriptionId: 'sub_dup', checkoutId: 'co_old', subscriptionStatus: 'active', migrateGrantId: 'grant-1' } + handleWebhookMock.mockResolvedValueOnce(created) + const handler = await mockPluginAndLoadHandler() + await handler({ context: {} } as never) + expect(isDuplicateBundleSubscription).toHaveBeenCalledWith('grant-1', 'sub_dup', 'co_old') + expect(upsertPaymentAccount).not.toHaveBeenCalled() + expect(redeemMigrateGrant).not.toHaveBeenCalled() + expect(updateWorkspace).not.toHaveBeenCalled() + + // The refund of the duplicate: the workspace's valid subscription is another one, so the ending is ignored. + getActivePaymentAccount.mockResolvedValue({ subscription_id: 'sub_valid', plan: 'pro' }) + handleWebhookMock.mockResolvedValueOnce({ ...created, event: 'subscription.canceled', subscriptionStatus: 'canceled' }) + await handler({ context: {} } as never) + expect(archiveActivePaymentAccount).not.toHaveBeenCalled() + expect(updateWorkspace).not.toHaveBeenCalled() + }) + it('uses up the Migrate grant a subscription was started from', async () => { const markMigrateGrantRedeemed = vi.fn().mockResolvedValue(undefined) vi.stubGlobal('useDatabaseProvider', vi.fn().mockReturnValue({ @@ -202,7 +224,7 @@ describe('billing webhook integration', () => { const handler = await mockPluginAndLoadHandler() await handler({ context: {} } as never) expect(markMigrateGrantRedeemed).toHaveBeenCalledWith('grant-1', 'sub_123') - expect(redeemMigrateGrant).toHaveBeenCalledWith(expect.anything(), 'grant-1', 'sub_123') + expect(redeemMigrateGrant).toHaveBeenCalledWith(expect.anything(), 'grant-1', 'sub_123', null) // The trial cap tells a Migrate trial apart by this mark. expect(upsertPaymentAccount).toHaveBeenCalledWith(expect.objectContaining({ pluginMetadata: expect.objectContaining({ trial_origin: 'migrate' }), diff --git a/tests/unit/migrate-bundle-subscription.test.ts b/tests/unit/migrate-bundle-subscription.test.ts index 4b8e5393..02fc752e 100644 --- a/tests/unit/migrate-bundle-subscription.test.ts +++ b/tests/unit/migrate-bundle-subscription.test.ts @@ -78,7 +78,55 @@ describe('bundle subscription: move to the list product', () => { expect(payment.moveBundleSubscriptionToList).not.toHaveBeenCalled() }) + describe('isDuplicateBundleSubscription', () => { + it('is true only for a bundle that already has a different subscription', async () => { + const { isDuplicateBundleSubscription } = await load() + db.getMigrateGrantById.mockResolvedValue(bundleGrant({ redeemed_subscription_id: 'sub_1' })) + expect(await isDuplicateBundleSubscription('grant-1', 'sub_2')).toBe(true) + expect(await isDuplicateBundleSubscription('grant-1', 'sub_1')).toBe(false) + db.getMigrateGrantById.mockResolvedValue(bundleGrant({ redeemed_subscription_id: null })) + expect(await isDuplicateBundleSubscription('grant-1', 'sub_2')).toBe(false) + db.getMigrateGrantById.mockResolvedValue(bundleGrant({ kind: 'trial', redeemed_subscription_id: 'sub_1' })) + expect(await isDuplicateBundleSubscription('grant-1', 'sub_2')).toBe(false) + }) + }) + + describe('money guards on redeem', () => { + const alarms = () => errorLog.mock.calls.map(call => String(call[0])).filter(line => line.includes('ALARM')) + + it('a second subscription for a grant that already has one raises an alarm and is not moved', async () => { + db.getMigrateGrantById.mockResolvedValue(bundleGrant({ redeemed_subscription_id: 'sub_1', checkout_id: 'co_new' })) + const { redeemMigrateGrant } = await load() + await redeemMigrateGrant(payment as never, 'grant-1', 'sub_2', 'co_old') + expect(alarms()).toEqual([expect.stringContaining('duplicate payment')]) + expect(db.markMigrateGrantRedeemed).not.toHaveBeenCalled() + expect(payment.moveBundleSubscriptionToList).not.toHaveBeenCalled() + }) + + it('the same subscription arriving twice (created, then updated) is not a duplicate', async () => { + db.getMigrateGrantById.mockResolvedValue(bundleGrant({ redeemed_subscription_id: 'sub_1', checkout_id: 'co_new' })) + const { redeemMigrateGrant } = await load() + await redeemMigrateGrant(payment as never, 'grant-1', 'sub_1', 'co_new') + expect(alarms()).toEqual([]) + }) + + it('a paid checkout that is not the current one is still honoured, with an alarm to check the amount', async () => { + db.getMigrateGrantById.mockResolvedValue(bundleGrant({ redeemed_subscription_id: null, checkout_id: 'co_new' })) + const { redeemMigrateGrant } = await load() + await redeemMigrateGrant(payment as never, 'grant-1', 'sub_1', 'co_old') + expect(alarms()).toEqual([expect.stringContaining('stale checkout paid')]) + expect(db.markMigrateGrantRedeemed).toHaveBeenCalledWith('grant-1', 'sub_1') + }) + }) + describe('reconciler', () => { + it('alarms on a redeemed bundle that has no subscription id', async () => { + db.listPendingMigrateBundles.mockResolvedValue([bundleGrant({ redeemed_subscription_id: null })]) + const { reconcileMigrateBundles } = await load() + expect(await reconcileMigrateBundles(payment as never, now)).toMatchObject({ checked: 1, alarms: 1 }) + expect(errorLog.mock.calls.some(call => String(call[0]).includes('without a subscription id'))).toBe(true) + }) + it('retries pending moves and counts those it fixed', async () => { db.listPendingMigrateBundles.mockResolvedValue([bundleGrant(), bundleGrant({ id: 'grant-2', redeemed_subscription_id: 'sub_2' })]) const { reconcileMigrateBundles } = await load() diff --git a/tests/unit/supabase-auth-provider.test.ts b/tests/unit/supabase-auth-provider.test.ts index 8bcca7ef..85e521dc 100644 --- a/tests/unit/supabase-auth-provider.test.ts +++ b/tests/unit/supabase-auth-provider.test.ts @@ -273,4 +273,29 @@ describe('supabase auth provider', () => { }, }) }) + + describe('ensureUserForProviderAccount', () => { + const input = { provider: 'github' as const, accountId: '4242', email: 'owner@example.com' } + const load = async (identities: Array>) => { + providerState.adminClient = { auth: { admin: { getUserById: vi.fn().mockResolvedValue({ data: { user: { id: 'u1', identities } }, error: null }) } } } + const { createSupabaseAuthProvider } = await import('../../server/providers/supabase-auth') + const { IdentityConflictError } = await import('../../server/providers/auth') + const provider = createSupabaseAuthProvider() + vi.spyOn(provider, 'getUserByProviderAccount').mockResolvedValue(null) + vi.spyOn(provider, 'getUserByEmail').mockResolvedValue({ id: 'u1', email: input.email, avatarUrl: null, provider: 'google' as never, providerAccountId: 'g-1' }) + return { provider, IdentityConflictError } + } + + it('refuses an email account that already has another GitHub identity', async () => { + const { provider, IdentityConflictError } = await load([{ provider: 'github', identity_data: { provider_id: '9999' } }]) + await expect(provider.ensureUserForProviderAccount(input)).rejects.toBeInstanceOf(IdentityConflictError) + }) + + it('does not mistake the last sign-in provider for a GitHub identity: Google-only or the same GitHub account passes', async () => { + const googleOnly = await load([{ provider: 'google', identity_data: { sub: 'g-1' } }]) + expect((await googleOnly.provider.ensureUserForProviderAccount(input)).id).toBe('u1') + const same = await load([{ provider: 'google', identity_data: { sub: 'g-1' } }, { provider: 'github', identity_data: { provider_id: '4242' } }]) + expect((await same.provider.ensureUserForProviderAccount(input)).id).toBe('u1') + }) + }) })