Skip to content

feat(audit): attribute MP writes to acting user via session context #64

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
chriskehayias merged 1 commit into from
May 21, 2026
Merged

feat(audit): attribute MP writes to acting user via session context #64

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
43 changes: 40 additions & 3 deletions src/lib/auth.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,40 @@ import { betterAuth, BetterAuthOptions } from "better-auth";
import { genericOAuth } from "better-auth/plugins";
import { customSession } from "better-auth/plugins";
import { nextCookies } from "better-auth/next-js";
import { MPHelper } from "@/lib/providers/ministry-platform";
import { sanitizeGuid } from "@/lib/providers/ministry-platform/utils/filter-sanitize";

const mpBaseUrl = process.env.MINISTRY_PLATFORM_BASE_URL!;

// Process-wide cache of User_GUID → MP User_ID. customSession runs on every
// getSession() call, so without a cache each request would do a dp_Users
// lookup. Mapping is stable per user, so an unbounded Map is fine in practice.
const userIdCache = new Map<string, number>();

async function resolveMpUserId(userGuid: string): Promise<number | null> {
const cached = userIdCache.get(userGuid);
if (cached !== undefined) return cached;
try {
const mp = new MPHelper();
const [record] = await mp.getTableRecords<{ User_ID: number }>({
table: "dp_Users",
filter: `User_GUID = '${sanitizeGuid(userGuid)}'`,
select: "User_ID",
top: 1,
});
if (record?.User_ID) {
userIdCache.set(userGuid, record.User_ID);
return record.User_ID;
}
return null;
} catch (err) {
// Never block session creation on this — the NonUser Write warning at
// write time will surface the missing attribution.
console.error("[customSession] resolveMpUserId failed", { userGuid, err });
return null;
}
}

