fix(security): authenticate searchContacts and getCurrentUserProfile

.claude/docs/TestCoverage.md

@@ -165,21 +165,33 @@ searchContactLogs("5; DROP")   →  filter: "Contact_ID = 5; DROP"
 `contactLogService.ts` is at **100% statements and 100% branches**. No test passes a non-numeric value,
 which is exactly why full coverage did not catch it.
 
-### 5.2 `searchContacts` — no authentication 🔴
+### 5.2 `searchContacts` — no authentication ✅ FIXED
 
-→ `.claude/TODO/server-action-search-contacts-unauthenticated.md`
+Was: a `'use server'` action with zero `getSession` calls, returning up to 20 contacts including
+email and mobile phone. `proxy.ts:8` allows all `/api` paths without a session, and every sibling
+action file did check. 100% statements, 100% branches, 5 passing tests, none of which asked the
+authorization question — because nothing in the code answered it.
 
-A `'use server'` action with zero `getSession` calls, returning up to 20 contacts including email and
-mobile phone. `proxy.ts:8` allows all `/api` paths without a session, and every sibling action file
-does check. 100% statements, 100% branches, 5 passing tests, none of which asks the authorization
-question — because nothing in the code answers it.
+Now: `searchContacts` calls `auth.api.getSession` and throws `Authentication required` before any
+other work. The check sits **before** the try/catch, so the auth failure surfaces as itself rather
+than being masked as `Failed to search contacts`, and the empty-search-term early return cannot
+become an unauthenticated success path. Three tests cover it: null session, session with no
+`user.id`, and rejection of an empty term while unauthenticated.
 
-### 5.3 `getCurrentUserProfile` — no authentication, no ownership check 🔴
+### 5.3 `getCurrentUserProfile` — no authentication, no ownership check ✅ FIXED
 
-→ `.claude/TODO/server-action-user-profile-unauthenticated.md`
+Was: took an arbitrary `User_GUID` and returned that user's profile **plus their roles and user
+groups** — disclosing the authorization model for any user whose GUID was known. 100% covered; both
+tests asserted pass-through.
 
-Takes an arbitrary `User_GUID` and returns that user's profile **plus their roles and user groups**.
-100% covered. Both tests assert pass-through.
+Now: the parameter is **gone**. The action reads `userGuid` from the session, which makes the
+ownership question unaskable rather than merely answered — there is no longer an argument for a
+caller to tamper with. It throws `Authentication required` with no session and `User GUID not found
+in session` when an authenticated session carries no GUID. `UserProvider` calls it with no argument
+and keeps `userGuid` only as an effect dependency so switching users still re-fetches.
+
+If a feature ever needs to read another user's profile, that is a separate, explicitly role-gated
+function — not a widening of this one.
 
 ### 5.4 Contact-log actions authenticate but never authorize 🟠
 

src/components/contact-lookup/actions.test.ts

@@ -1,7 +1,20 @@
 import { describe, it, expect, vi, beforeEach } from 'vitest';
 
