Commit 60eabb70 authored by Claude's avatar Claude

Make login credentials case- and format-insensitive

Postgres '=' is case-sensitive, so a user stored as 'Km...@gmail.com'
could not log in from a phone keyboard that lowercases the email field.
The lookup in AuthService returned null before Hash::check ever ran, so
this presented as "wrong password" and was invisible in login_history —
that table is only written once a user has been found.

On OC-Sport this affected 8 of 26 accounts, and had already produced one
duplicate registration: a user who could not get in simply signed up
again with the same address in lowercase.

- CredentialNormalizer: one canonical shape for emails and phones
- AuthService: case-insensitive email lookup, deterministically ordered
  so a pre-existing case-duplicate pair resolves to the account actually
  in use rather than an arbitrary row; phone lookup matches local and
  +20 forms
- User: set-mutators so new rows are stored canonical
- Migration: normalises existing rows, skipping and logging any that
  would collide, since those are duplicate accounts needing a human
  merge rather than a guess
Co-Authored-By: 's avatarClaude Opus 5 <noreply@anthropic.com>
parent 3d472374
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
use App\Domain\Identity\DTOs\AuthResult; use App\Domain\Identity\DTOs\AuthResult;
use App\Domain\Identity\Models\LoginHistory; use App\Domain\Identity\Models\LoginHistory;
use App\Domain\Shared\Helpers\CredentialNormalizer;
use App\Models\User; use App\Models\User;
use Illuminate\Support\Facades\Hash; use Illuminate\Support\Facades\Hash;
...@@ -14,9 +15,11 @@ class AuthService ...@@ -14,9 +15,11 @@ class AuthService
public function attempt(string $identifier, string $password, string $ip, ?string $userAgent = null): AuthResult public function attempt(string $identifier, string $password, string $ip, ?string $userAgent = null): AuthResult
{ {
$identifier = trim($identifier);
$user = filter_var($identifier, FILTER_VALIDATE_EMAIL) $user = filter_var($identifier, FILTER_VALIDATE_EMAIL)
? User::where('email', $identifier)->first() ? $this->findByEmail($identifier)
: User::where('phone', $identifier)->first(); : $this->findByPhone($identifier);
if (!$user) { if (!$user) {
return new AuthResult(success: false, reason: 'invalid_credentials'); return new AuthResult(success: false, reason: 'invalid_credentials');
...@@ -65,6 +68,48 @@ public function attempt(string $identifier, string $password, string $ip, ?strin ...@@ -65,6 +68,48 @@ public function attempt(string $identifier, string $password, string $ip, ?strin
return new AuthResult(success: true, user: $user); return new AuthResult(success: true, user: $user);
} }
/**
* Find a user by email, case-insensitively.
*
* Mail addresses are not case-sensitive, but Postgres '=' is, so a row
* stored as 'Km...@gmail.com' was unreachable to anyone whose keyboard
* lowercases the field. Ordering makes the result deterministic where a
* case-duplicate pair already exists: an exact match wins, then the
* account actually being used, so nobody is dropped into a stale
* duplicate. Those pairs still need a manual merge.
*/
private function findByEmail(string $email): ?User
{
return User::whereRaw('LOWER(email) = ?', [CredentialNormalizer::email($email)])
->orderByRaw('CASE WHEN email = ? THEN 0 ELSE 1 END', [$email])
->orderByRaw('last_login_at IS NULL')
->orderByDesc('last_login_at')
->orderByDesc('id')
->first();
}
/**
* Find a user by phone, tolerating the formats people actually type.
*
* Matches every stored shape of the number rather than the canonical
* form alone, so this keeps working on rows written before the
* normalisation migration ran.
*/
private function findByPhone(string $phone): ?User
{
$variants = CredentialNormalizer::phoneVariants($phone);
if ($variants === []) {
return null;
}
return User::whereIn('phone', $variants)
->orderByRaw('last_login_at IS NULL')
->orderByDesc('last_login_at')
->orderByDesc('id')
->first();
}
private function recordLogin(User $user, string $ip, ?string $userAgent, string $status, ?string $reason = null): void private function recordLogin(User $user, string $ip, ?string $userAgent, string $status, ?string $reason = null): void
{ {
LoginHistory::create([ LoginHistory::create([
......
<?php
namespace App\Domain\Shared\Helpers;
class CredentialNormalizer
{
/**
* Canonical storage form for an email address: trimmed and lowercased.
*
* Mailbox providers we serve treat addresses case-insensitively, but
* Postgres '=' does not. Storing one canonical form is what stops a
* stored capital from locking someone out of their own account.
*/
public static function email(?string $email): ?string
{
$email = trim((string) $email);
return $email === '' ? null : mb_strtolower($email, 'UTF-8');
}
/**
* Canonical storage form for a phone number: digits only, local format.
*
* Accepts the shapes people actually type — '+20 101 408 7672',
* '0 11 41078754' — and returns '01014087672'. Egypt country code is
* stripped so that the local and international forms of one number
* cannot end up as two different accounts.
*/
public static function phone(?string $phone): ?string
{
$digits = preg_replace('/[^0-9]/', '', (string) $phone) ?? '';
if ($digits === '') {
return null;
}
if (str_starts_with($digits, '20') && strlen($digits) === 12) {
$digits = '0' . substr($digits, 2);
}
return $digits;
}
/**
* Every stored shape a typed phone number might match.
*
* Lookups use this rather than the canonical form alone so that logins
* keep working on rows written before normalisation ran — including on
* client databases where the migration has not been applied yet.
*
* @return array<int, string>
*/
public static function phoneVariants(?string $phone): array
{
$local = self::phone($phone);
if ($local === null) {
return [];
}
$variants = [$local, trim((string) $phone)];
if (str_starts_with($local, '0')) {
$bare = substr($local, 1);
$variants[] = '+20' . $bare;
$variants[] = '20' . $bare;
}
return array_values(array_unique(array_filter($variants)));
}
}
...@@ -5,9 +5,11 @@ ...@@ -5,9 +5,11 @@
use App\Domain\Identity\Models\LoginHistory; use App\Domain\Identity\Models\LoginHistory;
use App\Domain\Identity\Models\Person; use App\Domain\Identity\Models\Person;
use App\Domain\Identity\Models\Role; use App\Domain\Identity\Models\Role;
use App\Domain\Shared\Helpers\CredentialNormalizer;
use App\Domain\Shared\Traits\Auditable; use App\Domain\Shared\Traits\Auditable;
use App\Domain\Shared\Traits\BelongsToAcademy; use App\Domain\Shared\Traits\BelongsToAcademy;
use App\Domain\Shared\Traits\HasUuid; use App\Domain\Shared\Traits\HasUuid;
use Illuminate\Database\Eloquent\Casts\Attribute;
use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Factories\HasFactory;
use Illuminate\Database\Eloquent\Relations\BelongsTo; use Illuminate\Database\Eloquent\Relations\BelongsTo;
use Illuminate\Database\Eloquent\Relations\BelongsToMany; use Illuminate\Database\Eloquent\Relations\BelongsToMany;
...@@ -67,6 +69,20 @@ protected function casts(): array ...@@ -67,6 +69,20 @@ protected function casts(): array
]; ];
} }
// ─── Credential normalisation ─────────────────────────────────
// Stored in one canonical shape so that login can match on equality.
// Set-only: nothing about how these are read changes.
protected function email(): Attribute
{
return Attribute::set(fn (?string $value) => CredentialNormalizer::email($value));
}
protected function phone(): Attribute
{
return Attribute::set(fn (?string $value) => CredentialNormalizer::phone($value));
}
// ─── Relationships ──────────────────────────────────────────── // ─── Relationships ────────────────────────────────────────────
public function person(): BelongsTo public function person(): BelongsTo
......
<?php
use App\Domain\Shared\Helpers\CredentialNormalizer;
use Illuminate\Database\Migrations\Migration;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Facades\Log;
/**
* Bring existing user credentials into the canonical shape that login now
* matches on: lowercase emails, digits-only local phone numbers.
*
* Runs against every client database on deploy, so it is deliberately
* non-destructive. A row that cannot be normalised without colliding with
* another account is left exactly as it is and logged — those pairs are
* duplicate registrations that need a human merge decision, and guessing
* which one to keep would be worse than leaving both.
*
* Uses the query builder rather than the User model on purpose: the
* BelongsToAcademy global scope resolves 'current_academy', which is not
* bound during migrations.
*/
return new class extends Migration
{
public function up(): void
{
$this->normalizeColumn('email', fn ($v) => CredentialNormalizer::email($v));
$this->normalizeColumn('phone', fn ($v) => CredentialNormalizer::phone($v));
}
public function down(): void
{
// Irreversible by design: the original casing and formatting are not
// recorded anywhere, and restoring them would re-break login.
}
private function normalizeColumn(string $column, callable $normalize): void
{
$skipped = [];
DB::table('users')
->select('id', $column)
->whereNotNull($column)
->orderBy('id')
->chunk(200, function ($rows) use ($column, $normalize, &$skipped) {
foreach ($rows as $row) {
$current = $row->{$column};
$normalized = $normalize($current);
if ($normalized === null || $normalized === $current) {
continue;
}
$taken = DB::table('users')
->where($column, $normalized)
->where('id', '!=', $row->id)
->exists();
if ($taken) {
$skipped[] = $row->id;
continue;
}
DB::table('users')
->where('id', $row->id)
->update([$column => $normalized]);
}
});
if ($skipped !== []) {
Log::warning('normalize_user_credentials: left {column} untouched on user ids {ids} — another account already holds the normalised value. These are duplicate registrations and need a manual merge.', [
'column' => $column,
'ids' => implode(', ', $skipped),
]);
}
}
};
<?php
namespace Tests\Unit;
use App\Domain\Shared\Helpers\CredentialNormalizer;
use PHPUnit\Framework\TestCase;
class CredentialNormalizerTest extends TestCase
{
public function test_email_is_lowercased_and_trimmed(): void
{
$this->assertSame('km97911801@gmail.com', CredentialNormalizer::email('Km97911801@gmail.com'));
$this->assertSame('a@icloud.com', CredentialNormalizer::email(' A@iCloud.com '));
}
public function test_blank_email_becomes_null(): void
{
$this->assertNull(CredentialNormalizer::email(null));
$this->assertNull(CredentialNormalizer::email(' '));
}
public function test_phone_is_reduced_to_local_digits(): void
{
$this->assertSame('01141078754', CredentialNormalizer::phone('0 11 41078754'));
$this->assertSame('01014087672', CredentialNormalizer::phone('+201014087672'));
$this->assertSame('01027203379', CredentialNormalizer::phone('01027203379'));
}
public function test_phone_variants_cover_local_and_international_forms(): void
{
$variants = CredentialNormalizer::phoneVariants('01014087672');
$this->assertContains('01014087672', $variants);
$this->assertContains('+201014087672', $variants);
$this->assertContains('201014087672', $variants);
}
public function test_local_and_international_input_agree(): void
{
$this->assertSame(
CredentialNormalizer::phone('01014087672'),
CredentialNormalizer::phone('+20 101 408 7672'),
);
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment