sign in with an email or a username
Only an address worked, though usernames are already unique and already validated as /^[a-zA-Z0-9._-]+$/ — which is what makes this safe: no `@`, so the two namespaces cannot overlap and there is nothing to guess. An `@` decides which lookup runs, and it decides it exactly. The field is still called `email` on the wire, because every client sends that name and renaming it would break them for nothing. Fixed a latent bug while passing: the JWT took `email` from the REQUEST rather than the row. Identical while an address was the only way in — and a username in the `email` claim the moment it was not, leaving the DAV door, the managed home path and everything else keyed on email looking up a user who does not exist. The login field is `type="text"`, not `type="email"`, or the browser refuses to submit a username and validates someone out of an account they have. `autoComplete="username"`, which is what password managers store beside a password for a field that takes either. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -46,13 +46,17 @@ export function Login() {
|
|||||||
|
|
||||||
<form ref={formRef} onSubmit={handleSubmit} className="grid gap-4">
|
<form ref={formRef} onSubmit={handleSubmit} className="grid gap-4">
|
||||||
<Label className="grid gap-2">
|
<Label className="grid gap-2">
|
||||||
<span className="text-duck-dark/70">Email</span>
|
<span className="text-duck-dark/70">Email or username</span>
|
||||||
<Input
|
<Input
|
||||||
className="h-11 bg-background/60 border-duck-dark/20 text-duck-dark placeholder:text-duck-dark/40"
|
className="h-11 bg-background/60 border-duck-dark/20 text-duck-dark placeholder:text-duck-dark/40"
|
||||||
type="email"
|
// `text`, not `email`: the browser refuses to submit a username under type="email", so the
|
||||||
|
// field would have validated the user out of an account they have.
|
||||||
|
type="text"
|
||||||
name="email"
|
name="email"
|
||||||
placeholder="you@example.com"
|
placeholder="you@example.com"
|
||||||
autoComplete="email"
|
// `username` rather than `email`, which is what password managers expect for a field that
|
||||||
|
// takes either — and it is the value they already store beside the password.
|
||||||
|
autoComplete="username"
|
||||||
/>
|
/>
|
||||||
</Label>
|
</Label>
|
||||||
|
|
||||||
|
|||||||
@@ -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);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -1,5 +1,5 @@
|
|||||||
import type { Handler } from 'hono';
|
import type { Handler } from 'hono';
|
||||||
import { getUserByEmail, getPasskeysByUserIdAndOrigin } from 'officerdb';
|
import { getUserByEmail, getUserByUsername, getPasskeysByUserIdAndOrigin } from 'officerdb';
|
||||||
import { sign } from '@@/jwt';
|
import { sign } from '@@/jwt';
|
||||||
import argon2 from 'argon2';
|
import argon2 from 'argon2';
|
||||||
import * as errors from '@@/custom-errors';
|
import * as errors from '@@/custom-errors';
|
||||||
@@ -8,7 +8,9 @@ import { isLockdown, noteBlocked } from './panic';
|
|||||||
const TEST_USERS: number[] = [];
|
const TEST_USERS: number[] = [];
|
||||||
|
|
||||||
export const signinHandler: Handler = async function (ctx) {
|
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
|
// 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
|
// 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
|
// 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();
|
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();
|
if (!dbUser) throw errors.UNAUTHORIZED();
|
||||||
|
|
||||||
const passkeys = await getPasskeysByUserIdAndOrigin(dbUser.id, origin);
|
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
|
// 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.
|
// 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 };
|
const tokenUser = { id, email, name, username, passkeys: passkeys.length };
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user