From dc6e722e3269a12d4059b144cfcea8b7aab1c42f Mon Sep 17 00:00:00 2001
From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
Date: Fri, 24 Jul 2026 02:16:47 -0700
Subject: [PATCH] fix: only retry urldecoded password when decoding changes it
 (LDAP badPwdCount double-increment)

Fixes #33657

Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
---
 lib/private/User/Manager.php   | 10 +++++--
 tests/lib/User/ManagerTest.php | 49 ++++++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/lib/private/User/Manager.php b/lib/private/User/Manager.php
index 5c899b7892f21..a56fa79490465 100644
--- a/lib/private/User/Manager.php
+++ b/lib/private/User/Manager.php
@@ -241,12 +241,18 @@ public function checkPasswordNoLogging($loginName, $password) {
 		// since http basic auth doesn't provide a standard way of handling non ascii password we allow password to be urlencoded
 		// we only do this decoding after using the plain password fails to maintain compatibility with any password that happens
 		// to contain urlencoded patterns by "accident".
-		$password = urldecode($password);
+		$decodedPassword = urldecode($password);
+		if ($decodedPassword === $password) {
+			// decoding the password did not change it, so retrying with the
+			// decoded value would just be a redundant duplicate authentication
+			// attempt (e.g. a second LDAP bind that double-increments badPwdCount).
+			return false;
+		}
 
 		foreach ($backends as $backend) {
 			if ($backend instanceof ICheckPasswordBackend || $backend->implementsActions(Backend::CHECK_PASSWORD)) {
 				/** @var ICheckPasswordBackend|UserInterface $backend */
-				$uid = $backend->checkPassword($loginName, $password);
+				$uid = $backend->checkPassword($loginName, $decodedPassword);
 				if ($uid !== false) {
 					return $this->getUserObject($uid, $backend);
 				}
diff --git a/tests/lib/User/ManagerTest.php b/tests/lib/User/ManagerTest.php
index f606ac4d80503..fe27d1d2fd6c3 100644
--- a/tests/lib/User/ManagerTest.php
+++ b/tests/lib/User/ManagerTest.php
@@ -180,6 +180,55 @@ public function testCheckPasswordNotSupported(): void {
 		$this->assertFalse($manager->checkPassword('foo', 'bar'));
 	}
 
+	public function testCheckPasswordWrongPasswordCallsBackendOnce(): void {
+		$backend = $this->createMock(\Test\Util\User\Dummy::class);
+		$backend->expects($this->once())
+			->method('checkPassword')
+			->with($this->equalTo('foo'), $this->equalTo('wrongpass'))
+			->willReturn(false);
+
+		$backend->expects($this->any())
+			->method('implementsActions')
+			->willReturnCallback(function ($actions) {
+				if ($actions === BACKEND::CHECK_PASSWORD) {
+					return true;
+				} else {
+					return false;
+				}
+			});
+
+		$manager = new Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger);
+		$manager->registerBackend($backend);
+
+		$this->assertFalse($manager->checkPassword('foo', 'wrongpass'));
+	}
+
+	public function testCheckPasswordUrlencodedRetriesWithDecodedValue(): void {
+		$backend = $this->createMock(\Test\Util\User\Dummy::class);
+		$backend->expects($this->exactly(2))
+			->method('checkPassword')
+			->willReturnMap([
+				['foo', '%C3%A9', false],
+				['foo', urldecode('%C3%A9'), 'foo'],
+			]);
+
+		$backend->expects($this->any())
+			->method('implementsActions')
+			->willReturnCallback(function ($actions) {
+				if ($actions === BACKEND::CHECK_PASSWORD) {
+					return true;
+				} else {
+					return false;
+				}
+			});
+
+		$manager = new Manager($this->config, $this->cacheFactory, $this->eventDispatcher, $this->logger);
+		$manager->registerBackend($backend);
+
+		$user = $manager->checkPassword('foo', '%C3%A9');
+		$this->assertTrue($user instanceof User);
+	}
+
 	public function testGetOneBackendExists(): void {
 		$backend = $this->createMock(\Test\Util\User\Dummy::class);
 		$backend->expects($this->once())