Skip to content

Commit

Permalink
fix: stop creating a Stripe account during web3.storage account creat…
Browse files Browse the repository at this point in the history
…ion (#2226)

Fixes #2225 

This eliminates a race condition causing the creation of multiple
customers in Stripe. New customers are now treated as if they're in the
free tier without actually giving them a stripe customer.
  • Loading branch information
e-schneid authored Mar 10, 2023
1 parent 34e64ea commit 425ac5c
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 127 deletions.
38 changes: 7 additions & 31 deletions packages/api/src/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { pagination } from './utils/pagination.js'
import { toPinStatusResponse } from './pins.js'
import { INVALID_REQUEST_ID, REQUIRED_REQUEST_ID, validateSearchParams } from './utils/psa.js'
import { magicLinkBypassForE2ETestingInTestmode } from './magic.link.js'
import { CustomerNotFound, getPaymentSettings, initializeBillingForNewUser, isStoragePriceName, savePaymentSettings } from './utils/billing.js'
import { CustomerNotFound, getPaymentSettings, isStoragePriceName, savePaymentSettings } from './utils/billing.js'

/**
* @typedef {string} UserId
Expand Down Expand Up @@ -107,25 +107,6 @@ const createMagicLinkRequestAuthenticator = (env) => async (request) => {
return authentication
}

/**
* Initialize a new user that just registered.
* @param {object} ctx
* @param {object} user
* @param {string} user.id
* @param {string} user.issuer
* @param {import('../src/utils/billing-types').UserCreationOptions} [userCreationOptions]
*/
async function initializeNewUser (ctx, user, userCreationOptions) {
await initializeBillingForNewUser(
{
customers: ctx.customers,
subscriptions: ctx.subscriptions,
user: { ...user, id: user.id.toString() },
userCreationOptions
}
)
}

/**
* @param {Request} request
* @param {UserRegistrationContext} env
Expand Down Expand Up @@ -155,15 +136,7 @@ async function loginOrRegister (request, env) {
return maintenanceHandler()
} else if (env.MODE === READ_WRITE) {
user = await env.db.upsertUser(parsed)
// initialize billing, etc, but only if the user was newly inserted
if (user.inserted) {
await initializeNewUser(
env,
{ ...user, id: user.id },
{ name: parsed.name, email: parsed.email }
)
} else {
// previously existing user. Update their customer record
if (!user.inserted) {
await updateUserCustomerContact(env, user, parsed)
}
} else if (env.MODE === READ_ONLY) {
Expand All @@ -182,8 +155,11 @@ async function loginOrRegister (request, env) {
* @param {import('../src/utils/billing-types').CustomerContact} contact
*/
async function updateUserCustomerContact (context, user, contact) {
const customer = await context.customers.getOrCreateForUser(user)
await context.customers.updateContact(customer.id, contact)
const customer = await context.customers.getForUser(user)

if (customer) {
await context.customers.updateContact(customer.id, contact)
}
}

/**
Expand Down
1 change: 1 addition & 0 deletions packages/api/src/utils/billing-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ export interface CustomersService {
getContact(customerId: Customer['id']): Promise<CustomerContact|CustomerNotFound>
updateContact(customerId: Customer['id'], contact: CustomerContact): Promise<CustomerNotFound|void>
getOrCreateForUser(user: BillingUser, userCreationOptions?: UserCreationOptions): Promise<Pick<Customer, 'id'>>
getForUser(user: BillingUser): Promise<Pick<Customer, 'id'>|null>
}

export type StoragePriceName = 'free' | 'lite' | 'pro'
Expand Down
30 changes: 12 additions & 18 deletions packages/api/src/utils/billing.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,24 +106,6 @@ export function hasOwnProperty (obj, prop) {
return Object.prototype.hasOwnProperty.call(obj, prop)
}

/**
* Initialize billing for a newly signed up user
* @param {object} ctx
* @param {import('./billing-types').CustomersService} ctx.customers
* @param {import('./billing-types').SubscriptionsService} ctx.subscriptions
* @param {import('./billing-types').BillingUser} ctx.user
* @param {import('./billing-types').UserCreationOptions} [ctx.userCreationOptions]
*/
export async function initializeBillingForNewUser (ctx) {
const { customers, user, userCreationOptions } = ctx
const customer = await customers.getOrCreateForUser(user, userCreationOptions)
await ctx.subscriptions.saveSubscription(customer.id, {
storage: {
price: storagePriceNames.free
}
})
}

/**
* Get a user's payment settings
* @param {object} ctx
Expand Down Expand Up @@ -190,6 +172,17 @@ export function createMockCustomerService (
customerIdMap = new Map()
) {
const mockCustomers = []
/**
* @type {import('src/utils/billing-types.js').CustomersService['getForUser']}
*/
async function getForUser (user) {
const existingCustomerForUser = await userCustomerService.getUserCustomer(user.id)
if (existingCustomerForUser) {
return { id: existingCustomerForUser.id }
}

return null
}
/**
* @type {import('src/utils/billing-types.js').CustomersService['getOrCreateForUser']}
*/
Expand Down Expand Up @@ -230,6 +223,7 @@ export function createMockCustomerService (
return {
getContact,
getOrCreateForUser,
getForUser,
updateContact,
mockCustomers
}
Expand Down
14 changes: 14 additions & 0 deletions packages/api/src/utils/stripe.js
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,20 @@ export class StripeCustomersService {
}
}

/**
* @param {import('./billing-types').BillingUser} user
* @returns {Promise<import('./billing-types').Customer|null>}
*/
async getForUser (user) {
const customer = await this.userCustomerService.getUserCustomer(user.id.toString())

if (!customer) return null

return {
id: customer.id
}
}

/**
* @param {import('./billing-types').BillingUser} user
* @param {import('./billing-types').UserCreationOptions} [options]
Expand Down
98 changes: 20 additions & 78 deletions packages/api/test/user.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import userUploads from './fixtures/pgrest/get-user-uploads.js'
import { AuthorizationTestContext } from './contexts/authorization.js'
import { userLoginPost } from '../src/user.js'
import { Magic } from '@magic-sdk/admin'
import { createMockCustomerService, createMockSubscriptionsService, createMockUserCustomerService, storagePriceNames } from '../src/utils/billing.js'
import { createMockCustomerService, createMockSubscriptionsService, createMockUserCustomerService } from '../src/utils/billing.js'

describe('GET /user/account', () => {
it('error if not authenticated with magic.link', async () => {
Expand Down Expand Up @@ -422,41 +422,7 @@ describe('GET /user/pins', () => {
})

describe('userLoginPost', function () {
it('brand new users have a storageSubscription with price=free', async function () {
const user1Authentication = {
issuer: `user1-${Math.random().toString().slice(2)}`,
publicAddress: `user1-${Math.random().toString().slice(2)}`,
email: '[email protected]'
}
const env = {
MODE: /** @type {const} */ ('rw'),
db: getDBClient(),
magic: new Magic(process.env.MAGIC_SECRET_KEY),
authenticateRequest: async () => user1Authentication,
customers: createMockCustomerService(),
subscriptions: createMockSubscriptionsService()
}
const request = new Request(new URL('/user/login', endpoint).toString(), {
method: 'post',
body: JSON.stringify({
data: {}
})
})
const response = await userLoginPost(request, env)
assert.equal(response.status, 200, 'response.status is as expected')

