fix(security): close IDOR in getCurrentUserProfile

.claude/playbooks/port-better-auth-1.6-userguid.md

@@ -81,9 +81,15 @@ plugin. Better Auth generates its own internal `user.id`; the MP `User_GUID` (th
 OAuth `sub`) is carried on the session as a **custom user `additionalField`**
 named `userGuid`, populated server-side from the OAuth profile via
 `mapProfileToUser`. **Everything MP-related keys off `userGuid`** — the client
-`UserProvider` calls `getCurrentUserProfile(userGuid)` to load the profile
+`UserProvider` calls `getCurrentUserProfile()` to load the profile
 (avatar, name). No `userGuid` → no profile → dead avatar/menu.
 
+> If your fork still declares that action as `getCurrentUserProfile(userGuid)`,
+> fix it separately: a server action is a caller-shaped POST endpoint, so the
+> parameter is an IDOR — any authenticated MP user can read another user's
+> profile. Derive the GUID from the session inside the action. Unrelated to the
+> 1.6 upgrade, but you will be looking right at the code.
+
 **The breaking change:** As of Better Auth **1.6**, the function that pulls
 additional fields off an OAuth provider profile stopped letting a field declared
 `input: false` through when a value is supplied. Two things changed versus the
@@ -643,7 +649,7 @@ unusual route-group layout — stop and ask the user before improvising.
   because `genericOAuth`'s `additionalFields` aren't inferred — e.g.
   `(session?.user as { userGuid?: string })?.userGuid`.
 - **The avatar/menu chain end-to-end:** `useSession()` → `session.user.userGuid`
-  → `UserProvider` → `getCurrentUserProfile(userGuid)` → `MPUserProfile`
+  → `UserProvider` → `getCurrentUserProfile()` → `MPUserProfile`
   (`Image_GUID`, names) → `Header` renders the photo + `UserMenu`. Any break in
   `userGuid` collapses the whole chain to a non-interactive fallback
   (a generic `UserCircleIcon`, not text initials).

.claude/references/DECISIONS.md

@@ -80,7 +80,7 @@ Architectural decisions captured by the context-engineering review at SHA `971c4
 **Date:** 2026-04-17
 **Status:** Accepted
 **Context:** A tempting design is to enrich the session object inside `customSession` with the user's full MP profile (roles, user groups, Contact_ID, Image_GUID). Every consumer would then read a single object. The downside is that `customSession` runs on every cache miss, and MP profile lookups require extra MP API calls (`dp_Users` + `dp_User_Roles` + `dp_User_User_Groups`).
-**Decision:** `customSession` in `src/lib/auth.ts:97-112` does only `firstName` / `lastName` splitting from `user.name` — no API calls. MP profile loading moves to the client, behind `UserProvider` (`src/contexts/user-context.tsx`), which calls the `getCurrentUserProfile(userGuid)` server action on mount and exposes `useUser()`.
+**Decision:** `customSession` in `src/lib/auth.ts:398-413` does only `firstName` / `lastName` splitting from `user.name` — no API calls. MP profile loading moves to the client, behind `UserProvider` (`src/contexts/user-context.tsx`), which calls the parameterless `getCurrentUserProfile()` server action on mount (the action re-derives the GUID from the session — a GUID parameter would be an IDOR) and exposes `useUser()`.
 **Consequences:** `getSession()` stays cheap. Sign-in does not break when MP is down. Client components that need roles/groups must mount under `UserProvider`; every page load incurs one extra round-trip. `UserService.getUserProfile()` issues three queries (profile + roles + groups).
 **Alternatives considered:**
 - **Enrich in `customSession`** — would hit MP API on every JWT refresh and couple sign-in availability to MP uptime.

.claude/references/auth/oauth-flow.md

@@ -119,7 +119,8 @@ mapProfileToUser: (profile) => {
    e. Creates account (accountId=sub, tokens) — storeAccountCookie: true
    f. Creates session → sets JWT cookie (cookieCache)
 8. Browser lands on callbackURL (app page)
-9. Client-side UserProvider reads session.user.userGuid → getCurrentUserProfile(userGuid)
+9. Client-side UserProvider reads session.user.userGuid (to decide whether to load)
+   → getCurrentUserProfile()  [server action re-derives the GUID from the session]
 ```
 
 ## Sign-in entry (verbatim from `src/app/signin/page.tsx`)

.claude/references/auth/sessions.md

