Skip to content
This repository was archived by the owner on May 29, 2026. It is now read-only.

Commit 35aae9f

Browse files
committed
fix(security): NotificationHistoryController — force recipient = caller for non-admins
`index()` is `#[NoAdminRequired]` and forwarded the caller-supplied `recipient` filter straight to the mapper with no scoping. Any authed Bob could call `/api/notification-history?recipient=alice` and read Alice's full history — channels, status, ruleId, dispatch timestamps, across tenants. This is the privacy/recon persistence vector flagged in the strict review. Fix: when the caller is not admin, force `recipient = currentUid` regardless of any value they passed; admins keep the free filter so operators can still audit. Existing 500-row page cap stays. Refs: #1419 review (blocker 3) — discussion_r3187494421
1 parent 265ee3b commit 35aae9f

1 file changed

Lines changed: 27 additions & 3 deletions

File tree

lib/Controller/NotificationHistoryController.php

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,11 @@
2929

3030
use OCA\OpenRegister\Db\NotificationHistoryMapper;
3131
use OCP\AppFramework\Controller;
32+
use OCP\AppFramework\Http;
3233
use OCP\AppFramework\Http\JSONResponse;
34+
use OCP\IGroupManager;
3335
use OCP\IRequest;
36+
use OCP\IUserSession;
3437

3538
/**
3639
* Class NotificationHistoryController.
@@ -49,7 +52,9 @@ class NotificationHistoryController extends Controller
4952
public function __construct(
5053
string $appName,
5154
IRequest $request,
52-
private readonly NotificationHistoryMapper $mapper
55+
private readonly NotificationHistoryMapper $mapper,
56+
private readonly IUserSession $userSession,
57+
private readonly IGroupManager $groupManager
5358
) {
5459
parent::__construct(appName: $appName, request: $request);
5560

@@ -70,9 +75,28 @@ public function __construct(
7075
*/
7176
public function index(): JSONResponse
7277
{
78+
$user = $this->userSession->getUser();
79+
if ($user === null) {
80+
return new JSONResponse(
81+
data: ['error' => 'Authentication required'],
82+
statusCode: Http::STATUS_UNAUTHORIZED
83+
);
84+
}
85+
7386
$filters = $this->extractFilters();
74-
$limit = $this->resolveLimit();
75-
$offset = $this->resolveOffset();
87+
88+
// SECURITY: non-admins may only read their OWN history. Force the
89+
// `recipient` filter to the current UID regardless of any value
90+
// the caller supplied — without this, any authed Bob could query
91+
// `?recipient=alice` and read Alice's full notification stream
92+
// (recipient list, channels, ruleId, dispatch timestamps), a
93+
// privacy + recon vector across tenants.
94+
if ($this->groupManager->isAdmin($user->getUID()) === false) {
95+
$filters['recipient'] = $user->getUID();
96+
}
97+
98+
$limit = $this->resolveLimit();
99+
$offset = $this->resolveOffset();
76100

77101
$results = $this->mapper->findFiltered(filters: $filters, limit: $limit, offset: $offset);
78102
$total = $this->mapper->countFiltered(filters: $filters);

0 commit comments

Comments
 (0)