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..0e7ea66 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 @@ -91,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/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/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/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/docs/username-migration.md b/docs/username-migration.md new file mode 100644 index 0000000..396c3b3 --- /dev/null +++ b/docs/username-migration.md @@ -0,0 +1,70 @@ +# 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 (`@`) 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 / 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 + `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 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 + ``` + + 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 username writes or the discovery endpoint from #82 until this +sequence and the #191 purge have completed successfully. + +## 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..0a2edae --- /dev/null +++ b/scripts/backfill-normalized-usernames.js @@ -0,0 +1,116 @@ +#!/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 { + 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']); +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 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}`); + } + + 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 readUsers(users); + 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.`); + } + + 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'); + } + 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(() => Promise.allSettled([ + mongoose.disconnect(), + ...(postgresPool ? [postgresPool.end()] : []), +])); 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/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/auth/index.js b/server/auth/index.js index 91219c9..d099bed 100644 --- a/server/auth/index.js +++ b/server/auth/index.js @@ -3,6 +3,8 @@ const CustomStrategy = require('passport-custom').Strategy; 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 @@ -31,20 +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', - email: 'cypress@example.com', - 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()); @@ -89,14 +80,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/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/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/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/models/user.js b/server/models/user.js index eb01155..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) { @@ -20,9 +22,6 @@ const schema = new mongoose.Schema({ type: Number, required: true, }, - email: { - type: String, - }, name: { type: String, required: true, @@ -30,6 +29,9 @@ const schema = new mongoose.Schema({ username: { type: String, }, + usernameNormalized: { + type: String, + }, wcaId: { type: String, }, @@ -71,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 new file mode 100644 index 0000000..cd5fd05 --- /dev/null +++ b/server/models/user.test.js @@ -0,0 +1,70 @@ +/** @jest-environment node */ +/* eslint-env jest */ + +jest.mock('../postgres/dualWrite', () => ({ mirrorUser: jest.fn() })); + +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'); + }); +}); + +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 fed3219..87bbe1d 100644 --- a/server/package.json +++ b/server/package.json @@ -38,6 +38,8 @@ "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", + "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 7fea99b..02ce1bf 100644 --- a/server/postgres/dualWrite.js +++ b/server/postgres/dualWrite.js @@ -58,15 +58,22 @@ 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, email, name, username, wca_id, preferences, avatar, - source_created_at, source_updated_at + 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 = EXCLUDED.email, + email = NULL, name = EXCLUDED.name, username = EXCLUDED.username, + username_normalized = EXCLUDED.username_normalized, wca_id = EXCLUDED.wca_id, preferences = EXCLUDED.preferences, avatar = EXCLUDED.avatar, @@ -76,16 +83,16 @@ 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.username_normalized, app.users.wca_id, app.users.preferences, app.users.avatar ) IS DISTINCT FROM ROW( - EXCLUDED.email, EXCLUDED.name, EXCLUDED.username, + EXCLUDED.username_normalized, EXCLUDED.wca_id, EXCLUDED.preferences, EXCLUDED.avatar @@ -94,9 +101,9 @@ const upsertUser = async (client, user, fallbackUpdatedAt = new Date()) => { `, [ id, wcaUserId, - user.email || null, 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 39503fe..e296886 100644 --- a/server/postgres/dualWrite.test.js +++ b/server/postgres/dualWrite.test.js @@ -18,9 +18,10 @@ const { const user = { id: 1234, - email: 'solver@example.com', + email: 'private@example.com', name: 'Test Solver', username: 'solver', + usernameNormalized: 'solver', wcaId: '2026TEST01', accessToken: 'must-not-be-mirrored', showWCAID: true, @@ -46,14 +47,22 @@ 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(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(values).toContain('solver@example.com'); + 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 25c9a74..9cbb555 100644 --- a/server/prisma/schema.prisma +++ b/server/prisma/schema.prisma @@ -6,9 +6,11 @@ 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? + 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/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'); + }); +}); diff --git a/server/username.js b/server/username.js new file mode 100644 index 0000000..5d58835 --- /dev/null +++ b/server/username.js @@ -0,0 +1,122 @@ +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 canonicalizeUsername = (value) => ( + typeof value === 'string' ? value.normalize('NFKC').trim() : value +); + +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 = canonicalizeUsername(value); + 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, + canonicalizeUsername, + findUserByUsername, + isUsernameDuplicateKeyError, + normalizeUsername, + searchUsersByUsernamePrefix, + updateUsername, +}; diff --git a/server/username.test.js b/server/username.test.js new file mode 100644 index 0000000..9dcf95b --- /dev/null +++ b/server/username.test.js @@ -0,0 +1,133 @@ +/** @jest-environment node */ +/* eslint-env jest */ + +const { + canonicalizeUsername, + 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('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, + usernameNormalized: undefined, + }); + }); + + it.each([ + '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) => { + 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..5ccdd16 --- /dev/null +++ b/server/usernameBackfill.js @@ -0,0 +1,145 @@ +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' && canonicalizeUsername(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 postgresUsers = []; + const invalid = []; + let empty = 0; + let privacyRemoved = 0; + let valid = 0; + + analyzed.forEach((entry) => { + const { user } = entry; + let targetUsername; + let targetUsernameNormalized; + + if (entry.error) { + invalid.push({ id: identifier(user), code: entry.error.code }); + const removePrivateValue = containsEmailMarker(user.username); + if (removePrivateValue) { + privacyRemoved += 1; + } else { + targetUsername = user.username; + } + } else if (!entry.usernameNormalized) { + empty += 1; + } else if (collisionKeys.has(entry.usernameNormalized)) { + targetUsername = entry.username; + } else { + valid += 1; + 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) => ({ + usernameNormalized, + users: grouped.get(usernameNormalized).map(({ user, username }) => ({ + id: identifier(user), + username, + })), + })); + + return { + operations, + report: { + scanned: users.length, + valid, + empty, + invalid, + collisions, + privacyRemoved, + pendingChanges: operations.length, + }, + postgresUsers, + }; +}; + +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 new file mode 100644 index 0000000..67ff93e --- /dev/null +++ b/server/usernameBackfill.test.js @@ -0,0 +1,114 @@ +/** @jest-environment node */ +/* eslint-env jest */ + +const { + assertUsernameRolloutReady, + planUsernameBackfill, +} = require('./usernameBackfill'); +const { normalizeUsername } = require('./username'); + +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' }, + { _id: '7', id: 7, username: 'private\uFF20example.com' }, + { _id: '8', id: 8, username: 'private\uFE6Bexample.com' }, + ]; + + const { + operations, postgresUsers, 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(users[6].username).toBeUndefined(); + expect(users[7].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' }, + { 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', () => { + 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' }, + { _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(); + }); +});