fix(security): authenticate searchContacts and getCurrentUserProfile

[chriskehayias]

Summary

Two 'use server' actions were reachable as unauthenticated POST endpoints. Server actions compile to callable endpoints and src/proxy.ts:8 lets all /api paths through without a session, so neither had anything standing between an anonymous caller and Ministry Platform PII.

Closes the two TODOs under .claude/TODO/.

searchContacts — no session check

Searched Contacts across First_Name, Last_Name, Nickname, Email_Address, and Mobile_Phone, returning up to 20 records including email address and mobile phone, to any caller. A one-character search term was enough.

Now calls auth.api.getSession and throws Authentication required before any other work.

Two placement details that are deliberate rather than stylistic:

• The check sits before the try/catch. Inside it, the existing catch would have rewritten the auth failure as Failed to search contacts — a misleading error, and one that hides a security condition behind a generic one. Sibling files put the check inside their try, but they also rethrow the original error, so the same masking does not occur there.
• Placing it first also stops the empty-search-term early return from being an unauthenticated success path. An anonymous searchContacts('') previously returned [] — a cheap oracle confirming the endpoint is live.

getCurrentUserProfile — no session check and no ownership check

Took an arbitrary User_GUID and returned that user's profile plus their roles and user groups (userService.ts:72-89) — i.e. it disclosed the authorization model for any user whose GUID was known. GUIDs are not usefully secret: session.user.userGuid is in the client-side session, and MP GUIDs appear in /contactlookup/[guid] URLs.

Rather than validate the parameter, the parameter is gone. The GUID now comes from the session:

export async function getCurrentUserProfile(): Promise<MPUserProfile | undefined> {
  const session = await auth.api.getSession({ headers: await headers() });
  if (!session?.user?.id) throw new Error('Authentication required');

  const userGuid = (session.user as { userGuid?: string }).userGuid;
  if (!userGuid) throw new Error('User GUID not found in session');
  ...
}

This makes the ownership question unaskable rather than merely answered — there is no argument left for a caller to tamper with, so no ownership comparison to get wrong later. The TODO recommended this over the reject-or-role-gate alternative, and it was available: the sole production caller (UserProvider) already passed the session's own GUID, so this is not a capability regression. UserProvider keeps userGuid as an effect dependency so switching users still re-fetches.

Cross-user profile reads, if ever needed, belong in a separate role-gated function rather than a widening of this one — noted in the JSDoc.

Tests

427 passing, up from 421. New cases: null session, session with no user.id, authenticated session missing userGuid, an unauthenticated empty search term, and — the one the old tests structurally could not ask — that a caller-supplied GUID cannot displace the session's:

await (getCurrentUserProfile as unknown as (id: string) => Promise<unknown>)('someone-elses-guid');
expect(mockGetUserProfile).toHaveBeenCalledWith('guid-123');
expect(mockGetUserProfile).not.toHaveBeenCalledWith('someone-elses-guid');

The type signature stops a compiled caller from passing a GUID; that test covers a hand-rolled POST at the endpoint, which the type system does not reach.

Both files were previously at 100% statement and branch coverage. Coverage could not flag a check that was never written — recorded in TestCoverage.md §5.2/§5.3, now updated from 🔴 to ✅ with the reasoning.

Also audited, deliberately unchanged

The other two 'use server' files with no getSession call are both fine:

• getMpTimezone — returns a non-sensitive server config string (an IANA zone), no PII.
• handleSignOut — sign-out is idempotent and must work without a valid session.

Verification

• npm run test:run — 30 files, 427 tests passing
• npm run lint — clean
• npm run build — compiles, TypeScript clean, 7/7 static pages

Out of scope

Two unrelated in-flight working-tree changes (investigate-emnapi-lockfile-drift.md, a deleted Better Auth playbook) were deliberately left unstaged and are not in this branch.

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!