diff --git a/src/apps/officer-web/Screens/Authentication/LandingPage/Login.tsx b/src/apps/officer-web/Screens/Authentication/LandingPage/Login.tsx index d9451daa..87496b9e 100644 --- a/src/apps/officer-web/Screens/Authentication/LandingPage/Login.tsx +++ b/src/apps/officer-web/Screens/Authentication/LandingPage/Login.tsx @@ -46,13 +46,17 @@ export function Login() {
diff --git a/src/servers/api/auth/signin-identifier.test.ts b/src/servers/api/auth/signin-identifier.test.ts new file mode 100644 index 00000000..0d653bc5 --- /dev/null +++ b/src/servers/api/auth/signin-identifier.test.ts @@ -0,0 +1,34 @@ +import { describe, expect, test } from 'bun:test'; + +// Signing in with an email or a username, and why the two can never be confused. +// +// Usernames are validated as `/^[a-zA-Z0-9._-]+$/` — no `@`. That single fact is what lets one field +// accept both without guessing, without querying twice, and without a username ever being able to +// impersonate an address. + +const USERNAME_RE = /^[a-zA-Z0-9._-]+$/; +const looksLikeEmail = (identifier: string) => identifier.includes('@'); + +describe('telling an email from a username', () => { + test('a username can never contain the character that identifies an email', () => { + expect(USERNAME_RE.test('pastilhas')).toBe(true); + expect(USERNAME_RE.test('a.b_c-d1')).toBe(true); + // The load-bearing one: if this ever passes, the two namespaces overlap and signin has to guess. + expect(USERNAME_RE.test('someone@example.com')).toBe(false); + }); + + test('an address routes to the email lookup', () => { + expect(looksLikeEmail('someone@example.com')).toBe(true); + }); + + test('a bare name routes to the username lookup', () => { + expect(looksLikeEmail('pastilhas')).toBe(false); + }); + + test('a name that merely contains a dot is still a username', () => { + // `jrg.conceicao` is a real username on this project's own server, and a naive "has a dot" test + // would have sent it to the email lookup and failed the login. + expect(looksLikeEmail('jrg.conceicao')).toBe(false); + expect(USERNAME_RE.test('jrg.conceicao')).toBe(true); + }); +}); diff --git a/src/servers/api/auth/signin.ts b/src/servers/api/auth/signin.ts index d4a93a11..c702656c 100755 --- a/src/servers/api/auth/signin.ts +++ b/src/servers/api/auth/signin.ts @@ -1,5 +1,5 @@ import type { Handler } from 'hono'; -import { getUserByEmail, getPasskeysByUserIdAndOrigin } from 'officerdb'; +import { getUserByEmail, getUserByUsername, getPasskeysByUserIdAndOrigin } from 'officerdb'; import { sign } from '@@/jwt'; import argon2 from 'argon2'; import * as errors from '@@/custom-errors'; @@ -8,7 +8,9 @@ import { isLockdown, noteBlocked } from './panic'; const TEST_USERS: number[] = []; export const signinHandler: Handler = async function (ctx) { - const { email, password } = ctx.get('body'); + // Still named `email` on the wire, because every client sends that field and renaming it would break + // them for no gain. What it holds is an IDENTIFIER: an email address or a username. + const { email: identifier, password } = ctx.get('body'); // A caller that sends neither Origin nor Referer — a server-to-server client, curl — leaves this // undefined. It was once unreachable: origin validation rejected those requests before this handler // ran, so the value was always a string by the time anything touched it. Then the flag that disabled @@ -24,7 +26,12 @@ export const signinHandler: Handler = async function (ctx) { throw errors.UNAUTHORIZED(); } - const dbUser = await getUserByEmail(email); + // An `@` decides it, and decides it exactly: usernames are `/^[a-zA-Z0-9._-]+$/`, so the two + // namespaces cannot overlap and there is no ambiguity to resolve by guessing or by querying twice. + const dbUser = + typeof identifier === 'string' && identifier.includes('@') + ? await getUserByEmail(identifier) + : await getUserByUsername(String(identifier ?? '')); if (!dbUser) throw errors.UNAUTHORIZED(); const passkeys = await getPasskeysByUserIdAndOrigin(dbUser.id, origin); @@ -40,7 +47,11 @@ export const signinHandler: Handler = async function (ctx) { // and what they can reach once inside is decided by their role's permissions at every request. A rule // that let a member hold a Gitea grant they could never sign in to use was not defence, just a wall. - const { id, name, username } = dbUser; + // `email` from the ROW, not from the request. It happened to be the same string while email was the + // only way in; signing in by username would otherwise put a username in the `email` claim and leave + // every downstream reader — the DAV door, the managed home path, anything keyed on email — looking up + // a user that does not exist. + const { id, name, username, email } = dbUser; const tokenUser = { id, email, name, username, passkeys: passkeys.length };