diff --git a/app/Http/Controllers/Auth/AuthenticatedSessionController.php b/app/Http/Controllers/Auth/AuthenticatedSessionController.php index 2824e7b8..662574d1 100644 --- a/app/Http/Controllers/Auth/AuthenticatedSessionController.php +++ b/app/Http/Controllers/Auth/AuthenticatedSessionController.php @@ -4,6 +4,7 @@ namespace App\Http\Controllers\Auth; use App\Http\Controllers\Controller; use App\Http\Requests\Auth\LoginRequest; +use App\Modules\Identity\Ldap\LdapSettings; use App\Modules\Identity\StartPages; use App\Modules\Platform\Settings\Setting; use App\Modules\Platform\Settings\Settings; @@ -25,6 +26,7 @@ class AuthenticatedSessionController extends Controller 'canResetPassword' => Route::has('password.request'), 'canRegister' => app(Settings::class)->get(Setting::ClientsCanRegister) === true, 'status' => $request->session()->get('status'), + 'usernameSignIn' => LdapSettings::current()->allowsUsernameSignIn(), ]); } diff --git a/app/Http/Requests/Auth/LoginRequest.php b/app/Http/Requests/Auth/LoginRequest.php index 7cf5ce77..579c77e9 100644 --- a/app/Http/Requests/Auth/LoginRequest.php +++ b/app/Http/Requests/Auth/LoginRequest.php @@ -4,7 +4,9 @@ namespace App\Http\Requests\Auth; use App\Models\User; use App\Modules\Identity\AccountLookup; +use App\Modules\Identity\Ldap\LdapAuthenticator; use App\Modules\Identity\Ldap\LdapProvisioner; +use App\Modules\Identity\Ldap\LdapSettings; use App\Modules\Identity\PasswordVerification; use App\Modules\Identity\SignIn; use App\Modules\Platform\Captcha\CaptchaForm; @@ -14,6 +16,7 @@ use Illuminate\Contracts\Validation\ValidationRule; use Illuminate\Foundation\Http\FormRequest; use Illuminate\Support\Facades\Auth; use Illuminate\Support\Facades\RateLimiter; +use Illuminate\Support\Facades\Validator; use Illuminate\Support\Str; use Illuminate\Validation\ValidationException; @@ -35,7 +38,12 @@ class LoginRequest extends FormRequest public function rules(): array { return [ - 'email' => ['required', 'string', 'email'], + // The field keeps its name either way: with a directory username + // attribute configured it also takes a username. See + // loginEmail(). + 'email' => LdapSettings::current()->allowsUsernameSignIn() + ? ['required', 'string', 'max:255'] + : ['required', 'string', 'email'], 'password' => ['required', 'string'], // Deliberately here rather than inside authenticate(): rules // run first, so a bot never reaches the credential check, and @@ -72,23 +80,32 @@ class LoginRequest extends FormRequest { $this->ensureIsNotRateLimited(); + // Anything that is not an address is a directory username, and from + // here on the login is for the address the directory holds for it. + $login = (string) $this->string('email'); + $byUsername = ! $this->isEmail($login); + $email = $byUsername ? app(LdapAuthenticator::class)->emailForUsername($login) : $login; + // Exact, for the reason SocialAuthenticator is: a collation that // folds accents would otherwise let somebody typing // admin@éxample.com be *identified* as admin@example.com. A // password still gates this one, so it was never the takeover the // social path was — but identifying the wrong account is the bug, // and the credential check is a second line rather than the rule. - $user = app(AccountLookup::class)->byEmail((string) $this->string('email')); + $user = $email !== null ? app(AccountLookup::class)->byEmail($email) : null; // A directory identity with no local account yet. Returns null // unless LDAP is on, auto-provisioning is on, and the bind // succeeds — so an unknown email costs nothing on an installation // that does not use a directory. - if ($user === null) { - $user = app(LdapProvisioner::class)->provision( - (string) $this->string('email'), - (string) $this->string('password'), - ); + if ($user === null && $email !== null) { + $user = app(LdapProvisioner::class)->provision($email, (string) $this->string('password')); + } + + // The directory is client-only. A username is a directory name, so + // it never leads to a staff account, whichever password is typed. + if ($byUsername && $user !== null && ! $user->isClient()) { + $user = null; } $verified = $this->verifyCredentials($user); @@ -118,6 +135,16 @@ class LoginRequest extends FormRequest return $pendingTwoFactor; } + /** + * By the same `email` rule that admitted every stored address, so an + * address it accepts and filter_var() does not (a dotless domain, a + * non-ASCII local part) is never mistaken for a username. + */ + private function isEmail(string $login): bool + { + return Validator::make(['email' => $login], ['email' => 'email'])->passes(); + } + /** * The account whose password checks out, or null. * diff --git a/app/Modules/Identity/Http/Controllers/LdapSettingsController.php b/app/Modules/Identity/Http/Controllers/LdapSettingsController.php index 5f087cfb..46d13f0f 100644 --- a/app/Modules/Identity/Http/Controllers/LdapSettingsController.php +++ b/app/Modules/Identity/Http/Controllers/LdapSettingsController.php @@ -56,6 +56,7 @@ class LdapSettingsController extends Controller 'user_filter' => $ldap->user_filter, 'email_attribute' => $ldap->email_attribute, 'name_attribute' => $ldap->name_attribute, + 'username_attribute' => $ldap->username_attribute, 'auto_provision' => $ldap->auto_provision, 'auto_approve' => $ldap->auto_approve, ], @@ -93,6 +94,7 @@ class LdapSettingsController extends Controller 'user_filter' => ['nullable', 'string', 'max:255'], 'email_attribute' => ['required', 'string', 'max:64'], 'name_attribute' => ['required', 'string', 'max:64'], + 'username_attribute' => ['nullable', 'string', 'max:64'], 'auto_provision' => ['required', 'boolean'], 'auto_approve' => ['required', 'boolean'], ]); @@ -110,6 +112,7 @@ class LdapSettingsController extends Controller 'user_filter' => $validated['user_filter'] ?? null, 'email_attribute' => $validated['email_attribute'], 'name_attribute' => $validated['name_attribute'], + 'username_attribute' => $validated['username_attribute'] ?? null, 'auto_provision' => (bool) $validated['auto_provision'], 'auto_approve' => (bool) $validated['auto_approve'], ]); diff --git a/app/Modules/Identity/Ldap/LdapAuthenticator.php b/app/Modules/Identity/Ldap/LdapAuthenticator.php index 86d95821..830a55b0 100644 --- a/app/Modules/Identity/Ldap/LdapAuthenticator.php +++ b/app/Modules/Identity/Ldap/LdapAuthenticator.php @@ -72,6 +72,20 @@ class LdapAuthenticator return $identity; } + /** + * The address a directory username belongs to, so a login typed as a + * username can carry on exactly as if the address had been typed. + * Null whenever username sign-in is off or the breaker is open. + */ + public function emailForUsername(string $username): ?string + { + if (! LdapSettings::current()->allowsUsernameSignIn() || $this->breakerOpen()) { + return null; + } + + return $this->directory->emailForUsername($username); + } + /** * Record what the directory told us about an account that already * exists, so an administrator can see which entry it corresponds to. @@ -104,13 +118,18 @@ class LdapAuthenticator return false; } - if (Cache::get(self::BREAKER_KEY) === true) { + if ($this->breakerOpen()) { return false; } return $this->enabled(); } + private function breakerOpen(): bool + { + return Cache::get(self::BREAKER_KEY) === true; + } + /** * Whether this account's password lives in the directory rather than * here — in which case the local hash is not consulted at all. diff --git a/app/Modules/Identity/Ldap/LdapDirectory.php b/app/Modules/Identity/Ldap/LdapDirectory.php index 9daf2e9c..b87ca201 100644 --- a/app/Modules/Identity/Ldap/LdapDirectory.php +++ b/app/Modules/Identity/Ldap/LdapDirectory.php @@ -25,6 +25,14 @@ interface LdapDirectory */ public function authenticate(string $email, string $password): ?LdapIdentity; + /** + * The address of the one entry whose username attribute matches, found + * with the service account. No bind as the person, so nothing is + * verified here: the caller still signs in by that address, through + * authenticate(). Null for no match, more than one, or any failure. + */ + public function emailForUsername(string $username): ?string; + /** * Exercise the configuration and report which stage failed, for the * settings screen's test button. This is the one place that is allowed diff --git a/app/Modules/Identity/Ldap/LdapRecordDirectory.php b/app/Modules/Identity/Ldap/LdapRecordDirectory.php index e8d9aa64..7b0fc854 100644 --- a/app/Modules/Identity/Ldap/LdapRecordDirectory.php +++ b/app/Modules/Identity/Ldap/LdapRecordDirectory.php @@ -42,7 +42,7 @@ class LdapRecordDirectory implements LdapDirectory $connection = LdapConnectionFactory::make($settings); $connection->connect(); - $entry = $this->findEntry($connection, $settings, $email); + $entry = $this->findEntry($connection, $settings, $settings->email_attribute, $email); if ($entry === null) { return null; @@ -66,6 +66,28 @@ class LdapRecordDirectory implements LdapDirectory } } + public function emailForUsername(string $username): ?string + { + $settings = LdapSettings::current(); + + if (! $settings->allowsUsernameSignIn()) { + return null; + } + + try { + $connection = LdapConnectionFactory::make($settings); + $connection->connect(); + + $entry = $this->findEntry($connection, $settings, (string) $settings->username_attribute, $username); + + return $entry === null ? null : $this->attribute($entry, $settings->email_attribute); + } catch (Throwable $e) { + Log::warning('LDAP username lookup could not be completed.', ['exception' => $e::class]); + + return null; + } + } + public function probe(?string $email = null, ?string $password = null): LdapProbeResult { $settings = LdapSettings::current(); @@ -95,7 +117,7 @@ class LdapRecordDirectory implements LdapDirectory } try { - $entry = $this->findEntry($connection, $settings, $email); + $entry = $this->findEntry($connection, $settings, $settings->email_attribute, $email); } catch (Throwable $e) { return LdapProbeResult::failed( LdapProbeResult::STAGE_SEARCH, @@ -131,21 +153,22 @@ class LdapRecordDirectory implements LdapDirectory } /** - * The one entry matching this address, or null. + * The one entry whose attribute holds this value (an address, or a + * username), or null. * - * Two results is a misconfiguration — two objects sharing an address — - * and choosing one of them is how you sign the wrong person in, so it - * fails closed. + * Two results is a misconfiguration — two objects sharing an address or + * a username — and choosing one of them is how you sign the wrong person + * in, so it fails closed. * * @return array|null */ - private function findEntry(Connection $connection, LdapSettings $settings, string $email): ?array + private function findEntry(Connection $connection, LdapSettings $settings, string $attribute, string $value): ?array { $query = $connection->query() ->in($settings->base_dn) - // The email goes through the builder, which escapes it. It is - // never concatenated into a filter string. - ->whereEquals($settings->email_attribute, $email); + // What the visitor typed goes through the builder, which + // escapes it. It is never concatenated into a filter string. + ->whereEquals($attribute, $value); if (is_string($settings->user_filter) && $settings->user_filter !== '') { // Admin-supplied, never visitor-supplied. diff --git a/app/Modules/Identity/Ldap/LdapSettings.php b/app/Modules/Identity/Ldap/LdapSettings.php index 71bf615a..40f013a8 100644 --- a/app/Modules/Identity/Ldap/LdapSettings.php +++ b/app/Modules/Identity/Ldap/LdapSettings.php @@ -25,6 +25,7 @@ use Illuminate\Database\Eloquent\Model; * @property string|null $user_filter * @property string $email_attribute * @property string $name_attribute + * @property string|null $username_attribute * @property bool $auto_provision * @property bool $auto_approve */ @@ -80,4 +81,15 @@ class LdapSettings extends Model && is_string($this->host) && $this->host !== '' && is_string($this->base_dn) && $this->base_dn !== ''; } + + /** + * Whether people may sign in with a directory username as well as an + * address: only once an administrator has named the attribute that + * holds it. + */ + public function allowsUsernameSignIn(): bool + { + return $this->usable() + && is_string($this->username_attribute) && $this->username_attribute !== ''; + } } diff --git a/database/migrations/2026_10_05_090000_add_username_attribute_to_ldap_settings.php b/database/migrations/2026_10_05_090000_add_username_attribute_to_ldap_settings.php new file mode 100644 index 00000000..40486e77 --- /dev/null +++ b/database/migrations/2026_10_05_090000_add_username_attribute_to_ldap_settings.php @@ -0,0 +1,28 @@ +string('username_attribute')->nullable(); + }); + } + + public function down(): void + { + Schema::table('ldap_settings', function (Blueprint $table) { + $table->dropColumn('username_attribute'); + }); + } +}; diff --git a/resources/js/pages/auth/login.tsx b/resources/js/pages/auth/login.tsx index a0477d4c..e1c8c246 100644 --- a/resources/js/pages/auth/login.tsx +++ b/resources/js/pages/auth/login.tsx @@ -26,9 +26,11 @@ interface LoginProps { status?: string; canResetPassword: boolean; canRegister: boolean; + /** A directory username attribute is configured, so the field also takes a username. */ + usernameSignIn: boolean; } -export default function Login({ status, canResetPassword, canRegister }: LoginProps) { +export default function Login({ status, canResetPassword, canRegister, usernameSignIn }: LoginProps) { const { t } = useTranslation(); const { flash } = usePage().props; const captcha = useRef(null); @@ -59,7 +61,12 @@ export default function Login({ status, canResetPassword, canRegister }: LoginPr }; return ( - + {/* The app-wide Toaster lives in the authenticated layout, so a @@ -75,14 +82,14 @@ export default function Login({ status, canResetPassword, canRegister }: LoginPr
- + setData('email', e.target.value)} placeholder="email@example.com" diff --git a/resources/js/pages/system/settings/ldap.tsx b/resources/js/pages/system/settings/ldap.tsx index 98b8a9e9..6c120852 100644 --- a/resources/js/pages/system/settings/ldap.tsx +++ b/resources/js/pages/system/settings/ldap.tsx @@ -33,6 +33,7 @@ interface LdapSettings { user_filter: string | null; email_attribute: string; name_attribute: string; + username_attribute: string | null; auto_provision: boolean; auto_approve: boolean; } @@ -68,6 +69,7 @@ export default function LdapSettingsPage({ ldap, encryptions, extension_availabl user_filter: ldap.user_filter ?? '', email_attribute: ldap.email_attribute, name_attribute: ldap.name_attribute, + username_attribute: ldap.username_attribute ?? '', auto_provision: ldap.auto_provision, auto_approve: ldap.auto_approve, }); @@ -268,6 +270,23 @@ export default function LdapSettingsPage({ ldap, encryptions, extension_availabl
+
+ + form.setData('username_attribute', e.target.value)} + /> +

+ {t( + 'Lets people sign in with their directory username as well as their address: uid, cn or sAMAccountName, for example. Leave it empty to sign in by address only.', + )} +

+ +
+
$entries + * @param array $entries */ function fakeDirectory(array $entries = []): FakeLdapDirectory { @@ -35,7 +35,7 @@ function fakeDirectory(array $entries = []): FakeLdapDirectory return $fake; } -function enableLdap(bool $autoProvision = false, bool $autoApprove = false): LdapSettings +function enableLdap(bool $autoProvision = false, bool $autoApprove = false, ?string $usernameAttribute = null): LdapSettings { $settings = LdapSettings::current(); $settings->forceFill([ @@ -44,6 +44,7 @@ function enableLdap(bool $autoProvision = false, bool $autoApprove = false): Lda 'base_dn' => 'dc=example,dc=test', 'auto_provision' => $autoProvision, 'auto_approve' => $autoApprove, + 'username_attribute' => $usernameAttribute, ])->save(); return $settings; @@ -512,3 +513,113 @@ test('a local account still confirms against its own hash, with LDAP on', functi ->assertRedirect() ->assertSessionHasNoErrors(); }); + +/* +|-------------------------------------------------------------------------- +| Signing in with a directory username +|-------------------------------------------------------------------------- +*/ + +test('a client signs in with their directory username', function () { + enableLdap(usernameAttribute: 'uid'); + $fake = fakeDirectory(['someone@example.test' => ['password' => 'directory-pass', 'username' => 'someone']]); + $client = User::factory()->client()->create(['email' => 'someone@example.test']); + + $this->post('/login', ['email' => 'someone', 'password' => 'directory-pass'])->assertRedirect(); + + $this->assertAuthenticatedAs($client); + expect($fake->lookedUpUsernames)->toBe(['someone']) + ->and($fake->attemptedEmails)->toBe(['someone@example.test']); +}); + +test('a username signs in a client the directory has not met yet, when auto-provisioning is on', function () { + enableLdap(autoProvision: true, autoApprove: true, usernameAttribute: 'uid'); + fakeDirectory(['newcomer@example.test' => ['password' => 'directory-pass', 'username' => 'newcomer', 'name' => 'New Comer']]); + + $this->post('/login', ['email' => 'newcomer', 'password' => 'directory-pass'])->assertRedirect(); + + $user = User::query()->where('email', 'newcomer@example.test')->sole(); + expect($user->isClient())->toBeTrue() + ->and($user->auth_source)->toBe(AuthSource::Ldap); + $this->assertAuthenticatedAs($user); +}); + +test('a username never signs in a staff account, even with its own password', function () { + enableLdap(usernameAttribute: 'uid'); + fakeDirectory(['admin@example.test' => ['password' => 'directory-pass', 'username' => 'admin']]); + User::factory()->create(['email' => 'admin@example.test']); + + $this->post('/login', ['email' => 'admin', 'password' => 'password'])->assertSessionHasErrors('email'); + $this->post('/login', ['email' => 'admin', 'password' => 'directory-pass'])->assertSessionHasErrors('email'); + + $this->assertGuest(); +}); + +test('a wrong password with a valid username is refused', function () { + enableLdap(usernameAttribute: 'uid'); + fakeDirectory(['someone@example.test' => ['password' => 'directory-pass', 'username' => 'someone']]); + User::factory()->client()->create(['email' => 'someone@example.test']); + + $this->post('/login', ['email' => 'someone', 'password' => 'wrong']) + ->assertSessionHasErrors(['email' => __('auth.failed')]); + + $this->assertGuest(); +}); + +test('an unknown username gets the same failure as a wrong password, and counts toward the limit', function () { + enableLdap(usernameAttribute: 'uid'); + fakeDirectory(); + + foreach (range(1, 5) as $_) { + $this->post('/login', ['email' => 'nobody', 'password' => 'whatever']) + ->assertSessionHasErrors(['email' => __('auth.failed')]); + } + + $this->post('/login', ['email' => 'nobody', 'password' => 'whatever']) + ->assertSessionHasErrors('email'); + expect(session('errors')->first('email'))->not->toBe(__('auth.failed')); +}); + +test('a username is only accepted once a username attribute is set', function () { + enableLdap(); + $fake = fakeDirectory(['someone@example.test' => ['password' => 'directory-pass', 'username' => 'someone']]); + User::factory()->client()->create(['email' => 'someone@example.test']); + + $this->post('/login', ['email' => 'someone', 'password' => 'directory-pass'])->assertSessionHasErrors('email'); + + expect($fake->calls)->toBe(0); + $this->assertGuest(); +}); + +test('an email address still signs in by email with username sign-in on', function () { + enableLdap(usernameAttribute: 'uid'); + $fake = fakeDirectory(['someone@example.test' => ['password' => 'directory-pass', 'username' => 'someone']]); + $client = User::factory()->client()->create(['email' => 'someone@example.test']); + + $this->post('/login', ['email' => 'someone@example.test', 'password' => 'directory-pass'])->assertRedirect(); + + $this->assertAuthenticatedAs($client); + expect($fake->lookedUpUsernames)->toBe([]); +}); + +test('the login page offers username sign-in only when it is configured', function () { + $this->get('/login')->assertInertia(fn ($page) => $page->where('usernameSignIn', false)); + + enableLdap(usernameAttribute: 'uid'); + + $this->get('/login')->assertInertia(fn ($page) => $page->where('usernameSignIn', true)); +}); + +// Decided by the same rule that admitted the address, or an account whose +// address the `email` rule accepts and filter_var does not (a dotless +// domain, say) would be taken for a username and locked out. +test('an address the email rule accepts is still an address with username sign-in on', function () { + enableLdap(usernameAttribute: 'uid'); + $fake = fakeDirectory(); + $client = User::factory()->client()->create(['email' => 'someone@localhost']); + + $this->post('/login', ['email' => 'someone@localhost', 'password' => 'password'])->assertRedirect(); + + $this->assertAuthenticatedAs($client); + expect($fake->lookedUpUsernames)->toBe([]); +}); diff --git a/tests/Feature/Identity/LdapSettingsTest.php b/tests/Feature/Identity/LdapSettingsTest.php index 39322381..a5e5716f 100644 --- a/tests/Feature/Identity/LdapSettingsTest.php +++ b/tests/Feature/Identity/LdapSettingsTest.php @@ -190,3 +190,16 @@ test('settings are only usable once they are complete', function () { $settings->forceFill(['active' => true, 'host' => 'h', 'base_dn' => 'b'])->save(); expect($settings->refresh()->usable())->toBe(extension_loaded('ldap')); }); + +test('the username attribute is optional, saved, and cleared when left empty', function () { + expect(LdapSettings::current()->username_attribute)->toBeNull(); + + $this->actingAs($this->admin)->patch('/system/settings/ldap', ldapPayload(['username_attribute' => 'uid']))->assertRedirect(); + expect(LdapSettings::current()->username_attribute)->toBe('uid'); + + $this->actingAs($this->admin)->get('/system/settings/ldap') + ->assertInertia(fn (AssertableInertia $page) => $page->where('ldap.username_attribute', 'uid')); + + $this->actingAs($this->admin)->patch('/system/settings/ldap', ldapPayload(['username_attribute' => '']))->assertRedirect(); + expect(LdapSettings::current()->username_attribute)->toBeNull(); +}); diff --git a/tests/Support/FakeLdapDirectory.php b/tests/Support/FakeLdapDirectory.php index 82e214f0..00d70269 100644 --- a/tests/Support/FakeLdapDirectory.php +++ b/tests/Support/FakeLdapDirectory.php @@ -25,8 +25,11 @@ class FakeLdapDirectory implements LdapDirectory /** @var list */ public array $attemptedEmails = []; + /** @var list */ + public array $lookedUpUsernames = []; + /** - * @param array $entries keyed by email + * @param array $entries keyed by email */ public function __construct(private readonly array $entries = []) {} @@ -48,6 +51,19 @@ class FakeLdapDirectory implements LdapDirectory ); } + public function emailForUsername(string $username): ?string + { + $this->calls++; + $this->lookedUpUsernames[] = $username; + + $matches = array_keys(array_filter( + $this->entries, + fn (array $entry): bool => ($entry['username'] ?? null) === $username, + )); + + return count($matches) === 1 ? $matches[0] : null; + } + public function probe(?string $email = null, ?string $password = null): LdapProbeResult { return LdapProbeResult::ok(LdapProbeResult::STAGE_SERVICE_BIND, 'Fake directory reachable.');