Merge pull request #21106 from nextcloud/fix/10809/user-pwd-change-loginname

use the loginname to verify the old password in user password changes

2 files changed, 29 insertions, 6 deletions

diff --git a/apps/settings/lib/Controller/ChangePasswordController.php b/apps/settings/lib/Controller/ChangePasswordController.php
index 439731b22eb..e6567bf9043 100644
--- a/apps/settings/lib/Controller/ChangePasswordController.php
+++ b/apps/settings/lib/Controller/ChangePasswordController.php
@@ -89,8 +89,9 @@ class ChangePasswordController extends Controller {
 	 * @BruteForceProtection(action=changePersonalPassword)
 	 */
 	public function changePersonalPassword(string $oldpassword = '', string $newpassword = null): JSONResponse {
+		$loginName = $this->userSession->getLoginName();
 		/** @var IUser $user */
-		$user = $this->userManager->checkPassword($this->userId, $oldpassword);
+		$user = $this->userManager->checkPassword($loginName, $oldpassword);
 		if ($user === false) {
 			$response = new JSONResponse([
 				'status' => 'error',
diff --git a/tests/Core/Controller/ChangePasswordControllerTest.php b/tests/Core/Controller/ChangePasswordControllerTest.php
index 175628552bc..21a80b61063 100644
--- a/tests/Core/Controller/ChangePasswordControllerTest.php
+++ b/tests/Core/Controller/ChangePasswordControllerTest.php
@@ -36,6 +36,8 @@ use OCP\IUserManager;
 class ChangePasswordControllerTest extends \Test\TestCase {
 	/** @var string */
 	private $userId = 'currentUser';
+	/** @var string */
+	private $loginName = 'ua1337';
 	/** @var IUserManager|\PHPUnit_Framework_MockObject_MockObject */
 	private $userManager;
 	/** @var Session|\PHPUnit_Framework_MockObject_MockObject */
@@ -75,9 +77,13 @@ class ChangePasswordControllerTest extends \Test\TestCase {
 	}
 
 	public function testChangePersonalPasswordWrongPassword() {
+		$this->userSession->expects($this->once())
+			->method('getLoginName')
+			->willReturn($this->loginName);
+
 		$this->userManager->expects($this->once())
 			->method('checkPassword')
-			->with($this->userId, 'old')
+			->with($this->loginName, 'old')
 			->willReturn(false);
 
 		$expects = new JSONResponse([
@@ -93,10 +99,14 @@ class ChangePasswordControllerTest extends \Test\TestCase {
 	}
 
 	public function testChangePersonalPasswordCommonPassword() {
+		$this->userSession->expects($this->once())
+			->method('getLoginName')
+			->willReturn($this->loginName);
+
 		$user = $this->getMockBuilder(IUser::class)->getMock();
 		$this->userManager->expects($this->once())
 			->method('checkPassword')
-			->with($this->userId, 'old')
+			->with($this->loginName, 'old')
 			->willReturn($user);
 
 		$user->expects($this->once())
@@ -116,10 +126,14 @@ class ChangePasswordControllerTest extends \Test\TestCase {
 	}
 
 	public function testChangePersonalPasswordNoNewPassword() {
+		$this->userSession->expects($this->once())
+			->method('getLoginName')
+			->willReturn($this->loginName);
+
 		$user = $this->getMockBuilder(IUser::class)->getMock();
 		$this->userManager->expects($this->once())
 			->method('checkPassword')
-			->with($this->userId, 'old')
+			->with($this->loginName, 'old')
 			->willReturn($user);
 
 		$expects = [
@@ -132,10 +146,14 @@ class ChangePasswordControllerTest extends \Test\TestCase {
 	}
 
 	public function testChangePersonalPasswordCantSetPassword() {
+		$this->userSession->expects($this->once())
+			->method('getLoginName')
+			->willReturn($this->loginName);
+
 		$user = $this->getMockBuilder(IUser::class)->getMock();
 		$this->userManager->expects($this->once())
 			->method('checkPassword')
-			->with($this->userId, 'old')
+			->with($this->loginName, 'old')
 			->willReturn($user);
 
 		$user->expects($this->once())
@@ -152,10 +170,14 @@ class ChangePasswordControllerTest extends \Test\TestCase {
 	}
 
 	public function testChangePersonalPassword() {
+		$this->userSession->expects($this->once())
+			->method('getLoginName')
+			->willReturn($this->loginName);
+
 		$user = $this->getMockBuilder(IUser::class)->getMock();
 		$this->userManager->expects($this->once())
 			->method('checkPassword')
-			->with($this->userId, 'old')
+			->with($this->loginName, 'old')
 			->willReturn($user);
 
 		$user->expects($this->once())