From dc9a5d72522080bd4156e05ea3f3a9d87614b262 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= <1005065+DeepDiver1975@users.noreply.github.com> Date: Mon, 1 Jun 2026 15:30:03 +0200 Subject: [PATCH 1/3] fix(settings): pass target user ID to sendEmail in setMailAddress subadmin path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In setMailAddress, the subadmin/non-admin code path was calling sendEmail($userId, ...) where $userId is the session caller's UID instead of $id (the target user's UID). This caused the verification token and confirmation link to be associated with the caller rather than the target, so clicking the link changed the caller's email instead of the target's. The admin path was already corrected in commit 3ff2884; this fixes the remaining subadmin path (GHSA-5945-5fp8-4283). Co-Authored-By: Claude Sonnet 4.6 Signed-off-by: Thomas Müller <1005065+DeepDiver1975@users.noreply.github.com> --- settings/Controller/UsersController.php | 2 +- .../Controller/UsersControllerTest.php | 74 +++++++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-) diff --git a/settings/Controller/UsersController.php b/settings/Controller/UsersController.php index b688e0fd554b..06382de44442 100644 --- a/settings/Controller/UsersController.php +++ b/settings/Controller/UsersController.php @@ -985,7 +985,7 @@ public function setMailAddress($id, $mailAddress) { } try { - if ($this->sendEmail($userId, $mailAddress)) { + if ($this->sendEmail($id, $mailAddress)) { return new DataResponse( [ 'status' => 'success', diff --git a/tests/Settings/Controller/UsersControllerTest.php b/tests/Settings/Controller/UsersControllerTest.php index aff872116e92..6f4d5edde18c 100644 --- a/tests/Settings/Controller/UsersControllerTest.php +++ b/tests/Settings/Controller/UsersControllerTest.php @@ -2276,6 +2276,80 @@ public function testSetEmailAddress($mailAddress, $isValid, $expectsUpdate, $can $this->assertSame($responseCode, $response->getStatus()); } + /** + * When a subadmin calls setMailAddress for a different user, the verification + * email and token must be stored under the target user's ID, not the caller's. + * Regression test for GHSA-5945-5fp8-4283 (subadmin path). + */ + public function testSetMailAddressSubadminSendsEmailToTargetNotCaller(): void { + $this->container['IsAdmin'] = false; + + $callerUser = $this->createMock(User::class); + $callerUser->method('getUID')->willReturn('subadmin'); + + $targetUser = $this->createMock(User::class); + $targetUser->method('getUID')->willReturn('targetuser'); + $targetUser->method('canChangeMailAddress')->willReturn(true); + + $subAdmin = $this->createMock(SubAdmin::class); + $subAdmin->method('isUserAccessible')->willReturn(true); + + $this->container['GroupManager'] + ->method('getSubAdmin') + ->willReturn($subAdmin); + + $this->container['UserSession'] + ->method('getUser') + ->willReturn($callerUser); + + $this->container['UserManager'] + ->method('get') + ->willReturnCallback(function ($id) use ($callerUser, $targetUser) { + return $id === 'subadmin' ? $callerUser : ($id === 'targetuser' ? $targetUser : null); + }); + + $this->container['Mailer'] + ->method('validateMailAddress') + ->willReturn(true); + + // Token must be read from and stored under 'targetuser', not 'subadmin' + $this->container['Config'] + ->method('getUserValue') + ->with('targetuser', 'owncloud', 'changeMail') + ->willReturn(''); + + $this->container['Config'] + ->expects($this->once()) + ->method('setUserValue') + ->with('targetuser', 'owncloud', 'changeMail', $this->anything()); + + $this->container['TimeFactory'] + ->method('getTime') + ->willReturn(12345); + + $this->container['SecureRandom'] + ->method('generate') + ->willReturn('SomeToken'); + + // Confirmation link must carry 'targetuser', not 'subadmin' + $this->container['URLGenerator'] + ->expects($this->once()) + ->method('linkToRouteAbsolute') + ->with('settings.Users.changeMail', $this->callback(function ($params) { + return isset($params['userId']) && $params['userId'] === 'targetuser'; + })) + ->willReturn('https://example.com/changeMail'); + + $message = $this->createMock(Message::class); + $this->container['Mailer'] + ->method('createMessage') + ->willReturn($message); + + $response = $this->container['UsersController']->setMailAddress('targetuser', 'new@example.com'); + $this->assertSame(Http::STATUS_OK, $response->getStatus()); + $this->assertSame('success', $response->getData()['status']); + } + public function testStatsAdmin(): void { $this->container['IsAdmin'] = true; From 82e008406bc481a367beedf0b0d9fb4c023c7e13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= <1005065+DeepDiver1975@users.noreply.github.com> Date: Mon, 1 Jun 2026 15:44:47 +0200 Subject: [PATCH 2/3] chore: add changelog entry for PR 41574 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Sonnet 4.6 Signed-off-by: Thomas Müller <1005065+DeepDiver1975@users.noreply.github.com> --- changelog/unreleased/41574 | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 changelog/unreleased/41574 diff --git a/changelog/unreleased/41574 b/changelog/unreleased/41574 new file mode 100644 index 000000000000..39b6397e0b2f --- /dev/null +++ b/changelog/unreleased/41574 @@ -0,0 +1,8 @@ +Bugfix: Fix subadmin email change updating caller's address instead of target's + +The verification token and confirmation link in the subadmin path of +setMailAddress were associated with the caller's account instead of the +target user's account. Clicking the confirmation link changed the +subadmin's email rather than the intended target's email. + +https://github.com/owncloud/core/pull/41574 From 31e21797faa62e29cd0669393d3f5f6a810c3ec3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= <1005065+DeepDiver1975@users.noreply.github.com> Date: Mon, 1 Jun 2026 15:46:48 +0200 Subject: [PATCH 3/3] chore: simplify user lookup callback in subadmin mail test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Sonnet 4.6 Signed-off-by: Thomas Müller <1005065+DeepDiver1975@users.noreply.github.com> --- tests/Settings/Controller/UsersControllerTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/Settings/Controller/UsersControllerTest.php b/tests/Settings/Controller/UsersControllerTest.php index 6f4d5edde18c..12ea792b58e3 100644 --- a/tests/Settings/Controller/UsersControllerTest.php +++ b/tests/Settings/Controller/UsersControllerTest.php @@ -2305,7 +2305,8 @@ public function testSetMailAddressSubadminSendsEmailToTargetNotCaller(): void { $this->container['UserManager'] ->method('get') ->willReturnCallback(function ($id) use ($callerUser, $targetUser) { - return $id === 'subadmin' ? $callerUser : ($id === 'targetuser' ? $targetUser : null); + $userMap = ['subadmin' => $callerUser, 'targetuser' => $targetUser]; + return $userMap[$id] ?? null; }); $this->container['Mailer']