// now ensure it has desired subscription
const gotUser = await env.db.getUser(user1Authentication.issuer, {})
const gotSubscription = await env.subscriptions.getSubscription(
(await env.customers.getOrCreateForUser({ id: gotUser._id })).id
)
assert.ok(!(gotSubscription instanceof Error), 'gotSubscription is not an error')
assert.ok(gotSubscription, 'gotSubscription is truthy')
assert.equal(gotSubscription.storage?.price, storagePriceNames.free)
})
it('login to email-originating user updates customer contact', async function () {
// we're going to create the user by doing userLoginPost the first time
// then on the second time we'll expect the customer to be updated
const user1Name1 = 'user1+1'
const user1Authentication1 = {
issuer: `user1-${Math.random().toString().slice(2)}`,
Expand All @@ -471,34 +437,22 @@ describe('userLoginPost', function () {
customers: createMockCustomerService(userCustomerService),
subscriptions: createMockSubscriptionsService()
}
const getCustomerForUserIssuer = async (issuer) => {
const user = await env.db.getUser(issuer, {})
const customer = await userCustomerService.getUserCustomer(user._id)
return customer
}
const createUserLoginRequest = () => new Request(new URL('/user/login', endpoint).toString(), {
method: 'post',
body: JSON.stringify({
data: {}
})
})
// do first userLoginPost request, which should create the user for the first time
const response1 = await userLoginPost(createUserLoginRequest(), {
...env,
authenticateRequest: async () => user1Authentication1
})
assert.equal(response1.status, 200, 'response.status is as expected')
// the user has been created

// after this first login, we expect the customer contact to have been set
const customer1 = await getCustomerForUserIssuer(user1Authentication1.issuer)
const contact1 = customer1 && await env.customers.getContact(customer1.id)
assert.deepEqual(contact1, {
name: user1Name1,
email: user1Authentication1.email
}, 'user1 contact saved based on request authentication in first userLoginPost')
// create the user
const { id } = await env.db.upsertUser({ ...user1Authentication1, name: user1Name1 })
// create the customer for the user
await env.customers.getOrCreateForUser(
{ id },
{ email: user1Authentication1.email, name: user1Name1 }
)

// now we're going to make the same userLoginPost request again, logging in to that same user.issuer, but with a new email
// now we're going to make a userLoginPost request but with a new email
const name2 = 'user1+2'
/** @type {import('../src/user.js').IssuedAuthentication} */
const user1Authentication2 = {
Expand Down Expand Up @@ -569,29 +523,17 @@ describe('userLoginPost', function () {
name: 'User 1'
}
}
// do first userLoginPost request, which should create the user for the first time
const response1 = await userLoginPost(createUserLoginViaGithubRequest(githubUserOauth1), {
...env,
authenticateRequest: async () => user1Authentication1
})
assert.equal(response1.status, 200, 'response.status is as expected')

// the user has been created
// let's make sure the customer contact was created based on the github oauth info
const getCustomerForUserIssuer = async (issuer) => {
const user = await env.db.getUser(issuer, {})
const customer = await userCustomerService.getUserCustomer(user._id)
return customer
}
const customer1 = await getCustomerForUserIssuer(user1Authentication1.issuer)
assert.ok(!(customer1 instanceof Error), 'no error finding customer for user after first login')
assert.ok(customer1, 'user has a customer after first login')
const contact1 = await env.customers.getContact(customer1.id)
assert.ok(!(contact1 instanceof Error), 'no error finding contact for customer after first login')
assert.deepEqual(contact1.email, user1Authentication1.email, 'customer contact has email from authentication after first login')
assert.deepEqual(contact1.name, githubUserOauth1.userInfo.name, 'customer contact has name from userLoginPost request body after first login')

// now we're going to make the same request again, logging in to that user, but with a new email and name
// create the user
const { id } = await env.db.upsertUser({
...user1Authentication1,
name: githubUserOauth1.userInfo.name
})
// create the customer for the user
const customer1 = await env.customers.getOrCreateForUser(
{ id },
{ email: user1Authentication1.email, name: githubUserOauth1.userInfo.name }
)

const name2 = {
emailLocalName: 'user1+2',
formatted: 'User 1+2'
Expand Down

0 comments on commit 425ac5c

Please sign in to comment.