From 55cf5cefb748990a95c0ff3c9dd51f57a2153e4b Mon Sep 17 00:00:00 2001 From: David Deal Date: Tue, 23 Jun 2026 09:47:08 -0700 Subject: [PATCH 01/10] feat(marketing): enforce FGA access on marketing BFF endpoints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add server-side authorization for the Marketing Impact and Campaigns BFF endpoints (LFXV2-2235). Previously these were UI-gated only. Changes: - Extend AccessCheckAccessType with marketing_dashboard_viewer, campaign_viewer, campaign_manager - Add Project interface fields for the marketing access probe (marketingDashboardViewer, campaignViewer, campaignManager) - Add AccessCheckService.checkMultipleAccess() for batched multi-relation checks on a single resource (avoids Map-by-id collision bug in checkAccess()) - New requireProjectAccess() middleware factory: reads foundationSlug from req.query, resolves via NATS (cached on req to prevent N+1), checks FGA relation fail-closed; gated behind MARKETING_ACCESS_ENFORCEMENT=true env flag for safe rollout - Mount marketing_dashboard_viewer on the 5 Marketing Impact analytics endpoints; campaign_viewer on GET campaign routes; campaign_manager on write routes - Wire marketing access probe through project controller and service (?marketing_access=true opt-in flag) - CampaignService sends foundationSlug on all campaign requests so the server can resolve and check the selected foundation No ED-persona fast-path — FGA relation is sole source of truth. Deploy flag-gated until tuples are confirmed across all foundations. ED-view enforcement tracked in LFXV2-2483; UI guards in LFXV2-2236. Refs: LFXV2-2235 Signed-off-by: David Deal --- .../app/shared/services/campaign.service.ts | 66 +++++--- .../server/controllers/project.controller.ts | 5 +- .../require-project-access.middleware.ts | 147 ++++++++++++++++++ .../src/server/routes/analytics.route.ts | 17 +- .../src/server/routes/campaigns.route.ts | 50 ++++-- .../server/services/access-check.service.ts | 79 ++++++++++ .../src/server/services/project.service.ts | 74 +++++++-- .../src/interfaces/access-check.interface.ts | 9 +- .../src/interfaces/project.interface.ts | 15 ++ 9 files changed, 397 insertions(+), 65 deletions(-) create mode 100644 apps/lfx-one/src/server/middleware/require-project-access.middleware.ts diff --git a/apps/lfx-one/src/app/shared/services/campaign.service.ts b/apps/lfx-one/src/app/shared/services/campaign.service.ts index 0770b8c95..95f8add63 100644 --- a/apps/lfx-one/src/app/shared/services/campaign.service.ts +++ b/apps/lfx-one/src/app/shared/services/campaign.service.ts @@ -28,29 +28,42 @@ import { } from '@lfx-one/shared/interfaces'; import { exhaustMap, last, map, Observable, of, take, takeWhile, timer } from 'rxjs'; +import { ProjectContextService } from './project-context.service'; import { SseService } from './sse.service'; @Injectable({ providedIn: 'root' }) export class CampaignService { private readonly http = inject(HttpClient); private readonly sse = inject(SseService); + private readonly projectContextService = inject(ProjectContextService); + + /** + * Returns the `foundationSlug` of the currently selected foundation. + * Sent on every campaign request so the server can resolve the foundation + * project and enforce the campaign_viewer / campaign_manager FGA relation. + * See LFXV2-2235. + */ + private get foundationSlug(): string | undefined { + return this.projectContextService.selectedFoundation()?.slug ?? undefined; + } public generateBrief(request: CampaignBriefRequest): Observable> { - return this.sse.connect('/api/campaigns/brief/generate', { - method: 'POST', - body: request, - }); + const slug = this.foundationSlug; + const url = slug ? `/api/campaigns/brief/generate?foundationSlug=${encodeURIComponent(slug)}` : '/api/campaigns/brief/generate'; + return this.sse.connect(url, { method: 'POST', body: request }); } public refineBrief(request: CampaignBriefRefineRequest): Observable> { - return this.sse.connect('/api/campaigns/brief/refine', { - method: 'POST', - body: request, - }); + const slug = this.foundationSlug; + const url = slug ? `/api/campaigns/brief/refine?foundationSlug=${encodeURIComponent(slug)}` : '/api/campaigns/brief/refine'; + return this.sse.connect(url, { method: 'POST', body: request }); } public createCampaign(request: CampaignCreateRequest): Observable<{ jobId: string; result?: CampaignCreateResponse; error?: string }> { - return this.http.post<{ jobId: string; result?: CampaignCreateResponse; error?: string }>('/api/campaigns/create', request); + const slug = this.foundationSlug; + return this.http.post<{ jobId: string; result?: CampaignCreateResponse; error?: string }>('/api/campaigns/create', request, { + ...(slug && { params: { foundationSlug: slug } }), + }); } public getCreateResult(jobId: string): Observable { @@ -70,58 +83,67 @@ export class CampaignService { } public getMonitorData(days: number = 30): Observable { - return this.http.get('/api/campaigns/monitor', { params: { days } }); + return this.http.get('/api/campaigns/monitor', { params: { days, ...(this.foundationSlug && { foundationSlug: this.foundationSlug }) } }); } public getLinkedInAccounts(): Observable { - return this.http.get('/api/campaigns/linkedin/accounts'); + return this.http.get('/api/campaigns/linkedin/accounts', { params: { ...(this.foundationSlug && { foundationSlug: this.foundationSlug }) } }); } public getLinkedInMonitorData(accountKey: string, days: number = 30): Observable { - return this.http.get('/api/campaigns/linkedin/monitor', { params: { days, accountKey } }); + return this.http.get('/api/campaigns/linkedin/monitor', { params: { days, accountKey, ...(this.foundationSlug && { foundationSlug: this.foundationSlug }) } }); } public getRedditAccounts(): Observable { - return this.http.get('/api/campaigns/reddit/accounts'); + return this.http.get('/api/campaigns/reddit/accounts', { params: { ...(this.foundationSlug && { foundationSlug: this.foundationSlug }) } }); } public getRedditMonitorData(accountKey: string, days: number = 30): Observable { - return this.http.get('/api/campaigns/reddit/monitor', { params: { days, accountKey } }); + return this.http.get('/api/campaigns/reddit/monitor', { params: { days, accountKey, ...(this.foundationSlug && { foundationSlug: this.foundationSlug }) } }); } public getMetaAccounts(): Observable { - return this.http.get('/api/campaigns/meta/accounts'); + return this.http.get('/api/campaigns/meta/accounts', { params: { ...(this.foundationSlug && { foundationSlug: this.foundationSlug }) } }); } public getMetaMonitorData(accountKey: string, days: number = 30): Observable { - return this.http.get('/api/campaigns/meta/monitor', { params: { days, accountKey } }); + return this.http.get('/api/campaigns/meta/monitor', { params: { days, accountKey, ...(this.foundationSlug && { foundationSlug: this.foundationSlug }) } }); } public getKeywords(days: number = 30): Observable { - return this.http.get('/api/campaigns/keywords', { params: { days } }); + return this.http.get('/api/campaigns/keywords', { params: { days, ...(this.foundationSlug && { foundationSlug: this.foundationSlug }) } }); } public getAudience(days: number = 30): Observable { - return this.http.get('/api/campaigns/audience', { params: { days } }); + return this.http.get('/api/campaigns/audience', { params: { days, ...(this.foundationSlug && { foundationSlug: this.foundationSlug }) } }); } public lookupHubSpotUtm(eventName: string): Observable { - return this.http.get('/api/campaigns/hubspot/utm', { params: { event_name: eventName } }); + return this.http.get('/api/campaigns/hubspot/utm', { params: { event_name: eventName, ...(this.foundationSlug && { foundationSlug: this.foundationSlug }) } }); } public createHubSpotUtm(eventName: string): Observable { - return this.http.post('/api/campaigns/hubspot/utm/create', {}, { params: { event_name: eventName } }); + const slug = this.foundationSlug; + return this.http.post('/api/campaigns/hubspot/utm/create', {}, { params: { event_name: eventName, ...(slug && { foundationSlug: slug }) } }); } public executeKeywordActions(request: BulkKeywordActionRequest): Observable { - return this.http.post('/api/campaigns/keywords/actions', request); + const slug = this.foundationSlug; + return this.http.post('/api/campaigns/keywords/actions', request, { + ...(slug && { params: { foundationSlug: slug } }), + }); } private pollJobStatus(jobId: string): Observable { const maxPolls = Math.ceil(300_000 / CAMPAIGN_JOB_POLL_INTERVAL_MS); + const slug = this.foundationSlug; return timer(0, CAMPAIGN_JOB_POLL_INTERVAL_MS).pipe( take(maxPolls), - exhaustMap(() => this.http.get(`/api/campaigns/jobs/${encodeURIComponent(jobId)}`)), + exhaustMap(() => + this.http.get(`/api/campaigns/jobs/${encodeURIComponent(jobId)}`, { + params: { ...(slug && { foundationSlug: slug }) }, + }) + ), takeWhile((status) => status.status === 'running', true) ); } diff --git a/apps/lfx-one/src/server/controllers/project.controller.ts b/apps/lfx-one/src/server/controllers/project.controller.ts index 1d0a8a606..531af9ac3 100644 --- a/apps/lfx-one/src/server/controllers/project.controller.ts +++ b/apps/lfx-one/src/server/controllers/project.controller.ts @@ -111,17 +111,18 @@ export class ProjectController { } const includeMeetingCoordinator = req.query['meeting_coordinator'] === 'true'; + const includeMarketingAccess = req.query['marketing_access'] === 'true'; // Check if slug is a uuid if (isUuid(slug)) { // If the slug is a uuid, get the project by id - const project = await this.projectService.getProjectById(req, slug, true, includeMeetingCoordinator); + const project = await this.projectService.getProjectById(req, slug, true, includeMeetingCoordinator, includeMarketingAccess); res.json(project); return; } // If the slug is not a uuid, get the project by slug - const project = await this.projectService.getProjectBySlug(req, slug, includeMeetingCoordinator); + const project = await this.projectService.getProjectBySlug(req, slug, includeMeetingCoordinator, includeMarketingAccess); // Log the success logger.success(req, 'get_project_by_slug', startTime, { diff --git a/apps/lfx-one/src/server/middleware/require-project-access.middleware.ts b/apps/lfx-one/src/server/middleware/require-project-access.middleware.ts new file mode 100644 index 000000000..ac14e9075 --- /dev/null +++ b/apps/lfx-one/src/server/middleware/require-project-access.middleware.ts @@ -0,0 +1,147 @@ +// Copyright The Linux Foundation and each contributor to LFX. +// SPDX-License-Identifier: MIT + +import { AccessCheckAccessType } from '@lfx-one/shared/interfaces'; +import { NextFunction, Request, RequestHandler, Response } from 'express'; + +import { AuthorizationError } from '../errors'; +import { logger } from '../services/logger.service'; +import { AccessCheckService } from '../services/access-check.service'; +import { ProjectService } from '../services/project.service'; + +/** + * SLUG_PATTERN matches valid foundation/project slugs (lowercase alphanumeric and hyphens). + * Must match the pattern enforced in analytics.controller.ts. + */ +const SLUG_PATTERN = /^[a-z0-9-]+$/; + +/** + * Symbol used to cache slug→UID lookups on the request object. + * Prevents N+1 NATS calls when multiple middleware-gated analytics endpoints + * fire in parallel on the same page load. + */ +const PROJECT_UID_CACHE = Symbol('projectUidCache'); + +/** + * Factory that returns an Express middleware enforcing an FGA relation on the + * foundation identified by `foundationSlug` in the request query string. + * + * Gate logic (all fail-closed): + * 1. Read `foundationSlug` from `req.query`; validate format. + * 2. Resolve slug → project UID via NATS (result cached on `req` to amortise + * parallel analytics calls that all share the same slug). + * 3. Check the FGA relation via `/access-check` on LFX_V2_SERVICE. + * 4. Call `next()` on success; `next(AuthorizationError)` on any failure. + * + * Rollout flag: the enforcement is **disabled by default** until tuples are + * confirmed across all foundations. Set the environment variable + * `MARKETING_ACCESS_ENFORCEMENT=true` to activate. + * + * @param relation The FGA relation to enforce (e.g. 'marketing_dashboard_viewer'). + */ +export function requireProjectAccess(relation: AccessCheckAccessType): RequestHandler { + const accessCheckService = new AccessCheckService(); + const projectService = new ProjectService(); + + return async (req: Request, _res: Response, next: NextFunction): Promise => { + // Rollout gate — deploy dark; enable once tuples are confirmed. + if (process.env['MARKETING_ACCESS_ENFORCEMENT'] !== 'true') { + next(); + return; + } + + const operation = `require_project_access_${relation}`; + + try { + const foundationSlug = req.query['foundationSlug']; + + if (!foundationSlug || typeof foundationSlug !== 'string' || !SLUG_PATTERN.test(foundationSlug)) { + logger.warning(req, operation, 'Missing or invalid foundationSlug — denying access', { + path: req.path, + relation, + foundation_slug: typeof foundationSlug === 'string' ? foundationSlug : '[invalid type]', + }); + next( + new AuthorizationError('Marketing access requires a valid foundationSlug', { + operation, + service: 'authorization', + path: req.path, + code: 'MARKETING_ACCESS_REQUIRED', + }) + ); + return; + } + + // Resolve slug → UID; cache on req to avoid N+1 NATS calls. + const cache = (req as Request & { [PROJECT_UID_CACHE]?: Record })[PROJECT_UID_CACHE] ?? {}; + (req as Request & { [PROJECT_UID_CACHE]?: Record })[PROJECT_UID_CACHE] = cache; + + let projectUid = cache[foundationSlug]; + + if (!projectUid) { + const natsResult = await projectService.getProjectIdBySlug(req, foundationSlug); + + if (!natsResult.exists || !natsResult.uid) { + logger.warning(req, operation, 'Foundation slug could not be resolved — denying access', { + path: req.path, + relation, + foundation_slug: foundationSlug, + }); + next( + new AuthorizationError('Foundation not found — marketing access denied', { + operation, + service: 'authorization', + path: req.path, + code: 'MARKETING_ACCESS_REQUIRED', + }) + ); + return; + } + + projectUid = natsResult.uid; + cache[foundationSlug] = projectUid; + } + + const hasAccess = await accessCheckService.checkSingleAccess(req, { + resource: 'project', + id: projectUid, + access: relation, + }); + + if (!hasAccess) { + logger.warning(req, operation, 'FGA check denied marketing access', { + path: req.path, + relation, + foundation_slug: foundationSlug, + project_uid: projectUid, + }); + next( + new AuthorizationError('Insufficient permissions for this resource', { + operation, + service: 'authorization', + path: req.path, + code: 'MARKETING_ACCESS_REQUIRED', + }) + ); + return; + } + + next(); + } catch (error) { + // Fail-closed: any unexpected error denies access rather than permitting it. + logger.warning(req, operation, 'Marketing access check threw unexpectedly — denying access', { + path: req.path, + relation, + error: error instanceof Error ? error.message : String(error), + }); + next( + new AuthorizationError('Marketing access check failed', { + operation, + service: 'authorization', + path: req.path, + code: 'MARKETING_ACCESS_REQUIRED', + }) + ); + } + }; +} diff --git a/apps/lfx-one/src/server/routes/analytics.route.ts b/apps/lfx-one/src/server/routes/analytics.route.ts index c64a16b12..43992ef58 100644 --- a/apps/lfx-one/src/server/routes/analytics.route.ts +++ b/apps/lfx-one/src/server/routes/analytics.route.ts @@ -4,6 +4,7 @@ import { Router } from 'express'; import { AnalyticsController } from '../controllers/analytics.controller'; +import { requireProjectAccess } from '../middleware/require-project-access.middleware'; const router = Router(); @@ -137,21 +138,25 @@ router.get('/org-involvement-event-attendance-monthly', (req, res, next) => anal router.get('/org-involvement-certified-employees-monthly', (req, res, next) => analyticsController.orgCertifiedEmployeesMonthly(req, res, next)); router.get('/org-involvement-training-enrollments', (req, res, next) => analyticsController.orgTrainingEnrollments(req, res, next)); +// Marketing Impact endpoints — require marketing_dashboard_viewer FGA relation. +// See LFXV2-2235. Enforcement is flag-gated (MARKETING_ACCESS_ENFORCEMENT=true). +const requireMarketingDashboardViewer = requireProjectAccess('marketing_dashboard_viewer'); + // Web activities summary endpoint (marketing dashboard) -router.get('/web-activities-summary', (req, res, next) => analyticsController.getWebActivitiesSummary(req, res, next)); +router.get('/web-activities-summary', requireMarketingDashboardViewer, (req, res, next) => analyticsController.getWebActivitiesSummary(req, res, next)); // Email CTR endpoint (marketing dashboard) -router.get('/email-ctr', (req, res, next) => analyticsController.getEmailCtr(req, res, next)); +router.get('/email-ctr', requireMarketingDashboardViewer, (req, res, next) => analyticsController.getEmailCtr(req, res, next)); // Social reach endpoint (marketing dashboard) -router.get('/social-reach', (req, res, next) => analyticsController.getSocialReach(req, res, next)); +router.get('/social-reach', requireMarketingDashboardViewer, (req, res, next) => analyticsController.getSocialReach(req, res, next)); // Keyword performance endpoint (marketing dashboard) -router.get('/keyword-performance', (req, res, next) => analyticsController.getKeywordPerformance(req, res, next)); +router.get('/keyword-performance', requireMarketingDashboardViewer, (req, res, next) => analyticsController.getKeywordPerformance(req, res, next)); // Social media endpoints (marketing dashboard) -router.get('/social-media', (req, res, next) => analyticsController.getSocialMedia(req, res, next)); -router.get('/social-media/monthly', (req, res, next) => analyticsController.getSocialMediaMonthly(req, res, next)); +router.get('/social-media', requireMarketingDashboardViewer, (req, res, next) => analyticsController.getSocialMedia(req, res, next)); +router.get('/social-media/monthly', requireMarketingDashboardViewer, (req, res, next) => analyticsController.getSocialMediaMonthly(req, res, next)); // North Star metrics endpoints (executive director dashboard) router.get('/member-retention', (req, res, next) => analyticsController.getMemberRetention(req, res, next)); diff --git a/apps/lfx-one/src/server/routes/campaigns.route.ts b/apps/lfx-one/src/server/routes/campaigns.route.ts index 5ca00ac9a..338aad1f3 100644 --- a/apps/lfx-one/src/server/routes/campaigns.route.ts +++ b/apps/lfx-one/src/server/routes/campaigns.route.ts @@ -4,26 +4,42 @@ import { Router } from 'express'; import { CampaignController } from '../controllers/campaign.controller'; +import { requireProjectAccess } from '../middleware/require-project-access.middleware'; const router = Router(); const campaignController = new CampaignController(); -router.post('/brief/generate', (req, res, next) => campaignController.generateBrief(req, res, next)); -router.post('/brief/refine', (req, res, next) => campaignController.refineBrief(req, res, next)); -router.post('/create', (req, res, next) => campaignController.createCampaign(req, res, next)); -router.get('/jobs/:jobId', (req, res, next) => campaignController.getJobStatus(req, res, next)); -router.get('/hubspot/utm', (req, res, next) => campaignController.lookupHubSpotUtm(req, res, next)); -router.post('/hubspot/utm/create', (req, res, next) => campaignController.createHubSpotUtm(req, res, next)); -router.get('/monitor', (req, res, next) => campaignController.getMonitorData(req, res, next)); -router.get('/linkedin/accounts', (req, res) => campaignController.getLinkedInAccounts(req, res)); -router.get('/linkedin/monitor', (req, res, next) => campaignController.getLinkedInMonitor(req, res, next)); -router.get('/reddit/accounts', (req, res) => campaignController.getRedditAccounts(req, res)); -router.get('/reddit/monitor', (req, res, next) => campaignController.getRedditMonitor(req, res, next)); -router.get('/meta/accounts', (req, res) => campaignController.getMetaAccounts(req, res)); -router.get('/meta/monitor', (req, res, next) => campaignController.getMetaMonitor(req, res, next)); -router.get('/keywords', (req, res, next) => campaignController.getKeywords(req, res, next)); -router.get('/audience', (req, res, next) => campaignController.getAudience(req, res, next)); -router.post('/keywords/actions', (req, res, next) => campaignController.executeKeywordActions(req, res, next)); -router.patch('/:campaignId/status', (req, res, next) => campaignController.updateCampaignStatus(req, res, next)); +// Campaign access enforcement — gated by FGA relation on the selected foundation. +// See LFXV2-2235. Enforcement is flag-gated (MARKETING_ACCESS_ENFORCEMENT=true). +// All routes require foundationSlug (sent by campaign.service.ts via LFXV2-2235). +// +// FGA model 14.1.0 (LFXV2-1760): both campaign_viewer and campaign_manager +// resolve to `executive_director or marketing_ops`, so every campaign_manager +// user also holds campaign_viewer. The viewer/manager split is semantic +// (future-proofing) and does not create a case where a manager can create but +// not poll. If the FGA model changes, revisit this split. +const requireCampaignViewer = requireProjectAccess('campaign_viewer'); +const requireCampaignManager = requireProjectAccess('campaign_manager'); + +// Write operations — require campaign_manager +router.post('/brief/generate', requireCampaignManager, (req, res, next) => campaignController.generateBrief(req, res, next)); +router.post('/brief/refine', requireCampaignManager, (req, res, next) => campaignController.refineBrief(req, res, next)); +router.post('/create', requireCampaignManager, (req, res, next) => campaignController.createCampaign(req, res, next)); +router.post('/hubspot/utm/create', requireCampaignManager, (req, res, next) => campaignController.createHubSpotUtm(req, res, next)); +router.post('/keywords/actions', requireCampaignManager, (req, res, next) => campaignController.executeKeywordActions(req, res, next)); +router.patch('/:campaignId/status', requireCampaignManager, (req, res, next) => campaignController.updateCampaignStatus(req, res, next)); + +// Read operations — require campaign_viewer +router.get('/jobs/:jobId', requireCampaignViewer, (req, res, next) => campaignController.getJobStatus(req, res, next)); +router.get('/hubspot/utm', requireCampaignViewer, (req, res, next) => campaignController.lookupHubSpotUtm(req, res, next)); +router.get('/monitor', requireCampaignViewer, (req, res, next) => campaignController.getMonitorData(req, res, next)); +router.get('/linkedin/accounts', requireCampaignViewer, (req, res) => campaignController.getLinkedInAccounts(req, res)); +router.get('/linkedin/monitor', requireCampaignViewer, (req, res, next) => campaignController.getLinkedInMonitor(req, res, next)); +router.get('/reddit/accounts', requireCampaignViewer, (req, res) => campaignController.getRedditAccounts(req, res)); +router.get('/reddit/monitor', requireCampaignViewer, (req, res, next) => campaignController.getRedditMonitor(req, res, next)); +router.get('/meta/accounts', requireCampaignViewer, (req, res) => campaignController.getMetaAccounts(req, res)); +router.get('/meta/monitor', requireCampaignViewer, (req, res, next) => campaignController.getMetaMonitor(req, res, next)); +router.get('/keywords', requireCampaignViewer, (req, res, next) => campaignController.getKeywords(req, res, next)); +router.get('/audience', requireCampaignViewer, (req, res, next) => campaignController.getAudience(req, res, next)); export default router; diff --git a/apps/lfx-one/src/server/services/access-check.service.ts b/apps/lfx-one/src/server/services/access-check.service.ts index 5272aa78e..95da6f975 100644 --- a/apps/lfx-one/src/server/services/access-check.service.ts +++ b/apps/lfx-one/src/server/services/access-check.service.ts @@ -118,6 +118,85 @@ export class AccessCheckService { return results.get(resource.id) || false; } + /** + * Check multiple access types on a single resource in one API call. + * + * Unlike {@link checkAccess}, which keys its Map by `resource.id` only (causing + * distinct access-type results for the same ID to collide), this method keys by + * `"id#access"` so every combination is preserved independently. + * + * Fails-closed: returns all-false on upstream error. + * + * @param req Express request object with auth context + * @param resourceType Resource type (e.g. 'project') + * @param id Resource unique identifier + * @param accessTypes Array of access types to check + * @returns Record mapping each access type to its boolean result + */ + public async checkMultipleAccess( + req: Request, + resourceType: AccessCheckResourceType, + id: string, + accessTypes: AccessCheckAccessType[] + ): Promise> { + const operationName = `check_multiple_access_${resourceType}`; + const startTime = logger.startOperation(req, operationName, { + resource_type: resourceType, + resource_id: id, + access_types: accessTypes, + }); + + const fallback = Object.fromEntries(accessTypes.map((a) => [a, false])) as Record; + + if (accessTypes.length === 0) { + return fallback; + } + + try { + const apiRequests = accessTypes.map((access) => `${resourceType}:${id}#${access}`); + const requestPayload: AccessCheckApiRequest = { requests: apiRequests }; + + const response = await this.microserviceProxy.proxyRequest( + req, + 'LFX_V2_SERVICE', + '/access-check', + 'POST', + undefined, + requestPayload + ); + + const result = { ...fallback }; + + for (let i = 0; i < accessTypes.length; i++) { + const access = accessTypes[i]; + const resultString = response.results[i]; + let hasAccess = false; + + if (resultString && typeof resultString === 'string') { + const parts = resultString.split('\t'); + if (parts.length >= 2) { + hasAccess = parts[1]?.toLowerCase() === 'true'; + } + } + + result[access] = hasAccess; + } + + logger.success(req, operationName, startTime, { + resource_id: id, + granted: accessTypes.filter((a) => result[a]), + }); + + return result; + } catch (error) { + logger.error(req, operationName, startTime, error, { + resource_id: id, + fallback_behavior: 'returning no access', + }); + return fallback; + } + } + /** * Add writer access field to multiple resources automatically * @param req Express request object with auth context diff --git a/apps/lfx-one/src/server/services/project.service.ts b/apps/lfx-one/src/server/services/project.service.ts index 97c0aff30..f1e2542cf 100644 --- a/apps/lfx-one/src/server/services/project.service.ts +++ b/apps/lfx-one/src/server/services/project.service.ts @@ -290,7 +290,13 @@ export class ProjectService { /** * Fetches a single project by ID */ - public async getProjectById(req: Request, uid: string, access: boolean = true, includeMeetingCoordinator: boolean = false): Promise { + public async getProjectById( + req: Request, + uid: string, + access: boolean = true, + includeMeetingCoordinator: boolean = false, + includeMarketingAccess: boolean = false + ): Promise { const project = await this.microserviceProxy.proxyRequest(req, 'LFX_V2_SERVICE', `/projects/${uid}`, 'GET'); if (!project) { @@ -307,28 +313,57 @@ export class ProjectService { // meeting_coordinator, so the extra round trip can't change the outcome. // Return the field as undefined (omitted) rather than false — false would be a // false-negative assertion since the role was never actually checked for writers. - if (writerProject.writer) { + if (writerProject.writer && !includeMeetingCoordinator && !includeMarketingAccess) { return writerProject; } + + let result: Project = writerProject; + // Only run the meeting_coordinator FGA check when the caller explicitly requests it. // This field is consumed by exactly one branch of writer.guard.ts; running it on every // GET /api/projects/:slug call would add a second sequential access-check round-trip // for all non-writer callers (guards, components, etc.) that never read the field. - if (!includeMeetingCoordinator) { - return writerProject; + if (includeMeetingCoordinator && !writerProject.writer) { + const isMeetingCoordinator = await this.accessCheckService + .checkSingleAccess(req, { resource: 'project', id: project.uid, access: 'meeting_coordinator' }) + .catch((error) => { + logger.warning(req, 'get_project_by_id', 'meeting coordinator check failed, skipping field', { + project_uid: project.uid, + error: error instanceof Error ? error.message : String(error), + }); + // Return undefined rather than false — false implies the check ran clean and found no + // role; undefined preserves the "unknown" semantics documented on Project.meetingCoordinator. + return undefined; + }); + result = { ...result, meetingCoordinator: isMeetingCoordinator }; } - const isMeetingCoordinator = await this.accessCheckService - .checkSingleAccess(req, { resource: 'project', id: project.uid, access: 'meeting_coordinator' }) - .catch((error) => { - logger.warning(req, 'get_project_by_id', 'meeting coordinator check failed, skipping field', { - project_uid: project.uid, - error: error instanceof Error ? error.message : String(error), + + // Marketing access probe — checked independently of writer status because the + // marketing_* FGA relations are separate from the project writer relation. + // Only populated when explicitly requested (opt-in) to avoid extra round-trips + // on every project fetch. Consumed by LFXV2-2236 Angular marketing guards. + if (includeMarketingAccess) { + const marketingResults = await this.accessCheckService + .checkMultipleAccess(req, 'project', project.uid, ['marketing_dashboard_viewer', 'campaign_viewer', 'campaign_manager']) + .catch((error) => { + logger.warning(req, 'get_project_by_id', 'marketing access check failed, skipping fields', { + project_uid: project.uid, + error: error instanceof Error ? error.message : String(error), + }); + return undefined; }); - // Return undefined rather than false — false implies the check ran clean and found no - // role; undefined preserves the "unknown" semantics documented on Project.meetingCoordinator. - return undefined; - }); - return { ...writerProject, meetingCoordinator: isMeetingCoordinator }; + + if (marketingResults) { + result = { + ...result, + marketingDashboardViewer: marketingResults['marketing_dashboard_viewer'], + campaignViewer: marketingResults['campaign_viewer'], + campaignManager: marketingResults['campaign_manager'], + }; + } + } + + return result; } return project; @@ -404,7 +439,12 @@ export class ProjectService { * Fetches a single project by slug using NATS for slug resolution * First resolves slug to ID via NATS, then fetches project data */ - public async getProjectBySlug(req: Request, projectSlug: string, includeMeetingCoordinator: boolean = false): Promise { + public async getProjectBySlug( + req: Request, + projectSlug: string, + includeMeetingCoordinator: boolean = false, + includeMarketingAccess: boolean = false + ): Promise { const natsResult = await this.getProjectIdBySlug(req, projectSlug); if (!natsResult.exists || !natsResult.uid) { @@ -416,7 +456,7 @@ export class ProjectService { } // Now fetch the project using the resolved ID - return this.getProjectById(req, natsResult.uid, true, includeMeetingCoordinator); + return this.getProjectById(req, natsResult.uid, true, includeMeetingCoordinator, includeMarketingAccess); } public async getProjectSettings(req: Request, uid: string): Promise { diff --git a/packages/shared/src/interfaces/access-check.interface.ts b/packages/shared/src/interfaces/access-check.interface.ts index 596bfd323..3fb526540 100644 --- a/packages/shared/src/interfaces/access-check.interface.ts +++ b/packages/shared/src/interfaces/access-check.interface.ts @@ -42,4 +42,11 @@ export type AccessCheckResourceType = | 'groupsio_service' | 'groupsio_mailing_list' | 'groupsio_member'; -export type AccessCheckAccessType = 'writer' | 'viewer' | 'organizer' | 'meeting_coordinator'; +export type AccessCheckAccessType = + | 'writer' + | 'viewer' + | 'organizer' + | 'meeting_coordinator' + | 'marketing_dashboard_viewer' + | 'campaign_viewer' + | 'campaign_manager'; diff --git a/packages/shared/src/interfaces/project.interface.ts b/packages/shared/src/interfaces/project.interface.ts index 7eee2d7d4..063b37e4e 100644 --- a/packages/shared/src/interfaces/project.interface.ts +++ b/packages/shared/src/interfaces/project.interface.ts @@ -21,6 +21,21 @@ export interface Project { * transiently. Do NOT treat as a definitive role denial. */ meetingCoordinator?: boolean; + /** + * Response-only marketing access probe fields — present only when the caller + * requested marketing access checks (`?marketing_access=true`). + * + * Each follows the same semantics as `meetingCoordinator`: + * - `true` — FGA check ran and user holds the relation. + * - `false` — FGA check ran; user does not hold the relation. + * - `undefined` — check was not requested or failed transiently. + * + * These are consumed by the Angular marketing guards (LFXV2-2236). + * FGA-backed — no ED-persona shortcut. + */ + marketingDashboardViewer?: boolean; + campaignViewer?: boolean; + campaignManager?: boolean; public: boolean; parent_uid: string; stage: ProjectStage | string; From a08209d175093b79dc767bb721ca8512ab884f34 Mon Sep 17 00:00:00 2001 From: David Deal Date: Tue, 23 Jun 2026 09:49:21 -0700 Subject: [PATCH 02/10] fix(review): use err field for error objects in warning logs Replace `error: error.message` with `err: error` in warning-level catch handlers so Pino serializes the full Error object including stack trace and custom properties. Refs: LFXV2-2235 Signed-off-by: David Deal --- .../server/middleware/require-project-access.middleware.ts | 2 +- apps/lfx-one/src/server/services/project.service.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/lfx-one/src/server/middleware/require-project-access.middleware.ts b/apps/lfx-one/src/server/middleware/require-project-access.middleware.ts index ac14e9075..ac6ee8a3b 100644 --- a/apps/lfx-one/src/server/middleware/require-project-access.middleware.ts +++ b/apps/lfx-one/src/server/middleware/require-project-access.middleware.ts @@ -132,7 +132,7 @@ export function requireProjectAccess(relation: AccessCheckAccessType): RequestHa logger.warning(req, operation, 'Marketing access check threw unexpectedly — denying access', { path: req.path, relation, - error: error instanceof Error ? error.message : String(error), + err: error, }); next( new AuthorizationError('Marketing access check failed', { diff --git a/apps/lfx-one/src/server/services/project.service.ts b/apps/lfx-one/src/server/services/project.service.ts index f1e2542cf..6217337fd 100644 --- a/apps/lfx-one/src/server/services/project.service.ts +++ b/apps/lfx-one/src/server/services/project.service.ts @@ -329,7 +329,7 @@ export class ProjectService { .catch((error) => { logger.warning(req, 'get_project_by_id', 'meeting coordinator check failed, skipping field', { project_uid: project.uid, - error: error instanceof Error ? error.message : String(error), + err: error, }); // Return undefined rather than false — false implies the check ran clean and found no // role; undefined preserves the "unknown" semantics documented on Project.meetingCoordinator. @@ -348,7 +348,7 @@ export class ProjectService { .catch((error) => { logger.warning(req, 'get_project_by_id', 'marketing access check failed, skipping fields', { project_uid: project.uid, - error: error instanceof Error ? error.message : String(error), + err: error, }); return undefined; }); From 90dbc73b2869da524c37cebdf34c3e65a6426fbf Mon Sep 17 00:00:00 2001 From: David Deal Date: Tue, 23 Jun 2026 09:52:22 -0700 Subject: [PATCH 03/10] fix(review): clarify docs and add audit-logging intent comment - Fix checkMultipleAccess docstring: accurately describes positional indexing into a Record rather than the incorrect "id#access keying" claim - Add comment in requireProjectAccess explaining why pre-error WARN logs are intentional on each deny path (security audit events, not accidental double-logging) - Document in campaigns.route.ts that campaign_manager implies campaign_viewer in FGA model 14.1.0 (both resolve to executive_director or marketing_ops), preventing a theoretical create-but-cannot-poll gap Refs: LFXV2-2235 Signed-off-by: David Deal --- .../middleware/require-project-access.middleware.ts | 4 ++++ apps/lfx-one/src/server/services/access-check.service.ts | 9 ++++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/apps/lfx-one/src/server/middleware/require-project-access.middleware.ts b/apps/lfx-one/src/server/middleware/require-project-access.middleware.ts index ac6ee8a3b..62984f20e 100644 --- a/apps/lfx-one/src/server/middleware/require-project-access.middleware.ts +++ b/apps/lfx-one/src/server/middleware/require-project-access.middleware.ts @@ -53,6 +53,10 @@ export function requireProjectAccess(relation: AccessCheckAccessType): RequestHa const operation = `require_project_access_${relation}`; try { + // Pre-error WARN logs on each deny path are intentional: authorization denials + // are security-relevant audit events and warrant a dedicated log entry before + // apiErrorHandler processes the AuthorizationError. This is an exception to + // the no-pre-next-error-logging rule that applies to business-logic controllers. const foundationSlug = req.query['foundationSlug']; if (!foundationSlug || typeof foundationSlug !== 'string' || !SLUG_PATTERN.test(foundationSlug)) { diff --git a/apps/lfx-one/src/server/services/access-check.service.ts b/apps/lfx-one/src/server/services/access-check.service.ts index 95da6f975..fd414aea1 100644 --- a/apps/lfx-one/src/server/services/access-check.service.ts +++ b/apps/lfx-one/src/server/services/access-check.service.ts @@ -121,10 +121,13 @@ export class AccessCheckService { /** * Check multiple access types on a single resource in one API call. * - * Unlike {@link checkAccess}, which keys its Map by `resource.id` only (causing - * distinct access-type results for the same ID to collide), this method keys by - * `"id#access"` so every combination is preserved independently. + * Unlike {@link checkAccess} — which maps its results by `resource.id` only, + * causing multiple access types for the same ID to collide (last-write-wins) — + * this method operates on a single ID and returns a `Record` keyed by + * `AccessCheckAccessType`, so every relation is preserved independently. * + * Results are matched positionally to `accessTypes` (same order as the upstream + * `/access-check` response), then stored as `record[accessType] = hasAccess`. * Fails-closed: returns all-false on upstream error. * * @param req Express request object with auth context From 9f4952894dc1903c35f0dc179bab41238234a287 Mon Sep 17 00:00:00 2001 From: David Deal Date: Tue, 23 Jun 2026 09:55:49 -0700 Subject: [PATCH 04/10] fix(review): improve comment clarity for audit boundary and FGA invariant source Co-authored-by: David Deal Signed-off-by: David Deal --- .../middleware/require-project-access.middleware.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/apps/lfx-one/src/server/middleware/require-project-access.middleware.ts b/apps/lfx-one/src/server/middleware/require-project-access.middleware.ts index 62984f20e..14739bb80 100644 --- a/apps/lfx-one/src/server/middleware/require-project-access.middleware.ts +++ b/apps/lfx-one/src/server/middleware/require-project-access.middleware.ts @@ -53,10 +53,11 @@ export function requireProjectAccess(relation: AccessCheckAccessType): RequestHa const operation = `require_project_access_${relation}`; try { - // Pre-error WARN logs on each deny path are intentional: authorization denials - // are security-relevant audit events and warrant a dedicated log entry before - // apiErrorHandler processes the AuthorizationError. This is an exception to - // the no-pre-next-error-logging rule that applies to business-logic controllers. + // Authorization denials are security-relevant audit events that warrant a + // dedicated WARN log entry before apiErrorHandler processes the AuthorizationError. + // This middleware is a security boundary, not a business-logic controller — the + // no-pre-next-error-logging rule applies to controllers where apiErrorHandler is + // the sole audit surface; here the denial itself is the auditable event. const foundationSlug = req.query['foundationSlug']; if (!foundationSlug || typeof foundationSlug !== 'string' || !SLUG_PATTERN.test(foundationSlug)) { From 309b6756c0d49f2f4503f4b9fcdfe7442c47f3df Mon Sep 17 00:00:00 2001 From: David Deal Date: Tue, 23 Jun 2026 10:33:31 -0700 Subject: [PATCH 05/10] style(marketing): apply prettier formatting to campaign service Signed-off-by: David Deal --- .../app/shared/services/campaign.service.ts | 46 ++++++++++++++----- 1 file changed, 35 insertions(+), 11 deletions(-) diff --git a/apps/lfx-one/src/app/shared/services/campaign.service.ts b/apps/lfx-one/src/app/shared/services/campaign.service.ts index 95f8add63..1022996dd 100644 --- a/apps/lfx-one/src/app/shared/services/campaign.service.ts +++ b/apps/lfx-one/src/app/shared/services/campaign.service.ts @@ -83,48 +83,72 @@ export class CampaignService { } public getMonitorData(days: number = 30): Observable { - return this.http.get('/api/campaigns/monitor', { params: { days, ...(this.foundationSlug && { foundationSlug: this.foundationSlug }) } }); + return this.http.get('/api/campaigns/monitor', { + params: { days, ...(this.foundationSlug && { foundationSlug: this.foundationSlug }) }, + }); } public getLinkedInAccounts(): Observable { - return this.http.get('/api/campaigns/linkedin/accounts', { params: { ...(this.foundationSlug && { foundationSlug: this.foundationSlug }) } }); + return this.http.get('/api/campaigns/linkedin/accounts', { + params: { ...(this.foundationSlug && { foundationSlug: this.foundationSlug }) }, + }); } public getLinkedInMonitorData(accountKey: string, days: number = 30): Observable { - return this.http.get('/api/campaigns/linkedin/monitor', { params: { days, accountKey, ...(this.foundationSlug && { foundationSlug: this.foundationSlug }) } }); + return this.http.get('/api/campaigns/linkedin/monitor', { + params: { days, accountKey, ...(this.foundationSlug && { foundationSlug: this.foundationSlug }) }, + }); } public getRedditAccounts(): Observable { - return this.http.get('/api/campaigns/reddit/accounts', { params: { ...(this.foundationSlug && { foundationSlug: this.foundationSlug }) } }); + return this.http.get('/api/campaigns/reddit/accounts', { + params: { ...(this.foundationSlug && { foundationSlug: this.foundationSlug }) }, + }); } public getRedditMonitorData(accountKey: string, days: number = 30): Observable { - return this.http.get('/api/campaigns/reddit/monitor', { params: { days, accountKey, ...(this.foundationSlug && { foundationSlug: this.foundationSlug }) } }); + return this.http.get('/api/campaigns/reddit/monitor', { + params: { days, accountKey, ...(this.foundationSlug && { foundationSlug: this.foundationSlug }) }, + }); } public getMetaAccounts(): Observable { - return this.http.get('/api/campaigns/meta/accounts', { params: { ...(this.foundationSlug && { foundationSlug: this.foundationSlug }) } }); + return this.http.get('/api/campaigns/meta/accounts', { + params: { ...(this.foundationSlug && { foundationSlug: this.foundationSlug }) }, + }); } public getMetaMonitorData(accountKey: string, days: number = 30): Observable { - return this.http.get('/api/campaigns/meta/monitor', { params: { days, accountKey, ...(this.foundationSlug && { foundationSlug: this.foundationSlug }) } }); + return this.http.get('/api/campaigns/meta/monitor', { + params: { days, accountKey, ...(this.foundationSlug && { foundationSlug: this.foundationSlug }) }, + }); } public getKeywords(days: number = 30): Observable { - return this.http.get('/api/campaigns/keywords', { params: { days, ...(this.foundationSlug && { foundationSlug: this.foundationSlug }) } }); + return this.http.get('/api/campaigns/keywords', { + params: { days, ...(this.foundationSlug && { foundationSlug: this.foundationSlug }) }, + }); } public getAudience(days: number = 30): Observable { - return this.http.get('/api/campaigns/audience', { params: { days, ...(this.foundationSlug && { foundationSlug: this.foundationSlug }) } }); + return this.http.get('/api/campaigns/audience', { + params: { days, ...(this.foundationSlug && { foundationSlug: this.foundationSlug }) }, + }); } public lookupHubSpotUtm(eventName: string): Observable { - return this.http.get('/api/campaigns/hubspot/utm', { params: { event_name: eventName, ...(this.foundationSlug && { foundationSlug: this.foundationSlug }) } }); + return this.http.get('/api/campaigns/hubspot/utm', { + params: { event_name: eventName, ...(this.foundationSlug && { foundationSlug: this.foundationSlug }) }, + }); } public createHubSpotUtm(eventName: string): Observable { const slug = this.foundationSlug; - return this.http.post('/api/campaigns/hubspot/utm/create', {}, { params: { event_name: eventName, ...(slug && { foundationSlug: slug }) } }); + return this.http.post( + '/api/campaigns/hubspot/utm/create', + {}, + { params: { event_name: eventName, ...(slug && { foundationSlug: slug }) } } + ); } public executeKeywordActions(request: BulkKeywordActionRequest): Observable { From db7e0b90949308f400d8d6396458e67f8c367833 Mon Sep 17 00:00:00 2001 From: David Deal Date: Tue, 23 Jun 2026 11:50:38 -0700 Subject: [PATCH 06/10] fix(marketing): address multi-tool review findings - Add requireMarketingDashboardViewer to /revenue-impact and /marketing-attribution routes (missed in original pass; both back the Marketing Impact page per LFXV2-2235 plan) - Guard checkMultipleAccess against upstream result-count mismatch: log warning and fail-closed when response.results.length differs from accessTypes.length - Normalize AuthorizationError client messages to a single generic string to eliminate slug-validity and slug-existence enumeration - Fix misleading PROJECT_UID_CACHE comment (cache is per-request, not per-page-load; helps when two requireProjectAccess instances stack on the same route) Refs: LFXV2-2235 Signed-off-by: David Deal --- .../middleware/require-project-access.middleware.ts | 8 ++++---- apps/lfx-one/src/server/routes/analytics.route.ts | 4 ++-- apps/lfx-one/src/server/services/access-check.service.ts | 9 +++++++++ 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/apps/lfx-one/src/server/middleware/require-project-access.middleware.ts b/apps/lfx-one/src/server/middleware/require-project-access.middleware.ts index 14739bb80..7a2ff59da 100644 --- a/apps/lfx-one/src/server/middleware/require-project-access.middleware.ts +++ b/apps/lfx-one/src/server/middleware/require-project-access.middleware.ts @@ -17,8 +17,8 @@ const SLUG_PATTERN = /^[a-z0-9-]+$/; /** * Symbol used to cache slug→UID lookups on the request object. - * Prevents N+1 NATS calls when multiple middleware-gated analytics endpoints - * fire in parallel on the same page load. + * Prevents redundant NATS calls when multiple requireProjectAccess middleware + * instances are stacked on the same route (e.g. reader + writer checks). */ const PROJECT_UID_CACHE = Symbol('projectUidCache'); @@ -67,7 +67,7 @@ export function requireProjectAccess(relation: AccessCheckAccessType): RequestHa foundation_slug: typeof foundationSlug === 'string' ? foundationSlug : '[invalid type]', }); next( - new AuthorizationError('Marketing access requires a valid foundationSlug', { + new AuthorizationError('Insufficient permissions for this resource', { operation, service: 'authorization', path: req.path, @@ -93,7 +93,7 @@ export function requireProjectAccess(relation: AccessCheckAccessType): RequestHa foundation_slug: foundationSlug, }); next( - new AuthorizationError('Foundation not found — marketing access denied', { + new AuthorizationError('Insufficient permissions for this resource', { operation, service: 'authorization', path: req.path, diff --git a/apps/lfx-one/src/server/routes/analytics.route.ts b/apps/lfx-one/src/server/routes/analytics.route.ts index 43992ef58..8561d3950 100644 --- a/apps/lfx-one/src/server/routes/analytics.route.ts +++ b/apps/lfx-one/src/server/routes/analytics.route.ts @@ -192,8 +192,8 @@ router.get('/board-meeting-participation-summary', (req, res, next) => analytics router.get('/event-growth', (req, res, next) => analyticsController.getEventGrowth(req, res, next)); router.get('/brand-reach', (req, res, next) => analyticsController.getBrandReach(req, res, next)); router.get('/brand-health', (req, res, next) => analyticsController.getBrandHealth(req, res, next)); -router.get('/revenue-impact', (req, res, next) => analyticsController.getRevenueImpact(req, res, next)); -router.get('/marketing-attribution', (req, res, next) => analyticsController.getMarketingAttribution(req, res, next)); +router.get('/revenue-impact', requireMarketingDashboardViewer, (req, res, next) => analyticsController.getRevenueImpact(req, res, next)); +router.get('/marketing-attribution', requireMarketingDashboardViewer, (req, res, next) => analyticsController.getMarketingAttribution(req, res, next)); // Multi-foundation summary endpoint (multi-foundation dashboard) router.get('/multi-foundation-summary', (req, res, next) => analyticsController.getMultiFoundationSummary(req, res, next)); diff --git a/apps/lfx-one/src/server/services/access-check.service.ts b/apps/lfx-one/src/server/services/access-check.service.ts index fd414aea1..ea91d1198 100644 --- a/apps/lfx-one/src/server/services/access-check.service.ts +++ b/apps/lfx-one/src/server/services/access-check.service.ts @@ -168,6 +168,15 @@ export class AccessCheckService { requestPayload ); + if (response.results.length !== accessTypes.length) { + logger.warning(req, operationName, 'access-check result count mismatch — failing closed', { + resource_id: id, + expected: accessTypes.length, + received: response.results.length, + }); + return fallback; + } + const result = { ...fallback }; for (let i = 0; i < accessTypes.length; i++) { From 9b0fe7f92ec420e143dfc86f0f97e4e386b989d4 Mon Sep 17 00:00:00 2001 From: David Deal Date: Tue, 23 Jun 2026 12:16:53 -0700 Subject: [PATCH 07/10] fix(review): address PR #1018 review feedback Address review comments from copilot-pull-request-reviewer and coderabbitai: - access-check.service.ts: checkMultipleAccess now throws on upstream errors and result-count mismatches instead of returning all-false fallback; probe callers (project.service.ts) already wrap in .catch() and return undefined, so fields are omitted rather than set to false on transient FGA failures (per copilot-pull-request-reviewer, coderabbitai) - require-project-access.middleware.ts: rephrase FGA deny log from "denied" to "did not grant access (denied or upstream failure)" since checkSingleAccess is fail-closed and the log fires for both denial and upstream error (per copilot-pull-request-reviewer) - campaign.service.ts: capture this.foundationSlug once per method (getMonitorData, getLinkedInAccounts, getLinkedInMonitorData, getRedditAccounts, getRedditMonitorData, getMetaAccounts, getMetaMonitorData, getKeywords, getAudience) to avoid double signal evaluation (per copilot-pull-request-reviewer) - docs/user/dashboards/index.md: fix Prettier prose-wrap formatting (line 12 exceeded printWidth:160 in CI environment) Resolves 5 review threads. Refs: LFXV2-2235 Signed-off-by: David Deal --- .../app/shared/services/campaign.service.ts | 27 ++++++++++++------- .../require-project-access.middleware.ts | 2 +- .../server/services/access-check.service.ts | 12 +++------ docs/user/dashboards/index.md | 3 ++- 4 files changed, 25 insertions(+), 19 deletions(-) diff --git a/apps/lfx-one/src/app/shared/services/campaign.service.ts b/apps/lfx-one/src/app/shared/services/campaign.service.ts index 1022996dd..cf65f0107 100644 --- a/apps/lfx-one/src/app/shared/services/campaign.service.ts +++ b/apps/lfx-one/src/app/shared/services/campaign.service.ts @@ -83,56 +83,65 @@ export class CampaignService { } public getMonitorData(days: number = 30): Observable { + const slug = this.foundationSlug; return this.http.get('/api/campaigns/monitor', { - params: { days, ...(this.foundationSlug && { foundationSlug: this.foundationSlug }) }, + params: { days, ...(slug && { foundationSlug: slug }) }, }); } public getLinkedInAccounts(): Observable { + const slug = this.foundationSlug; return this.http.get('/api/campaigns/linkedin/accounts', { - params: { ...(this.foundationSlug && { foundationSlug: this.foundationSlug }) }, + params: { ...(slug && { foundationSlug: slug }) }, }); } public getLinkedInMonitorData(accountKey: string, days: number = 30): Observable { + const slug = this.foundationSlug; return this.http.get('/api/campaigns/linkedin/monitor', { - params: { days, accountKey, ...(this.foundationSlug && { foundationSlug: this.foundationSlug }) }, + params: { days, accountKey, ...(slug && { foundationSlug: slug }) }, }); } public getRedditAccounts(): Observable { + const slug = this.foundationSlug; return this.http.get('/api/campaigns/reddit/accounts', { - params: { ...(this.foundationSlug && { foundationSlug: this.foundationSlug }) }, + params: { ...(slug && { foundationSlug: slug }) }, }); } public getRedditMonitorData(accountKey: string, days: number = 30): Observable { + const slug = this.foundationSlug; return this.http.get('/api/campaigns/reddit/monitor', { - params: { days, accountKey, ...(this.foundationSlug && { foundationSlug: this.foundationSlug }) }, + params: { days, accountKey, ...(slug && { foundationSlug: slug }) }, }); } public getMetaAccounts(): Observable { + const slug = this.foundationSlug; return this.http.get('/api/campaigns/meta/accounts', { - params: { ...(this.foundationSlug && { foundationSlug: this.foundationSlug }) }, + params: { ...(slug && { foundationSlug: slug }) }, }); } public getMetaMonitorData(accountKey: string, days: number = 30): Observable { + const slug = this.foundationSlug; return this.http.get('/api/campaigns/meta/monitor', { - params: { days, accountKey, ...(this.foundationSlug && { foundationSlug: this.foundationSlug }) }, + params: { days, accountKey, ...(slug && { foundationSlug: slug }) }, }); } public getKeywords(days: number = 30): Observable { + const slug = this.foundationSlug; return this.http.get('/api/campaigns/keywords', { - params: { days, ...(this.foundationSlug && { foundationSlug: this.foundationSlug }) }, + params: { days, ...(slug && { foundationSlug: slug }) }, }); } public getAudience(days: number = 30): Observable { + const slug = this.foundationSlug; return this.http.get('/api/campaigns/audience', { - params: { days, ...(this.foundationSlug && { foundationSlug: this.foundationSlug }) }, + params: { days, ...(slug && { foundationSlug: slug }) }, }); } diff --git a/apps/lfx-one/src/server/middleware/require-project-access.middleware.ts b/apps/lfx-one/src/server/middleware/require-project-access.middleware.ts index 7a2ff59da..4c12cb2e6 100644 --- a/apps/lfx-one/src/server/middleware/require-project-access.middleware.ts +++ b/apps/lfx-one/src/server/middleware/require-project-access.middleware.ts @@ -114,7 +114,7 @@ export function requireProjectAccess(relation: AccessCheckAccessType): RequestHa }); if (!hasAccess) { - logger.warning(req, operation, 'FGA check denied marketing access', { + logger.warning(req, operation, 'FGA check did not grant marketing access (denied or upstream failure)', { path: req.path, relation, foundation_slug: foundationSlug, diff --git a/apps/lfx-one/src/server/services/access-check.service.ts b/apps/lfx-one/src/server/services/access-check.service.ts index ea91d1198..1352c93d5 100644 --- a/apps/lfx-one/src/server/services/access-check.service.ts +++ b/apps/lfx-one/src/server/services/access-check.service.ts @@ -169,12 +169,9 @@ export class AccessCheckService { ); if (response.results.length !== accessTypes.length) { - logger.warning(req, operationName, 'access-check result count mismatch — failing closed', { - resource_id: id, - expected: accessTypes.length, - received: response.results.length, - }); - return fallback; + throw new Error( + `access-check result count mismatch: expected ${accessTypes.length}, received ${response.results.length}` + ); } const result = { ...fallback }; @@ -203,9 +200,8 @@ export class AccessCheckService { } catch (error) { logger.error(req, operationName, startTime, error, { resource_id: id, - fallback_behavior: 'returning no access', }); - return fallback; + throw error; } } diff --git a/docs/user/dashboards/index.md b/docs/user/dashboards/index.md index e76e5ad98..e6118375b 100644 --- a/docs/user/dashboards/index.md +++ b/docs/user/dashboards/index.md @@ -9,7 +9,8 @@ last_updated: 2026-05-22 intercom_collection: Dashboards --- -The Dashboard is the central starting point of LFX Self Serve. It adapts to show you relevant information based on who you are (your persona) and what you are viewing (your lens). +The Dashboard is the central starting point of LFX Self Serve. It adapts to show you relevant information based on who you are (your persona) and what you are +viewing (your lens). ## Lens system From b0fec6990226f2636403a2acfc68424f6daecf58 Mon Sep 17 00:00:00 2001 From: David Deal Date: Tue, 23 Jun 2026 12:20:11 -0700 Subject: [PATCH 08/10] style(marketing): fix prettier formatting in access-check service Collapse multi-line throw to single line; fits within printWidth:160. Refs: LFXV2-2235 Signed-off-by: David Deal --- apps/lfx-one/src/server/services/access-check.service.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/apps/lfx-one/src/server/services/access-check.service.ts b/apps/lfx-one/src/server/services/access-check.service.ts index 1352c93d5..229a59366 100644 --- a/apps/lfx-one/src/server/services/access-check.service.ts +++ b/apps/lfx-one/src/server/services/access-check.service.ts @@ -169,9 +169,7 @@ export class AccessCheckService { ); if (response.results.length !== accessTypes.length) { - throw new Error( - `access-check result count mismatch: expected ${accessTypes.length}, received ${response.results.length}` - ); + throw new Error(`access-check result count mismatch: expected ${accessTypes.length}, received ${response.results.length}`); } const result = { ...fallback }; From bb287b817b9bb19f3e74812020fa21f1ec44c53f Mon Sep 17 00:00:00 2001 From: David Deal Date: Tue, 23 Jun 2026 12:52:49 -0700 Subject: [PATCH 09/10] docs(marketing): fix stale JSDoc after review feedback on PR #1018 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review comments from copilot-pull-request-reviewer: - access-check.service.ts: update checkMultipleAccess JSDoc — method now throws on upstream error / result-count mismatch, not returns all-false; clarify callers must catch and decide how to fail (per copilot-pull-request-reviewer) - require-project-access.middleware.ts: clarify factory JSDoc — slug→UID cache is per-request (stacked middleware), not shared across parallel requests; remove misleading "parallel analytics calls" language (per copilot-pull-request-reviewer) Resolves 2 review threads. Third thread (job store IDOR) documented as pre-existing limitation, addressed in thread response. Signed-off-by: David Deal --- .../server/middleware/require-project-access.middleware.ts | 5 +++-- apps/lfx-one/src/server/services/access-check.service.ts | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/apps/lfx-one/src/server/middleware/require-project-access.middleware.ts b/apps/lfx-one/src/server/middleware/require-project-access.middleware.ts index 4c12cb2e6..d1bdf01cb 100644 --- a/apps/lfx-one/src/server/middleware/require-project-access.middleware.ts +++ b/apps/lfx-one/src/server/middleware/require-project-access.middleware.ts @@ -28,8 +28,9 @@ const PROJECT_UID_CACHE = Symbol('projectUidCache'); * * Gate logic (all fail-closed): * 1. Read `foundationSlug` from `req.query`; validate format. - * 2. Resolve slug → project UID via NATS (result cached on `req` to amortise - * parallel analytics calls that all share the same slug). + * 2. Resolve slug → project UID via NATS (result cached on `req` to avoid + * redundant NATS calls when multiple middleware instances are stacked on + * the same route — not shared across concurrent requests). * 3. Check the FGA relation via `/access-check` on LFX_V2_SERVICE. * 4. Call `next()` on success; `next(AuthorizationError)` on any failure. * diff --git a/apps/lfx-one/src/server/services/access-check.service.ts b/apps/lfx-one/src/server/services/access-check.service.ts index 229a59366..3892491ff 100644 --- a/apps/lfx-one/src/server/services/access-check.service.ts +++ b/apps/lfx-one/src/server/services/access-check.service.ts @@ -128,7 +128,8 @@ export class AccessCheckService { * * Results are matched positionally to `accessTypes` (same order as the upstream * `/access-check` response), then stored as `record[accessType] = hasAccess`. - * Fails-closed: returns all-false on upstream error. + * Throws on upstream error or result-count mismatch — callers must catch and + * decide how to fail (e.g. omit probe fields, return `undefined`, or deny). * * @param req Express request object with auth context * @param resourceType Resource type (e.g. 'project') From 70620bf6e1005fe004dd888008b2741197fafad2 Mon Sep 17 00:00:00 2001 From: David Deal Date: Tue, 23 Jun 2026 13:19:27 -0700 Subject: [PATCH 10/10] =?UTF-8?q?fix(marketing):=20address=20PR=20#1018=20?= =?UTF-8?q?review=20feedback=20=E2=80=94=20type=20safety=20and=20consisten?= =?UTF-8?q?cy?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review comments from copilot-pull-request-reviewer: - access-check.service.ts: make checkMultipleAccess generic over T extends AccessCheckAccessType; return type is now Record instead of Record, removing the unsound cast that implied every access type key was present regardless of what was requested (per copilot-pull-request-reviewer) - campaign.service.ts: capture this.foundationSlug once in lookupHubSpotUtm before building params, consistent with every other method in the service (per copilot-pull-request-reviewer) Resolves 2 review threads. Signed-off-by: David Deal --- apps/lfx-one/src/app/shared/services/campaign.service.ts | 3 ++- apps/lfx-one/src/server/services/access-check.service.ts | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/apps/lfx-one/src/app/shared/services/campaign.service.ts b/apps/lfx-one/src/app/shared/services/campaign.service.ts index cf65f0107..8a610ecab 100644 --- a/apps/lfx-one/src/app/shared/services/campaign.service.ts +++ b/apps/lfx-one/src/app/shared/services/campaign.service.ts @@ -146,8 +146,9 @@ export class CampaignService { } public lookupHubSpotUtm(eventName: string): Observable { + const slug = this.foundationSlug; return this.http.get('/api/campaigns/hubspot/utm', { - params: { event_name: eventName, ...(this.foundationSlug && { foundationSlug: this.foundationSlug }) }, + params: { event_name: eventName, ...(slug && { foundationSlug: slug }) }, }); } diff --git a/apps/lfx-one/src/server/services/access-check.service.ts b/apps/lfx-one/src/server/services/access-check.service.ts index 3892491ff..38ea77749 100644 --- a/apps/lfx-one/src/server/services/access-check.service.ts +++ b/apps/lfx-one/src/server/services/access-check.service.ts @@ -137,12 +137,12 @@ export class AccessCheckService { * @param accessTypes Array of access types to check * @returns Record mapping each access type to its boolean result */ - public async checkMultipleAccess( + public async checkMultipleAccess( req: Request, resourceType: AccessCheckResourceType, id: string, - accessTypes: AccessCheckAccessType[] - ): Promise> { + accessTypes: T[] + ): Promise> { const operationName = `check_multiple_access_${resourceType}`; const startTime = logger.startOperation(req, operationName, { resource_type: resourceType, @@ -150,7 +150,7 @@ export class AccessCheckService { access_types: accessTypes, }); - const fallback = Object.fromEntries(accessTypes.map((a) => [a, false])) as Record; + const fallback = Object.fromEntries(accessTypes.map((a) => [a, false])) as Record; if (accessTypes.length === 0) { return fallback;