@@ -50,7 +50,7 @@ user: {
 ## `customSession` (verbatim)
 
 ```typescript
-// src/lib/auth.ts:97-112
+// src/lib/auth.ts:398-413
 customSession(
   async ({ user, session }) => {
     // No API calls here — profile loading is handled by UserProvider

.claude/references/auth/user-identity.md

@@ -123,12 +123,13 @@ const userGuid = (session?.user as { userGuid?: string } | undefined)?.userGuid;
 
 ```typescript
 // src/contexts/user-context.tsx:29-49 (excerpt)
+// userGuid gates whether the load fires; it is NOT passed to the action.
 const userGuid = (session?.user as { userGuid?: string } | undefined)?.userGuid;
 
 const loadUserProfile = useCallback(async () => {
   if (!userGuid) { /* ... */ return; }
   // ...
-  const profile = await getCurrentUserProfile(userGuid);
+  const profile = await getCurrentUserProfile(); // no argument: the action re-derives the GUID server-side
   setUserProfile(profile ?? null);
 }, [userGuid]);
 ```

.claude/references/components/layout.md

@@ -15,14 +15,14 @@ related:
   - ../routing/README.md
   - ../services/README.md
   - tool-framework.md
-last_verified: 2026-04-17
+last_verified: 2026-09-13
 ---
 
 ## Purpose
 `AuthWrapper` is the server-side session gate used at the app-shell level; it redirects unauthenticated requests to `/signin` while preserving the original path+query as `callbackUrl`. `shared-actions/` holds server actions used across multiple features (currently just `getCurrentUserProfile`).
 
 ## Files
-- `src/components/layout/auth-wrapper.tsx` — server component, 20 lines
+- `src/components/layout/auth-wrapper.tsx` — server component, 31 lines
 - `src/components/layout/auth-wrapper.test.tsx` — redirect + callback preservation tests
 - `src/components/layout/index.ts` — barrel: `AuthWrapper`
 - `src/components/shared-actions/user.ts` — `getCurrentUserProfile` server action
@@ -36,6 +36,8 @@ last_verified: 2026-04-17
 - The `x-pathname` header is set upstream by the proxy (`src/proxy.ts`) so the server component can see the original requested URL; it falls back to `/` when absent.
 - `shared-actions/user.ts` is marked `'use server'` at the top of the file — all exports are server actions.
 - Shared actions re-validate auth inside each action (`auth.api.getSession(...)`) — they do not trust the caller.
+- `getCurrentUserProfile` takes **no parameters**. The MP `User_GUID` is derived from the session inside the action. A caller-supplied GUID would be a live IDOR: server actions are caller-shaped POST endpoints, so any authenticated MP user could have read another user's contact details, roles, and user groups.
+- The guard keys on a non-empty-string `session.user.userGuid` (declared `required: true` in `src/lib/auth.ts`), not `session.user.id` — the latter is Better Auth's internal ID and its presence does not prove an MP identity exists.
 
 ## API / Interface
 
@@ -65,19 +67,28 @@ export async function AuthWrapper({ children }: { children: React.ReactNode }) {
     redirect(`${signinUrl.pathname}${signinUrl.search}`);
   }
 
+  // A session without a userGuid is unusable: every MP lookup keys off userGuid,
+  // and without it the header avatar/menu never renders — which leaves the user
+  // with no way to even sign out (the trap behind the better-auth 1.6 regression).
+  // Route these broken sessions to a recovery page that CAN sign them out,
+  // rather than rendering a dead app. /session-error lives outside the (web)
+  // route group, so it is not wrapped by AuthWrapper and cannot redirect-loop.
+  const userGuid = (session.user as { userGuid?: string | null }).userGuid;
+  if (!userGuid) {
+    redirect("/session-error");
+  }
+
   return <>{children}</>;
 }
 ```
 
 ### `getCurrentUserProfile`
-Source: `src/components/shared-actions/user.ts:8`
+Source: `src/components/shared-actions/user.ts:25`
 ```typescript
-export async function getCurrentUserProfile(
-  id: string
-): Promise<MPUserProfile | undefined>
+export async function getCurrentUserProfile(): Promise<MPUserProfile | undefined>
 ```
 
-Implementation:
+Implementation (docstring elided — see source for the IDOR rationale):
 ```typescript
 'use server';
 
@@ -86,13 +97,13 @@ import { MPUserProfile } from "@/lib/providers/ministry-platform/types";
 import { UserService } from '@/services/userService';
 import { headers } from 'next/headers';
 
-export async function getCurrentUserProfile(id: string): Promise<MPUserProfile | undefined> {
+export async function getCurrentUserProfile(): Promise<MPUserProfile | undefined> {
   const session = await auth.api.getSession({ headers: await headers() });
-  if (!session?.user?.id) throw new Error('Unauthorized');
+  const userGuid = (session?.user as Record<string, unknown> | undefined)?.userGuid;
+  if (typeof userGuid !== 'string' || userGuid.length === 0) throw new Error('Unauthorized');
 
   const userService = await UserService.getInstance();
-  const userProfile = await userService.getUserProfile(id);
-  return userProfile;
+  return userService.getUserProfile(userGuid);
 }
 ```
 
@@ -101,30 +112,36 @@ export async function getCurrentUserProfile(id: string): Promise<MPUserProfile |
   1. `await headers()` (Next.js 16 async dynamic API)
   2. `auth.api.getSession({ headers })` — Better Auth reads the JWT cookie, returns `null` if invalid/missing
   3. On `null`, read `x-pathname` (set by `src/proxy.ts`), build `/signin?callbackUrl=<originalPath>`, call `next/navigation` `redirect()` (which throws internally to abort rendering)
-  4. On valid session, render `<>{children}</>`
+  4. On a session with no `userGuid`, `redirect("/session-error")` — that route sits outside the `(web)` group, so it is not itself wrapped and cannot loop
+  5. On valid session, render `<>{children}</>`
 
 - **`getCurrentUserProfile` flow**
-  1. Re-validate session via `auth.api.getSession()` — if no `session.user.id`, throw `Unauthorized`
+  1. Re-validate session via `auth.api.getSession()` — if `session.user.userGuid` is not a non-empty string, throw `Unauthorized`
   2. Await `UserService.getInstance()` (async singleton)
-  3. Delegate to `userService.getUserProfile(id)` and return the `MPUserProfile` (or `undefined`)
+  3. Delegate to `userService.getUserProfile(userGuid)` and return the `MPUserProfile` (or `undefined`)
 
 ## Shared Actions catalog
 | Export | File | Purpose |
 |---|---|---|
-| `getCurrentUserProfile(id)` | `src/components/shared-actions/user.ts:8` | Fetch the current user's MP profile (`MPUserProfile`) by `User_GUID`; throws `Unauthorized` if no session. Backed by `UserService.getUserProfile`. |
+| `getCurrentUserProfile()` | `src/components/shared-actions/user.ts:25` | Fetch the **calling** user's MP profile (`MPUserProfile`); `User_GUID` comes from the session, never from a parameter. Throws `Unauthorized` if the session has no `userGuid`. Backed by `UserService.getUserProfile`. |
 
 Guidelines (verbatim from `src/components/shared-actions/README.md`):
 - Place actions here when they are **used by multiple components across different features**, provide **shared utility**, or handle **cross-cutting concerns**.
 - Keep actions **co-located** when they are feature-specific or tightly coupled to a single feature's logic.
 
 ## Tests
-- `src/components/layout/auth-wrapper.test.tsx` — 4 cases:
+- `src/components/layout/auth-wrapper.test.tsx` — 6 cases:
   - redirects with `callbackUrl` from `x-pathname`
   - falls back to `/` when `x-pathname` is missing
   - preserves URL-encoded query params through the redirect
+  - redirects to `/session-error` when `userGuid` is absent
+  - redirects to `/session-error` when `userGuid` is `null`
   - returns children when authenticated
-- `src/components/shared-actions/user.test.ts` — 3 cases:
-  - passes `id` through to `UserService.getUserProfile` and returns the profile
+- `src/components/shared-actions/user.test.ts` — 6 cases:
+  - looks the profile up with the session's `userGuid` and returns it
+  - ignores a caller-forged argument (cast through `unknown`) and still uses the session GUID
+  - throws `Unauthorized` when the session has no `userGuid`
+  - throws `Unauthorized` when `userGuid` is an empty string
   - throws `Unauthorized` when `auth.api.getSession()` returns `null`
   - propagates service-layer errors
 
@@ -135,6 +152,7 @@ Both test files use `vi.hoisted()` to share mock references (required pattern 
 - **`redirect()` throws.** `next/navigation` `redirect()` aborts rendering by throwing a magic error. Do not wrap in try/catch; do not add code after the redirect call expecting it to run on the unauthenticated branch.
 - **`callbackUrl` relies on `x-pathname`.** If a route bypasses the proxy (or a future proxy matcher excludes it), `x-pathname` will be missing and unauthenticated users land on `/` after sign-in. Verify proxy matcher coverage in `src/proxy.ts` when adding new protected routes.
 - **Shared actions must re-validate auth.** `getCurrentUserProfile` calls `auth.api.getSession()` itself rather than trusting caller context. Any new action added here must do the same (see `../auth/README.md` for session access patterns).
+- **Never re-add an identity parameter.** `getCurrentUserProfile` is a CLAUDE.md rule-12 carve-out from the `AuthorizationService` gate on the grounds that it returns only the caller's own profile. That justification holds only because there is no GUID argument to forge; adding one re-opens the IDOR.
 
 ## Related docs
 - `../auth/README.md` — Better Auth session shape, `session.user.userGuid` vs `session.user.id`