mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
refactor(server): fold the in-container bridge edges
This commit is contained in:
@@ -6,7 +6,7 @@ import fs from 'fs';
|
||||
import Database from 'better-sqlite3';
|
||||
import { db, closeDb, reinitialize } from '../../db/database';
|
||||
import { VALID_INTERVALS } from './auto-backup.settings';
|
||||
import { invalidatePermissionsCache } from '../permissions/permissions.bridge';
|
||||
import { invalidatePermissionsCache } from '../permissions/permissions-cache';
|
||||
import { pluginsCodeRoot, pluginsDataRoot } from '../plugins/paths';
|
||||
import { stageExtractedPluginTrees, applyStagedRestoreNow } from '../plugins/plugin-backup';
|
||||
import { snapshotAllPluginDataDbs } from '../plugins/host/plugin-data.service';
|
||||
|
||||
@@ -0,0 +1,27 @@
|
||||
import type { PermissionLevel } from './permissions.service';
|
||||
|
||||
/**
|
||||
* The permissions cache — module state on purpose, in its own home (the
|
||||
* oauth.pending-codes.ts precedent) so every path that must see one shared
|
||||
* cache actually does: the container PermissionsService singleton, the
|
||||
* permissions.bridge instance the MCP _shared helper reaches, and the plain
|
||||
* backup restore path (backup.impl.ts), which is free functions by design and
|
||||
* has nothing to hang an injected service off. A restore rewrites
|
||||
* app_settings under the request path's feet; invalidating here is what keeps
|
||||
* every reader consistent afterwards.
|
||||
*/
|
||||
let cache: Map<string, PermissionLevel> | null = null;
|
||||
|
||||
export function getPermissionsCache(): Map<string, PermissionLevel> | null {
|
||||
return cache;
|
||||
}
|
||||
|
||||
/** Install a fresh cache map and return it (the loader fills it in place). */
|
||||
export function setPermissionsCache(next: Map<string, PermissionLevel>): Map<string, PermissionLevel> {
|
||||
cache = next;
|
||||
return next;
|
||||
}
|
||||
|
||||
export function invalidatePermissionsCache(): void {
|
||||
cache = null;
|
||||
}
|
||||
@@ -9,19 +9,15 @@ import { PermissionsService } from './permissions.service';
|
||||
* Inside the container, inject PermissionsService instead. Delete this file
|
||||
* when the MCP/OAuth mount moves behind the container.
|
||||
*
|
||||
* The permissions cache is module-scoped in permissions.service.ts, so this
|
||||
* instance and the container singleton share one cache — invalidating through
|
||||
* either flushes both (backup restores depend on this).
|
||||
* The permissions cache is module-scoped in permissions-cache.ts, so this
|
||||
* instance and the container singleton share one cache (backup restores
|
||||
* invalidate it through that module directly).
|
||||
*
|
||||
* Module-level construction is safe: `db` is the reinitialize-proof Proxy onto
|
||||
* the shared better-sqlite3 singleton.
|
||||
*/
|
||||
const permissions = new PermissionsService(new DatabaseService(db));
|
||||
|
||||
export function invalidatePermissionsCache(): void {
|
||||
permissions.invalidatePermissionsCache();
|
||||
}
|
||||
|
||||
export function checkPermission(
|
||||
actionKey: string,
|
||||
userRole: string,
|
||||
|
||||
@@ -1,6 +1,11 @@
|
||||
import { Injectable } from '@nestjs/common';
|
||||
import { DatabaseService } from '../database/database.service';
|
||||
import { logError } from '../audit/audit-log.logger';
|
||||
import {
|
||||
getPermissionsCache,
|
||||
setPermissionsCache,
|
||||
invalidatePermissionsCache as invalidateSharedCache,
|
||||
} from './permissions-cache';
|
||||
|
||||
/**
|
||||
* Permission levels (hierarchical, higher includes lower):
|
||||
@@ -58,21 +63,22 @@ export const PERMISSION_ACTIONS: PermissionAction[] = [
|
||||
|
||||
const ACTIONS_MAP = new Map(PERMISSION_ACTIONS.map(a => [a.key, a]));
|
||||
|
||||
// In-memory cache, invalidated on save. Deliberately MODULE-scoped, not
|
||||
// instance state: two instances exist at runtime (the container singleton and
|
||||
// the permissions.bridge instance for out-of-container consumers), and legacy
|
||||
// callers — backupService invalidating after a restore, integration tests —
|
||||
// must flush the same cache the request path reads. Both instances wrap the
|
||||
// same shared connection, so a single cache is also the correct data shape.
|
||||
let cache: Map<string, PermissionLevel> | null = null;
|
||||
// The in-memory cache is deliberately MODULE-scoped, not instance state, and
|
||||
// lives in ./permissions-cache: two service instances exist at runtime (the
|
||||
// container singleton and the permissions.bridge instance for
|
||||
// out-of-container consumers), and the backup restore path — plain functions,
|
||||
// no DI — must flush the same cache the request path reads. Both instances
|
||||
// wrap the same shared connection, so a single cache is also the correct
|
||||
// data shape.
|
||||
|
||||
@Injectable()
|
||||
export class PermissionsService {
|
||||
constructor(private readonly dbs: DatabaseService) {}
|
||||
|
||||
private loadPermissions(): Map<string, PermissionLevel> {
|
||||
if (cache) return cache;
|
||||
cache = new Map<string, PermissionLevel>();
|
||||
const cached = getPermissionsCache();
|
||||
if (cached) return cached;
|
||||
const cache = setPermissionsCache(new Map<string, PermissionLevel>());
|
||||
try {
|
||||
const rows = this.dbs.all<{ key: string; value: string }>(
|
||||
"SELECT key, value FROM app_settings WHERE key LIKE 'perm_%'"
|
||||
@@ -98,7 +104,7 @@ export class PermissionsService {
|
||||
}
|
||||
|
||||
invalidatePermissionsCache(): void {
|
||||
cache = null;
|
||||
invalidateSharedCache();
|
||||
}
|
||||
|
||||
getPermissionLevel(actionKey: string): PermissionLevel {
|
||||
|
||||
@@ -1,9 +1,11 @@
|
||||
import { Module } from '@nestjs/common';
|
||||
import { AddonsModule } from '../addons/addons.module';
|
||||
import { SystemNoticesController } from './system-notices.controller';
|
||||
import { SystemNoticesService } from './system-notices.service';
|
||||
|
||||
/** System-notices domain (L2 leaf module). Registered in AppModule. */
|
||||
/** System-notices domain. Registered in AppModule. */
|
||||
@Module({
|
||||
imports: [AddonsModule],
|
||||
controllers: [SystemNoticesController],
|
||||
providers: [SystemNoticesService],
|
||||
})
|
||||
|
||||
@@ -1,16 +1,21 @@
|
||||
import { Injectable } from '@nestjs/common';
|
||||
import type { SystemNoticeDto } from '@trek/shared';
|
||||
import { getActiveNoticesFor, dismissNotice } from '../../systemNotices/service';
|
||||
import { AddonsService } from '../addons/addons.service';
|
||||
|
||||
/**
|
||||
* Thin Nest wrapper around the existing system-notices service. The condition
|
||||
* evaluation, version gating, sorting and dismissal persistence all stay in the
|
||||
* upstream service — this only adapts it for DI, so behaviour is unchanged.
|
||||
* upstream service — this only adapts it for DI (and threads the injected
|
||||
* addon-enablement check into the condition context, so the plain modules
|
||||
* underneath carry no bridge import), so behaviour is unchanged.
|
||||
*/
|
||||
@Injectable()
|
||||
export class SystemNoticesService {
|
||||
constructor(private readonly addons: AddonsService) {}
|
||||
|
||||
getActiveFor(userId: number): SystemNoticeDto[] {
|
||||
return getActiveNoticesFor(userId) as SystemNoticeDto[];
|
||||
return getActiveNoticesFor(userId, (addonId) => this.addons.isAddonEnabled(addonId)) as SystemNoticeDto[];
|
||||
}
|
||||
|
||||
dismiss(userId: number, noticeId: string): boolean {
|
||||
|
||||
@@ -1,11 +1,15 @@
|
||||
import semver from 'semver';
|
||||
import { isAddonEnabled } from '../nest/addons/addons.bridge.js';
|
||||
import type { NoticeCondition, SystemNotice } from './types.js';
|
||||
|
||||
interface ConditionContext {
|
||||
user: { login_count: number; first_seen_version: string; role: string; noTrips: number };
|
||||
currentAppVersion: string;
|
||||
now: Date;
|
||||
/**
|
||||
* Addon-enablement check, threaded in by the caller (the Nest service passes
|
||||
* its injected AddonsService) so this plain module carries no bridge import.
|
||||
*/
|
||||
addonEnabled: (addonId: string) => boolean;
|
||||
}
|
||||
|
||||
// Custom predicate registry — extensible without modifying this file
|
||||
@@ -48,7 +52,7 @@ function evaluateOne(condition: NoticeCondition, ctx: ConditionContext): boolean
|
||||
return condition.roles.includes(ctx.user.role as 'admin' | 'user');
|
||||
|
||||
case 'addonEnabled':
|
||||
return isAddonEnabled(condition.addonId);
|
||||
return ctx.addonEnabled(condition.addonId);
|
||||
|
||||
case 'custom': {
|
||||
const fn = customPredicates.get(condition.id);
|
||||
|
||||
@@ -36,7 +36,10 @@ function severityWeight(s: string): number {
|
||||
return s === 'critical' ? 2 : s === 'warn' ? 1 : 0;
|
||||
}
|
||||
|
||||
export function getActiveNoticesFor(userId: number): SystemNoticeDTO[] {
|
||||
export function getActiveNoticesFor(
|
||||
userId: number,
|
||||
addonEnabled: (addonId: string) => boolean
|
||||
): SystemNoticeDTO[] {
|
||||
const user = db.prepare(
|
||||
'SELECT login_count, first_seen_version, role FROM users WHERE id = ?'
|
||||
).get(userId) as { login_count: number; first_seen_version: string; role: string } | undefined;
|
||||
@@ -56,7 +59,7 @@ export function getActiveNoticesFor(userId: number): SystemNoticeDTO[] {
|
||||
|
||||
const now = new Date();
|
||||
const currentAppVersion = getCurrentAppVersion();
|
||||
const ctx = { user: { ...user, noTrips: tripCount }, currentAppVersion, now };
|
||||
const ctx = { user: { ...user, noTrips: tripCount }, currentAppVersion, now, addonEnabled };
|
||||
const appVer = semver.coerce(currentAppVersion)?.version ?? '0.0.0';
|
||||
|
||||
const isStillDismissed = (n: SystemNotice): boolean => {
|
||||
|
||||
@@ -30,6 +30,7 @@ vi.mock('../../src/systemNotices/service', () => ({
|
||||
}));
|
||||
|
||||
import { SystemNoticesModule } from '../../src/nest/system-notices/system-notices.module';
|
||||
import { DatabaseModule } from '../../src/nest/database/database.module';
|
||||
import { TrekExceptionFilter } from '../../src/nest/common/trek-exception.filter';
|
||||
|
||||
const notice = {
|
||||
@@ -42,7 +43,10 @@ describe('System-notices e2e (real auth guard + temp SQLite)', () => {
|
||||
let app: Awaited<ReturnType<typeof build>>;
|
||||
|
||||
async function build() {
|
||||
const moduleRef = await Test.createTestingModule({ imports: [SystemNoticesModule] }).compile();
|
||||
// DatabaseModule is @Global in the real app; a partial graph has to
|
||||
// provide it for SystemNoticesModule's AddonsModule import (the
|
||||
// addons.e2e precedent).
|
||||
const moduleRef = await Test.createTestingModule({ imports: [DatabaseModule, SystemNoticesModule] }).compile();
|
||||
const nest = moduleRef.createNestApplication();
|
||||
nest.use(cookieParser());
|
||||
nest.useGlobalFilters(new TrekExceptionFilter());
|
||||
@@ -71,7 +75,7 @@ describe('System-notices e2e (real auth guard + temp SQLite)', () => {
|
||||
const res = await request(server).get('/api/system-notices/active').set('Cookie', sessionCookie(1));
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body).toEqual([notice]);
|
||||
expect(mockGetActive).toHaveBeenCalledWith(1);
|
||||
expect(mockGetActive).toHaveBeenCalledWith(1, expect.any(Function));
|
||||
});
|
||||
|
||||
it('204 with no body on a successful dismiss', async () => {
|
||||
|
||||
@@ -424,7 +424,7 @@ describe('Reorder budget items', () => {
|
||||
|
||||
// Restrict budget_edit to trip_owner only
|
||||
testDb.prepare("INSERT OR REPLACE INTO app_settings (key, value) VALUES ('perm_budget_edit', 'trip_owner')").run();
|
||||
const { invalidatePermissionsCache } = await import('../../src/nest/permissions/permissions.bridge');
|
||||
const { invalidatePermissionsCache } = await import('../../src/nest/permissions/permissions-cache');
|
||||
invalidatePermissionsCache();
|
||||
|
||||
const res = await request(app)
|
||||
@@ -527,7 +527,7 @@ describe('Budget edit permission enforcement', () => {
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
addTripMember(testDb, trip.id, member.id);
|
||||
|
||||
const { invalidatePermissionsCache } = await import('../../src/nest/permissions/permissions.bridge');
|
||||
const { invalidatePermissionsCache } = await import('../../src/nest/permissions/permissions-cache');
|
||||
testDb.prepare("INSERT OR REPLACE INTO app_settings (key, value) VALUES ('perm_budget_edit', 'trip_owner')").run();
|
||||
invalidatePermissionsCache();
|
||||
|
||||
@@ -548,7 +548,7 @@ describe('Budget edit permission enforcement', () => {
|
||||
addTripMember(testDb, trip.id, member.id);
|
||||
createBudgetItem(testDb, trip.id, { name: 'Item', category: 'Transport' });
|
||||
|
||||
const { invalidatePermissionsCache } = await import('../../src/nest/permissions/permissions.bridge');
|
||||
const { invalidatePermissionsCache } = await import('../../src/nest/permissions/permissions-cache');
|
||||
testDb.prepare("INSERT OR REPLACE INTO app_settings (key, value) VALUES ('perm_budget_edit', 'trip_owner')").run();
|
||||
invalidatePermissionsCache();
|
||||
|
||||
|
||||
@@ -72,7 +72,7 @@ import {
|
||||
addJourneyContributor,
|
||||
} from '../helpers/factories';
|
||||
import { authCookie } from '../helpers/auth';
|
||||
import { invalidatePermissionsCache } from '../../src/nest/permissions/permissions.bridge';
|
||||
import { invalidatePermissionsCache } from '../../src/nest/permissions/permissions-cache';
|
||||
|
||||
let nestApp: INestApplication;
|
||||
let app: Application;
|
||||
|
||||
@@ -56,7 +56,7 @@ import { resetTestDb, resetRateLimits } from '../helpers/test-db';
|
||||
import { createUser, createAdmin, createTrip, createPlace, addTripMember } from '../helpers/factories';
|
||||
import { authCookie } from '../helpers/auth';
|
||||
import { PlacesService } from '../../src/nest/places/places.service';
|
||||
import { invalidatePermissionsCache } from '../../src/nest/permissions/permissions.bridge';
|
||||
import { invalidatePermissionsCache } from '../../src/nest/permissions/permissions-cache';
|
||||
|
||||
let nestApp: INestApplication;
|
||||
let app: Application;
|
||||
|
||||
@@ -307,7 +307,7 @@ describe('Shared trip access', () => {
|
||||
addTripMember(testDb, trip.id, member.id);
|
||||
await request(app).post(`/api/trips/${trip.id}/share-link`).set('Cookie', authCookie(owner.id)).send({});
|
||||
|
||||
const { invalidatePermissionsCache } = await import('../../src/nest/permissions/permissions.bridge');
|
||||
const { invalidatePermissionsCache } = await import('../../src/nest/permissions/permissions-cache');
|
||||
testDb.prepare("INSERT OR REPLACE INTO app_settings (key, value) VALUES ('perm_share_manage', 'trip_member')").run();
|
||||
invalidatePermissionsCache();
|
||||
try {
|
||||
|
||||
@@ -44,7 +44,7 @@ import { runMigrations } from '../../src/db/migrations';
|
||||
import { resetTestDb, resetRateLimits } from '../helpers/test-db';
|
||||
import { createUser, createTrip, addTripMember } from '../helpers/factories';
|
||||
import { authCookie } from '../helpers/auth';
|
||||
import { invalidatePermissionsCache } from '../../src/nest/permissions/permissions.bridge';
|
||||
import { invalidatePermissionsCache } from '../../src/nest/permissions/permissions-cache';
|
||||
|
||||
let nestApp: INestApplication;
|
||||
let app: Application;
|
||||
|
||||
@@ -57,7 +57,7 @@ import { runMigrations } from '../../src/db/migrations';
|
||||
import { resetTestDb, resetRateLimits } from '../helpers/test-db';
|
||||
import { createUser, createAdmin, createTrip, addTripMember, createPlace, createReservation, createTag, createDayAccommodation, createBudgetItem, createPackingItem, createDayNote, createDayAssignment } from '../helpers/factories';
|
||||
import { authCookie } from '../helpers/auth';
|
||||
import { invalidatePermissionsCache } from '../../src/nest/permissions/permissions.bridge';
|
||||
import { invalidatePermissionsCache } from '../../src/nest/permissions/permissions-cache';
|
||||
|
||||
let nestApp: INestApplication;
|
||||
let app: Application;
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { runMigrations } from '../../../src/db/migrations';
|
||||
import { createTables } from '../../../src/db/schema';
|
||||
import { invalidatePermissionsCache } from '../../../src/nest/permissions/permissions.bridge';
|
||||
import { invalidatePermissionsCache } from '../../../src/nest/permissions/permissions-cache';
|
||||
import { PermissionsService } from '../../../src/nest/permissions/permissions.service';
|
||||
import { DatabaseService } from '../../../src/nest/database/database.service';
|
||||
import { addTripMember, createDay, createTrip, createUser } from '../../helpers/factories';
|
||||
|
||||
@@ -43,10 +43,8 @@ import { runMigrations } from '../../../src/db/migrations';
|
||||
import { logError } from '../../../src/nest/audit/audit-log.logger';
|
||||
import { DatabaseService } from '../../../src/nest/database/database.service';
|
||||
import { PermissionsService, PERMISSION_ACTIONS } from '../../../src/nest/permissions/permissions.service';
|
||||
import {
|
||||
checkPermission as bridgeCheckPermission,
|
||||
invalidatePermissionsCache as bridgeInvalidatePermissionsCache,
|
||||
} from '../../../src/nest/permissions/permissions.bridge';
|
||||
import { checkPermission as bridgeCheckPermission } from '../../../src/nest/permissions/permissions.bridge';
|
||||
import { invalidatePermissionsCache as invalidateSharedCache } from '../../../src/nest/permissions/permissions-cache';
|
||||
|
||||
const svc = new PermissionsService(new DatabaseService(testDb));
|
||||
|
||||
@@ -253,11 +251,12 @@ describe('permissions.bridge delegation', () => {
|
||||
// (checkPermission for a plain member flips with the stored level).
|
||||
svc.savePermissions({ trip_edit: 'trip_member' });
|
||||
expect(bridgeCheckPermission('trip_edit', 'user', 10, 20, true)).toBe(true);
|
||||
// Raw SQL write, then invalidate through the BRIDGE — the DI instance
|
||||
// must serve the fresh value (backup-restore relies on exactly this).
|
||||
// Raw SQL write, then invalidate through permissions-cache — the plain
|
||||
// function backup.impl.ts calls after a restore. Both service instances
|
||||
// must serve the fresh value afterwards.
|
||||
testDb.prepare('INSERT OR REPLACE INTO app_settings (key, value) VALUES (?, ?)').run('perm_trip_edit', 'trip_owner');
|
||||
expect(svc.getPermissionLevel('trip_edit')).toBe('trip_member'); // still cached
|
||||
bridgeInvalidatePermissionsCache();
|
||||
invalidateSharedCache();
|
||||
expect(svc.getPermissionLevel('trip_edit')).toBe('trip_owner');
|
||||
expect(bridgeCheckPermission('trip_edit', 'user', 10, 20, true)).toBe(false);
|
||||
});
|
||||
|
||||
@@ -25,6 +25,9 @@ const baseCtx = {
|
||||
user: { login_count: 5, first_seen_version: '1.0.0', role: 'user', noTrips: 1 },
|
||||
currentAppVersion: '2.0.0',
|
||||
now: new Date('2026-06-01T00:00:00Z'),
|
||||
// Threaded in by the caller since the addons.bridge import left this module;
|
||||
// false keeps every case below reading as it did.
|
||||
addonEnabled: () => false,
|
||||
};
|
||||
|
||||
describe('firstLogin', () => {
|
||||
@@ -100,3 +103,12 @@ describe('empty conditions', () => {
|
||||
expect(evaluate(baseNotice, baseCtx)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('addonEnabled', () => {
|
||||
it('asks the threaded-in check with the condition addon id', () => {
|
||||
const notice = noticeWith({ kind: 'addonEnabled', addonId: 'journey' });
|
||||
expect(evaluate(notice, { ...baseCtx, addonEnabled: (id) => id === 'journey' })).toBe(true);
|
||||
expect(evaluate(notice, { ...baseCtx, addonEnabled: (id) => id === 'vacay' })).toBe(false);
|
||||
expect(evaluate(notice, baseCtx)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user