-const { mockContactSearch } = vi.hoisted(() => ({
+const { mockContactSearch, mockGetSession } = vi.hoisted(() => ({
   mockContactSearch: vi.fn(),
+  mockGetSession: vi.fn(),
+}));
+
+vi.mock('@/lib/auth', () => ({
+  auth: {
+    api: {
+      getSession: mockGetSession,
+    },
+  },
+}));
+
+vi.mock('next/headers', () => ({
+  headers: vi.fn().mockResolvedValue(new Headers()),
 }));
 
 vi.mock('@/services/contactService', () => ({
@@ -14,9 +27,36 @@ vi.mock('@/services/contactService', () => ({
 
 import { searchContacts } from './actions';
 
+const mockAuthSession = {
+  user: { id: 'internal-id', userGuid: 'user-guid-123' },
+};
+
 describe('searchContacts', () => {
   beforeEach(() => {
     vi.clearAllMocks();
+    mockGetSession.mockResolvedValue(mockAuthSession);
+  });
+
+  it('should require authentication', async () => {
+    mockGetSession.mockResolvedValue(null);
+
+    await expect(searchContacts('John')).rejects.toThrow('Authentication required');
+    expect(mockContactSearch).not.toHaveBeenCalled();
+  });
+
+  it('should reject a session with no user id', async () => {
+    mockGetSession.mockResolvedValue({ user: { userGuid: 'user-guid-123' } });
+
+    await expect(searchContacts('John')).rejects.toThrow('Authentication required');
+    expect(mockContactSearch).not.toHaveBeenCalled();
+  });
+
+  it('should reject an unauthenticated caller before validating the search term', async () => {
+    mockGetSession.mockResolvedValue(null);
+
+    // The empty-term early return must not become an unauthenticated success path.
+    await expect(searchContacts('')).rejects.toThrow('Authentication required');
+    expect(mockContactSearch).not.toHaveBeenCalled();
   });
 
   it('should return results for valid search term', async () => {

src/components/contact-lookup/actions.ts

@@ -2,19 +2,29 @@
 
 import { ContactService } from '@/services/contactService';
 import { ContactSearch } from '@/lib/dto';
+import { auth } from '@/lib/auth';
+import { headers } from 'next/headers';
 
 export async function searchContacts(searchTerm: string): Promise<ContactSearch[]> {
+  // Server actions compile to callable POST endpoints and src/proxy.ts lets all
+  // /api paths through without a session, so this check is the only thing
+  // standing between an anonymous caller and 20 contacts' emails and phones.
+  const session = await auth.api.getSession({ headers: await headers() });
+  if (!session?.user?.id) {
+    throw new Error('Authentication required');
+  }
+
   try {
     if (!searchTerm || searchTerm.trim().length === 0) {
       return [];
     }
 
     const contactService = await ContactService.getInstance();
     const results = await contactService.contactSearch(searchTerm.trim());
-    
+
     return results;
   } catch (error) {
     console.error('Error searching contacts:', error);
     throw new Error('Failed to search contacts');
   }
-}
+}

src/components/shared-actions/user.test.ts

@@ -1,7 +1,20 @@
 import { describe, it, expect, vi, beforeEach } from 'vitest';
 
-const { mockGetUserProfile } = vi.hoisted(() => ({
+const { mockGetUserProfile, mockGetSession } = vi.hoisted(() => ({
   mockGetUserProfile: vi.fn(),
+  mockGetSession: vi.fn(),
+}));
+
+vi.mock('@/lib/auth', () => ({
+  auth: {
+    api: {
+      getSession: mockGetSession,
+    },
+  },
+}));
+
+vi.mock('next/headers', () => ({
+  headers: vi.fn().mockResolvedValue(new Headers()),
 }));
 
 vi.mock('@/services/userService', () => ({
@@ -14,36 +27,85 @@ vi.mock('@/services/userService', () => ({
 
 import { getCurrentUserProfile } from './user';
 
+const mockAuthSession = {
+  user: { id: 'internal-id', userGuid: 'guid-123' },
+};
+
+const mockProfile = {
+  User_ID: 1,
+  User_GUID: 'guid-123',
+  Contact_ID: 100,
+  First_Name: 'John',
+  Nickname: 'Johnny',
+  Last_Name: 'Doe',
+  Email_Address: 'john@example.com',
+  Mobile_Phone: null,
+  Image_GUID: null,
+  roles: ['Admin'],
+  userGroups: ['Staff'],
+};
+
 describe('getCurrentUserProfile', () => {
   beforeEach(() => {
     vi.clearAllMocks();
   });
 
-  it('should call UserService with correct ID', async () => {
-    const mockProfile = {
-      User_ID: 1,
-      User_GUID: 'guid-123',
-      Contact_ID: 100,
-      First_Name: 'John',
-      Nickname: 'Johnny',
-      Last_Name: 'Doe',
-      Email_Address: 'john@example.com',
-      Mobile_Phone: null,
-      Image_GUID: null,
-      roles: ['Admin'],
-      userGroups: ['Staff'],
-    };
+  it('should require authentication', async () => {
+    mockGetSession.mockResolvedValueOnce(null);
+
+    await expect(getCurrentUserProfile()).rejects.toThrow('Authentication required');
+    expect(mockGetUserProfile).not.toHaveBeenCalled();
+  });
+
+  it('should reject a session with no user id', async () => {
+    mockGetSession.mockResolvedValueOnce({ user: { userGuid: 'guid-123' } });
+
+    await expect(getCurrentUserProfile()).rejects.toThrow('Authentication required');
+    expect(mockGetUserProfile).not.toHaveBeenCalled();
+  });
+
+  it('should reject an authenticated session that carries no userGuid', async () => {
+    mockGetSession.mockResolvedValueOnce({ user: { id: 'internal-id' } });
+
+    await expect(getCurrentUserProfile()).rejects.toThrow('User GUID not found in session');
+    expect(mockGetUserProfile).not.toHaveBeenCalled();
+  });
+
+  it("should look up the profile using the session's own User_GUID", async () => {
+    mockGetSession.mockResolvedValueOnce(mockAuthSession);
     mockGetUserProfile.mockResolvedValueOnce(mockProfile);
 
-    const result = await getCurrentUserProfile('guid-123');
+    const result = await getCurrentUserProfile();
 
     expect(mockGetUserProfile).toHaveBeenCalledWith('guid-123');
     expect(result).toEqual(mockProfile);
   });
 
+  it('should ignore any caller-supplied GUID and use the session GUID', async () => {
+    mockGetSession.mockResolvedValueOnce(mockAuthSession);
+    mockGetUserProfile.mockResolvedValueOnce(mockProfile);
+
+    // A hostile caller can still POST an argument at the compiled endpoint; the
+    // action takes no parameters, so the victim's GUID must never be used.
+    await (getCurrentUserProfile as unknown as (id: string) => Promise<unknown>)(
+      'someone-elses-guid'
+    );
+
+    expect(mockGetUserProfile).toHaveBeenCalledWith('guid-123');
+    expect(mockGetUserProfile).not.toHaveBeenCalledWith('someone-elses-guid');
+  });
+
+  it('should return undefined when MP has no matching user', async () => {
+    mockGetSession.mockResolvedValueOnce(mockAuthSession);
+    mockGetUserProfile.mockResolvedValueOnce(undefined);
+
+    await expect(getCurrentUserProfile()).resolves.toBeUndefined();
+  });
+
   it('should propagate errors', async () => {
+    mockGetSession.mockResolvedValueOnce(mockAuthSession);
     mockGetUserProfile.mockRejectedValueOnce(new Error('Service error'));
 
-    await expect(getCurrentUserProfile('guid-123')).rejects.toThrow('Service error');
+    await expect(getCurrentUserProfile()).rejects.toThrow('Service error');
   });
 });