From 777f711ef367327bfefbd1f0786602f8d4eba95e Mon Sep 17 00:00:00 2001 From: Cailyn Sinclair Date: Sun, 12 Jul 2026 20:43:15 -0700 Subject: [PATCH 1/4] Remove WCA email from user data Request only public WCA identity, allowlist login fields, and stop MongoDB and PostgreSQL writes. Add a value-free two-phase purge with an enforced privacy rollback floor so the compatibility column can remain safely empty until a later drop. --- .gitignore | 1 + README.md | 20 +++++-- README_DEPLOYMENT.md | 47 +++++++++++++++++ client/src/components/Header.jsx | 16 +++--- client/src/lib/wcaAuth.js | 12 +++++ client/src/lib/wcaAuth.test.js | 15 ++++++ scripts/deploy.sh | 13 +++++ scripts/test-deploy.sh | 40 ++++++++++++++ server/auth/index.js | 11 +--- server/auth/wcaProfile.js | 11 ++++ server/auth/wcaProfile.test.js | 28 ++++++++++ server/models/user.js | 3 -- server/models/user.test.js | 25 +++++++++ server/package.json | 1 + server/postgres/dualWrite.js | 9 ++-- server/postgres/dualWrite.test.js | 7 +-- server/prisma/schema.prisma | 1 + server/privacy/purgeUserEmails.js | 42 +++++++++++++++ server/privacy/userEmailPurge.js | 33 ++++++++++++ server/privacy/userEmailPurge.test.js | 76 +++++++++++++++++++++++++++ 20 files changed, 379 insertions(+), 32 deletions(-) create mode 100644 client/src/lib/wcaAuth.js create mode 100644 client/src/lib/wcaAuth.test.js create mode 100644 server/auth/wcaProfile.js create mode 100644 server/auth/wcaProfile.test.js create mode 100644 server/models/user.test.js create mode 100644 server/privacy/purgeUserEmails.js create mode 100644 server/privacy/userEmailPurge.js create mode 100644 server/privacy/userEmailPurge.test.js diff --git a/.gitignore b/.gitignore index c4b0b9f..0b56507 100644 --- a/.gitignore +++ b/.gitignore @@ -43,3 +43,4 @@ client/config/.env.development # Local build/worktree scratch .turbo .worktrees +.privacy-email-cutover diff --git a/README.md b/README.md index 363d1e4..557bfba 100644 --- a/README.md +++ b/README.md @@ -50,6 +50,19 @@ yarn start:client For more on the internals and contributing, check out the [wiki](https://github.com/coder13/LetsCube/wiki) +## Identity privacy + +Let's Cube requests only the WCA OAuth `public` scope. WCA profile data is +allowlisted at login, and the application does not request, retain, mirror, +return, log, analyze, or search WCA email addresses or dates of birth. Product +identity uses the WCA numeric user ID internally and exposes a WCA ID only when +the user has explicitly enabled that existing profile preference. + +Any user-discovery feature must accept only its documented username and visible +WCA ID formats. An email-like input must not be treated as an identifier or +produce an existence signal. This is a required invariant for the Friend System, +not a future discovery mode. + ## Metrics The server stores pseudonymous room and authentication events in both the @@ -71,9 +84,10 @@ access codes, OAuth credentials, chat content, scramble text, or solve times. ## PostgreSQL dual writes New MongoDB writes are mirrored into PostgreSQL without changing application -reads. PostgreSQL receives users and preferences, rooms and participant state, -attempts, durable solve results, and sanitized analytics events. OAuth access -tokens are deliberately not copied. Writes use deterministic UUIDs and upserts, +reads. PostgreSQL receives public identity and preferences, rooms and +participant state, attempts, durable solve results, and sanitized analytics +events. OAuth access tokens are deliberately not copied. Writes use +deterministic UUIDs and upserts, so retries and future backfills are idempotent. Live room saves mirror only the attempts and results changed by that save; complete room snapshots are reserved for explicit backfills. Changing a room event explicitly replaces that room's diff --git a/README_DEPLOYMENT.md b/README_DEPLOYMENT.md index ce85f15..9274f4e 100644 --- a/README_DEPLOYMENT.md +++ b/README_DEPLOYMENT.md @@ -187,6 +187,53 @@ Keep this setting disabled until the legacy scheduler is redesigned. Setting it to `true` explicitly restores the old mode for development or controlled testing. +## WCA Email Privacy Cutover + +The privacy cutover is deliberately separate from ordinary database migrations. +PostgreSQL migrations run before a new application image is healthy, so using a +migration to purge data could let the automatic rollback restore an image that +writes the data again. + +Perform the cutover in this order: + +1. Deploy this release to both application services with `DEPLOY_TARGET=all`. +2. Confirm the API and socket health checks pass, the browser's WCA authorization + request has exactly `scope=public`, and a new login succeeds. +3. From `/opt/letscube`, record the running safe commit as the rollback floor: + + ```bash + git rev-parse HEAD | tee .privacy-email-cutover + ``` + + Future `scripts/deploy.sh` runs refuse any commit that does not descend from + this floor. Do not use direct Compose commands or a saved image tag to restore + an older release after creating the marker. +4. Run the purge from the newly deployed image: + + ```bash + docker compose -f compose.yml -f compose.prod.yml --env-file .env.prod run --rm --no-deps api node server/privacy/purgeUserEmails.js + ``` + + The command reports record counts only. It never prints field values and + exits unsuccessfully if either database still has a value. +5. Run the same command a second time. Every matched, modified, cleared, and + remaining count should be zero; this verifies that the operation is + idempotent. +6. Create and verify a fresh post-cutover backup. Remove every pre-cutover local + and remote backup according to the storage provider's secure deletion + procedure; normal retention is not sufficient for historical private data. + +If the initial deployment rolls back before step 3, do not create the marker or +run the purge. Correct the release and redeploy it first. After step 3, the +privacy-safe commit is the minimum supported rollback image. + +The PostgreSQL `app.users.email` column remains temporarily as an always-empty +compatibility column. Current dual writes never send it a value and clear a +legacy value whenever they update an existing row. Removing the column is phase two: +first deploy an application release that no longer references the compatibility +column, advance every supported rollback image to that release, and only then +apply a migration that drops it. Do not combine that drop with this cutover. + ### Health Checks The API and socket processes expose dependency-aware health endpoints. diff --git a/client/src/components/Header.jsx b/client/src/components/Header.jsx index a30e337..113ee0c 100644 --- a/client/src/components/Header.jsx +++ b/client/src/components/Header.jsx @@ -1,5 +1,4 @@ import React from 'react'; -import qs from 'qs'; import PropTypes from 'prop-types'; import { connect } from 'react-redux'; import { Link } from 'react-router-dom'; @@ -15,6 +14,7 @@ import Button from '@material-ui/core/Button'; import MenuItem from '@material-ui/core/MenuItem'; import { apiOrigin } from '../lib/fetch'; import { getNameFromId } from '../lib/events'; +import { getWcaAuthorizationUrl } from '../lib/wcaAuth'; const useStyles = makeStyles(() => ({ root: { @@ -57,13 +57,13 @@ function Header({ user, room }) { }; const login = () => { - localStorage.setItem('letscube.redirect_uri', `${document.location.origin}/wca-redirect`); - const url = `${process.env.REACT_APP_WCA_ORIGIN}/oauth/authorize?${qs.stringify({ - response_type: 'code', - scope: 'public dob email', - redirect_uri: `${document.location.origin}/wca-redirect`, - client_id: process.env.REACT_APP_WCA_CLIENT_ID, - })}`; + const redirectUri = `${document.location.origin}/wca-redirect`; + localStorage.setItem('letscube.redirect_uri', redirectUri); + const url = getWcaAuthorizationUrl({ + origin: process.env.REACT_APP_WCA_ORIGIN, + clientId: process.env.REACT_APP_WCA_CLIENT_ID, + redirectUri, + }); window.location = url; }; diff --git a/client/src/lib/wcaAuth.js b/client/src/lib/wcaAuth.js new file mode 100644 index 0000000..53fb131 --- /dev/null +++ b/client/src/lib/wcaAuth.js @@ -0,0 +1,12 @@ +import qs from 'qs'; + +export const WCA_OAUTH_SCOPE = 'public'; + +export const getWcaAuthorizationUrl = ({ origin, clientId, redirectUri }) => ( + `${origin}/oauth/authorize?${qs.stringify({ + response_type: 'code', + scope: WCA_OAUTH_SCOPE, + redirect_uri: redirectUri, + client_id: clientId, + })}` +); diff --git a/client/src/lib/wcaAuth.test.js b/client/src/lib/wcaAuth.test.js new file mode 100644 index 0000000..5bd42b2 --- /dev/null +++ b/client/src/lib/wcaAuth.test.js @@ -0,0 +1,15 @@ +import { getWcaAuthorizationUrl, WCA_OAUTH_SCOPE } from './wcaAuth'; + +describe('WCA authorization request', () => { + it('requests only the public identity scope', () => { + const url = new URL(getWcaAuthorizationUrl({ + origin: 'https://www.worldcubeassociation.org', + clientId: 'client-id', + redirectUri: 'https://letscube.net/wca-redirect', + })); + + expect(WCA_OAUTH_SCOPE).toBe('public'); + expect(url.searchParams.get('scope')).toBe('public'); + expect(url.searchParams.get('scope')).not.toMatch(/dob|email/); + }); +}); diff --git a/scripts/deploy.sh b/scripts/deploy.sh index b177e38..62605b5 100755 --- a/scripts/deploy.sh +++ b/scripts/deploy.sh @@ -58,6 +58,19 @@ esac current_commit="$(git rev-parse HEAD)" +privacy_cutover_file="${PRIVACY_EMAIL_CUTOVER_FILE:-$APP_DIR/.privacy-email-cutover}" +if [ -f "$privacy_cutover_file" ]; then + privacy_cutover_commit="$(tr -d '[:space:]' < "$privacy_cutover_file")" + if ! [[ "$privacy_cutover_commit" =~ ^[0-9a-f]{40}$ ]]; then + echo "Invalid email privacy cutover commit in $privacy_cutover_file" >&2 + exit 2 + fi + if ! git merge-base --is-ancestor "$privacy_cutover_commit" "$current_commit"; then + echo "Refusing to deploy $current_commit: it predates the email privacy cutover." >&2 + exit 1 + fi +fi + if [ "$DEPLOY_TARGET" = "auto" ]; then missing_services=() for service in api socket nginx; do diff --git a/scripts/test-deploy.sh b/scripts/test-deploy.sh index ef82299..c707956 100755 --- a/scripts/test-deploy.sh +++ b/scripts/test-deploy.sh @@ -26,6 +26,9 @@ fake_git() { ;; fetch|merge) ;; + merge-base) + [ "${FAKE_CUTOVER_ANCESTOR:-true}" = "true" ] + ;; diff) if [ -n "${FAKE_CHANGED_PATH:-}" ]; then printf '%s\0' "$FAKE_CHANGED_PATH" @@ -269,7 +272,44 @@ run_rollback_test() { assert_contains "$(<"$log")" "git show $previous_commit:compose.prod.yml" } +run_privacy_floor_test() { + local scenario="$TEST_ROOT/privacy-floor" + local app_dir="$scenario/app" + local log="$scenario/commands.log" + local output="$scenario/output.log" + local git_state="$scenario/git-state" + local rollback_marker="$scenario/rollback" + local previous_commit="2222222222222222222222222222222222222222" + local current_commit="1111111111111111111111111111111111111111" + + mkdir -p "$scenario/tmp" + prepare_app "$app_dir" + printf '%s\n' "$previous_commit" > "$app_dir/.privacy-email-cutover" + + if PATH="$FAKE_BIN:$PATH" \ + APP_DIR="$app_dir" \ + DEPLOY_TEST_LOG="$log" \ + ENV_FILE=.env.prod \ + FAKE_CURRENT_COMMIT="$current_commit" \ + FAKE_CUTOVER_ANCESTOR=false \ + FAKE_GIT_STATE="$git_state" \ + FAKE_PREVIOUS_COMMIT="$previous_commit" \ + FAKE_ROLLBACK_MARKER="$rollback_marker" \ + TMPDIR="$scenario/tmp" \ + bash "$app_dir/scripts/deploy.sh" > "$output" 2>&1; then + echo 'Expected a pre-cutover deployment to be rejected.' >&2 + exit 1 + fi + + assert_contains "$(<"$output")" 'predates the email privacy cutover' + if grep -q '^docker ' "$log"; then + echo 'Privacy floor rejection ran Docker commands.' >&2 + exit 1 + fi +} + run_bootstrap_test run_rollback_test +run_privacy_floor_test echo 'Deploy script checks passed.' diff --git a/server/auth/index.js b/server/auth/index.js index 91219c9..525ce30 100644 --- a/server/auth/index.js +++ b/server/auth/index.js @@ -3,6 +3,7 @@ const CustomStrategy = require('passport-custom').Strategy; const { URLSearchParams } = require('url'); const { User } = require('../models'); const metrics = require('../metrics'); +const { buildWcaUserUpdate } = require('./wcaProfile'); const checkStatus = async (res) => { if (res.ok) { // res.status >= 200 && res.status < 300 @@ -37,7 +38,6 @@ module.exports = (app, passport) => { id: +(process.env.LETSCUBE_TEST_USER_ID || 990001), name: 'Cypress Test User', username: 'cypress', - email: 'cypress@example.com', wcaId: '2026TEST01', accessToken: `test-token-${code}`, avatar: {}, @@ -89,14 +89,7 @@ module.exports = (app, passport) => { User.findOneAndUpdate({ id: +profile.id, - }, { - id: +profile.id, - name: profile.name, - email: profile.email, - wcaId: profile.wca_id, - accessToken: tokenRes.access_token, - avatar: profile.avatar, - }, { + }, buildWcaUserUpdate(profile, tokenRes.access_token), { upsert: true, useFindAndModify: false, new: true, diff --git a/server/auth/wcaProfile.js b/server/auth/wcaProfile.js new file mode 100644 index 0000000..d41a023 --- /dev/null +++ b/server/auth/wcaProfile.js @@ -0,0 +1,11 @@ +const buildWcaUserUpdate = (profile, accessToken) => ({ + id: Number(profile.id), + name: profile.name, + wcaId: profile.wca_id, + accessToken, + avatar: profile.avatar, +}); + +module.exports = { + buildWcaUserUpdate, +}; diff --git a/server/auth/wcaProfile.test.js b/server/auth/wcaProfile.test.js new file mode 100644 index 0000000..0769ee3 --- /dev/null +++ b/server/auth/wcaProfile.test.js @@ -0,0 +1,28 @@ +/** @jest-environment node */ +/* eslint-env jest */ + +const { buildWcaUserUpdate } = require('./wcaProfile'); + +describe('WCA profile ingestion', () => { + it('allowlists the identity fields used by LetsCube', () => { + const update = buildWcaUserUpdate({ + id: '1234', + name: 'Test Solver', + wca_id: '2026TEST01', + avatar: { thumb_url: 'avatar.png' }, + email: 'private@example.com', + dob: '2000-01-01', + unrecognized_field: 'do not retain', + }, 'oauth-token'); + + expect(update).toEqual({ + id: 1234, + name: 'Test Solver', + wcaId: '2026TEST01', + accessToken: 'oauth-token', + avatar: { thumb_url: 'avatar.png' }, + }); + expect(update).not.toHaveProperty('email'); + expect(update).not.toHaveProperty('dob'); + }); +}); diff --git a/server/models/user.js b/server/models/user.js index eb01155..053e993 100644 --- a/server/models/user.js +++ b/server/models/user.js @@ -20,9 +20,6 @@ const schema = new mongoose.Schema({ type: Number, required: true, }, - email: { - type: String, - }, name: { type: String, required: true, diff --git a/server/models/user.test.js b/server/models/user.test.js new file mode 100644 index 0000000..874ac54 --- /dev/null +++ b/server/models/user.test.js @@ -0,0 +1,25 @@ +/** @jest-environment node */ +/* eslint-env jest */ + +const mongoose = require('mongoose'); +const UserSchema = require('./user'); + +const User = mongoose.models.UserPrivacyTest + || mongoose.model('UserPrivacyTest', UserSchema); + +describe('user privacy', () => { + it('does not define or serialize an email field', () => { + const user = new User({ + id: 1234, + name: 'Test Solver', + accessToken: 'oauth-token', + showWCAID: true, + }); + user.set('email', 'historical@example.com', { strict: false }); + + expect(UserSchema.path('email')).toBeUndefined(); + expect(user.toObject()).not.toHaveProperty('email'); + expect(user.toJSON()).not.toHaveProperty('email'); + expect(user.toObject()).not.toHaveProperty('accessToken'); + }); +}); diff --git a/server/package.json b/server/package.json index fed3219..99e154c 100644 --- a/server/package.json +++ b/server/package.json @@ -38,6 +38,7 @@ "postgres:migrate:status": "prisma migrate status --config prisma.config.mjs", "postgres:schema:check": "prisma migrate diff --exit-code --from-config-datasource --to-schema prisma/schema.prisma --config prisma.config.mjs", "postgres:schema:validate": "prisma validate --config prisma.config.mjs", + "privacy:purge-user-emails": "node privacy/purgeUserEmails.js", "test": "jest --passWithNoTests", "test:ci": "yarn test" }, diff --git a/server/postgres/dualWrite.js b/server/postgres/dualWrite.js index 7fea99b..ec04f6f 100644 --- a/server/postgres/dualWrite.js +++ b/server/postgres/dualWrite.js @@ -60,11 +60,11 @@ const upsertUser = async (client, user, fallbackUpdatedAt = new Date()) => { const updatedAt = sourceDate(user.updatedAt, fallbackUpdatedAt); await client.query(` INSERT INTO app.users ( - id, wca_user_id, email, name, username, wca_id, preferences, avatar, + id, wca_user_id, name, username, wca_id, preferences, avatar, source_created_at, source_updated_at - ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) + ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9) ON CONFLICT (wca_user_id) DO UPDATE SET - email = EXCLUDED.email, + email = NULL, name = EXCLUDED.name, username = EXCLUDED.username, wca_id = EXCLUDED.wca_id, @@ -76,14 +76,12 @@ const upsertUser = async (client, user, fallbackUpdatedAt = new Date()) => { OR ( app.users.source_updated_at = EXCLUDED.source_updated_at AND ROW( - app.users.email, app.users.name, app.users.username, app.users.wca_id, app.users.preferences, app.users.avatar ) IS DISTINCT FROM ROW( - EXCLUDED.email, EXCLUDED.name, EXCLUDED.username, EXCLUDED.wca_id, @@ -94,7 +92,6 @@ const upsertUser = async (client, user, fallbackUpdatedAt = new Date()) => { `, [ id, wcaUserId, - user.email || null, user.name, user.username || null, user.wcaId || null, diff --git a/server/postgres/dualWrite.test.js b/server/postgres/dualWrite.test.js index 39503fe..9bfeb96 100644 --- a/server/postgres/dualWrite.test.js +++ b/server/postgres/dualWrite.test.js @@ -18,7 +18,7 @@ const { const user = { id: 1234, - email: 'solver@example.com', + email: 'private@example.com', name: 'Test Solver', username: 'solver', wcaId: '2026TEST01', @@ -46,13 +46,14 @@ describe('PostgreSQL dual writer', () => { expect(stableId('user', 1234)).not.toBe(stableId('room', 1234)); }); - it('mirrors users without copying OAuth access tokens', async () => { + it('mirrors users without copying private profile or OAuth fields', async () => { await mirrorUser(user); expect(client.query).toHaveBeenCalledTimes(1); const values = client.query.mock.calls[0][1]; expect(values).toContain(1234); - expect(values).toContain('solver@example.com'); + expect(client.query.mock.calls[0][0]).toContain('email = NULL'); + expect(values).not.toContain('private@example.com'); expect(values).not.toContain('must-not-be-mirrored'); }); diff --git a/server/prisma/schema.prisma b/server/prisma/schema.prisma index 25c9a74..fa4c43c 100644 --- a/server/prisma/schema.prisma +++ b/server/prisma/schema.prisma @@ -6,6 +6,7 @@ datasource db { model User { id String @id @db.Uuid wcaUserId BigInt @unique(map: "users_wca_user_id_key") @map("wca_user_id") + /// Compatibility-only during the privacy cutover. This column must remain null. email String? name String username String? diff --git a/server/privacy/purgeUserEmails.js b/server/privacy/purgeUserEmails.js new file mode 100644 index 0000000..4de0bf7 --- /dev/null +++ b/server/privacy/purgeUserEmails.js @@ -0,0 +1,42 @@ +const mongoose = require('mongoose'); +const database = require('../database'); +const logger = require('../logger'); +const { pool } = require('../postgres'); +const { purgeUserEmails } = require('./userEmailPurge'); + +const report = (summary) => { + logger.info( + '[PRIVACY] User email purge result: ' + + `MongoDB matched=${summary.mongoMatched} modified=${summary.mongoModified} ` + + `remaining=${summary.mongoRemaining}; PostgreSQL cleared=${summary.postgresCleared} ` + + `remaining=${summary.postgresRemaining}`, + ); +}; + +const run = async () => { + const connection = await database.connect(); + + try { + return await purgeUserEmails({ + mongoUsers: connection.connection.collection('users'), + postgresClient: pool, + report, + }); + } finally { + await Promise.allSettled([ + mongoose.disconnect(), + pool.end(), + ]); + } +}; + +if (require.main === module) { + run().catch((err) => { + logger.error(err); + process.exitCode = 1; + }); +} + +module.exports = { + run, +}; diff --git a/server/privacy/userEmailPurge.js b/server/privacy/userEmailPurge.js new file mode 100644 index 0000000..9a79996 --- /dev/null +++ b/server/privacy/userEmailPurge.js @@ -0,0 +1,33 @@ +const purgeUserEmails = async ({ mongoUsers, postgresClient, report = () => {} }) => { + const mongoResult = await mongoUsers.updateMany( + { email: { $exists: true } }, + { $unset: { email: '' } }, + ); + const mongoRemaining = await mongoUsers.countDocuments({ email: { $exists: true } }); + + const postgresResult = await postgresClient.query( + 'UPDATE app.users SET email = NULL WHERE email IS NOT NULL', + ); + const postgresVerification = await postgresClient.query( + 'SELECT count(*)::int AS remaining FROM app.users WHERE email IS NOT NULL', + ); + + const summary = { + mongoMatched: mongoResult.matchedCount, + mongoModified: mongoResult.modifiedCount, + mongoRemaining, + postgresCleared: postgresResult.rowCount, + postgresRemaining: postgresVerification.rows[0].remaining, + }; + report(summary); + + if (mongoRemaining !== 0 || summary.postgresRemaining !== 0) { + throw new Error('User email purge verification failed'); + } + + return summary; +}; + +module.exports = { + purgeUserEmails, +}; diff --git a/server/privacy/userEmailPurge.test.js b/server/privacy/userEmailPurge.test.js new file mode 100644 index 0000000..947cb05 --- /dev/null +++ b/server/privacy/userEmailPurge.test.js @@ -0,0 +1,76 @@ +/** @jest-environment node */ +/* eslint-env jest */ + +const { purgeUserEmails } = require('./userEmailPurge'); + +const runPurge = async ({ matchedCount, modifiedCount, postgresCleared }) => { + const mongoUsers = { + updateMany: jest.fn().mockResolvedValue({ matchedCount, modifiedCount }), + countDocuments: jest.fn().mockResolvedValue(0), + }; + const postgresClient = { + query: jest.fn() + .mockResolvedValueOnce({ rowCount: postgresCleared }) + .mockResolvedValueOnce({ rows: [{ remaining: 0 }] }), + }; + const report = jest.fn(); + const summary = await purgeUserEmails({ mongoUsers, postgresClient, report }); + + return { + mongoUsers, postgresClient, report, summary, + }; +}; + +describe('user email purge', () => { + it('removes every MongoDB field and clears every PostgreSQL copy', async () => { + const { + mongoUsers, postgresClient, report, summary, + } = await runPurge({ matchedCount: 4, modifiedCount: 4, postgresCleared: 4 }); + + expect(mongoUsers.updateMany).toHaveBeenCalledWith( + { email: { $exists: true } }, + { $unset: { email: '' } }, + ); + expect(postgresClient.query).toHaveBeenNthCalledWith( + 1, + 'UPDATE app.users SET email = NULL WHERE email IS NOT NULL', + ); + expect(summary).toEqual({ + mongoMatched: 4, + mongoModified: 4, + mongoRemaining: 0, + postgresCleared: 4, + postgresRemaining: 0, + }); + expect(report).toHaveBeenCalledWith(summary); + }); + + it('is idempotent and reports zero changes on a rerun', async () => { + const { summary } = await runPurge({ + matchedCount: 0, + modifiedCount: 0, + postgresCleared: 0, + }); + + expect(summary).toEqual(expect.objectContaining({ + mongoMatched: 0, + mongoModified: 0, + postgresCleared: 0, + })); + }); + + it('fails when either data store still contains a value', async () => { + const mongoUsers = { + updateMany: jest.fn().mockResolvedValue({ matchedCount: 1, modifiedCount: 0 }), + countDocuments: jest.fn().mockResolvedValue(1), + }; + const postgresClient = { + query: jest.fn() + .mockResolvedValueOnce({ rowCount: 0 }) + .mockResolvedValueOnce({ rows: [{ remaining: 0 }] }), + }; + + await expect(purgeUserEmails({ mongoUsers, postgresClient })) + .rejects.toThrow('User email purge verification failed'); + }); +}); From 72e14945d5446db7fb88bc44a9fb533ba3e7e023 Mon Sep 17 00:00:00 2001 From: Cailyn Sinclair Date: Sun, 12 Jul 2026 20:48:10 -0700 Subject: [PATCH 2/4] Make email purge fail closed Exit unsuccessfully when MongoDB cannot be reached and clear the PostgreSQL compatibility column independently of guarded user upserts. This prevents connection failures or equal timestamps from leaving legacy email values behind. --- server/database.js | 2 +- server/database.test.js | 32 +++++++++++++++++++++++++++++++ server/postgres/dualWrite.js | 6 ++++++ server/postgres/dualWrite.test.js | 11 ++++++++--- 4 files changed, 47 insertions(+), 4 deletions(-) create mode 100644 server/database.test.js diff --git a/server/database.js b/server/database.js index 90533f5..44543be 100644 --- a/server/database.js +++ b/server/database.js @@ -12,7 +12,7 @@ module.exports.connect = async () => { logger.debug('[MONGODB] Connected to database.', { url: config.mongodb }); }).catch((err) => { logger.error('[MONGODB] Error when connecting to database', err); - process.exit(); + process.exit(1); }); return mongoose; diff --git a/server/database.test.js b/server/database.test.js new file mode 100644 index 0000000..dec2416 --- /dev/null +++ b/server/database.test.js @@ -0,0 +1,32 @@ +/** @jest-environment node */ +/* eslint-env jest */ + +jest.mock('mongoose', () => ({ + connect: jest.fn(), + set: jest.fn(), +})); +jest.mock('./logger', () => ({ + debug: jest.fn(), + error: jest.fn(), +})); + +const mongoose = require('mongoose'); +const database = require('./database'); + +describe('MongoDB connection failure', () => { + it('exits unsuccessfully so one-off privacy commands fail closed', async () => { + const connectionError = new Error('MongoDB unavailable'); + const exitError = new Error('process exit'); + mongoose.connect.mockRejectedValue(connectionError); + const exit = jest.spyOn(process, 'exit').mockImplementation((code) => { + exitError.code = code; + throw exitError; + }); + + await expect(database.connect()).rejects.toBe(exitError); + expect(exit).toHaveBeenCalledWith(1); + expect(exitError.code).toBe(1); + + exit.mockRestore(); + }); +}); diff --git a/server/postgres/dualWrite.js b/server/postgres/dualWrite.js index ec04f6f..6a5c869 100644 --- a/server/postgres/dualWrite.js +++ b/server/postgres/dualWrite.js @@ -58,6 +58,12 @@ const upsertUser = async (client, user, fallbackUpdatedAt = new Date()) => { const id = stableId('user', wcaUserId); const updatedAt = sourceDate(user.updatedAt, fallbackUpdatedAt); + // Clear the compatibility column independently so a stale or identical + // source timestamp cannot make the guarded upsert preserve a legacy value. + await client.query( + 'UPDATE app.users SET email = NULL WHERE wca_user_id = $1 AND email IS NOT NULL', + [wcaUserId], + ); await client.query(` INSERT INTO app.users ( id, wca_user_id, name, username, wca_id, preferences, avatar, diff --git a/server/postgres/dualWrite.test.js b/server/postgres/dualWrite.test.js index 9bfeb96..fb26cdb 100644 --- a/server/postgres/dualWrite.test.js +++ b/server/postgres/dualWrite.test.js @@ -49,10 +49,15 @@ describe('PostgreSQL dual writer', () => { it('mirrors users without copying private profile or OAuth fields', async () => { await mirrorUser(user); - expect(client.query).toHaveBeenCalledTimes(1); - const values = client.query.mock.calls[0][1]; + expect(client.query).toHaveBeenCalledTimes(2); + expect(client.query).toHaveBeenNthCalledWith( + 1, + 'UPDATE app.users SET email = NULL WHERE wca_user_id = $1 AND email IS NOT NULL', + [1234], + ); + const values = client.query.mock.calls[1][1]; expect(values).toContain(1234); - expect(client.query.mock.calls[0][0]).toContain('email = NULL'); + expect(client.query.mock.calls[1][0]).toContain('email = NULL'); expect(values).not.toContain('private@example.com'); expect(values).not.toContain('must-not-be-mirrored'); }); From 1b9237990bd0b9739d9d22642e9c1b1392f47378 Mon Sep 17 00:00:00 2001 From: Cailyn Sinclair Date: Sun, 12 Jul 2026 20:52:08 -0700 Subject: [PATCH 3/4] Normalize usernames for indexed lookup Centralize username validation and add a collision-aware MongoDB backfill. Mirror normalized keys through an additive PostgreSQL migration for safe friend discovery. --- README.md | 4 + client/src/components/common/Profile.jsx | 9 +- docs/username-migration.md | 59 +++++++++ scripts/backfill-normalized-usernames.js | 77 +++++++++++ server/api.js | 55 ++------ server/api.test.js | 76 +++++++++++ server/models/user.js | 26 ++++ server/models/user.test.js | 45 +++++++ server/package.json | 1 + server/postgres/dualWrite.js | 10 +- server/postgres/dualWrite.test.js | 3 + .../migration.sql | 5 + server/prisma/schema.prisma | 1 + server/username.js | 117 ++++++++++++++++ server/username.test.js | 125 ++++++++++++++++++ server/usernameBackfill.js | 106 +++++++++++++++ server/usernameBackfill.test.js | 68 ++++++++++ 17 files changed, 736 insertions(+), 51 deletions(-) create mode 100644 docs/username-migration.md create mode 100644 scripts/backfill-normalized-usernames.js create mode 100644 server/api.test.js create mode 100644 server/prisma/migrations/20260712120000_add_normalized_usernames/migration.sql create mode 100644 server/username.js create mode 100644 server/username.test.js create mode 100644 server/usernameBackfill.js create mode 100644 server/usernameBackfill.test.js diff --git a/README.md b/README.md index 557bfba..0e7ea66 100644 --- a/README.md +++ b/README.md @@ -105,3 +105,7 @@ TLS connections can set `PGSSL=true` and provide a CA with `PGSSL_CA`; certificate verification is enabled by default. PostgreSQL failures are logged but do not fail the corresponding MongoDB-backed application operation during this migration phase. + +Username lookup uses a separately normalized, uniquely indexed key while +preserving display casing. See [the normalized username migration](docs/username-migration.md) +for the required collision audit, production order, and rollback procedure. diff --git a/client/src/components/common/Profile.jsx b/client/src/components/common/Profile.jsx index 9a9bda0..dd4aa38 100644 --- a/client/src/components/common/Profile.jsx +++ b/client/src/components/common/Profile.jsx @@ -19,12 +19,9 @@ const validate = (username) => { return ''; } - if (username.indexOf(' ') > -1) { - return 'Username cannot contains spaces'; - } - - if (username.length >= 16) { - return 'Username cannot be longer than 16 characters'; + const normalizedUsername = username.normalize('NFKC').trim(); + if (normalizedUsername.length > 15 || !/^[A-Za-z0-9_-]+$/.test(normalizedUsername)) { + return 'Use 1–15 letters, numbers, underscores, or hyphens'; } return ''; diff --git a/docs/username-migration.md b/docs/username-migration.md new file mode 100644 index 0000000..1f3ae37 --- /dev/null +++ b/docs/username-migration.md @@ -0,0 +1,59 @@ +# Normalized username migration + +MongoDB remains the user source of truth. Usernames are optional and use two +fields: `username` stores trimmed display casing, while `usernameNormalized` +stores the lowercase lookup key. The normalized value is intentionally not +returned by user serialization. + +Valid new usernames contain 1–15 ASCII letters, numbers, underscores, or +hyphens. Empty input unsets both fields. Legacy invalid values are preserved but +left without a normalized key, so they are not discoverable until their owner +chooses a valid username. The privacy exception is an invalid value containing +an email marker (`@`), which is removed instead of retained. Case-insensitive +legacy collisions are also preserved and reported for explicit resolution; the +migration never picks a winner or silently renames an account. + +## Production order + +Issue #191 is a hard prerequisite. Merge and deploy its WCA email scope removal +and data purge before enabling username discovery. + +1. Back up MongoDB and schedule a short maintenance window for username writes. +2. Apply the committed PostgreSQL migrations. The nullable + `app.users.username_normalized` column and its unique index are additive, so + the previous application version continues to run. +3. From the release checkout, audit MongoDB without changing data: + + ```bash + yarn workspace letscube-server usernames:backfill + ``` + +4. Save the JSON report. Resolve every reported collision with the account + owners when practical. Invalid raw values are deliberately omitted from the + report, and email-like invalid values are removed rather than copied or + logged. +5. Apply the idempotent backfill and create or verify the sparse unique index: + + ```bash + yarn workspace letscube-server usernames:backfill --apply --create-index + ``` + +6. Run the dry-run command again. `pendingChanges` must be `0`; unresolved + collision and invalid-value reports may remain because those users are + intentionally undiscoverable. +7. Deploy the API and verify a casing-only username change, a conflicting + change (`409 USERNAME_TAKEN`), and an invalid change + (`400 INVALID_USERNAME`). PostgreSQL receives the normalized field through + the existing non-blocking dual writer whenever MongoDB users are saved. + +Do not enable the discovery endpoint from #82 until this sequence and the #191 +purge have completed. + +## Rollback + +Do not drop the MongoDB index or PostgreSQL column when rolling the application +back. Both additions are backward-compatible and the previous application +ignores them. Disable username discovery and username edits while the old code +is serving because it does not maintain `usernameNormalized`. Before rolling +forward again, rerun the dry-run and apply commands to reconcile any writes made +by the old application. diff --git a/scripts/backfill-normalized-usernames.js b/scripts/backfill-normalized-usernames.js new file mode 100644 index 0000000..a10b4f8 --- /dev/null +++ b/scripts/backfill-normalized-usernames.js @@ -0,0 +1,77 @@ +#!/usr/bin/env node +/* eslint-disable import/no-extraneous-dependencies, no-console */ +const path = require('path'); +const mongoose = require('mongoose'); + +process.env.GETCONFIG_ROOT = process.env.GETCONFIG_ROOT + || path.join(__dirname, '../server/config'); + +const config = require('../server/runtimeConfig'); +const { planUsernameBackfill } = require('../server/usernameBackfill'); + +const args = new Set(process.argv.slice(2)); +const supportedArgs = new Set(['--apply', '--create-index']); +const unknownArg = [...args].find((arg) => !supportedArgs.has(arg)); + +const duplicateNormalizedUsernames = (collection) => collection.aggregate([ + { $match: { usernameNormalized: { $exists: true } } }, + { $group: { _id: '$usernameNormalized', count: { $sum: 1 } } }, + { $match: { count: { $gt: 1 } } }, + { $limit: 1 }, +]).toArray(); + +const run = async () => { + if (unknownArg) { + throw new Error(`Unknown argument: ${unknownArg}`); + } + + const apply = args.has('--apply'); + const createIndex = args.has('--create-index'); + if (createIndex && !apply) { + throw new Error('--create-index must be used with --apply'); + } + + mongoose.set('strictQuery', false); + await mongoose.connect(config.mongodb, { autoIndex: false }); + const users = mongoose.connection.collection('users'); + const documents = await users.find({}, { + projection: { + _id: 1, + id: 1, + username: 1, + usernameNormalized: 1, + }, + }).toArray(); + const plan = planUsernameBackfill(documents); + + console.log(JSON.stringify({ + mode: apply ? 'apply' : 'dry-run', + ...plan.report, + }, null, 2)); + + if (!apply) { + return; + } + + if (plan.operations.length) { + const result = await users.bulkWrite(plan.operations, { ordered: false }); + console.log(`Updated ${result.modifiedCount} user records.`); + } + + if (createIndex) { + const duplicates = await duplicateNormalizedUsernames(users); + if (duplicates.length) { + throw new Error('Duplicate normalized usernames remain; unique index was not created'); + } + const name = await users.createIndex( + { usernameNormalized: 1 }, + { name: 'users_username_normalized_unique', sparse: true, unique: true }, + ); + console.log(`Verified MongoDB index ${name}.`); + } +}; + +run().catch((err) => { + console.error(err.message); + process.exitCode = 1; +}).finally(() => mongoose.disconnect()); diff --git a/server/api.js b/server/api.js index 7e7643a..dff3c8a 100644 --- a/server/api.js +++ b/server/api.js @@ -3,6 +3,7 @@ const express = require('express'); const router = express.Router(); const { User } = require('./models'); const auth = require('./middlewares/auth.js'); +const { updateUsername } = require('./username'); const PREFERENCE_KEYS = new Set([ 'showWCAID', @@ -14,57 +15,27 @@ const PREFERENCE_KEYS = new Set([ module.exports = () => { const sendError = (res, err) => { - res.status(err.statusCode || 500).send({ + const body = { status: err.statusCode, message: err.message || 'Error occured while retrieving data; contact Kleb', - }); + }; + if (err.code) { + body.code = err.code; + } + res.status(err.statusCode || 500).send(body); }; router.get('/me', auth, (req, res) => { res.json(req.user.toObject()); }); - router.put('/updateUsername', auth, (req, res) => { - // TODO: server side validation of username - // TODO: refactor - const { username } = req.body; - if (username === undefined) { - return sendError(res, { - statusCode: 400, - message: 'Missing username from request', - }); - } - - if (username === '') { - req.user.username = username; - return req.user.save().then((u) => { - res.json(u.toObject()); - }); + router.put('/updateUsername', auth, async (req, res) => { + try { + const user = await updateUsername(User, req.user, req.body.username); + return res.json(user.toObject()); + } catch (err) { + return sendError(res, err); } - - User.findOne({ - username: { - $regex: new RegExp(`^${username}$`, 'i'), - }, - }).then((user) => { - if (user && user.id !== req.user.id) { - sendError(res, { - statusCode: 500, - message: 'User with username already exists', - }); - return null; - } - - req.user.username = username.trim(); - return req.user.save().then((u) => { - res.json(u.toObject()); - }); - }).catch((err) => { - sendError(res, { - statusCode: 500, - message: err, - }); - }); }); router.put('/updatePreference', auth, async (req, res) => { diff --git a/server/api.test.js b/server/api.test.js new file mode 100644 index 0000000..bbc1944 --- /dev/null +++ b/server/api.test.js @@ -0,0 +1,76 @@ +/** @jest-environment node */ +/* eslint-env jest */ + +jest.mock('./models', () => ({ + User: { findOne: jest.fn() }, +})); +jest.mock('./middlewares/auth.js', () => (req, res, next) => next()); + +const { User } = require('./models'); +const router = require('./api')(); + +const updateUsernameHandler = router.stack + .find((layer) => layer.route && layer.route.path === '/updateUsername') + .route.stack[1].handle; + +const response = () => { + const res = { + json: jest.fn(), + send: jest.fn(), + status: jest.fn(), + }; + res.status.mockReturnValue(res); + return res; +}; + +describe('username API responses', () => { + beforeEach(() => jest.clearAllMocks()); + + it('returns stable 400 details for invalid input', async () => { + const req = { + body: { username: 'name@example.com' }, + user: { id: 1, save: jest.fn() }, + }; + const res = response(); + + await updateUsernameHandler(req, res); + + expect(res.status).toHaveBeenCalledWith(400); + expect(res.send).toHaveBeenCalledWith(expect.objectContaining({ + code: 'INVALID_USERNAME', + status: 400, + })); + expect(User.findOne).not.toHaveBeenCalled(); + }); + + it('returns stable 409 details for a case-insensitive conflict', async () => { + User.findOne.mockResolvedValue({ id: 2 }); + const req = { + body: { username: 'CUBER' }, + user: { id: 1, save: jest.fn() }, + }; + const res = response(); + + await updateUsernameHandler(req, res); + + expect(User.findOne).toHaveBeenCalledWith({ usernameNormalized: 'cuber' }); + expect(res.status).toHaveBeenCalledWith(409); + expect(res.send).toHaveBeenCalledWith(expect.objectContaining({ + code: 'USERNAME_TAKEN', + status: 409, + })); + expect(req.user.save).not.toHaveBeenCalled(); + }); + + it('returns stable 400 details when username is omitted', async () => { + const res = response(); + + await updateUsernameHandler({ body: {}, user: { id: 1 } }, res); + + expect(res.status).toHaveBeenCalledWith(400); + expect(res.send).toHaveBeenCalledWith(expect.objectContaining({ + code: 'MISSING_USERNAME', + status: 400, + })); + }); +}); diff --git a/server/models/user.js b/server/models/user.js index 053e993..c6426ff 100644 --- a/server/models/user.js +++ b/server/models/user.js @@ -1,9 +1,11 @@ const mongoose = require('mongoose'); const { mirrorUser } = require('../postgres/dualWrite'); +const { normalizeUsername } = require('../username'); const redactUser = (doc, ret) => { delete ret.email; delete ret.accessToken; + delete ret.usernameNormalized; if (!doc.showWCAID) { delete ret.wcaId; if (!doc.preferRealName) { @@ -27,6 +29,9 @@ const schema = new mongoose.Schema({ username: { type: String, }, + usernameNormalized: { + type: String, + }, wcaId: { type: String, }, @@ -68,6 +73,27 @@ const schema = new mongoose.Schema({ getters: true, transform: redactUser, }, + autoIndex: false, +}); + +schema.index( + { usernameNormalized: 1 }, + { name: 'users_username_normalized_unique', sparse: true, unique: true }, +); + +schema.pre('validate', function normalizeChangedUsername(next) { + if (!this.isNew && !this.isModified('username')) { + return next(); + } + + try { + const normalized = normalizeUsername(this.username); + this.username = normalized.username; + this.usernameNormalized = normalized.usernameNormalized; + return next(); + } catch (err) { + return next(err); + } }); schema.virtual('displayName').get(function () { diff --git a/server/models/user.test.js b/server/models/user.test.js index 874ac54..cd5fd05 100644 --- a/server/models/user.test.js +++ b/server/models/user.test.js @@ -1,6 +1,8 @@ /** @jest-environment node */ /* eslint-env jest */ +jest.mock('../postgres/dualWrite', () => ({ mirrorUser: jest.fn() })); + const mongoose = require('mongoose'); const UserSchema = require('./user'); @@ -23,3 +25,46 @@ describe('user privacy', () => { expect(user.toObject()).not.toHaveProperty('accessToken'); }); }); + +describe('user username schema', () => { + it('declares an explicit sparse unique normalized username index', () => { + expect(UserSchema.indexes()).toContainEqual([ + { usernameNormalized: 1 }, + expect.objectContaining({ + name: 'users_username_normalized_unique', + sparse: true, + unique: true, + }), + ]); + expect(UserSchema.options.autoIndex).toBe(false); + }); + + it('normalizes direct username writes through the shared validation path', async () => { + const user = new User({ + id: 1, + name: 'Test User', + accessToken: 'secret', + username: ' MixedCase_1 ', + }); + + await user.validate(); + + expect(user.username).toBe('MixedCase_1'); + expect(user.usernameNormalized).toBe('mixedcase_1'); + expect(user.toObject()).not.toHaveProperty('usernameNormalized'); + }); + + it('rejects invalid direct username writes', async () => { + const user = new User({ + id: 1, + name: 'Test User', + accessToken: 'secret', + username: 'not.an.email@example.com', + }); + + await expect(user.validate()).rejects.toMatchObject({ + code: 'INVALID_USERNAME', + statusCode: 400, + }); + }); +}); diff --git a/server/package.json b/server/package.json index 99e154c..87bbe1d 100644 --- a/server/package.json +++ b/server/package.json @@ -39,6 +39,7 @@ "postgres:schema:check": "prisma migrate diff --exit-code --from-config-datasource --to-schema prisma/schema.prisma --config prisma.config.mjs", "postgres:schema:validate": "prisma validate --config prisma.config.mjs", "privacy:purge-user-emails": "node privacy/purgeUserEmails.js", + "usernames:backfill": "node ../scripts/backfill-normalized-usernames.js", "test": "jest --passWithNoTests", "test:ci": "yarn test" }, diff --git a/server/postgres/dualWrite.js b/server/postgres/dualWrite.js index 6a5c869..02ce1bf 100644 --- a/server/postgres/dualWrite.js +++ b/server/postgres/dualWrite.js @@ -66,13 +66,14 @@ const upsertUser = async (client, user, fallbackUpdatedAt = new Date()) => { ); await client.query(` INSERT INTO app.users ( - id, wca_user_id, name, username, wca_id, preferences, avatar, - source_created_at, source_updated_at - ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9) + id, wca_user_id, name, username, username_normalized, wca_id, + preferences, avatar, source_created_at, source_updated_at + ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) ON CONFLICT (wca_user_id) DO UPDATE SET email = NULL, name = EXCLUDED.name, username = EXCLUDED.username, + username_normalized = EXCLUDED.username_normalized, wca_id = EXCLUDED.wca_id, preferences = EXCLUDED.preferences, avatar = EXCLUDED.avatar, @@ -84,12 +85,14 @@ const upsertUser = async (client, user, fallbackUpdatedAt = new Date()) => { AND ROW( app.users.name, app.users.username, + app.users.username_normalized, app.users.wca_id, app.users.preferences, app.users.avatar ) IS DISTINCT FROM ROW( EXCLUDED.name, EXCLUDED.username, + EXCLUDED.username_normalized, EXCLUDED.wca_id, EXCLUDED.preferences, EXCLUDED.avatar @@ -100,6 +103,7 @@ const upsertUser = async (client, user, fallbackUpdatedAt = new Date()) => { wcaUserId, user.name, user.username || null, + user.usernameNormalized || null, user.wcaId || null, { showWCAID: !!user.showWCAID, diff --git a/server/postgres/dualWrite.test.js b/server/postgres/dualWrite.test.js index fb26cdb..e296886 100644 --- a/server/postgres/dualWrite.test.js +++ b/server/postgres/dualWrite.test.js @@ -21,6 +21,7 @@ const user = { email: 'private@example.com', name: 'Test Solver', username: 'solver', + usernameNormalized: 'solver', wcaId: '2026TEST01', accessToken: 'must-not-be-mirrored', showWCAID: true, @@ -59,7 +60,9 @@ describe('PostgreSQL dual writer', () => { expect(values).toContain(1234); expect(client.query.mock.calls[1][0]).toContain('email = NULL'); expect(values).not.toContain('private@example.com'); + expect(values.slice(3, 5)).toEqual(['solver', 'solver']); expect(values).not.toContain('must-not-be-mirrored'); + expect(client.query.mock.calls[1][0]).toContain('username_normalized'); }); it('mirrors a room snapshot, participants, attempts, and solves', async () => { diff --git a/server/prisma/migrations/20260712120000_add_normalized_usernames/migration.sql b/server/prisma/migrations/20260712120000_add_normalized_usernames/migration.sql new file mode 100644 index 0000000..92a80fd --- /dev/null +++ b/server/prisma/migrations/20260712120000_add_normalized_usernames/migration.sql @@ -0,0 +1,5 @@ +ALTER TABLE app.users + ADD COLUMN username_normalized text; + +CREATE UNIQUE INDEX users_username_normalized_key + ON app.users (username_normalized); diff --git a/server/prisma/schema.prisma b/server/prisma/schema.prisma index fa4c43c..9cbb555 100644 --- a/server/prisma/schema.prisma +++ b/server/prisma/schema.prisma @@ -10,6 +10,7 @@ model User { email String? name String username String? + usernameNormalized String? @unique(map: "users_username_normalized_key") @map("username_normalized") wcaId String? @map("wca_id") preferences Json @default("{}") @db.JsonB avatar Json @default("{}") @db.JsonB diff --git a/server/username.js b/server/username.js new file mode 100644 index 0000000..18b198b --- /dev/null +++ b/server/username.js @@ -0,0 +1,117 @@ +const USERNAME_MAX_LENGTH = 15; +const USERNAME_SEARCH_DEFAULT_LIMIT = 20; +const USERNAME_SEARCH_MAX_LIMIT = 50; +const USERNAME_PATTERN = /^[A-Za-z0-9_-]+$/; + +class UsernameError extends Error { + constructor(code, message, statusCode = 400) { + super(message); + this.name = 'UsernameError'; + this.code = code; + this.statusCode = statusCode; + } +} + +const invalidUsername = () => new UsernameError( + 'INVALID_USERNAME', + 'Username may only contain letters, numbers, underscores, and hyphens', +); + +const normalizeUsername = (value, { allowEmpty = true } = {}) => { + if (value === undefined || value === null) { + if (allowEmpty) { + return { username: undefined, usernameNormalized: undefined }; + } + throw invalidUsername(); + } + + if (typeof value !== 'string') { + throw invalidUsername(); + } + + const username = value.normalize('NFKC').trim(); + if (!username) { + if (allowEmpty) { + return { username: undefined, usernameNormalized: undefined }; + } + throw invalidUsername(); + } + + if (username.length > USERNAME_MAX_LENGTH || !USERNAME_PATTERN.test(username)) { + throw invalidUsername(); + } + + return { + username, + usernameNormalized: username.toLowerCase(), + }; +}; + +const usernameConflict = () => new UsernameError( + 'USERNAME_TAKEN', + 'Username is already in use', + 409, +); + +const isUsernameDuplicateKeyError = (err) => err && err.code === 11000 && ( + (err.keyPattern && err.keyPattern.usernameNormalized) + || (err.message && err.message.includes('users_username_normalized_unique')) +); + +const findUserByUsername = (User, value) => { + const { usernameNormalized } = normalizeUsername(value, { allowEmpty: false }); + return User.findOne({ usernameNormalized }); +}; + +const searchUsersByUsernamePrefix = (User, value, requestedLimit) => { + const { usernameNormalized } = normalizeUsername(value, { allowEmpty: false }); + const parsedLimit = Number(requestedLimit); + const limit = Number.isInteger(parsedLimit) && parsedLimit > 0 + ? Math.min(parsedLimit, USERNAME_SEARCH_MAX_LIMIT) + : USERNAME_SEARCH_DEFAULT_LIMIT; + + return User.find({ + usernameNormalized: { + $gte: usernameNormalized, + $lt: `${usernameNormalized}\uffff`, + }, + }).sort({ usernameNormalized: 1 }).limit(limit); +}; + +const updateUsername = async (User, user, value) => { + if (value === undefined) { + throw new UsernameError('MISSING_USERNAME', 'Missing username from request'); + } + + const normalized = normalizeUsername(value); + if (normalized.usernameNormalized) { + const existingUser = await findUserByUsername(User, normalized.username); + if (existingUser && existingUser.id.toString() !== user.id.toString()) { + throw usernameConflict(); + } + } + + user.username = normalized.username; + user.usernameNormalized = normalized.usernameNormalized; + + try { + return await user.save(); + } catch (err) { + if (isUsernameDuplicateKeyError(err)) { + throw usernameConflict(); + } + throw err; + } +}; + +module.exports = { + USERNAME_MAX_LENGTH, + USERNAME_SEARCH_DEFAULT_LIMIT, + USERNAME_SEARCH_MAX_LIMIT, + UsernameError, + findUserByUsername, + isUsernameDuplicateKeyError, + normalizeUsername, + searchUsersByUsernamePrefix, + updateUsername, +}; diff --git a/server/username.test.js b/server/username.test.js new file mode 100644 index 0000000..7c04f4b --- /dev/null +++ b/server/username.test.js @@ -0,0 +1,125 @@ +/** @jest-environment node */ +/* eslint-env jest */ + +const { + findUserByUsername, + normalizeUsername, + searchUsersByUsernamePrefix, + updateUsername, +} = require('./username'); + +describe('username normalization and lookup', () => { + it('trims display values, preserves casing, and creates a lowercase key', () => { + expect(normalizeUsername(' FastCuber_7 ')).toEqual({ + username: 'FastCuber_7', + usernameNormalized: 'fastcuber_7', + }); + }); + + it('represents empty optional usernames as absent fields', () => { + expect(normalizeUsername(' ')).toEqual({ + username: undefined, + usernameNormalized: undefined, + }); + }); + + it.each([ + 'cuber.name', + 'cuber+name', + 'cuber@example.com', + 'two cubers', + '1234567890123456', + ])('rejects invalid input without treating it as a lookup: %s', (username) => { + expect(() => normalizeUsername(username)).toThrow(expect.objectContaining({ + code: 'INVALID_USERNAME', + statusCode: 400, + })); + }); + + it('uses normalized equality instead of a regular expression for exact lookup', () => { + const User = { findOne: jest.fn().mockReturnValue('query') }; + + expect(findUserByUsername(User, 'Cuber_1')).toBe('query'); + expect(User.findOne).toHaveBeenCalledWith({ usernameNormalized: 'cuber_1' }); + }); + + it('uses an indexed lexical range and caps prefix results', () => { + const limit = jest.fn().mockReturnValue('query'); + const sort = jest.fn().mockReturnValue({ limit }); + const User = { find: jest.fn().mockReturnValue({ sort }) }; + + expect(searchUsersByUsernamePrefix(User, 'CuBer', 1000)).toBe('query'); + expect(User.find).toHaveBeenCalledWith({ + usernameNormalized: { + $gte: 'cuber', + $lt: 'cuber\uffff', + }, + }); + expect(sort).toHaveBeenCalledWith({ usernameNormalized: 1 }); + expect(limit).toHaveBeenCalledWith(50); + }); + + it('rejects metacharacters before issuing a prefix query', () => { + const User = { find: jest.fn() }; + + expect(() => searchUsersByUsernamePrefix(User, 'cube.*')).toThrow(expect.objectContaining({ + code: 'INVALID_USERNAME', + })); + expect(User.find).not.toHaveBeenCalled(); + }); +}); + +describe('username updates', () => { + it('saves normalized fields and allows display casing changes for the same user', async () => { + const saved = { toObject: jest.fn() }; + const user = { id: 1, save: jest.fn().mockResolvedValue(saved) }; + const User = { findOne: jest.fn().mockResolvedValue({ id: 1 }) }; + + await expect(updateUsername(User, user, ' Cuber ')).resolves.toBe(saved); + expect(user).toEqual(expect.objectContaining({ + username: 'Cuber', + usernameNormalized: 'cuber', + })); + }); + + it('returns a stable conflict before writing another user\'s username', async () => { + const user = { id: 1, save: jest.fn() }; + const User = { findOne: jest.fn().mockResolvedValue({ id: 2 }) }; + + await expect(updateUsername(User, user, 'Cuber')).rejects.toMatchObject({ + code: 'USERNAME_TAKEN', + statusCode: 409, + }); + expect(user.save).not.toHaveBeenCalled(); + }); + + it('maps a unique-index race to the same stable conflict', async () => { + const duplicate = Object.assign(new Error('users_username_normalized_unique'), { code: 11000 }); + const user = { id: 1, save: jest.fn().mockRejectedValue(duplicate) }; + const User = { findOne: jest.fn().mockResolvedValue(null) }; + + await expect(updateUsername(User, user, 'Cuber')).rejects.toMatchObject({ + code: 'USERNAME_TAKEN', + statusCode: 409, + }); + }); + + it('unsets an optional username without querying for a conflict', async () => { + const user = { id: 1, username: 'Old', usernameNormalized: 'old' }; + user.save = jest.fn().mockResolvedValue(user); + const User = { findOne: jest.fn() }; + + await updateUsername(User, user, ''); + + expect(user.username).toBeUndefined(); + expect(user.usernameNormalized).toBeUndefined(); + expect(User.findOne).not.toHaveBeenCalled(); + }); + + it('returns a stable error when the request omits username', async () => { + await expect(updateUsername({}, { id: 1 }, undefined)).rejects.toMatchObject({ + code: 'MISSING_USERNAME', + statusCode: 400, + }); + }); +}); diff --git a/server/usernameBackfill.js b/server/usernameBackfill.js new file mode 100644 index 0000000..5f11aaa --- /dev/null +++ b/server/usernameBackfill.js @@ -0,0 +1,106 @@ +const { normalizeUsername } = require('./username'); + +const identifier = (user) => (user.id === undefined ? user._id.toString() : user.id); +const hasOwn = (value, key) => Object.prototype.hasOwnProperty.call(value, key); +const containsEmailMarker = (value) => typeof value === 'string' && value.includes('@'); + +const changedOperation = (user, set, unset) => { + const changedSet = Object.entries(set).some(([key, value]) => user[key] !== value); + const changedUnset = Object.keys(unset).some((key) => hasOwn(user, key)); + if (!changedSet && !changedUnset) { + return null; + } + + const update = {}; + if (Object.keys(set).length) { + update.$set = set; + } + if (Object.keys(unset).length) { + update.$unset = unset; + } + + return { + updateOne: { + filter: { _id: user._id }, + update, + }, + }; +}; + +const planUsernameBackfill = (users) => { + const analyzed = users.map((user) => { + try { + return { user, ...normalizeUsername(user.username) }; + } catch (err) { + return { user, error: err }; + } + }); + const grouped = new Map(); + + analyzed.forEach((entry) => { + if (!entry.usernameNormalized) { + return; + } + const matches = grouped.get(entry.usernameNormalized) || []; + matches.push(entry); + grouped.set(entry.usernameNormalized, matches); + }); + + const collisionKeys = new Set( + [...grouped.entries()].filter(([, entries]) => entries.length > 1).map(([key]) => key), + ); + const operations = []; + const invalid = []; + let empty = 0; + let valid = 0; + + analyzed.forEach((entry) => { + const { user } = entry; + let operation; + + if (entry.error) { + invalid.push({ id: identifier(user), code: entry.error.code }); + operation = changedOperation(user, {}, { + ...(containsEmailMarker(user.username) ? { username: '' } : {}), + usernameNormalized: '', + }); + } else if (!entry.usernameNormalized) { + empty += 1; + operation = changedOperation(user, {}, { username: '', usernameNormalized: '' }); + } else if (collisionKeys.has(entry.usernameNormalized)) { + operation = changedOperation(user, { username: entry.username }, { usernameNormalized: '' }); + } else { + valid += 1; + operation = changedOperation(user, { + username: entry.username, + usernameNormalized: entry.usernameNormalized, + }, {}); + } + + if (operation) { + operations.push(operation); + } + }); + + const collisions = [...collisionKeys].sort().map((usernameNormalized) => ({ + usernameNormalized, + users: grouped.get(usernameNormalized).map(({ user, username }) => ({ + id: identifier(user), + username, + })), + })); + + return { + operations, + report: { + scanned: users.length, + valid, + empty, + invalid, + collisions, + pendingChanges: operations.length, + }, + }; +}; + +module.exports = { planUsernameBackfill }; diff --git a/server/usernameBackfill.test.js b/server/usernameBackfill.test.js new file mode 100644 index 0000000..51b7b5c --- /dev/null +++ b/server/usernameBackfill.test.js @@ -0,0 +1,68 @@ +/** @jest-environment node */ +/* eslint-env jest */ + +const { planUsernameBackfill } = require('./usernameBackfill'); + +const applyOperations = (users, operations) => { + operations.forEach(({ updateOne }) => { + const user = users.find(({ _id }) => _id === updateOne.filter._id); + Object.assign(user, updateOne.update.$set || {}); + Object.keys(updateOne.update.$unset || {}).forEach((key) => delete user[key]); + }); +}; + +describe('normalized username backfill', () => { + it('plans valid, empty, invalid, and colliding legacy records safely', () => { + const users = [ + { _id: '1', id: 1, username: ' SoloCuber ' }, + { _id: '2', id: 2, username: 'FastCuber' }, + { + _id: '3', id: 3, username: 'fastcuber', usernameNormalized: 'stale', + }, + { _id: '4', id: 4, username: '' }, + { + _id: '5', id: 5, username: 'legacy.name', usernameNormalized: 'legacy.name', + }, + { _id: '6', id: 6, username: 'private@example.com' }, + ]; + + const { operations, report } = planUsernameBackfill(users); + applyOperations(users, operations); + + expect(users[0]).toMatchObject({ + username: 'SoloCuber', usernameNormalized: 'solocuber', + }); + expect(users[1].usernameNormalized).toBeUndefined(); + expect(users[2].usernameNormalized).toBeUndefined(); + expect(users[3].username).toBeUndefined(); + expect(users[4]).toMatchObject({ username: 'legacy.name' }); + expect(users[4].usernameNormalized).toBeUndefined(); + expect(users[5].username).toBeUndefined(); + expect(report.collisions).toEqual([{ + usernameNormalized: 'fastcuber', + users: [ + { id: 2, username: 'FastCuber' }, + { id: 3, username: 'fastcuber' }, + ], + }]); + expect(report.invalid).toEqual([ + { id: 5, code: 'INVALID_USERNAME' }, + { id: 6, code: 'INVALID_USERNAME' }, + ]); + expect(JSON.stringify(report.invalid)).not.toContain('private@example.com'); + }); + + it('is idempotent after applying the planned operations', () => { + const users = [ + { _id: '1', id: 1, username: ' SoloCuber ' }, + { _id: '2', id: 2, username: 'FastCuber' }, + { _id: '3', id: 3, username: 'fastcuber' }, + { _id: '4', id: 4, username: 'legacy.name' }, + { _id: '5', id: 5, username: 'private@example.com' }, + ]; + const first = planUsernameBackfill(users); + applyOperations(users, first.operations); + + expect(planUsernameBackfill(users).operations).toEqual([]); + }); +}); From 35e3198afc1293194e61f0102495edeead85c464 Mon Sep 17 00:00:00 2001 From: Cailyn Sinclair Date: Sun, 12 Jul 2026 21:13:16 -0700 Subject: [PATCH 4/4] Harden normalized username rollout Reconcile MongoDB username targets into PostgreSQL and fail index creation while collisions remain. Canonicalize privacy markers and keep test authentication on the shared normalization path. --- docs/username-migration.md | 37 ++++++---- scripts/backfill-normalized-usernames.js | 59 +++++++++++++--- server/auth/index.js | 17 ++--- server/auth/testUser.js | 18 +++++ server/auth/testUser.test.js | 30 ++++++++ server/username.js | 7 +- server/username.test.js | 8 +++ server/usernameBackfill.js | 67 ++++++++++++++---- server/usernameBackfill.test.js | 50 ++++++++++++- server/usernamePostgresBackfill.js | 90 ++++++++++++++++++++++++ server/usernamePostgresBackfill.test.js | 66 +++++++++++++++++ 11 files changed, 396 insertions(+), 53 deletions(-) create mode 100644 server/auth/testUser.js create mode 100644 server/auth/testUser.test.js create mode 100644 server/usernamePostgresBackfill.js create mode 100644 server/usernamePostgresBackfill.test.js diff --git a/docs/username-migration.md b/docs/username-migration.md index 1f3ae37..396c3b3 100644 --- a/docs/username-migration.md +++ b/docs/username-migration.md @@ -9,14 +9,15 @@ Valid new usernames contain 1–15 ASCII letters, numbers, underscores, or hyphens. Empty input unsets both fields. Legacy invalid values are preserved but left without a normalized key, so they are not discoverable until their owner chooses a valid username. The privacy exception is an invalid value containing -an email marker (`@`), which is removed instead of retained. Case-insensitive -legacy collisions are also preserved and reported for explicit resolution; the +an email marker (`@`) after NFKC canonicalization, which is removed instead of +retained. This includes compatibility forms such as fullwidth and small `@`. +Case-insensitive legacy collisions are reported for explicit resolution; the migration never picks a winner or silently renames an account. ## Production order -Issue #191 is a hard prerequisite. Merge and deploy its WCA email scope removal -and data purge before enabling username discovery. +Issue #191 / PR #192 is a hard prerequisite. Its WCA email scope removal and +data purge must remain in the branch and be deployed before username discovery. 1. Back up MongoDB and schedule a short maintenance window for username writes. 2. Apply the committed PostgreSQL migrations. The nullable @@ -29,25 +30,35 @@ and data purge before enabling username discovery. ``` 4. Save the JSON report. Resolve every reported collision with the account - owners when practical. Invalid raw values are deliberately omitted from the - report, and email-like invalid values are removed rather than copied or - logged. -5. Apply the idempotent backfill and create or verify the sparse unique index: + owners before rollout. Do not deploy username writes or discovery while any + collision remains: index creation fails closed rather than leaving the name + available for a third account to claim. Invalid raw values are deliberately + omitted from the report, and email-like invalid values are removed rather + than copied or logged. +5. After the collision report is empty, apply the idempotent backfill, reconcile + existing PostgreSQL usernames by WCA user ID, and create or verify the sparse + unique MongoDB index: ```bash yarn workspace letscube-server usernames:backfill --apply --create-index ``` -6. Run the dry-run command again. `pendingChanges` must be `0`; unresolved - collision and invalid-value reports may remain because those users are - intentionally undiscoverable. + The PostgreSQL reconciliation uses MongoDB's planned target for each WCA user + and verifies exact equality without printing stored values. It clears both + PostgreSQL username fields for email-like legacy values. When PostgreSQL is + intentionally disabled with `POSTGRES_ENABLED=false`, the command records a + disabled status and skips that secondary store; rerun with PostgreSQL enabled + before bringing the mirror back into service. +6. Run the dry-run command again. `pendingChanges`, `privacyRemoved`, and the + collision report must all be empty or zero. The apply command must report + zero MongoDB private values and zero PostgreSQL mismatches. 7. Deploy the API and verify a casing-only username change, a conflicting change (`409 USERNAME_TAKEN`), and an invalid change (`400 INVALID_USERNAME`). PostgreSQL receives the normalized field through the existing non-blocking dual writer whenever MongoDB users are saved. -Do not enable the discovery endpoint from #82 until this sequence and the #191 -purge have completed. +Do not enable username writes or the discovery endpoint from #82 until this +sequence and the #191 purge have completed successfully. ## Rollback diff --git a/scripts/backfill-normalized-usernames.js b/scripts/backfill-normalized-usernames.js index a10b4f8..0a2edae 100644 --- a/scripts/backfill-normalized-usernames.js +++ b/scripts/backfill-normalized-usernames.js @@ -7,7 +7,11 @@ process.env.GETCONFIG_ROOT = process.env.GETCONFIG_ROOT || path.join(__dirname, '../server/config'); const config = require('../server/runtimeConfig'); -const { planUsernameBackfill } = require('../server/usernameBackfill'); +const { + assertUsernameRolloutReady, + planUsernameBackfill, +} = require('../server/usernameBackfill'); +const { reconcilePostgresUsernames } = require('../server/usernamePostgresBackfill'); const args = new Set(process.argv.slice(2)); const supportedArgs = new Set(['--apply', '--create-index']); @@ -20,6 +24,17 @@ const duplicateNormalizedUsernames = (collection) => collection.aggregate([ { $limit: 1 }, ]).toArray(); +const readUsers = (collection) => collection.find({}, { + projection: { + _id: 1, + id: 1, + username: 1, + usernameNormalized: 1, + }, +}).toArray(); + +let postgresPool; + const run = async () => { if (unknownArg) { throw new Error(`Unknown argument: ${unknownArg}`); @@ -34,14 +49,7 @@ const run = async () => { mongoose.set('strictQuery', false); await mongoose.connect(config.mongodb, { autoIndex: false }); const users = mongoose.connection.collection('users'); - const documents = await users.find({}, { - projection: { - _id: 1, - id: 1, - username: 1, - usernameNormalized: 1, - }, - }).toArray(); + const documents = await readUsers(users); const plan = planUsernameBackfill(documents); console.log(JSON.stringify({ @@ -58,7 +66,35 @@ const run = async () => { console.log(`Updated ${result.modifiedCount} user records.`); } + const verifiedPlan = planUsernameBackfill(await readUsers(users)); + if (verifiedPlan.report.pendingChanges || verifiedPlan.report.privacyRemoved) { + throw new Error('MongoDB username backfill verification failed'); + } + + let postgres = { status: 'disabled' }; + if (config.postgres.enabled) { + // Avoid creating a PostgreSQL pool for dry runs or explicitly disabled mirrors. + // eslint-disable-next-line global-require + ({ pool: postgresPool } = require('../server/postgres')); + postgres = { + status: 'reconciled', + ...await reconcilePostgresUsernames({ + client: postgresPool, + users: verifiedPlan.postgresUsers, + }), + }; + } + + console.log(JSON.stringify({ + verification: { + mongoPendingChanges: verifiedPlan.report.pendingChanges, + mongoPrivateValuesRemaining: verifiedPlan.report.privacyRemoved, + postgres, + }, + }, null, 2)); + if (createIndex) { + assertUsernameRolloutReady(verifiedPlan.report); const duplicates = await duplicateNormalizedUsernames(users); if (duplicates.length) { throw new Error('Duplicate normalized usernames remain; unique index was not created'); @@ -74,4 +110,7 @@ const run = async () => { run().catch((err) => { console.error(err.message); process.exitCode = 1; -}).finally(() => mongoose.disconnect()); +}).finally(() => Promise.allSettled([ + mongoose.disconnect(), + ...(postgresPool ? [postgresPool.end()] : []), +])); diff --git a/server/auth/index.js b/server/auth/index.js index 525ce30..d099bed 100644 --- a/server/auth/index.js +++ b/server/auth/index.js @@ -4,6 +4,7 @@ const { URLSearchParams } = require('url'); const { User } = require('../models'); const metrics = require('../metrics'); const { buildWcaUserUpdate } = require('./wcaProfile'); +const { upsertTestUser } = require('./testUser'); const checkStatus = async (res) => { if (res.ok) { // res.status >= 200 && res.status < 300 @@ -32,19 +33,9 @@ module.exports = (app, passport) => { if (process.env.LETSCUBE_TEST_AUTH === 'true') { try { - const user = await User.findOneAndUpdate({ - id: +(process.env.LETSCUBE_TEST_USER_ID || 990001), - }, { - id: +(process.env.LETSCUBE_TEST_USER_ID || 990001), - name: 'Cypress Test User', - username: 'cypress', - wcaId: '2026TEST01', - accessToken: `test-token-${code}`, - avatar: {}, - }, { - upsert: true, - useFindAndModify: false, - new: true, + const user = await upsertTestUser(User, { + code, + userId: +(process.env.LETSCUBE_TEST_USER_ID || 990001), }); done(null, user.toObject()); diff --git a/server/auth/testUser.js b/server/auth/testUser.js new file mode 100644 index 0000000..2f4f65c --- /dev/null +++ b/server/auth/testUser.js @@ -0,0 +1,18 @@ +const { normalizeUsername } = require('../username'); + +const upsertTestUser = (User, { code, userId }) => User.findOneAndUpdate({ + id: userId, +}, { + id: userId, + name: 'Cypress Test User', + ...normalizeUsername('cypress'), + wcaId: '2026TEST01', + accessToken: `test-token-${code}`, + avatar: {}, +}, { + upsert: true, + useFindAndModify: false, + new: true, +}); + +module.exports = { upsertTestUser }; diff --git a/server/auth/testUser.test.js b/server/auth/testUser.test.js new file mode 100644 index 0000000..7eae187 --- /dev/null +++ b/server/auth/testUser.test.js @@ -0,0 +1,30 @@ +/** @jest-environment node */ +/* eslint-env jest */ + +const { upsertTestUser } = require('./testUser'); + +describe('Cypress test authentication user', () => { + it('writes the normalized username explicitly through findOneAndUpdate', async () => { + const saved = { id: 990001 }; + const User = { findOneAndUpdate: jest.fn().mockResolvedValue(saved) }; + + await expect(upsertTestUser(User, { + code: 'test-code', + userId: 990001, + })).resolves.toBe(saved); + + expect(User.findOneAndUpdate).toHaveBeenCalledWith( + { id: 990001 }, + expect.objectContaining({ + id: 990001, + username: 'cypress', + usernameNormalized: 'cypress', + }), + { + upsert: true, + useFindAndModify: false, + new: true, + }, + ); + }); +}); diff --git a/server/username.js b/server/username.js index 18b198b..5d58835 100644 --- a/server/username.js +++ b/server/username.js @@ -17,6 +17,10 @@ const invalidUsername = () => new UsernameError( 'Username may only contain letters, numbers, underscores, and hyphens', ); +const canonicalizeUsername = (value) => ( + typeof value === 'string' ? value.normalize('NFKC').trim() : value +); + const normalizeUsername = (value, { allowEmpty = true } = {}) => { if (value === undefined || value === null) { if (allowEmpty) { @@ -29,7 +33,7 @@ const normalizeUsername = (value, { allowEmpty = true } = {}) => { throw invalidUsername(); } - const username = value.normalize('NFKC').trim(); + const username = canonicalizeUsername(value); if (!username) { if (allowEmpty) { return { username: undefined, usernameNormalized: undefined }; @@ -109,6 +113,7 @@ module.exports = { USERNAME_SEARCH_DEFAULT_LIMIT, USERNAME_SEARCH_MAX_LIMIT, UsernameError, + canonicalizeUsername, findUserByUsername, isUsernameDuplicateKeyError, normalizeUsername, diff --git a/server/username.test.js b/server/username.test.js index 7c04f4b..9dcf95b 100644 --- a/server/username.test.js +++ b/server/username.test.js @@ -2,6 +2,7 @@ /* eslint-env jest */ const { + canonicalizeUsername, findUserByUsername, normalizeUsername, searchUsersByUsernamePrefix, @@ -16,6 +17,11 @@ describe('username normalization and lookup', () => { }); }); + it('canonicalizes compatibility characters before validation', () => { + expect(canonicalizeUsername(' cuber\uFF20example.com ')).toBe('cuber@example.com'); + expect(canonicalizeUsername('cuber\uFE6Bexample.com')).toBe('cuber@example.com'); + }); + it('represents empty optional usernames as absent fields', () => { expect(normalizeUsername(' ')).toEqual({ username: undefined, @@ -27,6 +33,8 @@ describe('username normalization and lookup', () => { 'cuber.name', 'cuber+name', 'cuber@example.com', + 'cuber\uFF20example.com', + 'cuber\uFE6Bexample.com', 'two cubers', '1234567890123456', ])('rejects invalid input without treating it as a lookup: %s', (username) => { diff --git a/server/usernameBackfill.js b/server/usernameBackfill.js index 5f11aaa..5ccdd16 100644 --- a/server/usernameBackfill.js +++ b/server/usernameBackfill.js @@ -1,8 +1,10 @@ -const { normalizeUsername } = require('./username'); +const { canonicalizeUsername, normalizeUsername } = require('./username'); const identifier = (user) => (user.id === undefined ? user._id.toString() : user.id); const hasOwn = (value, key) => Object.prototype.hasOwnProperty.call(value, key); -const containsEmailMarker = (value) => typeof value === 'string' && value.includes('@'); +const containsEmailMarker = (value) => ( + typeof value === 'string' && canonicalizeUsername(value).includes('@') +); const changedOperation = (user, set, unset) => { const changedSet = Object.entries(set).some(([key, value]) => user[key] !== value); @@ -50,36 +52,57 @@ const planUsernameBackfill = (users) => { [...grouped.entries()].filter(([, entries]) => entries.length > 1).map(([key]) => key), ); const operations = []; + const postgresUsers = []; const invalid = []; let empty = 0; + let privacyRemoved = 0; let valid = 0; analyzed.forEach((entry) => { const { user } = entry; - let operation; + let targetUsername; + let targetUsernameNormalized; if (entry.error) { invalid.push({ id: identifier(user), code: entry.error.code }); - operation = changedOperation(user, {}, { - ...(containsEmailMarker(user.username) ? { username: '' } : {}), - usernameNormalized: '', - }); + const removePrivateValue = containsEmailMarker(user.username); + if (removePrivateValue) { + privacyRemoved += 1; + } else { + targetUsername = user.username; + } } else if (!entry.usernameNormalized) { empty += 1; - operation = changedOperation(user, {}, { username: '', usernameNormalized: '' }); } else if (collisionKeys.has(entry.usernameNormalized)) { - operation = changedOperation(user, { username: entry.username }, { usernameNormalized: '' }); + targetUsername = entry.username; } else { valid += 1; - operation = changedOperation(user, { - username: entry.username, - usernameNormalized: entry.usernameNormalized, - }, {}); + targetUsername = entry.username; + targetUsernameNormalized = entry.usernameNormalized; } + const operation = changedOperation( + user, + { + ...(targetUsername === undefined ? {} : { username: targetUsername }), + ...(targetUsernameNormalized === undefined + ? {} : { usernameNormalized: targetUsernameNormalized }), + }, + { + ...(targetUsername === undefined ? { username: '' } : {}), + ...(targetUsernameNormalized === undefined ? { usernameNormalized: '' } : {}), + }, + ); + if (operation) { operations.push(operation); } + + postgresUsers.push({ + wcaUserId: user.id, + username: targetUsername, + usernameNormalized: targetUsernameNormalized, + }); }); const collisions = [...collisionKeys].sort().map((usernameNormalized) => ({ @@ -98,9 +121,25 @@ const planUsernameBackfill = (users) => { empty, invalid, collisions, + privacyRemoved, pendingChanges: operations.length, }, + postgresUsers, }; }; -module.exports = { planUsernameBackfill }; +const assertUsernameRolloutReady = (report) => { + if (report.pendingChanges) { + throw new Error('Username backfill verification still has pending changes'); + } + if (report.collisions.length) { + throw new Error( + `Cannot create username index: ${report.collisions.length} collision group(s) require resolution`, + ); + } +}; + +module.exports = { + assertUsernameRolloutReady, + planUsernameBackfill, +}; diff --git a/server/usernameBackfill.test.js b/server/usernameBackfill.test.js index 51b7b5c..67ff93e 100644 --- a/server/usernameBackfill.test.js +++ b/server/usernameBackfill.test.js @@ -1,7 +1,11 @@ /** @jest-environment node */ /* eslint-env jest */ -const { planUsernameBackfill } = require('./usernameBackfill'); +const { + assertUsernameRolloutReady, + planUsernameBackfill, +} = require('./usernameBackfill'); +const { normalizeUsername } = require('./username'); const applyOperations = (users, operations) => { operations.forEach(({ updateOne }) => { @@ -24,9 +28,13 @@ describe('normalized username backfill', () => { _id: '5', id: 5, username: 'legacy.name', usernameNormalized: 'legacy.name', }, { _id: '6', id: 6, username: 'private@example.com' }, + { _id: '7', id: 7, username: 'private\uFF20example.com' }, + { _id: '8', id: 8, username: 'private\uFE6Bexample.com' }, ]; - const { operations, report } = planUsernameBackfill(users); + const { + operations, postgresUsers, report, + } = planUsernameBackfill(users); applyOperations(users, operations); expect(users[0]).toMatchObject({ @@ -38,6 +46,8 @@ describe('normalized username backfill', () => { expect(users[4]).toMatchObject({ username: 'legacy.name' }); expect(users[4].usernameNormalized).toBeUndefined(); expect(users[5].username).toBeUndefined(); + expect(users[6].username).toBeUndefined(); + expect(users[7].username).toBeUndefined(); expect(report.collisions).toEqual([{ usernameNormalized: 'fastcuber', users: [ @@ -48,8 +58,17 @@ describe('normalized username backfill', () => { expect(report.invalid).toEqual([ { id: 5, code: 'INVALID_USERNAME' }, { id: 6, code: 'INVALID_USERNAME' }, + { id: 7, code: 'INVALID_USERNAME' }, + { id: 8, code: 'INVALID_USERNAME' }, ]); + expect(report.privacyRemoved).toBe(3); expect(JSON.stringify(report.invalid)).not.toContain('private@example.com'); + expect(postgresUsers.slice(5)).toEqual([ + { wcaUserId: 6, username: undefined, usernameNormalized: undefined }, + { wcaUserId: 7, username: undefined, usernameNormalized: undefined }, + { wcaUserId: 8, username: undefined, usernameNormalized: undefined }, + ]); + expect(JSON.stringify(postgresUsers.slice(5))).not.toContain('private'); }); it('is idempotent after applying the planned operations', () => { @@ -59,10 +78,37 @@ describe('normalized username backfill', () => { { _id: '3', id: 3, username: 'fastcuber' }, { _id: '4', id: 4, username: 'legacy.name' }, { _id: '5', id: 5, username: 'private@example.com' }, + { _id: '6', id: 6, username: 'private\uFF20example.com' }, + { _id: '7', id: 7, username: 'private\uFE6Bexample.com' }, ]; const first = planUsernameBackfill(users); applyOperations(users, first.operations); expect(planUsernameBackfill(users).operations).toEqual([]); }); + + it('fails index rollout while any valid collision remains unclaimed', () => { + const users = [ + { _id: '1', id: 1, username: 'FastCuber' }, + { _id: '2', id: 2, username: 'fastcuber' }, + ]; + const first = planUsernameBackfill(users); + applyOperations(users, first.operations); + const verified = planUsernameBackfill(users); + + expect(verified.report.pendingChanges).toBe(0); + expect(() => assertUsernameRolloutReady(verified.report)) + .toThrow('collision group(s) require resolution'); + expect(normalizeUsername('FASTCUBER').usernameNormalized) + .toBe(verified.report.collisions[0].usernameNormalized); + }); + + it('allows index rollout only after the verified plan is collision-free', () => { + const users = [{ + _id: '1', id: 1, username: 'SoloCuber', usernameNormalized: 'solocuber', + }]; + const { report } = planUsernameBackfill(users); + + expect(() => assertUsernameRolloutReady(report)).not.toThrow(); + }); }); diff --git a/server/usernamePostgresBackfill.js b/server/usernamePostgresBackfill.js new file mode 100644 index 0000000..79975a6 --- /dev/null +++ b/server/usernamePostgresBackfill.js @@ -0,0 +1,90 @@ +const DEFAULT_BATCH_SIZE = 500; + +const validUser = ({ wcaUserId }) => { + const id = Number(wcaUserId); + return Number.isSafeInteger(id) && id > 0; +}; + +const expectedValues = (users) => users.flatMap((user) => [ + Number(user.wcaUserId), + user.username || null, + user.usernameNormalized || null, +]); + +const expectedRows = (users) => users.map((user, index) => { + const offset = index * 3; + return `($${offset + 1}::bigint, $${offset + 2}::text, $${offset + 3}::text)`; +}).join(', '); + +const reconcileBatch = async (client, users) => { + const values = expectedValues(users); + const rows = expectedRows(users); + const updated = await client.query(` + WITH expected(wca_user_id, username, username_normalized) AS ( + VALUES ${rows} + ) + UPDATE app.users AS users + SET username = expected.username, + username_normalized = expected.username_normalized, + ingested_at = now() + FROM expected + WHERE users.wca_user_id = expected.wca_user_id + AND ROW(users.username, users.username_normalized) + IS DISTINCT FROM ROW(expected.username, expected.username_normalized) + `, values); + const verification = await client.query(` + WITH expected(wca_user_id, username, username_normalized) AS ( + VALUES ${rows} + ) + SELECT count(*)::int AS remaining + FROM app.users AS users + JOIN expected USING (wca_user_id) + WHERE ROW(users.username, users.username_normalized) + IS DISTINCT FROM ROW(expected.username, expected.username_normalized) + `, values); + + return { + modified: updated.rowCount, + remaining: verification.rows[0].remaining, + }; +}; + +const reconcilePostgresUsernames = async ({ + client, + users, + batchSize = DEFAULT_BATCH_SIZE, +}) => { + if (!Number.isInteger(batchSize) || batchSize <= 0) { + throw new Error('PostgreSQL username reconciliation requires a positive batch size'); + } + const eligibleUsers = users.filter(validUser); + if (eligibleUsers.length !== users.length) { + throw new Error('PostgreSQL username reconciliation requires a valid WCA user ID'); + } + const batches = Array.from( + { length: Math.ceil(eligibleUsers.length / batchSize) }, + (unused, index) => eligibleUsers.slice(index * batchSize, (index + 1) * batchSize), + ); + + const summary = await batches.reduce(async (previous, batch) => { + const accumulated = await previous; + const result = await reconcileBatch(client, batch); + return { + modified: accumulated.modified + result.modified, + remaining: accumulated.remaining + result.remaining, + }; + }, Promise.resolve({ modified: 0, remaining: 0 })); + + if (summary.remaining) { + throw new Error('PostgreSQL username reconciliation verification failed'); + } + + return { + considered: eligibleUsers.length, + ...summary, + }; +}; + +module.exports = { + reconcilePostgresUsernames, +}; diff --git a/server/usernamePostgresBackfill.test.js b/server/usernamePostgresBackfill.test.js new file mode 100644 index 0000000..9bfd667 --- /dev/null +++ b/server/usernamePostgresBackfill.test.js @@ -0,0 +1,66 @@ +/** @jest-environment node */ +/* eslint-env jest */ + +const { reconcilePostgresUsernames } = require('./usernamePostgresBackfill'); + +describe('PostgreSQL username reconciliation', () => { + it('reconciles canonical targets by WCA user ID without raw-value matching', async () => { + const client = { + query: jest.fn() + .mockResolvedValueOnce({ rowCount: 2 }) + .mockResolvedValueOnce({ rows: [{ remaining: 0 }] }), + }; + const users = [ + { wcaUserId: 1, username: 'Cuber', usernameNormalized: 'cuber' }, + { wcaUserId: 2, username: undefined, usernameNormalized: undefined }, + ]; + + await expect(reconcilePostgresUsernames({ client, users })).resolves.toEqual({ + considered: 2, + modified: 2, + remaining: 0, + }); + + expect(client.query).toHaveBeenCalledTimes(2); + expect(client.query.mock.calls[0][0]).toContain('wca_user_id'); + expect(client.query.mock.calls[0][0]).not.toContain('LIKE'); + expect(client.query.mock.calls[0][1]).toEqual([1, 'Cuber', 'cuber', 2, null, null]); + expect(client.query.mock.calls[1][0]).toContain('IS DISTINCT FROM'); + }); + + it('is idempotent when PostgreSQL already matches MongoDB targets', async () => { + const client = { + query: jest.fn() + .mockResolvedValueOnce({ rowCount: 0 }) + .mockResolvedValueOnce({ rows: [{ remaining: 0 }] }), + }; + + await expect(reconcilePostgresUsernames({ + client, + users: [{ wcaUserId: 1, username: 'Cuber', usernameNormalized: 'cuber' }], + })).resolves.toMatchObject({ modified: 0, remaining: 0 }); + }); + + it('fails closed when exact post-update verification finds a mismatch', async () => { + const client = { + query: jest.fn() + .mockResolvedValueOnce({ rowCount: 0 }) + .mockResolvedValueOnce({ rows: [{ remaining: 1 }] }), + }; + + await expect(reconcilePostgresUsernames({ + client, + users: [{ wcaUserId: 2, username: undefined, usernameNormalized: undefined }], + })).rejects.toThrow('PostgreSQL username reconciliation verification failed'); + }); + + it('fails closed instead of skipping a target without a WCA user ID', async () => { + const client = { query: jest.fn() }; + + await expect(reconcilePostgresUsernames({ + client, + users: [{ wcaUserId: undefined, username: 'orphan', usernameNormalized: 'orphan' }], + })).rejects.toThrow('requires a valid WCA user ID'); + expect(client.query).not.toHaveBeenCalled(); + }); +});