From 1075fec53fe68855fd9d57b9cfceb0a3369a5416 Mon Sep 17 00:00:00 2001 From: Marshall Polaris Date: Sat, 18 Jun 2022 14:31:39 -0700 Subject: [PATCH] Clean up unclean user names (#543) * Clean the user's display name on update. The user's display name should always be clean (see for example functions/src/create-user.ts). However, change-user-info.ts does not enforce this, thus potentially allowing a malicious user to change their name to something that doesn't satisfy the rules for clean display names. Note: this cannot happen currently because all callers (in profile.tsx) clean the name. However, doing it here is good defense in depth (similar to how the userName is cleaned). * Update display name max length to 30 * Add a script to hunt down too-long display names * Make util.isProd a function * Don't access admin.firestore() on top level of utils.ts Co-authored-by: Jonas Wagner --- common/util/clean-username.ts | 2 +- functions/src/analytics.ts | 2 +- functions/src/change-user-info.ts | 9 ++++++- functions/src/resolve-market.ts | 2 +- functions/src/scripts/clean-display-names.ts | 26 ++++++++++++++++++++ functions/src/stripe.ts | 2 +- functions/src/utils.ts | 11 +++++---- 7 files changed, 44 insertions(+), 10 deletions(-) create mode 100644 functions/src/scripts/clean-display-names.ts diff --git a/common/util/clean-username.ts b/common/util/clean-username.ts index 5c295aeb..10da8376 100644 --- a/common/util/clean-username.ts +++ b/common/util/clean-username.ts @@ -7,6 +7,6 @@ export const cleanUsername = (name: string, maxLength = 25) => { .substring(0, maxLength) } -export const cleanDisplayName = (displayName: string, maxLength = 25) => { +export const cleanDisplayName = (displayName: string, maxLength = 30) => { return displayName.replace(/\s+/g, ' ').substring(0, maxLength).trim() } diff --git a/functions/src/analytics.ts b/functions/src/analytics.ts index 9c01fe36..d178ee0f 100644 --- a/functions/src/analytics.ts +++ b/functions/src/analytics.ts @@ -5,7 +5,7 @@ import { PROD_CONFIG } from '../../common/envs/prod' import { isProd } from './utils' -const key = isProd ? PROD_CONFIG.amplitudeApiKey : DEV_CONFIG.amplitudeApiKey +const key = isProd() ? PROD_CONFIG.amplitudeApiKey : DEV_CONFIG.amplitudeApiKey const amp = Amplitude.init(key ?? '') diff --git a/functions/src/change-user-info.ts b/functions/src/change-user-info.ts index ab15eb70..118d5c67 100644 --- a/functions/src/change-user-info.ts +++ b/functions/src/change-user-info.ts @@ -5,7 +5,10 @@ import { getUser } from './utils' import { Contract } from '../../common/contract' import { Comment } from '../../common/comment' import { User } from '../../common/user' -import { cleanUsername } from '../../common/util/clean-username' +import { + cleanUsername, + cleanDisplayName, +} from '../../common/util/clean-username' import { removeUndefinedProps } from '../../common/util/object' import { Answer } from '../../common/answer' @@ -63,6 +66,10 @@ export const changeUser = async ( } } + if (update.name) { + update.name = cleanDisplayName(update.name) + } + const userRef = firestore.collection('users').doc(user.id) const userUpdate: Partial = removeUndefinedProps(update) diff --git a/functions/src/resolve-market.ts b/functions/src/resolve-market.ts index 894f1492..43cb4839 100644 --- a/functions/src/resolve-market.ts +++ b/functions/src/resolve-market.ts @@ -129,7 +129,7 @@ export const resolveMarket = functions const openBets = bets.filter((b) => !b.isSold && !b.sale) const loanPayouts = getLoanPayouts(openBets) - if (!isProd) + if (!isProd()) console.log( 'payouts:', payouts, diff --git a/functions/src/scripts/clean-display-names.ts b/functions/src/scripts/clean-display-names.ts new file mode 100644 index 00000000..8dd2a3d3 --- /dev/null +++ b/functions/src/scripts/clean-display-names.ts @@ -0,0 +1,26 @@ +// For a while, we didn't enforce that display names would be clean in the `updateUserInfo` +// cloud function, so this script hunts down unclean ones. + +import * as admin from 'firebase-admin' +import { initAdmin } from './script-init' +import { cleanDisplayName } from '../../../common/util/clean-username' +import { log, writeUpdatesAsync, UpdateSpec } from '../utils' +initAdmin() +const firestore = admin.firestore() + +if (require.main === module) { + const usersColl = firestore.collection('users') + usersColl.get().then(async (userSnaps) => { + log(`Loaded ${userSnaps.size} users.`) + const updates = userSnaps.docs.reduce((acc, u) => { + const name = u.data().name + if (name != cleanDisplayName(name)) { + acc.push({ doc: u.ref, fields: { name: cleanDisplayName(name) } }) + } + return acc + }, [] as UpdateSpec[]) + log(`Found ${updates.length} users to update:`, updates) + await writeUpdatesAsync(firestore, updates) + log(`Updated all users.`) + }) +} diff --git a/functions/src/stripe.ts b/functions/src/stripe.ts index 6fbc182c..67309aa8 100644 --- a/functions/src/stripe.ts +++ b/functions/src/stripe.ts @@ -28,7 +28,7 @@ const initStripe = () => { } // manage at https://dashboard.stripe.com/test/products?active=true -const manticDollarStripePrice = isProd +const manticDollarStripePrice = isProd() ? { 500: 'price_1KFQXcGdoFKoCJW770gTNBrm', 1000: 'price_1KFQp1GdoFKoCJW7Iu0dsF65', diff --git a/functions/src/utils.ts b/functions/src/utils.ts index 9cac3409..a95ac858 100644 --- a/functions/src/utils.ts +++ b/functions/src/utils.ts @@ -15,7 +15,7 @@ export const logMemory = () => { } } -type UpdateSpec = { +export type UpdateSpec = { doc: admin.firestore.DocumentReference fields: { [k: string]: unknown } } @@ -36,8 +36,9 @@ export const writeUpdatesAsync = async ( } } -export const isProd = - admin.instanceId().app.options.projectId === 'mantic-markets' +export const isProd = () => { + return admin.instanceId().app.options.projectId === 'mantic-markets' +} export const getDoc = async (collection: string, doc: string) => { const snap = await admin.firestore().collection(collection).doc(doc).get() @@ -69,6 +70,7 @@ export const getPrivateUser = (userId: string) => { } export const getUserByUsername = async (username: string) => { + const firestore = admin.firestore() const snap = await firestore .collection('users') .where('username', '==', username) @@ -77,13 +79,12 @@ export const getUserByUsername = async (username: string) => { return snap.empty ? undefined : (snap.docs[0].data() as User) } -const firestore = admin.firestore() - const updateUserBalance = ( userId: string, delta: number, isDeposit = false ) => { + const firestore = admin.firestore() return firestore.runTransaction(async (transaction) => { const userDoc = firestore.doc(`users/${userId}`) const userSnap = await transaction.get(userDoc)