External Auth: Updated user external auth id matching to be stricter

Added tests to cover.
Thanks to whale120 for reporting.

app/Access/ExternalBaseUserProvider.php

@@ -3,11 +3,17 @@
 namespace BookStack\Access;
 
 use BookStack\Users\Models\User;
+use BookStack\Users\UserRepo;
 use Illuminate\Contracts\Auth\Authenticatable;
 use Illuminate\Contracts\Auth\UserProvider;
 
 class ExternalBaseUserProvider implements UserProvider
 {
+    public function __construct(
+        protected UserRepo $userRepo,
+    ) {
+    }
+
     /**
      * Retrieve a user by their unique identifier.
      */
@@ -44,9 +50,7 @@ class ExternalBaseUserProvider implements UserProvider
      */
     public function retrieveByCredentials(array $credentials): ?Authenticatable
     {
-        return User::query()
-            ->where('external_auth_id', $credentials['external_auth_id'])
-            ->first();
+        return $this->userRepo->getByExternalAuthId($credentials['external_auth_id']);
     }
 
     /**

app/Access/RegistrationService.php

@@ -52,9 +52,7 @@ class RegistrationService
      */
     public function findOrRegister(string $name, string $email, string $externalId): User
     {
-        $user = User::query()
-            ->where('external_auth_id', '=', $externalId)
-            ->first();
+        $user = $this->userRepo->getByExternalAuthId($externalId);
 
         if (is_null($user)) {
             $userData = [

app/App/Providers/AuthServiceProvider.php

@@ -10,6 +10,7 @@ use BookStack\Access\LoginService;
 use BookStack\Access\RegistrationService;
 use BookStack\Api\ApiTokenGuard;
 use BookStack\Users\Models\User;
+use BookStack\Users\UserRepo;
 use Illuminate\Support\Facades\Auth;
 use Illuminate\Support\ServiceProvider;
 use Illuminate\Validation\Rules\Password;
@@ -60,7 +61,7 @@ class AuthServiceProvider extends ServiceProvider
     public function register(): void
     {
         Auth::provider('external-users', function () {
-            return new ExternalBaseUserProvider();
+            return new ExternalBaseUserProvider($this->app[UserRepo::class]);
         });
 
         // Bind and provide the default system user as a singleton to the app instance when needed.

app/Users/UserRepo.php

@@ -51,6 +51,25 @@ class UserRepo
         return User::query()->where('slug', '=', $slug)->firstOrFail();
     }
 
+    /**
+     * Get a user by their external auth ID value.
+     * Returns null if no matching user found.
+     */
+    public function getByExternalAuthId(string $externalId): User|null
+    {
+        // We only really expect at most one user from the search, but as an extra layer of defence against database
+        // normalisation we search possible matches exactly against the value.
+        $users = User::query()->where('external_auth_id', '=', $externalId)->get();
+
+        foreach ($users as $user) {
+            if ($user->external_auth_id === $externalId) {
+                return $user;
+            }
+        }
+
+        return null;
+    }
+
     /**
      * Create a new basic instance of user with the given pre-validated data.
      *

database/migrations/2026_07_27_201402_update_users_external_auth_id_collation.php

@@ -0,0 +1,31 @@
+<?php
+
+use Illuminate\Database\Migrations\Migration;
+use Illuminate\Database\Schema\Blueprint;
+use Illuminate\Support\Facades\Schema;
+
+return new class extends Migration
+{
+    /**
+     * Run the migrations.
+     */
+    public function up(): void
+    {
+        Schema::table('users', function (Blueprint $table) {
+            $table->string('external_auth_id')
+                ->collation('utf8mb4_bin')
+                ->change();
+        });
+    }
+
+    /**
+     * Reverse the migrations.
+     */
+    public function down(): void
+    {
+        Schema::table('users', function (Blueprint $table) {
+            $table->string('external_auth_id')
+                ->change();
+        });
+    }
+};

tests/Auth/LdapTest.php

@@ -207,6 +207,36 @@ class LdapTest extends TestCase
         $this->assertDatabaseHas('users', ['email' => $this->mockUser->email, 'email_confirmed' => false, 'external_auth_id' => 'cooluser456']);
     }
 
+    public function test_login_uses_exact_match_for_external_auth_id_values()
+    {
+        config()->set(['services.ldap.id_attribute' => 'my_custom_id']);
+        // External auth id the same as we expect below but different casing
+        User::query()->forceCreate([
+            'email'            => '[email protected]',
+            'external_auth_id' => 'CoolUser456',
+            'email_confirmed'  => true,
+            'name'             => 'Barry Scott',
+        ]);
+
+        $this->commonLdapMocks(1, 1, 1, 2, 1);
+        $ldapDn = 'cn=test-user,dc=test' . config('services.ldap.base_dn');
+        $this->mockLdap->shouldReceive('searchAndGetEntries')->times(1)
+            ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
+            ->andReturn(['count' => 1, 0 => [
+                'cn'           => [$this->mockUser->name],
+                'dn'           => $ldapDn,
+                'my_custom_id' => ['cooluser456'],
+                'mail'         => [$this->mockUser->email],
+            ]]);
+
+        $resp = $this->mockUserLogin();
+        $resp->assertRedirect('/');
+
+        $this->assertDatabaseHas('users', ['email' => $this->mockUser->email]);
+        $this->assertEquals($this->mockUser->email, user()->email);
+        $this->assertEquals('cooluser456', user()->external_auth_id);
+    }
+
     public function test_user_filter_default_placeholder_format()
     {
         config()->set('services.ldap.user_filter', '(&(uid={user}))');

tests/Auth/OidcTest.php

@@ -471,7 +471,28 @@ class OidcTest extends TestCase
         $this->assertEquals('xXBennyTheGeezXx', $user->external_auth_id);
     }
 
-    public function test_auth_uses_mulitple_display_name_claims_if_configured()
+    public function test_auth_uses_external_id_as_exact_value()
+    {
+        // External auth id the same as we expect below but different casing
+        User::query()->forceCreate([
+            'email'            => '[email protected]',
+            'external_auth_id' => 'Benni202',
+            'email_confirmed'  => true,
+            'name'             => 'Barry Scott',
+        ]);
+
+        $resp = $this->runLogin([
+            'email'            => '[email protected]',
+            'sub'              => 'benni202',
+        ]);
+        $resp->assertRedirect('/');
+
+        $this->assertDatabaseHas('users', ['email' => '[email protected]']);
+        $this->assertEquals('[email protected]', user()->email);
+        $this->assertEquals('benni202', user()->external_auth_id);
+    }
+
+    public function test_auth_uses_multiple_display_name_claims_if_configured()
     {
         config()->set(['oidc.display_name_claims' => 'first_name|last_name']);
 

tests/Auth/Saml2Test.php

@@ -419,6 +419,29 @@ class Saml2Test extends TestCase
         $acsPost->assertSee('A user with the email [email protected] already exists but with different credentials');
     }
 
+    public function test_login_uses_exact_match_for_external_auth_values()
+    {
+        $this->post('/saml2/login');
+        config()->set(['saml2.onelogin.strict' => false]);
+
+        // Make the user pre-existing in DB with auth_id of different casing
+        User::query()->forceCreate([
+            'email'            => '[email protected]',
+            'external_auth_id' => 'UsEr',
+            'email_confirmed'  => true,
+            'name'             => 'Barry Scott',
+        ]);
+
+        $this->followingRedirects()->post('/saml2/acs', ['SAMLResponse' => $this->acsPostData]);
+
+        $this->assertTrue($this->isAuthenticated());
+        $this->assertDatabaseHas('users', [
+            'email' => '[email protected]',
+        ]);
+        $this->assertEquals('[email protected]', user()->email);
+        $this->assertEquals('user', user()->external_auth_id);
+    }
+
     public function test_login_request_contains_expected_default_authncontext()
     {
         $authReq = $this->getAuthnRequest();