diff --git a/src/lib/server/identity/auth.test.ts b/src/lib/server/identity/auth.test.ts index 752fd4a..3817898 100644 --- a/src/lib/server/identity/auth.test.ts +++ b/src/lib/server/identity/auth.test.ts @@ -1,7 +1,18 @@ import { readFileSync } from 'node:fs'; import { describe, expect, it, onTestFinished, vi } from 'vitest'; +import { createInstance } from '../instance.ts'; import { createFileTestDb } from '../test-db.ts'; -import { type Admission, createAuth, isPlaceholderSecret, MIN_PASSWORD_LENGTH } from './auth.ts'; +import { + type Admission, + CLIENT_IP_HEADER, + createAuth, + isNameTooLong, + isPlaceholderSecret, + MAX_NAME_LENGTH, + MIN_PASSWORD_LENGTH +} from './auth.ts'; +import { createRegistration } from './registration.ts'; +import { createFirstAdmin } from './setup.ts'; const secret = 'test-secret-test-secret-test-secret-0123'; const body = { email: 'ada@example.com', password: 'correct horse battery', name: 'Ada' }; @@ -313,3 +324,219 @@ describe('createAuth', () => { } }); }); + +describe('the name cap', () => { + const NAME_TOO_LONG = 'The name is too long.'; + // Better Auth's limiter keeps its counters for the whole process (20 sign-ups per address per ten + // minutes), so each request here comes from an address of its own. + let caller = 0; + const signUp = (payload: Record) => { + caller += 1; + return new Request('http://localhost:3000/api/auth/sign-up/email', { + method: 'POST', + headers: { + 'content-type': 'application/json', + origin: 'http://localhost:3000', + [CLIENT_IP_HEADER]: `10.${caller >> 8}.${caller & 255}.1` + }, + body: JSON.stringify(payload) + }); + }; + const emoji = '\u{1F600}'; // one code point, two UTF-16 units + + describe('isNameTooLong counts Unicode code points on the trimmed name', () => { + it('is 100', () => { + expect(MAX_NAME_LENGTH).toBe(100); + }); + + it.each([ + ['', false], + ['Ada', false], + ['a'.repeat(100), false], + ['a'.repeat(101), true], + // Two UTF-16 units each, one code point each: 100 fit, 101 do not. + [emoji.repeat(100), false], + [emoji.repeat(101), true], + [`${'a'.repeat(99)}${emoji}`, false], + [`${'a'.repeat(100)}${emoji}`, true], + // Lone surrogates are one code point per unit. + ['\ud800'.repeat(100), false], + ['\ud800'.repeat(101), true], + // Trimmed first, with the same trim the forms use (Unicode whitespace included). + [` ${'a'.repeat(100)} `, false], + [`\u00a0\u3000${'a'.repeat(100)}\ufeff\n`, false], + [`${' '.repeat(5000)}x${' '.repeat(5000)}`, false], + [`${'a'.repeat(50)} ${'b'.repeat(50)} c`, true], + ['x'.repeat(512 * 1024), true] + ])('%j.length=%#', (name, tooLong) => { + expect(isNameTooLong(name)).toBe(tooLong); + }); + }); + + async function http(auth: Awaited>['auth'], name: string) { + return auth.handler(signUp({ ...body, name })); + } + + it('refuses a direct POST /api/auth/sign-up/email with a longer name, with a fixed error, storing nothing', async () => { + const { auth, db } = await setup(); + + const res = await http(auth, 'x'.repeat(MAX_NAME_LENGTH + 1)); + + expect(res.status).toBe(400); + expect(await res.json()).toMatchObject({ code: 'NAME_TOO_LONG', message: NAME_TOO_LONG }); + expect(await db.user.count()).toBe(0); + expect(await db.account.count()).toBe(0); + expect(await db.session.count()).toBe(0); + expect(await db.learner.count()).toBe(0); + }); + + it('refuses the server-side call too, and any size of name, without echoing it', async () => { + const { auth, db } = await setup(); + const huge = `${'x'.repeat(512 * 1024)}@secret-marker`; + + const refusal = await auth.api.signUpEmail({ body: { ...body, name: huge } }).then( + () => null, + (error: unknown) => error + ); + + expect(refusal).toMatchObject({ statusCode: 400, body: { code: 'NAME_TOO_LONG' } }); + expect(JSON.stringify((refusal as { body: unknown }).body)).not.toContain('secret-marker'); + expect(await db.user.count()).toBe(0); + }); + + it.each([ + ['100 characters', 'a'.repeat(100)], + ['100 emoji', emoji.repeat(100)], + ['100 characters with spaces around them', ` ${'a'.repeat(100)} `] + ])('accepts %s, and stores the trimmed name', async (_label, name) => { + const { auth, db } = await setup(); + + const res = await http(auth, name); + + expect(res.status).toBe(200); + expect((await db.user.findFirstOrThrow()).name).toBe(name.trim()); + }); + + it('refuses 101 emoji', async () => { + const { auth, db } = await setup(); + + const res = await http(auth, emoji.repeat(101)); + + expect(res.status).toBe(400); + expect(await db.user.count()).toBe(0); + }); + + it('cannot be got round with padding: a name that is short once trimmed is stored trimmed', async () => { + const { auth, db } = await setup(); + + const res = await http(auth, `${' '.repeat(300_000)}Ada${' '.repeat(300_000)}`); + + expect(res.status).toBe(200); + expect((await db.user.findFirstOrThrow()).name).toBe('Ada'); + }); + + it('answers a closed sign-up as closed, whatever the name: a too-long name is not told apart', async () => { + const { auth, db } = await setup(() => null); + + const long = await http(auth, 'x'.repeat(1000)); + const short = await http(auth, 'Ada'); + + expect(long.status).toBe(403); + expect(short.status).toBe(403); + expect(await db.user.count()).toBe(0); + }); + + it('makes no Learner for a refused name, not even the named Local one', async () => { + const { auth, db } = await setup(() => ({ learnerId: 'local', role: 'admin' })); + + const res = await http(auth, 'x'.repeat(1000)); + + expect(res.status).toBe(400); + expect(await db.learner.count()).toBe(0); + expect(await db.user.count()).toBe(0); + }); + + it('answers a long name the same for an email that has an Account and one that has not: no probe', async () => { + const { auth, db } = await setup(); + await auth.api.signUpEmail({ body }); + const before = await db.user.count(); + const long = (email: string) => + auth.handler(signUp({ ...body, email, name: 'x'.repeat(MAX_NAME_LENGTH + 1) })); + + const known = await long(body.email); + const unknown = await long('nobody@example.com'); + + expect(known.status).toBe(400); + expect(unknown.status).toBe(400); + expect(await known.json()).toEqual(await unknown.json()); + expect(await db.user.count()).toBe(before); + }); + + it('still answers a closed sign-up first: 403 for a long name, a blank one, a known email or not', async () => { + const { auth, db } = await setup(() => null); + const post = (email: string, name: string) => auth.handler(signUp({ ...body, email, name })); + + for (const name of ['x'.repeat(1000), ' ', '']) { + expect((await post(body.email, name)).status, JSON.stringify(name.slice(0, 5))).toBe(403); + } + expect(await db.user.count()).toBe(0); + }); + + describe('a blank name', () => { + it.each([ + ['empty', ''], + ['spaces', ' '], + ['Unicode whitespace', '\u00a0\u3000\ufeff\n\t'] + ])('is refused over HTTP, %s, with a fixed error, storing nothing', async (_label, name) => { + const { auth, db } = await setup(); + + const res = await auth.handler(signUp({ ...body, name })); + + expect(res.status).toBe(400); + expect(await res.json()).toMatchObject({ code: 'NAME_REQUIRED', message: 'Enter a name.' }); + expect(await db.user.count()).toBe(0); + expect(await db.learner.count()).toBe(0); + }); + + it('is refused by the server-side call too, storing nothing', async () => { + const { auth, db } = await setup(); + + await expect(auth.api.signUpEmail({ body: { ...body, name: ' ' } })).rejects.toMatchObject({ + statusCode: 400, + body: { code: 'NAME_REQUIRED' } + }); + expect(await db.user.count()).toBe(0); + }); + + it('is refused for a known email the same as for an unknown one: no probe', async () => { + const { auth } = await setup(); + await auth.api.signUpEmail({ body }); + const blank = (email: string) => auth.handler(signUp({ ...body, email, name: ' ' })); + + const known = await blank(body.email); + const unknown = await blank('nobody@example.com'); + + expect(known.status).toBe(400); + expect(await known.json()).toEqual(await unknown.json()); + }); + }); + + it('refuses the first admin with a longer name, and leaves the Instance not set up', async () => { + const db = await createFileTestDb(); + const instance = await createInstance(db); + const auth = createAuth({ + db, + origin: 'http://localhost:3000', + secret, + admitSignUp: createRegistration({ instance }) + }); + + await expect( + createFirstAdmin({ instance, auth, db }, { ...body, name: 'x'.repeat(101) }) + ).rejects.toMatchObject({ body: { code: 'NAME_TOO_LONG' } }); + + expect(instance.mode()).toBeNull(); + expect(await db.user.count()).toBe(0); + expect(await db.learner.count()).toBe(0); + }); +}); diff --git a/src/lib/server/identity/auth.ts b/src/lib/server/identity/auth.ts index 4d10d40..4fc981a 100644 --- a/src/lib/server/identity/auth.ts +++ b/src/lib/server/identity/auth.ts @@ -43,6 +43,22 @@ export function isPlaceholderSecret(secret: string): boolean { const value = secret.trim().toLowerCase(); return value === BETTER_AUTH_DEFAULT_SECRET || value.startsWith('change-me'); } + +/** The longest name an Account may have: 100 characters, counted in Unicode code points on the trimmed name. */ +export const MAX_NAME_LENGTH = 100; + +/** + * Whether a name is over the cap. The name is trimmed first (the forms store it trimmed), then counted by + * code point, so an emoji is one character and not its two UTF-16 units. Cheap on any size of input: a code + * point is one or two units, so up to 100 units is always within the cap and over 200 never is. + */ +export function isNameTooLong(name: string): boolean { + const trimmed = name.trim(); + if (trimmed.length <= MAX_NAME_LENGTH) return false; + if (trimmed.length > 2 * MAX_NAME_LENGTH) return true; + return [...trimmed].length > MAX_NAME_LENGTH; +} + export const MIN_PASSWORD_LENGTH = 12; export const MAX_PASSWORD_LENGTH = 128; @@ -102,6 +118,21 @@ const HTTP_SIGN_IN_PATHS = ['/sign-in/email']; const signUpClosed = () => new APIError('FORBIDDEN', { message: 'Sign-up is closed' }); +// Fixed messages and codes: the forms map the code to their own text, and the name is never echoed. +const nameTooLong = () => + new APIError('BAD_REQUEST', { message: 'The name is too long.', code: 'NAME_TOO_LONG' }); +const nameRequired = () => + new APIError('BAD_REQUEST', { message: 'Enter a name.', code: 'NAME_REQUIRED' }); + +/** + * The refusal for a name, or null if it is fine: blank once trimmed, or over the cap. Which characters a + * name may hold is another question (a product rule) and is not asked here. + */ +function nameRefusal(name: string): APIError | null { + if (name.trim() === '') return nameRequired(); + return isNameTooLong(name) ? nameTooLong() : null; +} + /** Better Auth over our database: email + password and the admin plugin, nothing else. */ export function createAuth({ db, @@ -168,6 +199,12 @@ export function createAuth({ before: async (user, ctx) => { const admission = ctx && (await admitSignUp({ email: user.email })); if (!admission) throw signUpClosed(); + // After the admission, so a closed sign-up answers the same whatever the name; before the + // Learner below, so a refused name leaves nothing behind. Whatever path made the Account + // (the form, POST /sign-up/email, a later one) comes through here. For a sign-up over HTTP + // the request hook below has already asked, so this is the backstop. + const refusal = nameRefusal(user.name); + if (refusal) throw refusal; // A named Learner (the Local one) already exists or is made here and is never removed. // A fresh one is only named here and made in `after`, once the Account row is in, so a // failed insert (a duplicate email, a concurrent sign-up) leaves no Learner behind. @@ -182,6 +219,9 @@ export function createAuth({ return { data: { ...user, + // Stored trimmed, as the forms send it: the cap counts the trimmed name, so padding + // must not be kept either. + name: user.name.trim(), learnerId, ...(admission.role ? { role: admission.role } : {}) } @@ -200,11 +240,16 @@ export function createAuth({ } }, // Asked first, so a closed sign-up answers 403 whether or not the email exists (otherwise the - // duplicate-email 422 would reveal which emails have Accounts). The database hook is the backstop. + // duplicate-email 422 would reveal which emails have Accounts). The name is checked next, for the same + // reason: Better Auth looks the email up before the database hook runs, so a name refused only there + // would answer 400 for an email with no Account and 422 for one that has. The database hook is the backstop. hooks: { before: createAuthMiddleware(async (ctx) => { if (ctx.path !== '/sign-up/email') return; if (!(await admitSignUp({ email: ctx.body?.email }))) throw signUpClosed(); + const name = ctx.body?.name; + const refusal = typeof name === 'string' ? nameRefusal(name) : null; + if (refusal) throw refusal; }) }, disabledPaths: [...UNUSED_ADMIN_PATHS, ...UNUSED_CORE_PATHS, ...HTTP_SIGN_IN_PATHS], diff --git a/src/routes/setup/+page.server.ts b/src/routes/setup/+page.server.ts index df67d6b..f04bc8d 100644 --- a/src/routes/setup/+page.server.ts +++ b/src/routes/setup/+page.server.ts @@ -1,5 +1,7 @@ import { fail, redirect } from '@sveltejs/kit'; +import { isAPIError } from 'better-auth/api'; import type { MultiUserChoice } from '$lib/server/app'; +import { isNameTooLong, MAX_NAME_LENGTH } from '$lib/server/identity/auth'; import { forwardSetCookies } from '$lib/server/identity/auth-handler'; import { InvalidFirstAdminError, MultiUserNotAvailableError } from '$lib/server/identity/setup'; import { InstanceAlreadySetUpError } from '$lib/server/instance'; @@ -19,6 +21,7 @@ const FIELD_MESSAGES = { email: 'Enter a valid email address.', password: 'Use 12 to 128 characters.' } as const; +const NAME_TOO_LONG = `Use at most ${MAX_NAME_LENGTH} characters.`; const ALREADY_SET_UP = 'This instance has already been set up.'; const text = (data: FormData, key: string) => { const value = data.get(key); @@ -72,10 +75,21 @@ export const actions: Actions = { if (locals.instance.mode() !== null) { return fail(409, adminStep(typed, { message: ALREADY_SET_UP })); } + // Before the Instance is claimed, so a name over the cap leaves nothing to undo. Better Auth's hook + // refuses it too (`NAME_TOO_LONG`), which is what the catch below maps if one ever gets this far. + if (isNameTooLong(input.name)) { + return fail(400, adminStep(typed, { field: 'name', message: NAME_TOO_LONG })); + } let headers: Headers; try { ({ headers } = await locals.setup.createFirstAdmin(input)); } catch (error) { + if (isAPIError(error) && error.body?.code === 'NAME_TOO_LONG') { + return fail(400, adminStep(typed, { field: 'name', message: NAME_TOO_LONG })); + } + if (isAPIError(error) && error.body?.code === 'NAME_REQUIRED') { + return fail(400, adminStep(typed, { field: 'name', message: FIELD_MESSAGES.name })); + } if (error instanceof InvalidFirstAdminError) { return fail( 400, diff --git a/src/routes/setup/setup-action.test.ts b/src/routes/setup/setup-action.test.ts index b03c376..449ce95 100644 --- a/src/routes/setup/setup-action.test.ts +++ b/src/routes/setup/setup-action.test.ts @@ -1,4 +1,5 @@ import { isRedirect } from '@sveltejs/kit'; +import { APIError } from 'better-auth/api'; import { afterEach, describe, expect, it, vi } from 'vitest'; import type { MultiUserChoice } from '$lib/server/app'; import { InvalidFirstAdminError, MultiUserNotAvailableError } from '$lib/server/identity/setup'; @@ -187,6 +188,94 @@ describe('step 2: create the admin', () => { } ); + describe('the name cap: 100 characters, counted in code points on the trimmed name', () => { + const emoji = '\u{1F600}'; + const open = (createFirstAdmin = vi.fn(async () => ({ headers: new Headers() }))) => ({ + createFirstAdmin, + locals: { setup: setupWith({ kind: 'open' }, createFirstAdmin) } + }); + + it('refuses a longer name on its field before calling createFirstAdmin, keeping what was typed', async () => { + const { createFirstAdmin, locals } = open(); + const name = 'x'.repeat(101); + + const result = await run(actions.admin, { ...admin, name }, locals); + + expect(result).toMatchObject({ + status: 400, + data: { + step: 'admin', + field: 'name', + message: 'Use at most 100 characters.', + name, + email: admin.email + } + }); + expect(JSON.stringify(result)).not.toContain(admin.password); + expect(createFirstAdmin).not.toHaveBeenCalled(); + }); + + it.each([ + ['exactly 100 characters', 'x'.repeat(100), true], + ['101 characters', 'x'.repeat(101), false], + ['100 emoji, 200 UTF-16 units', emoji.repeat(100), true], + ['101 emoji', emoji.repeat(101), false], + ['100 characters with spaces around them', ` ${'x'.repeat(100)} `, true], + ['101 characters with spaces around them', ` ${'x'.repeat(101)} `, false], + ['half a megabyte', 'x'.repeat(512 * 1024), false] + ])('%s: %s', async (_label, name, accepted) => { + const { createFirstAdmin, locals } = open(); + + await run(actions.admin, { ...admin, name }, locals); + + expect(createFirstAdmin).toHaveBeenCalledTimes(accepted ? 1 : 0); + }); + + it('says the Instance is already set up before it says anything about the name', async () => { + const { createFirstAdmin, locals } = open(); + + const result = await run( + actions.admin, + { ...admin, name: 'x'.repeat(101) }, + { ...locals, instance: { mode: () => 'local' } } + ); + + expect(result).toMatchObject({ status: 409 }); + expect(createFirstAdmin).not.toHaveBeenCalled(); + }); + + it('maps Better Auth\u2019s own refusal of a blank name, should one get through, to the form\u2019s blank-name error', async () => { + const refusal = new APIError('BAD_REQUEST', { + message: 'Enter a name.', + code: 'NAME_REQUIRED' + }); + const { locals } = open(vi.fn(async () => Promise.reject(refusal))); + + const result = await run(actions.admin, admin, locals); + + expect(result).toMatchObject({ + status: 400, + data: { field: 'name', message: 'Enter a name.' } + }); + }); + + it('maps Better Auth\u2019s own refusal of a long name, should one get through, to the same field error', async () => { + const refusal = new APIError('BAD_REQUEST', { + message: 'The name is too long.', + code: 'NAME_TOO_LONG' + }); + const { locals } = open(vi.fn(async () => Promise.reject(refusal))); + vi.spyOn(console, 'error').mockImplementation(() => {}); + + const result = await run(actions.admin, admin, locals); + + expect(result).toMatchObject({ + status: 400, + data: { field: 'name', message: 'Use at most 100 characters.' } + }); + }); + }); + it('refuses where Multi-user is not open, saying what to add to .env, without calling createFirstAdmin', async () => { const createFirstAdmin = vi.fn(); const message = 'Add BETTER_AUTH_SECRET to your .env.'; diff --git a/src/routes/signup/seed-open-instance.ts b/src/routes/signup/seed-open-instance.ts index da6e3ea..3b221f3 100644 --- a/src/routes/signup/seed-open-instance.ts +++ b/src/routes/signup/seed-open-instance.ts @@ -1,7 +1,8 @@ // Test-only fixture, run with `bun` by signup.multi.e2e.ts: makes a migrated database a Multi-user // Instance whose Registration Mode is already Open, so that a server started on it offers Sign up. -// Nothing in the app imports this file, and the app has no code path that opens registration: this -// writes the rows a finished setup and the Users toggle (C34b) would leave behind. +// Nothing in the app imports this file. The app can open registration itself (the admin's toggle on the +// Users page, C34b), but only for a signed-in admin of a running Instance: this writes the rows a finished +// setup and that toggle would leave behind, so a test can start a server that is already open. import { createDb } from '../../lib/server/db.ts'; const url = process.argv[2]; diff --git a/src/routes/signup/sign-up.test.ts b/src/routes/signup/sign-up.test.ts index c8e4425..7273ba6 100644 --- a/src/routes/signup/sign-up.test.ts +++ b/src/routes/signup/sign-up.test.ts @@ -188,6 +188,100 @@ describe('what is wrong with the form', () => { } ); + describe('the name cap: 100 characters, counted in code points on the trimmed name', () => { + const emoji = '\u{1F600}'; + const refusal = (values: Form) => harness().post(values); + + it('refuses a longer name on its field before calling Better Auth, keeping what was typed', async () => { + const { post, signUpEmail } = harness(); + const name = 'x'.repeat(101); + + const result = await post({ ...ada, name }); + + expect(result).toMatchObject({ + status: 400, + data: { field: 'name', message: 'Use at most 100 characters.', name, email: ada.email } + }); + expect(JSON.stringify(result)).not.toContain(ada.password); + expect(signUpEmail).not.toHaveBeenCalled(); + }); + + it.each([ + ['exactly 100 characters', 'x'.repeat(100), true], + ['101 characters', 'x'.repeat(101), false], + ['100 emoji, 200 UTF-16 units', emoji.repeat(100), true], + ['101 emoji', emoji.repeat(101), false], + ['100 characters with spaces around them', ` ${'x'.repeat(100)} `, true], + ['101 characters with spaces around them', ` ${'x'.repeat(101)} `, false], + ['half a megabyte', 'x'.repeat(512 * 1024), false] + ])('%s: %s', async (_label, name, accepted) => { + const { post, signUpEmail } = harness(); + + await post({ ...ada, name }); + + expect(signUpEmail).toHaveBeenCalledTimes(accepted ? 1 : 0); + if (accepted) + expect(signUpEmail.mock.calls[0][0]).toMatchObject({ body: { name: name.trim() } }); + }); + + it('answers a blank name before a long one, and a long name before a bad email or password', async () => { + expect(await refusal({ ...ada, name: ' ' })).toMatchObject({ + data: { field: 'name', message: 'Enter a name.' } + }); + expect( + await refusal({ ...ada, name: 'x'.repeat(101), email: 'nope', password: 'short' }) + ).toMatchObject({ data: { field: 'name', message: 'Use at most 100 characters.' } }); + }); + + it('counts the refused attempt against the limit like any other', async () => { + const throttled = harness({ throttle: createThrottle({ addressLimit: 2 }) }); + await throttled.post({ ...ada, name: 'x'.repeat(101) }); + await throttled.post({ ...ada, name: 'x'.repeat(101) }); + + expect(await throttled.post(ada)).toMatchObject({ status: 429 }); + }); + + it('maps Better Auth\u2019s own refusal of a blank name to the form\u2019s blank-name error', async () => { + const raced = harness({ + auth: { + api: { + signUpEmail: async () => { + throw new APIError('BAD_REQUEST', { + message: 'Enter a name.', + code: 'NAME_REQUIRED' + }); + } + } + } + }); + + expect(await raced.post(ada)).toMatchObject({ + status: 400, + data: { field: 'name', message: 'Enter a name.' } + }); + }); + + it('maps Better Auth\u2019s own refusal of a long name to the same field error', async () => { + const raced = harness({ + auth: { + api: { + signUpEmail: async () => { + throw new APIError('BAD_REQUEST', { + message: 'The name is too long.', + code: 'NAME_TOO_LONG' + }); + } + } + } + }); + + expect(await raced.post(ada)).toMatchObject({ + status: 400, + data: { field: 'name', message: 'Use at most 100 characters.' } + }); + }); + }); + it('accepts a password of exactly 12 and exactly 128 characters', async () => { for (const password of ['x'.repeat(12), 'x'.repeat(128)]) { const { post, signUpEmail } = harness(); diff --git a/src/routes/signup/sign-up.ts b/src/routes/signup/sign-up.ts index 898232b..2c34e4a 100644 --- a/src/routes/signup/sign-up.ts +++ b/src/routes/signup/sign-up.ts @@ -1,13 +1,19 @@ import { fail, type RequestEvent, redirect } from '@sveltejs/kit'; import { isAPIError } from 'better-auth/api'; import { z } from 'zod'; -import { MAX_PASSWORD_LENGTH, MIN_PASSWORD_LENGTH } from '$lib/server/identity/auth'; +import { + isNameTooLong, + MAX_NAME_LENGTH, + MAX_PASSWORD_LENGTH, + MIN_PASSWORD_LENGTH +} from '$lib/server/identity/auth'; import { forwardSetCookies } from '$lib/server/identity/auth-handler'; import type { Throttle } from '$lib/server/identity/throttle'; /** Fixed texts only: an error's own message (Prisma's carry paths and query text) never reaches the form. */ const MESSAGES = { name: 'Enter a name.', + nameLong: `Use at most ${MAX_NAME_LENGTH} characters.`, email: 'Enter an email address like name@example.com.', passwordShort: `Use at least ${MIN_PASSWORD_LENGTH} characters.`, passwordLong: `Use at most ${MAX_PASSWORD_LENGTH} characters.`, @@ -45,6 +51,10 @@ function known(error: unknown): { status: 400 | 403; field?: Field; message: str case 'USER_ALREADY_EXISTS': case 'USER_ALREADY_EXISTS_USE_ANOTHER_EMAIL': return { status: 400, field: 'email', message: MESSAGES.emailUnavailable }; + case 'NAME_REQUIRED': + return { status: 400, field: 'name', message: MESSAGES.name }; + case 'NAME_TOO_LONG': + return { status: 400, field: 'name', message: MESSAGES.nameLong }; case 'INVALID_EMAIL': return { status: 400, field: 'email', message: MESSAGES.email }; case 'PASSWORD_TOO_SHORT': @@ -111,6 +121,7 @@ export function createSignUp(throttle: Throttle) { const email = typed.email.trim(); const password = text(data, 'password'); if (name === '') return refused(400, typed, MESSAGES.name, 'name'); + if (isNameTooLong(name)) return refused(400, typed, MESSAGES.nameLong, 'name'); if (!z.email().safeParse(email).success) return refused(400, typed, MESSAGES.email, 'email'); if (password.length < MIN_PASSWORD_LENGTH) { return refused(400, typed, MESSAGES.passwordShort, 'password'); diff --git a/src/routes/signup/signup.multi.e2e.ts b/src/routes/signup/signup.multi.e2e.ts index e8ac56e..3bba51f 100644 --- a/src/routes/signup/signup.multi.e2e.ts +++ b/src/routes/signup/signup.multi.e2e.ts @@ -5,12 +5,13 @@ import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { test as base, expect } from '@playwright/test'; -// Two kinds of server, because the Multi-user server on 4174 starts invite-only and nothing in the app -// opens registration yet (the Users toggle is C34b): -// - the 4174 server itself shows the closed state (it is invite-only, as every new Instance is); +// Two kinds of server, because the Multi-user server on 4174 starts invite-only (as every new Instance does) +// and the Users page's toggle (C34b) can open registration, but a test that flipped it would change the +// shared server for every other file: +// - the 4174 server itself shows the closed state; // - for the open state this file starts its own server from the same build, on a fresh database and a free -// port, whose Instance a fixture made Multi-user and Open (seed-open-instance.ts). It is a test fixture -// only: the app gains no way to open registration. +// port, whose Instance a fixture made Multi-user and Open (seed-open-instance.ts), the state the toggle +// leaves behind. The seed is a test fixture only: nothing in the app imports it. const LEARNER = { name: 'Grace Hopper', @@ -197,6 +198,104 @@ test.describe('registration open', () => { await context.close(); }); + test('a name over 100 characters is refused on its field, keeping what was typed but never the password', async ({ + browser, + open + }) => { + const context = await browser.newContext(); + const page = await context.newPage(); + await page.goto(`${open.url}/signup`); + const tooLong = 'N'.repeat(101); + + await page.getByLabel('Name').fill(tooLong); + await page.getByLabel('Email').fill('longname@example.com'); + await page.getByLabel('Password').fill(LEARNER.password); + await page.getByRole('button', { name: 'Create account →' }).click(); + + await expect(page.getByText('Use at most 100 characters.')).toBeVisible(); + await expect(page.getByLabel('Name')).toHaveAttribute('aria-invalid', 'true'); + await expect(page.getByLabel('Name')).toHaveValue(tooLong); + await expect(page.getByLabel('Email')).toHaveValue('longname@example.com'); + await expect(page.getByLabel('Password')).toHaveValue(''); + await expect(page).toHaveURL(/\/signup/); + await context.close(); + }); + + test('a name of exactly 100 characters, emoji counted once each, is accepted', async ({ + browser, + open + }) => { + const context = await browser.newContext(); + const page = await context.newPage(); + await page.goto(`${open.url}/signup`); + // 100 characters, 200 UTF-16 units: the browser must not count them twice. + const hundred = '\u{1F600}'.repeat(100); + + await page.getByLabel('Name').fill(hundred); + await page.getByLabel('Email').fill('hundred@example.com'); + await page.getByLabel('Password').fill(LEARNER.password); + await page.getByRole('button', { name: 'Create account →' }).click(); + + await expect(page).toHaveURL(`${open.url}/`); + await expect(page.getByRole('navigation', { name: 'Primary' })).toContainText(hundred); + await context.close(); + }); + + test('a direct POST to /api/auth/sign-up/email with a longer name is refused, and nothing is stored', async ({ + request, + open + }) => { + const response = await request.post(`${open.url}/api/auth/sign-up/email`, { + headers: { origin: open.url }, + data: { + name: 'N'.repeat(101), + email: 'direct-long@example.com', + password: LEARNER.password + } + }); + + expect(response.status()).toBe(400); + expect(await response.json()).toMatchObject({ + code: 'NAME_TOO_LONG', + message: 'The name is too long.' + }); + expect(response.headers()['set-cookie']).toBeUndefined(); + // No Account was made: the Sign in form does not know that email, whatever the password. + const signIn = await request.post(`${open.url}/signin`, { + headers: { origin: open.url, accept: 'text/html' }, + form: { email: 'direct-long@example.com', password: LEARNER.password } + }); + expect(signIn.status()).toBe(400); + expect(await signIn.text()).toContain('Email or password is wrong.'); + }); + + test('a refused name is refused the same for an email that has an Account, so it is no way to find one out', async ({ + request, + open + }) => { + const post = (email: string, name: string) => + request.post(`${open.url}/api/auth/sign-up/email`, { + headers: { origin: open.url }, + data: { name, email, password: LEARNER.password } + }); + + // LEARNER signed up in the first test of this file, so that email has an Account. + const known = await post(LEARNER.email, 'N'.repeat(101)); + const unknown = await post('nobody-long@example.com', 'N'.repeat(101)); + const blankKnown = await post(LEARNER.email, ' '); + const blankUnknown = await post('nobody-blank@example.com', ' '); + + expect(known.status()).toBe(400); + expect(unknown.status()).toBe(400); + expect(await known.json()).toEqual(await unknown.json()); + expect(blankKnown.status()).toBe(400); + expect(await blankKnown.json()).toMatchObject({ + code: 'NAME_REQUIRED', + message: 'Enter a name.' + }); + expect(await blankKnown.json()).toEqual(await blankUnknown.json()); + }); + test('an email that cannot be used gets a message that does not say why', async ({ browser, open