refactor(notifications): bell loaders read through owner access, no bypass
countUnreadNotifications/loadNotificationList took a caller-supplied recipientID with module-level admin bypass — one misuse away from reading someone else's inbox. They now take the actor (CampaignUser) and read with overrideAccess: false under canReadOwnNotifications, keeping the explicit recipient filter as a second fence. Pin: owner isolation at the loader level in campaignNotifications.int.spec.ts. Co-authored-by: Francisco Solla <fsolla@users.noreply.github.com>
This commit is contained in:
parent
6705db80eb
commit
7a876ec334
3 changed files with 59 additions and 12 deletions
|
|
@ -43,7 +43,7 @@ export const loadNotificationBellData = async (): Promise<NotificationBellData |
|
|||
if (!user) return null
|
||||
|
||||
const payload = await getPayload({ config })
|
||||
const unreadCount = await countUnreadNotifications(payload, user.id)
|
||||
const unreadCount = await countUnreadNotifications(payload, user)
|
||||
|
||||
return {
|
||||
unreadCount,
|
||||
|
|
@ -56,7 +56,7 @@ export const listCampaignNotifications = async (): Promise<NotificationListItem[
|
|||
if (!user) return []
|
||||
|
||||
const payload = await getPayload({ config })
|
||||
return loadNotificationList(payload, user.id)
|
||||
return loadNotificationList(payload, user)
|
||||
}
|
||||
|
||||
export const markAllCampaignNotificationsRead = async () =>
|
||||
|
|
|
|||
|
|
@ -1,13 +1,13 @@
|
|||
import 'server-only'
|
||||
|
||||
/** Trusted server reads in this module use admin bypass (`overrideAccess: true`). */
|
||||
import type { Payload } from 'payload'
|
||||
|
||||
import type {
|
||||
NotificationListItem,
|
||||
NotificationPayload,
|
||||
NotificationType,
|
||||
} from '@/lib/notificationContract'
|
||||
import type { Notification } from '@/payload-types'
|
||||
import type { CampaignUser, Notification } from '@/payload-types'
|
||||
|
||||
export type { NotificationListItem } from '@/lib/notificationContract'
|
||||
|
||||
|
|
@ -33,33 +33,40 @@ const toNotificationListItem = (doc: Notification): NotificationListItem | null
|
|||
}
|
||||
}
|
||||
|
||||
// Reads go through collection access (`canReadOwnNotifications` scopes to the
|
||||
// recipient) with the explicit recipient filter as a second fence: the actor
|
||||
// is the only thing these loaders accept, so a caller cannot ask for someone
|
||||
// else's inbox. No admin bypass — that is what the create/update paths are
|
||||
// for, and they live in the domain hooks/actions with their own justifications.
|
||||
export const countUnreadNotifications = async (
|
||||
payload: import('payload').Payload,
|
||||
recipientID: number,
|
||||
payload: Payload,
|
||||
user: CampaignUser,
|
||||
): Promise<number> => {
|
||||
const result = await payload.count({
|
||||
collection: 'notification',
|
||||
where: {
|
||||
and: [{ recipient: { equals: recipientID } }, { readAt: { exists: false } }],
|
||||
and: [{ recipient: { equals: user.id } }, { readAt: { exists: false } }],
|
||||
},
|
||||
overrideAccess: true,
|
||||
user,
|
||||
overrideAccess: false,
|
||||
})
|
||||
|
||||
return result.totalDocs
|
||||
}
|
||||
|
||||
export const loadNotificationList = async (
|
||||
payload: import('payload').Payload,
|
||||
recipientID: number,
|
||||
payload: Payload,
|
||||
user: CampaignUser,
|
||||
limit = 40,
|
||||
): Promise<NotificationListItem[]> => {
|
||||
const result = await payload.find({
|
||||
collection: 'notification',
|
||||
where: { recipient: { equals: recipientID } },
|
||||
where: { recipient: { equals: user.id } },
|
||||
depth: 0,
|
||||
limit,
|
||||
sort: '-createdAt',
|
||||
overrideAccess: true,
|
||||
user,
|
||||
overrideAccess: false,
|
||||
})
|
||||
|
||||
return result.docs
|
||||
|
|
|
|||
|
|
@ -4,6 +4,10 @@ import { getPayload, type Payload } from 'payload'
|
|||
import { beforeAll, describe, expect, it } from 'vitest'
|
||||
|
||||
import config from '@/payload.config'
|
||||
import {
|
||||
countUnreadNotifications,
|
||||
loadNotificationList,
|
||||
} from '@/utilities/notification/notificationList'
|
||||
|
||||
import { installCampaignFixtures } from '../helpers/campaignFixtures'
|
||||
|
||||
|
|
@ -69,6 +73,42 @@ describe('notification access', () => {
|
|||
|
||||
expect(visible.docs.map((doc) => doc.id)).toEqual([own.id])
|
||||
})
|
||||
|
||||
it('reads the bell loaders through owner-scoped access (no bypass)', async () => {
|
||||
const fixtures = campaignFixtures()
|
||||
const [owner, other] = await Promise.all([
|
||||
fixtures.createCampaignUser('advisor'),
|
||||
fixtures.createCampaignUser('advisor'),
|
||||
])
|
||||
|
||||
const own = await payload.create({
|
||||
collection: 'notification',
|
||||
data: {
|
||||
recipient: owner.id,
|
||||
type: 'municipality_update',
|
||||
payload: { title: 'Sino — própria', detail: 'Detalhe', href: '/campanha' },
|
||||
},
|
||||
overrideAccess: true,
|
||||
})
|
||||
await payload.create({
|
||||
collection: 'notification',
|
||||
data: {
|
||||
recipient: other.id,
|
||||
type: 'new_supporter',
|
||||
payload: { title: 'Sino — alheia', detail: 'Detalhe', href: '/campanha' },
|
||||
},
|
||||
overrideAccess: true,
|
||||
})
|
||||
|
||||
// The loaders take the actor, not a recipientID — a caller cannot ask for
|
||||
// someone else's inbox (Pass 4: the previous recipientID+bypass shape
|
||||
// trusted every call site).
|
||||
await expect(loadNotificationList(payload, owner)).resolves.toEqual([
|
||||
expect.objectContaining({ id: own.id }),
|
||||
])
|
||||
await expect(countUnreadNotifications(payload, owner)).resolves.toBe(1)
|
||||
await expect(countUnreadNotifications(payload, other)).resolves.toBe(1)
|
||||
})
|
||||
})
|
||||
|
||||
describe('pushSubscription access', () => {
|
||||
|
|
|
|||
Loading…
Reference in a new issue