Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
136 changes: 136 additions & 0 deletions __tests__/security/novu-payload-allowlist.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
/**
* ADR 20 for the notification layer.
*
* The list-helper allowlists (`org-scope-payload-allowlist.test.ts`) pin what
* an organization can READ through the scoped queries. Nothing pinned what it
* can be SENT. That gap matters because `RecordingPayload.recordingUrl` puts a
* live media URL in a notification body, and the only thing keeping it away
* from an operator is that `notifyRecordingAvailable` is called with
* `getEventAttendeeIds(...)` — a participant list — rather than a roster.
*
* That is exactly the shape ADR 20 was written about: "the accident happened to
* be mostly right... but it held only because no one had yet added a field to a
* select statement". A future change widening that recipient list to
* `rosterForOrg(orgId, VISIBILITY_ROLES)` would leak the URL with no test
* failing, so these assertions pin the two halves of the rule:
*
* 1. Content-bearing payloads are only ever sent to participant-derived
* recipient lists.
* 2. The org-roster dispatchers carry no content field at all.
*
* Source-level assertions for the same reason the sibling suite gives: what
* matters is which recipient list each trigger reaches for.
*/

import { readFileSync } from "fs";
import { join } from "path";

const read = (rel: string) => readFileSync(join(process.cwd(), rel), "utf8");

const ORG_WORKFLOWS = "lib/novu/org-workflows.ts";
const WORKFLOWS = "lib/novu/workflows.ts";
const RECORDING_HANDLERS = "lib/stream/recording-handlers.ts";

/**
* Fields ADR 20 names as session CONTENT. A payload carrying one of these may
* only reach the two people who were in the session.
*/
const CONTENT_FIELDS = [
"recordingUrl",
"fileUrl",
"storagePath",
"requestNotes",
"feedbackFromConsultee",
"feedbackFromConsultant",
"cancellationNotes",
"transcript",
];

