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 <ltlygwayh@gmail.com>
This commit is contained in:
Marshall Polaris 2022-06-18 14:31:39 -07:00 committed by GitHub
parent 08632a3a07
commit 1075fec53f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 44 additions and 10 deletions

View File

@ -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()
}

View File

@ -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 ?? '')

View File

@ -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<User> = removeUndefinedProps(update)

View File

@ -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,

View File

@ -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.`)
})
}

View File

@ -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',

View File

@ -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 <T>(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)