serfowiandClaude Opus 5
c617bc9687
Acts on an adversarial review of the M1 auth and authorization code. Five findings fixed; the rest recorded in SECURITY-FINDINGS.md as the M1 exit criteria rather than left in a tool transcript. - auth: serve /phone-number/request-password-reset, /phone-number/ reset-password and /sign-in/phone-number as 404. better-auth's phoneNumber() registers all three unconditionally -- they are NOT gated on emailAndPassword.enabled:false. Left live they form a silent second credential path: request-password-reset stores an OTP and sends no SMS (sendPasswordResetOTP was never configured, so the owner is never told), reset-password mints a bcrypt credential row, and sign-in/phone-number then accepts it forever with no OTP. The OTP gate still applies, so this is not remote unauthenticated takeover -- it converts one momentary OTP compromise into permanent access the victim cannot see or rotate. - auth: drop bearer(). It accepts the plaintext sessions.token column as an Authorization credential, making any single leaked row a replayable login. The mobile client it was added for is hypothetical. - auth: pin E.164 via phoneNumberValidator, and add toE164/isE164 to @linkder/shared. phone is UNIQUE and bans are per-account, so "+34600111222" and "0034600111222" being separately storable meant one handset could hold two accounts and a ban was escapable by retyping. 15 tests. - auth: NEXT_PUBLIC_APP_URL now throws in production instead of falling back to localhost, which was silently dropping Secure and the __Secure- prefix from the production session cookie. - pro.upsertProfile: actually apply requiresReReview. It was computed, returned to the client and never acted on, so a verified plumber could become a verified electrician in another city by ignoring a response flag. Now demotes to pending in the same transaction and audits it. Trade changes count as material (they did not before) -- the licence is per-trade. Needed a verified -> pending edge in VERIFICATION_GRAPH, which did not exist. Removed two untracked scratch repro files. The impersonation repro depended on bearer() for transport and no longer applies as written; the underlying finding (resolveSession drops impersonatedBy, so admin actions are audited as the victim) is open and documented. typecheck, lint, build clean; 127 tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
62 lines
2.1 KiB
TypeScript
62 lines
2.1 KiB
TypeScript
import { describe, expect, it } from 'vitest';
|
|
import { isE164, phoneLast4, toE164 } from '../src/phone';
|
|
|
|
/**
|
|
* These are security tests, not formatting tests. `users.phone` is UNIQUE and
|
|
* bans are per-account, so any pair of inputs that normalises to two different
|
|
* strings for one real handset is a way to hold two accounts and to escape a ban.
|
|
*/
|
|
describe('toE164', () => {
|
|
it('passes through an already-normalised number', () => {
|
|
expect(toE164('+34600111222')).toBe('+34600111222');
|
|
});
|
|
|
|
it('collapses every separator style a human types to ONE stored form', () => {
|
|
const forms = [
|
|
'+34 600 111 222',
|
|
'+34-600-111-222',
|
|
'+34 (600) 111.222',
|
|
' +34600111222 ',
|
|
'0034600111222',
|
|
'0034 600 111 222',
|
|
];
|
|
const normalised = new Set(forms.map(toE164));
|
|
expect(normalised).toEqual(new Set(['+34600111222']));
|
|
});
|
|
|
|
it('refuses a bare national number rather than guessing a country', () => {
|
|
// Guessing would attach one person's account to another person's number.
|
|
expect(toE164('600111222')).toBeNull();
|
|
expect(toE164('0600111222')).toBeNull();
|
|
});
|
|
|
|
it('rejects junk, empties and letters', () => {
|
|
expect(toE164(null)).toBeNull();
|
|
expect(toE164(undefined)).toBeNull();
|
|
expect(toE164('')).toBeNull();
|
|
expect(toE164(' ')).toBeNull();
|
|
expect(toE164('+34600ABC222')).toBeNull();
|
|
expect(toE164('not a phone')).toBeNull();
|
|
});
|
|
|
|
it('enforces E.164 length and a nonzero country digit', () => {
|
|
expect(toE164('+3460011')).toBeNull(); // too short
|
|
expect(toE164('+3460011122233344')).toBeNull(); // too long
|
|
expect(toE164('+0600111222')).toBeNull(); // country code cannot start with 0
|
|
});
|
|
});
|
|
|
|
describe('isE164', () => {
|
|
it('accepts only the canonical stored form', () => {
|
|
expect(isE164('+34600111222')).toBe(true);
|
|
expect(isE164('0034600111222')).toBe(false);
|
|
expect(isE164('+34 600 111 222')).toBe(false);
|
|
});
|
|
});
|
|
|
|
describe('phoneLast4', () => {
|
|
it('returns the last four digits for masked display', () => {
|
|
expect(phoneLast4('+34600111222')).toBe('1222');
|
|
});
|
|
});
|