describe("ADR 20 — org-roster notifications carry no session content", () => {
it("no org-roster payload type declares a content field", () => {
const src = read(ORG_WORKFLOWS);
// Everything in org-workflows.ts dispatches to rosterForOrg(), so no
// payload defined or forwarded there may name a content field.
for (const field of CONTENT_FIELDS) {
expect(src).not.toContain(field);
}
});

it("the roster resolver is the only recipient source in org-workflows", () => {
const src = read(ORG_WORKFLOWS);
// Guards against someone importing getEventAttendeeIds here and blurring
// the two audiences into one file.
expect(src).not.toContain("getEventAttendeeIds");
expect(src).toContain("rosterForOrg");
});

it("recording notifications go to participants, never a roster", () => {
const src = read(RECORDING_HANDLERS);

// Asserting that `getEventAttendeeIds` merely APPEARS is too weak — it would
// still pass if the notifier were handed a roster while the resolver sat
// unused elsewhere in the file. So bind the two: take the identifier
// actually passed as the recipient argument, and require THAT identifier to
// be the one assigned from the attendee resolver.
const call = /notifyRecordingAvailable\(\s*([A-Za-z_$][\w$]*)\s*,/.exec(src);
expect(call).not.toBeNull();
const recipientVar = call![1];

const assignedFromResolver = new RegExp(
`(?:const|let|var)\\s+${recipientVar}\\s*=\\s*await\\s+getEventAttendeeIds\\(`,
);
expect(src).toMatch(assignedFromResolver);

// And the roster resolvers must not be reachable from this file at all, so
// the recipient list cannot be rebuilt from one further down.
expect(src).not.toContain("rosterForOrg");
expect(src).not.toContain("VISIBILITY_ROLES");
expect(src).not.toContain("OPERATOR_ROLES");
});
Comment thread
teetangh marked this conversation as resolved.

it("RecordingPayload is still the only content-bearing shared payload", () => {
const src = read(WORKFLOWS);
// If a second payload grows a content field, this fails and whoever added
// it has to come and think about who receives it.
const carriers = CONTENT_FIELDS.filter((f) => src.includes(f));
expect(carriers).toEqual(["recordingUrl"]);
});
});

describe("ADR 23 — dual-context payloads are attributable", () => {
const SCOPED_PAYLOADS = [
"AppointmentPayload",
"PaymentSuccessPayload",
"BookingRequestPayload",
"RecordingPayload",
];

it.each(SCOPED_PAYLOADS)("%s composes NotificationScope", (name) => {
const src = read(WORKFLOWS);
expect(src).toContain(`export type ${name} = NotificationScope & {`);
});

it("notificationScope keeps scope and organizationId consistent", async () => {
const { notificationScope } = await import("@/lib/novu/workflows");

expect(notificationScope(null)).toEqual({
organizationId: null,
scope: "personal",
});
expect(notificationScope(undefined)).toEqual({
organizationId: null,
scope: "personal",
});
expect(notificationScope("org_1", "Acme")).toEqual({
organizationId: "org_1",
scope: "org",
orgName: "Acme",
});
// A personal notification must not carry an org name — it would render an
// attribution the scope contradicts.
expect(notificationScope(null, "Acme")).toEqual({
organizationId: null,
scope: "personal",
});
});
});
12 changes: 11 additions & 1 deletion actions/maintenance/freeze-appointments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import crypto from "crypto";
import * as Sentry from "@sentry/nextjs";

import { notifyAppointmentCancelled } from "@/lib/novu/service";
import { notificationScope } from "@/lib/novu/workflows";
import { notificationHref } from "@/lib/novu/resolve-href";
import { createRefund } from "@/lib/payments";
import prisma from "@/lib/prisma";

Expand All @@ -23,6 +25,8 @@ type NotificationPayload = {
function buildCancellationNotification(params: {
appointmentId: string;
appointmentType: string;
/** ADR 23 — routes the notification to the dashboard owning the session. */
organizationId: string | null;
consultantUser: { id: string; name: string | null } | null | undefined;
participantIds: string[];
planTitle: string;
Expand All @@ -46,7 +50,8 @@ function buildCancellationNotification(params: {
consulteeName: params.consulteeName || "Participants",
planTitle: params.planTitle,
dateTime: params.dateTime,
dashboardUrl: "/dashboard",
...notificationScope(params.organizationId),
dashboardUrl: notificationHref(params.organizationId, "appointments"),
reason: "Scheduled platform maintenance",
cancelledBy: "system",
},
Expand Down Expand Up @@ -227,6 +232,7 @@ export async function freezeAppointments(

const notif = buildCancellationNotification({
appointmentId: appointment.id,
organizationId: appointment.organizationId,
appointmentType: "CONSULTATION",
consultantUser:
consultation.consultationPlan?.consultantProfile?.user,
Expand Down Expand Up @@ -258,6 +264,7 @@ export async function freezeAppointments(

const notif = buildCancellationNotification({
appointmentId: appointment.id,
organizationId: appointment.organizationId,
appointmentType: "SUBSCRIPTION",
consultantUser:
subscription.subscriptionPlan?.consultantProfile?.user,
Expand Down Expand Up @@ -286,6 +293,7 @@ export async function freezeAppointments(
);
const notif = buildCancellationNotification({
appointmentId: appointment.id,
organizationId: appointment.organizationId,
appointmentType: "WEBINAR",
consultantUser: webinar.webinarPlan?.consultantProfile?.user,
participantIds: webinarParticipantIds,
Expand All @@ -310,6 +318,7 @@ export async function freezeAppointments(
);
const notif = buildCancellationNotification({
appointmentId: appointment.id,
organizationId: appointment.organizationId,
appointmentType: "CLASS",
consultantUser: classEvent.classPlan?.consultantProfile?.user,
participantIds: classParticipantIds,
Expand All @@ -329,6 +338,7 @@ export async function freezeAppointments(

const notif = buildCancellationNotification({
appointmentId: appointment.id,
organizationId: appointment.organizationId,
appointmentType: "TRIAL",
consultantUser: trial.consultantProfile?.user,
participantIds: trial.consulteeProfile?.user?.id
Expand Down
14 changes: 12 additions & 2 deletions app/api/appointments/[appointmentId]/cancel/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@ import * as Sentry from "@sentry/nextjs";
import prisma from "@/lib/prisma";
import { NextRequest, NextResponse } from "next/server";
import { CancellationReason } from "@prisma/client";
import { notifyAppointmentCancelled } from "@/lib/novu";
import {
notifyAppointmentCancelled,
} from "@/lib/novu";
import { notificationScope } from "@/lib/novu/workflows";
import { notificationHref } from "@/lib/novu/resolve-href";
import { CancelAppointmentSchema } from "@/schemas/appointments";
import {
logConsultationCancelled,
Expand Down Expand Up @@ -416,12 +420,18 @@ export async function POST(
].filter((id): id is string => !!id);
if (userIds.length > 0) {
void notifyAppointmentCancelled(userIds, {
...notificationScope(appointment.organizationId),
appointmentId,
appointmentType: notificationMeta.appointmentType,
consultantName: notificationMeta.consultantName || "Consultant",
consulteeName: notificationMeta.consulteeName || "Consultee",
planTitle: notificationMeta.planTitle || "N/A",
dateTime: notificationMeta.dateTime,
dashboardUrl: "/dashboard",
// Both parties receive one payload, so the href has to suit either.
dashboardUrl: notificationHref(
appointment.organizationId,
"appointments",
),
reason: validatedData.reason || undefined,
cancelledBy:
notificationMeta.cancelledBy === notificationMeta.consultantUserId
Expand Down
12 changes: 9 additions & 3 deletions app/api/appointments/[appointmentId]/reschedule/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ import {
AppointmentNotFoundError,
} from "@/utils/errors/RescheduleErrors";
import { notifyAppointmentRescheduled } from "@/lib/novu/service";
import { notificationScope } from "@/lib/novu/workflows";
import { notificationHref } from "@/lib/novu/resolve-href";
import { logActivity } from "@/lib/activity/log-activity";
import { getAppUrl } from "@/lib/url";
import { hasActiveDisputeForAppointment } from "@/lib/payments/dispute-guard";
import {
CLASS_EVENT_ALLOWED_FROM,
Expand Down Expand Up @@ -585,13 +586,18 @@ export async function POST(
: "class";

if (uniqueUserIds.length > 0) {
const baseUrl = getAppUrl();
void notifyAppointmentRescheduled(uniqueUserIds, {
...notificationScope(appointment.organizationId),
appointmentType,
consultantName: plan?.consultantProfile?.user?.name ?? "Consultant",
consulteeName: requestedBy?.user?.name ?? "Participant",
planTitle: plan?.title ?? "Unknown",
dashboardUrl: `${baseUrl}/dashboard`,
// Group events fan out to every attendee, so one href must serve
// them all — org route when org-hosted, router bounce otherwise.
dashboardUrl: notificationHref(
appointment.organizationId,
"appointments",
),
}).catch((err) =>
console.error("[reschedule] Failed to send notification:", err),
);
Expand Down
7 changes: 7 additions & 0 deletions app/api/novu/preferences/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ export async function GET() {
trialNotifications: true,
subscriptionAlerts: true,
marketingEmails: false,
orgBillingAlerts: true,
orgMembershipAlerts: true,
orgProgramAlerts: true,
quietHoursEnabled: false,
quietHoursStart: null,
quietHoursEnd: null,
Expand Down Expand Up @@ -101,6 +104,10 @@ export async function PUT(req: NextRequest) {
trialNotifications: updated.trialNotifications,
subscriptionAlerts: updated.subscriptionAlerts,
marketingEmails: updated.marketingEmails,
// ADR 23 — org categories
orgBillingAlerts: updated.orgBillingAlerts,
orgMembershipAlerts: updated.orgMembershipAlerts,
orgProgramAlerts: updated.orgProgramAlerts,
});

return NextResponse.json(updated);
Expand Down
4 changes: 4 additions & 0 deletions app/api/novu/subscriber/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ export async function POST() {
phone: true,
image: true,
timezone: true,
// ADR 23 — routing preference for operators who own a workspace.
orgWorkspaceProfile: { select: { notificationRoutingMode: true } },
},
});

Expand All @@ -41,6 +43,8 @@ export async function POST() {
phone: user.phone || undefined,
avatar: user.image || undefined,
locale: "en",
routingMode:
user.orgWorkspaceProfile?.notificationRoutingMode ?? undefined,
});

return NextResponse.json({ success: true });
Expand Down
24 changes: 21 additions & 3 deletions app/api/slots/request-for-approval/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@ import { AppointmentStatus } from "@prisma/client";
import { lockSlotBooking, unlockSlotBooking } from "@/utils/appointmentlock";
import { SlotLockError } from "@/utils/errors/SlotLockError";
import { SlotValidationService } from "@/utils/slotAllocation/SlotValidationService";
import { notifyNewBookingRequest } from "@/lib/novu";
import {
notifyNewBookingRequest,
} from "@/lib/novu";
import { notificationScope } from "@/lib/novu/workflows";
import { scopedHref } from "@/lib/novu/resolve-href";
import { RequestForApprovalSchema } from "@/schemas/slots";
import { requestApprovalLimiter, applyRateLimit } from "@/lib/rate-limit";
import { ensureConsulteeProfile } from "@/lib/profiles/ensure-consultee-profile";
Expand Down Expand Up @@ -230,15 +234,29 @@ export async function POST(req: NextRequest) {
}),
);

// Fire-and-forget: notify consultant of new booking request
// Fire-and-forget: notify consultant of new booking request.
//
// ADR 23 — the link used to hardcode the personal Requests page even for
// an org-hosted plan, where the request is not listed: the personal scope
// pins organizationId: null. Single recipient with a known side, so this
// resolves to a precise route rather than the /dashboard bounce.
const requestOrgId = consultation.appointment?.organizationId ?? null;
void notifyNewBookingRequest(
consultation.consultationPlan.consultantProfile.user.id,
{
...notificationScope(requestOrgId),
consulteeName: consultation.requestedBy.user.name || "A consultee",
planTitle: consultation.consultationPlan.title,
appointmentType: "CONSULTATION",
requestedDateTime: startTime.toISOString(),
dashboardUrl: `/dashboard/consultant/${consultation.consultationPlan.consultantProfile.id}/requests`,
dashboardUrl: scopedHref({
organizationId: requestOrgId,
surface: "requests",
personal: {
kind: "consultant",
profileId: consultation.consultationPlan.consultantProfile.id,
},
}),
Comment thread
teetangh marked this conversation as resolved.
},
);

Expand Down
6 changes: 6 additions & 0 deletions app/api/webhooks/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
notifyDisputeCreated,
notifyDisputeResolved,
} from "@/lib/novu";
import { notificationScope } from "@/lib/novu/workflows";
import {
notifyOrgInvoicePaid,
notifyOrgWalletTopupConfirmed,
Expand Down Expand Up @@ -958,6 +959,11 @@ export async function handleRefundCreated(

// --- Novu notification (fire-and-forget) ---
void notifyRefundProcessed(payment.userId, {
// Payment.organizationId is the org tag (#PaymentOrgTag), so a refund
// inherits the org-ness of the payment it reverses. dashboardUrl stays a
// router bounce deliberately: this goes to the PAYER, and an org billing
// page is not readable by a LEARNER whose booking was org-sponsored.
...notificationScope(payment.organizationId),
amount,
currency,
dashboardUrl: `${getAppUrl()}/dashboard`,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import { DashboardErrorBoundary } from "@/components/DashboardErrorBoundary";
import { isActiveRoute } from "@/components/dashboard/route-active";
import { LinkPendingIcon } from "@/components/ui/NavLink";
import { OrganizationSwitcher } from "@/components/dashboard/OrganizationSwitcher";
import { useNovuSubscriberSync } from "@/hooks/useNovuSubscriberSync";
import { NotificationInbox } from "@/components/notifications/NotificationInbox";
import { signOutEverywhere } from "@/lib/auth/sign-out";

Expand Down Expand Up @@ -85,6 +86,11 @@ export function OrgWorkspaceShell({
userImage: string | null;
children: React.ReactNode;
}) {
// ADR 23 — see the org layout. This tree also carries a bell but never
// synced the subscriber behind it, and it is where the routing-mode
// preference is set, so the sync has to run here for that to take effect.
useNovuSubscriberSync();

// usePathname() returns the URL-encoded path, while orgWorkspaceId (from
// route params) is decoded — decode so basePath.slice + isActiveRoute compare
// like-for-like even for ids that need encoding. `?? ""` guards a null path;
Expand Down
Loading
Loading