fix(notifications): enforce settings access permission

Keep notification preferences hidden and inaccessible unless the user has notification access, while preserving the demo restriction.

Fixes #1057
This commit is contained in:
neon
2026-08-21 16:01:56 +00:00
parent 3b9bae7525
commit ffd1a4fd0f
10 changed files with 148 additions and 12 deletions
+4
View File
@@ -1,11 +1,15 @@
/// <reference types="vite/client" />
import 'vue-router'
import type { Permission } from '@bookorbit/types'
import type { RouteLocationNormalizedLoaded } from 'vue-router'
declare module 'vue-router' {
interface RouteMeta {
public?: boolean
requiredPermission?: Permission
forbiddenPermission?: Permission
permissionFallback?: string
maxWidth?: string
title?: string | ((to: RouteLocationNormalizedLoaded) => string)
}
+2 -2
View File
@@ -53,7 +53,7 @@ import NotificationSheet from '@/features/notifications/components/NotificationS
import { useNotifications } from '@/features/notifications/composables/useNotifications'
import { useWhatsNew } from '@/features/whats-new/composables/useWhatsNew'
import UserAvatar from '@/components/UserAvatar.vue'
import { DEFAULT_FORMAT_PRIORITY, LOCALE_LABELS, type Locale } from '@bookorbit/types'
import { DEFAULT_FORMAT_PRIORITY, LOCALE_LABELS, Permission, type Locale } from '@bookorbit/types'
import { useThemeStore } from '@/stores/theme'
import { useLocaleStore } from '@/stores/locale'
import { getFormatColor } from '@/features/book/lib/format-colors'
@@ -108,7 +108,7 @@ function navigateToAchievements() {
const canChangePassword = computed(
() => !isDemoRestrictedAccount.value && user.value?.provisioningMethod !== 'oidc' && user.value?.provisioningMethod !== 'shared',
)
const canAccessNotifications = computed(() => hasPermission('notification_access') && !isDemoRestrictedAccount.value)
const canAccessNotifications = computed(() => hasPermission(Permission.NotificationAccess) && !isDemoRestrictedAccount.value)
const GLOBAL_SEARCH_ROW_HEIGHT = 84
const GLOBAL_SEARCH_OVERSCAN = 4
const GLOBAL_SEARCH_VIEWPORT_HEIGHT = 512
@@ -1,6 +1,7 @@
import { describe, expect, it, vi, beforeEach } from 'vitest'
import { mount, RouterLinkStub } from '@vue/test-utils'
import { computed } from 'vue'
import { Permission } from '@bookorbit/types'
import SettingsNav from '../components/SettingsNav.vue'
const permState = {
@@ -76,12 +77,18 @@ describe('SettingsNav', () => {
expect(labels).toContain('Restrictions')
})
it('shows notifications for a normal account', () => {
expect(itemLabels(mountNav())).toContain('Notifications')
it('hides notifications without notification access', () => {
expect(itemLabels(mountNav())).not.toContain('Notifications')
})
it('hides notifications for a demo restricted account', () => {
expect(itemLabels(mountNav({ demo: true }))).not.toContain('Notifications')
it('shows notifications with notification access or superuser status', () => {
expect(itemLabels(mountNav({ perms: [Permission.NotificationAccess] }))).toContain('Notifications')
expect(itemLabels(mountNav({ su: true }))).toContain('Notifications')
})
it('hides notifications for a demo-restricted account even when otherwise allowed', () => {
expect(itemLabels(mountNav({ perms: [Permission.NotificationAccess], demo: true }))).not.toContain('Notifications')
expect(itemLabels(mountNav({ su: true, demo: true }))).not.toContain('Notifications')
})
})
@@ -242,6 +249,8 @@ describe('SettingsNav', () => {
it('never returns a page the user cannot open', async () => {
expect(await search(mountNav(), 'users')).toEqual([])
expect(await search(mountNav(), 'notifications')).toEqual([])
expect(await search(mountNav({ perms: [Permission.NotificationAccess] }), 'notifications')).toContain('Notifications')
})
it('shows an empty state when nothing matches', async () => {
@@ -1,6 +1,7 @@
import { describe, expect, it, vi } from 'vitest'
import { mount, RouterLinkStub } from '@vue/test-utils'
import { computed } from 'vue'
import { Permission } from '@bookorbit/types'
import SettingsSidebar from '../components/SettingsSidebar.vue'
const permState = {
@@ -21,10 +22,10 @@ vi.mock('@/features/auth/composables/usePermissions', () => ({
}),
}))
function mountSidebar(opts?: { rail?: boolean; su?: boolean; perms?: string[] }) {
function mountSidebar(opts?: { rail?: boolean; su?: boolean; perms?: string[]; demo?: boolean }) {
permState.isSuperuser = opts?.su ?? false
permState.permissions = opts?.perms ?? []
permState.demoRestricted = false
permState.demoRestricted = opts?.demo ?? false
return mount(SettingsSidebar, {
props: { isRail: opts?.rail ?? false },
global: {
@@ -83,4 +84,17 @@ describe('SettingsSidebar', () => {
expect(mountSidebar({ rail: true }).findAll('[data-testid="rail-group"]')).toHaveLength(1)
expect(mountSidebar({ rail: true, perms: ['manage_libraries'] }).findAll('[data-testid="rail-group"]')).toHaveLength(2)
})
it('applies notification access rules in collapsed rail mode', () => {
const labels = (opts?: Parameters<typeof mountSidebar>[0]) =>
mountSidebar({ ...opts, rail: true })
.findAll('[data-testid="settings-sidebar-rail-item"]')
.map((node) => node.text())
expect(labels()).not.toContain('Notifications')
expect(labels({ perms: [Permission.NotificationAccess] })).toContain('Notifications')
expect(labels({ su: true })).toContain('Notifications')
expect(labels({ perms: [Permission.NotificationAccess], demo: true })).not.toContain('Notifications')
expect(labels({ su: true, demo: true })).not.toContain('Notifications')
})
})
@@ -1,6 +1,7 @@
import { describe, expect, it } from 'vitest'
import type { RouteRecordRaw } from 'vue-router'
import { BookMarked, Highlighter, LibraryBig } from '@lucide/vue'
import { Permission } from '@bookorbit/types'
import { routes } from '@/router'
import { SETTINGS_NAV, findSettingsNavItem, firstVisibleSettingsRoute, visibleSettingsNav, type SettingsNavContext } from '../lib/settings-nav'
@@ -56,11 +57,31 @@ describe('visibleSettingsNav', () => {
expect(visibleSettingsNav(SUPERUSER).map((group) => group.id)).toEqual(['you', 'library', 'devices', 'server'])
})
it('filters children as well as top level items', () => {
const you = visibleSettingsNav({ ...NOBODY, isDemoRestricted: true })[0]
it('hides notifications without notification access', () => {
const you = visibleSettingsNav(NOBODY)[0]
expect(you?.items.map((item) => item.id)).not.toContain('notifications')
})
it('shows notifications with notification access or superuser status', () => {
const permitted = visibleSettingsNav({ ...NOBODY, permissions: [Permission.NotificationAccess] })[0]
const superuser = visibleSettingsNav(SUPERUSER)[0]
expect(permitted?.items.map((item) => item.id)).toContain('notifications')
expect(superuser?.items.map((item) => item.id)).toContain('notifications')
})
it('hides notifications from demo-restricted accounts even when otherwise allowed', () => {
const permittedDemo = visibleSettingsNav({
...NOBODY,
permissions: [Permission.NotificationAccess],
isDemoRestricted: true,
})[0]
const superuserDemo = visibleSettingsNav({ ...SUPERUSER, isDemoRestricted: true })[0]
expect(permittedDemo?.items.map((item) => item.id)).not.toContain('notifications')
expect(superuserDemo?.items.map((item) => item.id)).not.toContain('notifications')
})
it('only exposes the metadata pages the permission allows', () => {
const library = visibleSettingsNav({ ...NOBODY, permissions: ['manage_libraries'] }).find((group) => group.id === 'library')
const ids = library?.items.map((item) => item.id) ?? []
@@ -207,7 +207,7 @@ export const SETTINGS_NAV: readonly SettingsNavGroup[] = [
descriptionKey: 'settings.nav.descriptions.notifications',
icon: Bell,
keywords: 'notify email digest alerts events',
isVisible: (context) => !context.isDemoRestricted,
isVisible: (context) => !context.isDemoRestricted && (context.isSuperuser || context.permissions.includes(Permission.NotificationAccess)),
},
{
id: 'privacy',
@@ -1,5 +1,6 @@
import { describe, expect, it } from 'vitest'
import type { RouteRecordRaw } from 'vue-router'
import { Permission } from '@bookorbit/types'
import { routes } from '@/router'
type MissingRoute = {
@@ -45,6 +46,16 @@ describe('router title metadata', () => {
})
describe('router redirects', () => {
it('protects the notification settings route from direct navigation', () => {
const route = findRoute(routes, 'settings-notifications')
expect(route?.meta).toMatchObject({
requiredPermission: Permission.NotificationAccess,
forbiddenPermission: Permission.DemoRestricted,
permissionFallback: 'settings-account-profile',
})
})
it('redirects the legacy Integrations tab URL to the Readwise page', () => {
const route = findRoute(routes, 'settings-integrations')
expect(route?.redirect).toBeTypeOf('function')
@@ -1,5 +1,6 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { ref, type Ref } from 'vue'
import { Permission } from '@bookorbit/types'
import { registerAuthGuard } from './auth.guard'
@@ -8,6 +9,8 @@ const mocks = vi.hoisted(() => ({
openChangePassword: vi.fn<(required: boolean) => void>(),
user: null as unknown as Ref<{
isDefaultPassword: boolean
isSuperuser: boolean
permissions: string[]
provisioningMethod: string
settings: { achievementPreferences: { enabled: boolean } }
}>,
@@ -40,6 +43,8 @@ describe('registerAuthGuard', () => {
mocks.openChangePassword.mockReset()
mocks.user = ref({
isDefaultPassword: false,
isSuperuser: false,
permissions: [],
provisioningMethod: 'local',
settings: { achievementPreferences: { enabled: false } },
})
@@ -64,4 +69,57 @@ describe('registerAuthGuard', () => {
await expect(guard({ name: 'achievements', path: '/achievements', fullPath: '/achievements', meta: {} })).resolves.toBe(true)
})
it('redirects a direct route visit when the required permission is missing', async () => {
if (!guard) throw new Error('Expected guard to be registered')
await expect(
guard({
name: 'settings-notifications',
path: '/settings/account/notifications',
fullPath: '/settings/account/notifications',
meta: {
requiredPermission: Permission.NotificationAccess,
forbiddenPermission: Permission.DemoRestricted,
permissionFallback: 'settings-account-profile',
},
}),
).resolves.toEqual({ name: 'settings-account-profile' })
})
it('allows a direct route visit with notification access or superuser status', async () => {
if (!guard) throw new Error('Expected guard to be registered')
const route = {
name: 'settings-notifications',
path: '/settings/account/notifications',
fullPath: '/settings/account/notifications',
meta: { requiredPermission: Permission.NotificationAccess },
}
mocks.user.value.permissions = [Permission.NotificationAccess]
await expect(guard(route)).resolves.toBe(true)
mocks.user.value.permissions = []
mocks.user.value.isSuperuser = true
await expect(guard(route)).resolves.toBe(true)
})
it('redirects demo-restricted users even when notification access would otherwise allow the route', async () => {
if (!guard) throw new Error('Expected guard to be registered')
mocks.user.value.isSuperuser = true
mocks.user.value.permissions = [Permission.NotificationAccess, Permission.DemoRestricted]
await expect(
guard({
name: 'settings-notifications',
path: '/settings/account/notifications',
fullPath: '/settings/account/notifications',
meta: {
requiredPermission: Permission.NotificationAccess,
forbiddenPermission: Permission.DemoRestricted,
permissionFallback: 'settings-account-profile',
},
}),
).resolves.toEqual({ name: 'settings-account-profile' })
})
})
+13
View File
@@ -3,6 +3,10 @@ import { useAuth } from '@/features/auth/composables/useAuth'
import { useChangePasswordDialog } from '@/composables/useChangePasswordDialog'
import { useSetupStatus } from '@/features/auth/composables/useSetupStatus'
function permissionFallback(name: string | undefined) {
return name ? { name } : { path: '/' }
}
export function registerAuthGuard(router: Router): void {
router.beforeEach(async (to) => {
const { fetchSetupStatus, allowRegistration } = useSetupStatus()
@@ -41,6 +45,15 @@ export function registerAuthGuard(router: Router): void {
if (to.path !== '/') return { path: '/' }
}
const permissions = user.value.permissions ?? []
if (to.meta.forbiddenPermission && permissions.includes(to.meta.forbiddenPermission)) {
return permissionFallback(to.meta.permissionFallback)
}
if (to.meta.requiredPermission && !user.value.isSuperuser && !permissions.includes(to.meta.requiredPermission)) {
return permissionFallback(to.meta.permissionFallback)
}
if (to.name === 'achievements' && user.value.settings.achievementPreferences?.enabled === false) {
return { name: 'settings-account-profile' }
}
+7 -1
View File
@@ -6,6 +6,7 @@ import {
type RouteLocationNormalizedLoaded,
type RouteRecordRaw,
} from 'vue-router'
import { Permission } from '@bookorbit/types'
import { normalizeEmailTab } from '@/features/email/lib/email-tabs'
import { normalizeMetadataTab, type MetadataTab } from '@/features/settings/lib/metadata-tabs'
import { normalizeReaderTab, type ReaderTab } from '@/features/settings/lib/reader-tabs'
@@ -173,7 +174,12 @@ export const routes: RouteRecordRaw[] = [
name: 'settings-notifications',
component: () => import('@/features/notifications/components/NotificationPreferences.vue'),
props: { embedded: true },
meta: { title: () => t('titles.account.notifications') },
meta: {
requiredPermission: Permission.NotificationAccess,
forbiddenPermission: Permission.DemoRestricted,
permissionFallback: 'settings-account-profile',
title: () => t('titles.account.notifications'),
},
},
{
path: 'account/restrictions',