From 2268126b67612febc261817e1d01f44ab61c40ed Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Wed, 15 Jul 2026 21:44:12 -0700 Subject: [PATCH 1/2] fix: unreachable LDAP backend must not block local/DB account login Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> --- lib/private/User/Manager.php | 15 ++++++++-- tests/lib/User/ManagerTest.php | 51 ++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 2 deletions(-) diff --git a/lib/private/User/Manager.php b/lib/private/User/Manager.php index 5c899b7892f21..06deb7a947e39 100644 --- a/lib/private/User/Manager.php +++ b/lib/private/User/Manager.php @@ -9,6 +9,7 @@ use OC\Hooks\PublicEmitter; use OC\Memcache\WithLocalCache; +use OC\ServerNotAvailableException; use OCP\Config\IUserConfig; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\EventDispatcher\IEventDispatcher; @@ -231,7 +232,12 @@ public function checkPasswordNoLogging($loginName, $password) { foreach ($backends as $backend) { if ($backend instanceof ICheckPasswordBackend || $backend->implementsActions(Backend::CHECK_PASSWORD)) { /** @var ICheckPasswordBackend $backend */ - $uid = $backend->checkPassword($loginName, $password); + try { + $uid = $backend->checkPassword($loginName, $password); + } catch (ServerNotAvailableException $e) { + $this->logger->warning('Unable to check password against user backend', ['exception' => $e]); + continue; + } if ($uid !== false) { return $this->getUserObject($uid, $backend); } @@ -246,7 +252,12 @@ public function checkPasswordNoLogging($loginName, $password) { foreach ($backends as $backend) { if ($backend instanceof ICheckPasswordBackend || $backend->implementsActions(Backend::CHECK_PASSWORD)) { /** @var ICheckPasswordBackend|UserInterface $backend */ - $uid = $backend->checkPassword($loginName, $password); + try { + $uid = $backend->checkPassword($loginName, $password); + } catch (ServerNotAvailableException $e) { + $this->logger->warning('Unable to check password against user backend', ['exception' => $e]); + continue; + } if ($uid !== false) { return $this->getUserObject($uid, $backend); } diff --git a/tests/lib/User/ManagerTest.php b/tests/lib/User/ManagerTest.php index f606ac4d80503..f7f4adc535212 100644 --- a/tests/lib/User/ManagerTest.php +++ b/tests/lib/User/ManagerTest.php @@ -9,6 +9,7 @@ namespace Test\User; use OC\AllConfig; +use OC\ServerNotAvailableException; use OC\USER\BACKEND; use OC\User\Database; use OC\User\Manager; @@ -165,6 +166,56 @@ public function testCheckPassword(): void { $this->assertTrue($user instanceof User); } + public function testCheckPasswordNoLoggingContinuesAfterUnavailableBackend(): void { + $exception = new ServerNotAvailableException(); + $unavailableBackend = $this->createMock(\Test\Util\User\Dummy::class); + $unavailableBackend->expects($this->once()) + ->method('checkPassword') + ->with('foo', 'bar') + ->willThrowException($exception); + $unavailableBackend->method('implementsActions') + ->with(BACKEND::CHECK_PASSWORD) + ->willReturn(true); + + $databaseBackend = $this->createMock(Database::class); + $databaseBackend->expects($this->once()) + ->method('checkPassword') + ->with('foo', 'bar') + ->willReturn('foo'); + + $this->logger->expects($this->once()) + ->method('warning') + ->with('Unable to check password against user backend', ['exception' => $exception]); + + $manager = new Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); + $manager->registerBackend($unavailableBackend); + $manager->registerBackend($databaseBackend); + + $user = $manager->checkPasswordNoLogging('foo', 'bar'); + $this->assertInstanceOf(User::class, $user); + $this->assertSame('foo', $user->getUID()); + } + + public function testCheckPasswordNoLoggingReturnsFalseForUnavailableBackend(): void { + $exception = new ServerNotAvailableException(); + $backend = $this->createMock(\Test\Util\User\Dummy::class); + $backend->method('checkPassword') + ->with('foo', 'bar') + ->willThrowException($exception); + $backend->method('implementsActions') + ->with(BACKEND::CHECK_PASSWORD) + ->willReturn(true); + + $this->logger->expects($this->atLeastOnce()) + ->method('warning') + ->with('Unable to check password against user backend', ['exception' => $exception]); + + $manager = new Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); + $manager->registerBackend($backend); + + $this->assertFalse($manager->checkPasswordNoLogging('foo', 'bar')); + } + public function testCheckPasswordNotSupported(): void { $backend = $this->createMock(\Test\Util\User\Dummy::class); $backend->expects($this->never()) From 18c7c677dd455fcd929a6aed3d5d31fb46f9c276 Mon Sep 17 00:00:00 2001 From: mvanhorn Date: Thu, 16 Jul 2026 22:07:12 -0700 Subject: [PATCH 2/2] fix(user): include backend name in unavailable-backend warning Address review: log which backend was unreachable in the checkPasswordNoLogging warnings, using getBackendName() for IUserBackend backends and get_class() otherwise. Parenthesize the ternary so the concatenation and instanceof do not bind ahead of it. Adapt ManagerTest to stub getBackendName() and expect the backend name in the message. Signed-off-by: mvanhorn --- lib/private/User/Manager.php | 4 ++-- tests/lib/User/ManagerTest.php | 8 ++++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/private/User/Manager.php b/lib/private/User/Manager.php index 06deb7a947e39..2dc37f97b0bf7 100644 --- a/lib/private/User/Manager.php +++ b/lib/private/User/Manager.php @@ -235,7 +235,7 @@ public function checkPasswordNoLogging($loginName, $password) { try { $uid = $backend->checkPassword($loginName, $password); } catch (ServerNotAvailableException $e) { - $this->logger->warning('Unable to check password against user backend', ['exception' => $e]); + $this->logger->warning('Unable to check password against user backend: ' . ($backend instanceof IUserBackend ? $backend->getBackendName() : get_class($backend)), ['exception' => $e]); continue; } if ($uid !== false) { @@ -255,7 +255,7 @@ public function checkPasswordNoLogging($loginName, $password) { try { $uid = $backend->checkPassword($loginName, $password); } catch (ServerNotAvailableException $e) { - $this->logger->warning('Unable to check password against user backend', ['exception' => $e]); + $this->logger->warning('Unable to check password against user backend: ' . ($backend instanceof IUserBackend ? $backend->getBackendName() : get_class($backend)), ['exception' => $e]); continue; } if ($uid !== false) { diff --git a/tests/lib/User/ManagerTest.php b/tests/lib/User/ManagerTest.php index f7f4adc535212..0c86a4ca043c9 100644 --- a/tests/lib/User/ManagerTest.php +++ b/tests/lib/User/ManagerTest.php @@ -176,6 +176,8 @@ public function testCheckPasswordNoLoggingContinuesAfterUnavailableBackend(): vo $unavailableBackend->method('implementsActions') ->with(BACKEND::CHECK_PASSWORD) ->willReturn(true); + $unavailableBackend->method('getBackendName') + ->willReturn('Dummy'); $databaseBackend = $this->createMock(Database::class); $databaseBackend->expects($this->once()) @@ -185,7 +187,7 @@ public function testCheckPasswordNoLoggingContinuesAfterUnavailableBackend(): vo $this->logger->expects($this->once()) ->method('warning') - ->with('Unable to check password against user backend', ['exception' => $exception]); + ->with('Unable to check password against user backend: Dummy', ['exception' => $exception]); $manager = new Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); $manager->registerBackend($unavailableBackend); @@ -205,10 +207,12 @@ public function testCheckPasswordNoLoggingReturnsFalseForUnavailableBackend(): v $backend->method('implementsActions') ->with(BACKEND::CHECK_PASSWORD) ->willReturn(true); + $backend->method('getBackendName') + ->willReturn('Dummy'); $this->logger->expects($this->atLeastOnce()) ->method('warning') - ->with('Unable to check password against user backend', ['exception' => $exception]); + ->with('Unable to check password against user backend: Dummy', ['exception' => $exception]); $manager = new Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger); $manager->registerBackend($backend);