From 7022abe37dbde85cc5f3fd90e4486b03f80534d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Baki=20Burak=20=C3=96=C4=9F=C3=BCn?= <63836730+bakiburakogun@users.noreply.github.com> Date: Sat, 29 Aug 2026 05:41:36 +0300 Subject: [PATCH] feat: let apps define the actor of an activity via the activity manager MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CurrentUser::getUID() read the user straight from the session, so an action performed outside of a user's session could not be attributed to anybody. File activities created from a background job end up rendered as "remote account" did something, because FilesHooks asks CurrentUser for the actor. IManager::setCurrentUserId() already exists for exactly this, and OC\Activity\Manager::getCurrentUserId() honours it, but the activity app never consulted it. Read the actor from the activity manager instead of the session. With no override the result is unchanged for a request that has a session, because the manager returns the session user itself; when there is neither a session nor a valid feed token it throws, which is caught here so getUID() keeps returning null. Signed-off-by: Baki Burak Öğün <63836730+bakiburakogun@users.noreply.github.com> --- lib/CurrentUser.php | 21 ++++++++++++---- tests/CurrentUserTest.php | 53 +++++++++++++++++++++++---------------- 2 files changed, 48 insertions(+), 26 deletions(-) diff --git a/lib/CurrentUser.php b/lib/CurrentUser.php index e9fe71f7b..31d79bb33 100644 --- a/lib/CurrentUser.php +++ b/lib/CurrentUser.php @@ -7,6 +7,7 @@ namespace OCA\Activity; +use OCP\Activity\IManager as IActivityManager; use OCP\IRequest; use OCP\IUser; use OCP\IUserSession; @@ -22,6 +23,7 @@ public function __construct( protected readonly IRequest $request, protected readonly IManager $shareManager, protected readonly IFactory $l10nFactory, + protected readonly IActivityManager $activityManager, ) { } @@ -53,14 +55,23 @@ public function getUserIdentifier(): string { } /** - * Get the current user id from the session + * Get the current user id + * + * Apps can override who an action is attributed to with + * IManager::setCurrentUserId(). That is the only way to name an actor when the + * action happens outside of that user's session, e.g. from a background job. + * Without an override the manager reads the session, and on a request without + * a session the owner of the activity feed token. */ public function getUID(): ?string { - $user = $this->userSession->getUser(); - if ($user instanceof IUser) { - return $user->getUID(); + try { + $userId = $this->activityManager->getCurrentUserId(); + } catch (\UnexpectedValueException) { + // No override, no session and no valid feed token + return null; } - return null; + + return $userId === '' ? null : $userId; } /** diff --git a/tests/CurrentUserTest.php b/tests/CurrentUserTest.php index 2c7151ea7..41a3fbddc 100644 --- a/tests/CurrentUserTest.php +++ b/tests/CurrentUserTest.php @@ -24,8 +24,8 @@ use Exception; use OCA\Activity\CurrentUser; +use OCP\Activity\IManager as IActivityManager; use OCP\IRequest; -use OCP\IUser; use OCP\IUserSession; use OCP\L10N\IFactory; use OCP\Share\Exceptions\ShareNotFound; @@ -49,6 +49,7 @@ class CurrentUserTest extends TestCase { protected IUserSession&MockObject $userSession; protected IManager&MockObject $shareManager; protected IFactory&MockObject $l10nFactory; + protected IActivityManager&MockObject $activityManager; protected function setUp(): void { parent::setUp(); @@ -57,6 +58,7 @@ protected function setUp(): void { $this->userSession = $this->createMock(IUserSession::class); $this->shareManager = $this->createMock(IManager::class); $this->l10nFactory = $this->createMock(IFactory::class); + $this->activityManager = $this->createMock(IActivityManager::class); $this->request->method('getScriptName')->willReturn('/public.php'); } @@ -68,6 +70,7 @@ protected function getInstance(array $methods = []): CurrentUser|MockObject { $this->request, $this->shareManager, $this->l10nFactory, + $this->activityManager, ); } @@ -77,6 +80,7 @@ protected function getInstance(array $methods = []): CurrentUser|MockObject { $this->request, $this->shareManager, $this->l10nFactory, + $this->activityManager, ]) ->onlyMethods($methods) ->getMock(); @@ -108,39 +112,46 @@ public function testGetUserIdentifier(?string $cachedIdentifier, string|int|null $this->assertSame($expected, $instance->getUserIdentifier()); } - protected function getUserMock(string $uid): IUser { - $user = $this->createMock(IUser::class); - $user->expects($this->once()) - ->method('getUID') - ->willReturn($uid); - return $user; - } - public static function dataGetUID(): array { return [ - [null, null], ['uid', 'uid'], ['test', 'test'], + ['', null], ]; } #[DataProvider('dataGetUID')] - public function testGetUID(?string $uid, ?string $expected): void { - if ($uid === null) { - $this->userSession->expects($this->never()) - ->method('getUser'); - return; - } + public function testGetUID(string $currentUserId, ?string $expected): void { + $this->activityManager->expects($this->once()) + ->method('getCurrentUserId') + ->willReturn($currentUserId); + $this->userSession->expects($this->never()) + ->method('getUser'); - $user = $this->getUserMock($uid); $instance = $this->getInstance(); - $this->userSession->expects($this->once()) - ->method('getUser') - ->willReturn($user); - $this->assertSame($expected, $instance->getUID()); } + public function testGetUIDUsesTheActivityManagerOverride(): void { + $this->activityManager->method('getCurrentUserId') + ->willReturn('attributed-user'); + $this->userSession->expects($this->never()) + ->method('getUser'); + + $instance = $this->getInstance(); + $this->assertSame('attributed-user', $instance->getUID()); + } + + public function testGetUIDIsNullWithoutASessionOrAToken(): void { + $this->activityManager->method('getCurrentUserId') + ->willThrowException(new \UnexpectedValueException('The token is invalid')); + $this->userSession->expects($this->never()) + ->method('getUser'); + + $instance = $this->getInstance(); + $this->assertNull($instance->getUID()); + } + protected function getShareMock(array $share): IShare|Exception|null { if (empty($share)) { return null;