const options = {
baseURL: process.env.BETTER_AUTH_URL || process.env.NEXTAUTH_URL,
secret: process.env.BETTER_AUTH_SECRET || process.env.NEXTAUTH_SECRET,
Expand Down Expand Up @@ -96,14 +127,20 @@ export const auth = betterAuth({
...(options.plugins ?? []),
customSession(
async ({ user, session }) => {
// No API calls here — profile loading is handled by UserProvider
// on the client side via getCurrentUserProfile(). This keeps
// getSession() fast and avoids hitting the MP API on every request.
// Profile loading still happens client-side via UserProvider /
// getCurrentUserProfile(). The only server-side lookup we do here is
// User_ID, cached in-memory after the first resolution per process,
// so it costs at most one MP call per (user × container).
const userGuid = (user as { userGuid?: string | null }).userGuid;
const userId: number | null = userGuid
? await resolveMpUserId(userGuid)
: null;
return {
user: {
...user,
firstName: user.name?.split(" ")[0] || "",
lastName: user.name?.split(" ").slice(1).join(" ") || "",
userId,
},
session,
};
Expand Down
143 changes: 119 additions & 24 deletions src/services/contactLogService.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@ const {
mockUpdateTableRecords,
mockDeleteTableRecords,
mockGetDomainInfo,
mockGetActingUserIdForWrite,
} = vi.hoisted(() => ({
mockGetTableRecords: vi.fn(),
mockCreateTableRecords: vi.fn(),
mockUpdateTableRecords: vi.fn(),
mockDeleteTableRecords: vi.fn(),
mockGetDomainInfo: vi.fn(),
mockGetActingUserIdForWrite: vi.fn(),
}));

vi.mock('@/lib/providers/ministry-platform', () => {
Expand All @@ -26,6 +28,14 @@ vi.mock('@/lib/providers/ministry-platform', () => {
};
});

vi.mock('@/services/sessionContextService', () => ({
SessionContextService: {
getInstance: () => ({
getActingUserIdForWrite: mockGetActingUserIdForWrite,
}),
},
}));

import { ContactLogService } from '@/services/contactLogService';
import { DomainTimezoneService } from '@/services/domainTimezoneService';

Expand All @@ -36,6 +46,8 @@ describe('ContactLogService', () => {
mockUpdateTableRecords.mockReset();
mockDeleteTableRecords.mockReset();
mockGetDomainInfo.mockReset();
mockGetActingUserIdForWrite.mockReset();
mockGetActingUserIdForWrite.mockResolvedValue(500);
mockGetDomainInfo.mockResolvedValue({
TimeZoneName: 'America/New_York',
DisplayName: 'Test',
Expand Down Expand Up @@ -186,17 +198,49 @@ describe('ContactLogService', () => {
Feedback_Entry_ID: null,
});

expect(mockCreateTableRecords).toHaveBeenCalledWith('Contact_Log', [
expect.objectContaining({
Contact_ID: 42,
Contact_Date: '2026-05-17 00:00:00',
Notes: 'Test note',
Made_By: 100,
}),
]);
expect(mockCreateTableRecords).toHaveBeenCalledWith(
'Contact_Log',
[
expect.objectContaining({
Contact_ID: 42,
Contact_Date: '2026-05-17 00:00:00',
Notes: 'Test note',
Made_By: 100,
}),
],
{ $userId: 500 },
);
expect(mockGetActingUserIdForWrite).toHaveBeenCalledWith({
table: 'Contact_Log',
operation: 'create',
});
expect(result).toEqual(mockCreated);
});

it('omits $userId param when SessionContextService resolves to null (anonymous write)', async () => {
mockGetActingUserIdForWrite.mockResolvedValueOnce(null);
mockCreateTableRecords.mockResolvedValueOnce([{ Contact_Log_ID: 1 }]);

const service = await ContactLogService.getInstance();
await service.createContactLog({
Contact_ID: 42,
Contact_Date: '2026-05-17',
Contact_Log_Type_ID: 1,
Made_By: 100,
Notes: 'Test note',
Planned_Contact_ID: null,
Contact_Successful: null,
Original_Contact_Log_Entry: null,
Feedback_Entry_ID: null,
});

expect(mockCreateTableRecords).toHaveBeenCalledWith(
'Contact_Log',
expect.any(Array),
undefined,
);
});

it('converts a UTC-tagged Contact_Date into MP-TZ wall-clock', async () => {
mockCreateTableRecords.mockResolvedValueOnce([{ Contact_Log_ID: 1 }]);

Expand All @@ -214,9 +258,11 @@ describe('ContactLogService', () => {
Feedback_Entry_ID: null,
});

expect(mockCreateTableRecords).toHaveBeenCalledWith('Contact_Log', [
expect.objectContaining({ Contact_Date: '2026-05-16 23:33:00' }),
]);
expect(mockCreateTableRecords).toHaveBeenCalledWith(
'Contact_Log',
[expect.objectContaining({ Contact_Date: '2026-05-16 23:33:00' })],
{ $userId: 500 },
);
});

it('throws when API returns empty result', async () => {
Expand Down Expand Up @@ -261,12 +307,20 @@ describe('ContactLogService', () => {
const service = await ContactLogService.getInstance();
const result = await service.updateContactLog(1, { Notes: 'Updated note' });

expect(mockUpdateTableRecords).toHaveBeenCalledWith('Contact_Log', [
expect.objectContaining({
Contact_Log_ID: 1,
Notes: 'Updated note',
}),
]);
expect(mockUpdateTableRecords).toHaveBeenCalledWith(
'Contact_Log',
[
expect.objectContaining({
Contact_Log_ID: 1,
Notes: 'Updated note',
}),
],
{ $userId: 500 },
);
expect(mockGetActingUserIdForWrite).toHaveBeenCalledWith({
table: 'Contact_Log',
operation: 'update',
});
expect(result).toEqual(mockUpdated);
});

Expand All @@ -276,12 +330,30 @@ describe('ContactLogService', () => {
const service = await ContactLogService.getInstance();
await service.updateContactLog(1, { Contact_Date: '2026-05-17' });

expect(mockUpdateTableRecords).toHaveBeenCalledWith('Contact_Log', [
expect.objectContaining({
Contact_Log_ID: 1,
Contact_Date: '2026-05-17 00:00:00',
}),
]);
expect(mockUpdateTableRecords).toHaveBeenCalledWith(
'Contact_Log',
[
expect.objectContaining({
Contact_Log_ID: 1,
Contact_Date: '2026-05-17 00:00:00',
}),
],
{ $userId: 500 },
);
});

it('omits $userId param when SessionContextService resolves to null (anonymous update)', async () => {
mockGetActingUserIdForWrite.mockResolvedValueOnce(null);
mockUpdateTableRecords.mockResolvedValueOnce([{ Contact_Log_ID: 1 }]);

const service = await ContactLogService.getInstance();
await service.updateContactLog(1, { Notes: 'Anon update' });

expect(mockUpdateTableRecords).toHaveBeenCalledWith(
'Contact_Log',
expect.any(Array),
undefined,
);
});

it('regression: round-tripping the same edit does not shift the date', async () => {
Expand All @@ -298,6 +370,7 @@ describe('ContactLogService', () => {

for (const call of mockUpdateTableRecords.mock.calls) {
expect(call[1][0].Contact_Date).toBe('2026-05-17 00:00:00');
expect(call[2]).toEqual({ $userId: 500 });
}
});

Expand All @@ -318,7 +391,29 @@ describe('ContactLogService', () => {
const service = await ContactLogService.getInstance();
await service.deleteContactLog(42);

expect(mockDeleteTableRecords).toHaveBeenCalledWith('Contact_Log', [42]);
expect(mockDeleteTableRecords).toHaveBeenCalledWith(
'Contact_Log',
[42],
{ $userId: 500 },
);
expect(mockGetActingUserIdForWrite).toHaveBeenCalledWith({
table: 'Contact_Log',
operation: 'delete',
});
});

it('omits $userId param when SessionContextService resolves to null (anonymous delete)', async () => {
mockGetActingUserIdForWrite.mockResolvedValueOnce(null);
mockDeleteTableRecords.mockResolvedValueOnce(undefined);

const service = await ContactLogService.getInstance();
await service.deleteContactLog(42);

expect(mockDeleteTableRecords).toHaveBeenCalledWith(
'Contact_Log',
[42],
undefined,
);
});

it('should propagate delete errors', async () => {
Expand Down
27 changes: 23 additions & 4 deletions src/services/contactLogService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { ContactLogTypes } from "@/lib/providers/ministry-platform/models/Contac
import { ContactLogSchema, ContactLogInput } from "@/lib/providers/ministry-platform/models/ContactLogSchema";
import { MPHelper } from "@/lib/providers/ministry-platform";
import { DomainTimezoneService } from "@/services/domainTimezoneService";
import { SessionContextService } from "@/services/sessionContextService";

/**
* ContactLogService - Singleton service for managing contact log operations
Expand Down Expand Up @@ -146,9 +147,15 @@ export class ContactLogService {
const mpDate = await tz.toMpSqlDatetime(Contact_Date);
console.log('ContactLogService.createContactLog - MP-TZ SQL date:', mpDate);

const $userId = await SessionContextService.getInstance().getActingUserIdForWrite({
table: "Contact_Log",
operation: "create",
});

const result = await this.mp!.createTableRecords(
"Contact_Log",
[{ ...validatedRest, Contact_Date: mpDate }]
[{ ...validatedRest, Contact_Date: mpDate }],
$userId !== null ? { $userId } : undefined
);

if (!result || result.length === 0) {
Expand Down Expand Up @@ -191,9 +198,15 @@ export class ContactLogService {
...(mpDate !== undefined ? { Contact_Date: mpDate } : {}),
};

const $userId = await SessionContextService.getInstance().getActingUserIdForWrite({
table: "Contact_Log",
operation: "update",
});

const result = await this.mp!.updateTableRecords(
"Contact_Log",
[updateData]
[updateData],
$userId !== null ? { $userId } : undefined
);

if (!result || result.length === 0) {
Expand All @@ -211,10 +224,16 @@ export class ContactLogService {
*/
public async deleteContactLog(contactLogId: number): Promise<void> {
console.log('ContactLogService.deleteContactLog - Deleting log:', contactLogId);


const $userId = await SessionContextService.getInstance().getActingUserIdForWrite({
table: "Contact_Log",
operation: "delete",
});

await this.mp!.deleteTableRecords(
"Contact_Log",
[contactLogId]
[contactLogId],
$userId !== null ? { $userId } : undefined
);

console.log('ContactLogService.deleteContactLog - Successfully deleted');
Expand Down
Loading
Loading