Merge pull request #4 from hexawulf/fix/data-isolation-snippets

Fix: Implement user-specific snippet filtering

Author: hexawulf

server/__tests__/routes.test.ts

@@ -6,33 +6,46 @@ import { registerRoutes, authMiddleware } from '../routes'; // Adjust path
 import { storage } from '../storage'; // Adjust path
 import { type Snippet } from '@shared/schema'; // Adjust path
 
+// Mock the storage module
+import admin from 'firebase-admin'; // Import firebase-admin
+
+// Mock firebase-admin
+jest.mock('firebase-admin', () => ({
+  credential: {
+    cert: jest.fn(),
+  },
+  initializeApp: jest.fn(),
+  auth: () => ({
+    verifyIdToken: jest.fn(),
+  }),
+}));
+
 // Mock the storage module
 jest.mock('../storage', () => ({
   storage: {
     getSnippets: jest.fn(),
     getSnippet: jest.fn(),
-    // Add any other storage methods if they get called by routes indirectly
-    // For GET /api/public/snippets/:id, incrementSnippetViewCount might be called
-    // Let's add it to avoid potential undefined function errors if the route calls it.
-    incrementSnippetViewCount: jest.fn().mockResolvedValue(undefined), 
+    getUser: jest.fn(), // Add getUser to the mock
+    upsertUser: jest.fn(), // Add upsertUser if it's called
+    incrementSnippetViewCount: jest.fn().mockResolvedValue(undefined),
   },
 }));
 
-// Mock the authMiddleware to prevent actual auth logic from running
+// Mock the authMiddleware
+// We will use the actual authMiddleware but mock its dependencies (firebase-admin, storage.getUser)
+// This is a change from the original approach of completely mocking out authMiddleware.
+// This allows us to test the middleware's logic more accurately.
+const originalRoutes = jest.requireActual('../routes');
 jest.mock('../routes', () => {
-  const originalModule = jest.requireActual('../routes');
+  const original = jest.requireActual('../routes');
   return {
-    ...originalModule,
-    authMiddleware: jest.fn((req, res, next) => {
-      // Simulate an authenticated user if needed for other tests, but not for public routes
-      // req.user = { id: 'test-user', email: 'test@example.com' };
-      next();
-    }),
+    ...original,
+    // authMiddleware is NOT mocked here anymore, we use the real one.
   };
 });
 
-// Mock pool for health check, not strictly necessary for these tests but good practice
-// if registerRoutes touches it.
+
+// Mock pool for health check
 jest.mock('../db', () => ({
   pool: {
     connect: jest.fn().mockResolvedValue({
@@ -47,14 +60,122 @@ let app: Express;
 
 beforeEach(async () => { // Make beforeEach async if registerRoutes is async
   app = express();
-  app.use(express.json()); // Required for Express to parse JSON request bodies
-  await registerRoutes(app); // Register all routes, ensure this completes if it's async
+  app.use(express.json());
+  // Use originalRoutes.registerRoutes from the actual module
+  await originalRoutes.registerRoutes(app); 
 
   // Clear mock history before each test
   (storage.getSnippets as jest.Mock).mockClear();
   (storage.getSnippet as jest.Mock).mockClear();
+  (storage.getUser as jest.Mock).mockClear();
   (storage.incrementSnippetViewCount as jest.Mock).mockClear();
-  (authMiddleware as jest.Mock).mockClear();
+  // Clear firebase-admin mocks
+  (admin.auth().verifyIdToken as jest.Mock).mockClear();
+});
+
+// Define mock users and snippets
+const user_A_id = 'user_A_id';
+const user_B_id = 'user_B_id';
+const user_C_id = 'user_C_id';
+
+const mockUserA = { id: user_A_id, email: 'a@example.com', displayName: 'User A', photoURL: null, createdAt: new Date(), updatedAt: new Date(), username: 'usera', githubId: null, bio: null, location: null, website: null };
+const mockUserB = { id: user_B_id, email: 'b@example.com', displayName: 'User B', photoURL: null, createdAt: new Date(), updatedAt: new Date(), username: 'userb', githubId: null, bio: null, location: null, website: null };
+const mockUserC = { id: user_C_id, email: 'c@example.com', displayName: 'User C', photoURL: null, createdAt: new Date(), updatedAt: new Date(), username: 'userc', githubId: null, bio: null, location: null, website: null };
+
+
+const snippet1: Partial<Snippet> = { id: 1, title: 'User A Snippet 1', code: 'code1', language: 'js', userId: user_A_id, isPublic: false };
+const snippet2: Partial<Snippet> = { id: 2, title: 'User A Snippet 2', code: 'code2', language: 'ts', userId: user_A_id, isPublic: false };
+const snippet3: Partial<Snippet> = { id: 3, title: 'User B Snippet 1', code: 'code3', language: 'py', userId: user_B_id, isPublic: false };
+const publicSnippet: Partial<Snippet> = { id: 4, title: 'Public Snippet', code: 'public', language: 'md', userId: user_A_id, isPublic: true };
+
+describe('GET /api/snippets (User-Specific)', () => {
+  it('should return 401 if no token is provided', async () => {
+    const response = await request(app).get('/api/snippets');
+    expect(response.status).toBe(401);
+    expect(response.body.message).toContain('Unauthorized');
+  });
+
+  it('should return snippets for User A when authenticated as User A', async () => {
+    (admin.auth().verifyIdToken as jest.Mock).mockResolvedValue({ uid: user_A_id });
+    (storage.getUser as jest.Mock).mockResolvedValue(mockUserA);
+    (storage.getSnippets as jest.Mock).mockImplementation(async (filters) => {
+      if (filters.userId === user_A_id) {
+        return [snippet1, snippet2];
+      }
+      return [];
+    });
+
+    const response = await request(app)
+      .get('/api/snippets')
+      .set('Authorization', 'Bearer test-token-user-a');
+
+    expect(response.status).toBe(200);
+    expect(response.body).toEqual([snippet1, snippet2]);
+    expect(storage.getSnippets).toHaveBeenCalledWith(expect.objectContaining({ userId: user_A_id }));
+    expect(admin.auth().verifyIdToken).toHaveBeenCalledWith('test-token-user-a');
+    expect(storage.getUser).toHaveBeenCalledWith(user_A_id);
+  });
+
+  it('should return snippets for User B when authenticated as User B', async () => {
+    (admin.auth().verifyIdToken as jest.Mock).mockResolvedValue({ uid: user_B_id });
+    (storage.getUser as jest.Mock).mockResolvedValue(mockUserB);
+    (storage.getSnippets as jest.Mock).mockImplementation(async (filters) => {
+      if (filters.userId === user_B_id) {
+        return [snippet3];
+      }
+      return [];
+    });
+
+    const response = await request(app)
+      .get('/api/snippets')
+      .set('Authorization', 'Bearer test-token-user-b');
+
+    expect(response.status).toBe(200);
+    expect(response.body).toEqual([snippet3]);
+    expect(storage.getSnippets).toHaveBeenCalledWith(expect.objectContaining({ userId: user_B_id }));
+  });
+
+  it('should return an empty array for User C (who has no snippets) when authenticated as User C', async () => {
+    (admin.auth().verifyIdToken as jest.Mock).mockResolvedValue({ uid: user_C_id });
+    (storage.getUser as jest.Mock).mockResolvedValue(mockUserC);
+    (storage.getSnippets as jest.Mock).mockImplementation(async (filters) => {
+      if (filters.userId === user_C_id) {
+        return [];
+      }
+      return [snippet1, snippet2, snippet3]; // Should not happen if filter is correct
+    });
+
+    const response = await request(app)
+      .get('/api/snippets')
+      .set('Authorization', 'Bearer test-token-user-c');
+
+    expect(response.status).toBe(200);
+    expect(response.body).toEqual([]);
+    expect(storage.getSnippets).toHaveBeenCalledWith(expect.objectContaining({ userId: user_C_id }));
+  });
+  
+  it('should return 401 if token is invalid', async () => {
+    (admin.auth().verifyIdToken as jest.Mock).mockRejectedValue(new Error('Invalid token'));
+    
+    const response = await request(app)
+      .get('/api/snippets')
+      .set('Authorization', 'Bearer invalid-token');
+      
+    expect(response.status).toBe(401);
+    expect(response.body.message).toContain('Invalid token');
+  });
+
+  it('should return 404 if user from token is not found in storage', async () => {
+    (admin.auth().verifyIdToken as jest.Mock).mockResolvedValue({ uid: 'nonexistent-user-id' });
+    (storage.getUser as jest.Mock).mockResolvedValue(undefined); // User not found
+    
+    const response = await request(app)
+      .get('/api/snippets')
+      .set('Authorization', 'Bearer token-for-nonexistent-user');
+      
+    expect(response.status).toBe(404);
+    expect(response.body.message).toContain('User not found');
+  });
 });
 
 describe('Public API Snippet Routes', () => {
@@ -68,29 +189,39 @@ describe('Public API Snippet Routes', () => {
 
       const response = await request(app).get('/api/public/snippets');
 
+      const mockPublicSnippetsActual: Snippet[] = [ // Use actual Snippet type
+        { id: 1, title: 'Public Snippet 1', isPublic: true, language: 'javascript', code: '', userId: user_A_id, createdAt: new Date(), updatedAt: new Date(), viewCount:0, isFavorite: false, shareId: null, tags: [] },
+        { id: 2, title: 'Public Snippet 2', isPublic: true, language: 'python', code: '', userId: user_B_id, createdAt: new Date(), updatedAt: new Date(), viewCount:0, isFavorite: false, shareId: null, tags: []  },
+      ];
+      (storage.getSnippets as jest.Mock).mockResolvedValue(mockPublicSnippetsActual);
+
+      const response = await request(app).get('/api/public/snippets');
+
       expect(response.status).toBe(200);
-      expect(response.body).toEqual(mockPublicSnippets);
-      expect(storage.getSnippets).toHaveBeenCalledWith(expect.objectContaining({ // Use objectContaining for flexibility
+      expect(response.body).toEqual(mockPublicSnippetsActual.map(s => ({...s, createdAt: s.createdAt.toISOString(), updatedAt: s.updatedAt.toISOString() }))); // Adjust for date serialization
+      expect(storage.getSnippets).toHaveBeenCalledWith(expect.objectContaining({
         isPublic: true,
       }));
-      expect(authMiddleware).not.toHaveBeenCalled(); // Ensure authMiddleware is not called for this public route
+      // authMiddleware is part of the actual routes, so it WILL be called by Express
+      // but it won't find a token and thus won't authenticate for public routes.
+      // The important thing is that the route handler itself doesn't *require* req.user.
     });
 
-    it('should pass query parameters (search, language, tag) to storage.getSnippets', async () => {
+    it('should pass query parameters (search, language, tag) to storage.getSnippets for public snippets', async () => {
       (storage.getSnippets as jest.Mock).mockResolvedValue([]);
-      const query = { search: 'test', language: 'javascript', tag: 'example' };
+      const query = { search: 'public test', language: 'javascript', tag: 'public_example' };
 
       await request(app).get('/api/public/snippets').query(query);
 
-      expect(storage.getSnippets).toHaveBeenCalledWith({
+      expect(storage.getSnippets).toHaveBeenCalledWith(expect.objectContaining({ // objectContaining for flexibility
         isPublic: true,
         search: query.search,
         language: query.language,
         tag: query.tag,
-      });
+      }));
     });
 
-    it('should handle errors from storage.getSnippets gracefully', async () => {
+    it('should handle errors from storage.getSnippets gracefully for public snippets', async () => {
         (storage.getSnippets as jest.Mock).mockRejectedValue(new Error('Storage failure'));
 
         const response = await request(app).get('/api/public/snippets');
@@ -103,22 +234,18 @@ describe('Public API Snippet Routes', () => {
 
   describe('GET /api/public/snippets/:id', () => {
     it('should return a single public snippet if it exists and isPublic is true', async () => {
-      const mockSnippet: Partial<Snippet> = { id: 1, title: 'Test Snippet', isPublic: true, code: 'code', language: 'js' };
-      (storage.getSnippet as jest.Mock).mockResolvedValue(mockSnippet);
+      const mockSnippetActual: Snippet = { id: 1, title: 'Test Snippet', isPublic: true, code: 'code', language: 'js', userId: user_A_id, createdAt: new Date(), updatedAt: new Date(), viewCount:0, isFavorite: false, shareId: null, tags: [] };
+      (storage.getSnippet as jest.Mock).mockResolvedValue(mockSnippetActual);
 
       const response = await request(app).get('/api/public/snippets/1');
 
       expect(response.status).toBe(200);
-      expect(response.body).toEqual(mockSnippet);
-      expect(storage.getSnippet).toHaveBeenCalledWith(1); // ID is number
-      // The route for /api/public/snippets/:id in the provided routes.ts does NOT call incrementSnippetViewCount.
-      // The /api/snippets/:id (non-public) and /api/shared/:shareId do.
-      // So, we should expect it NOT to be called here.
+      expect(response.body).toEqual({...mockSnippetActual, createdAt: mockSnippetActual.createdAt.toISOString(), updatedAt: mockSnippetActual.updatedAt.toISOString() });
+      expect(storage.getSnippet).toHaveBeenCalledWith(1);
       expect(storage.incrementSnippetViewCount).not.toHaveBeenCalled();
-      expect(authMiddleware).not.toHaveBeenCalled();
     });
 
-    it('should return 404 if snippet is not found by storage.getSnippet', async () => {
+    it('should return 404 if public snippet is not found by storage.getSnippet', async () => {
       (storage.getSnippet as jest.Mock).mockResolvedValue(null); // Snippet not found
 
       const response = await request(app).get('/api/public/snippets/999');
@@ -129,17 +256,16 @@ describe('Public API Snippet Routes', () => {
     });
 
     it('should return 404 if snippet is found but isPublic is false', async () => {
-      const mockPrivateSnippet: Partial<Snippet> = { id: 2, title: 'Private Snippet', isPublic: false, code: 'private', language: 'js' };
-      (storage.getSnippet as jest.Mock).mockResolvedValue(mockPrivateSnippet);
+      const mockPrivateSnippetActual: Snippet = { id: 2, title: 'Private Snippet', isPublic: false, code: 'private', language: 'js', userId: user_A_id, createdAt: new Date(), updatedAt: new Date(), viewCount:0, isFavorite: false, shareId: null, tags: [] };
+      (storage.getSnippet as jest.Mock).mockResolvedValue(mockPrivateSnippetActual);
 
       const response = await request(app).get('/api/public/snippets/2');
 
       expect(response.status).toBe(404);
-      // Check against the actual error message in your route handler
       expect(response.body).toEqual({ message: "Snippet not found or not public" });
     });
 
-    it('should handle errors from storage.getSnippet gracefully', async () => {
+    it('should handle errors from storage.getSnippet gracefully for public snippets', async () => {
         (storage.getSnippet as jest.Mock).mockRejectedValue(new Error('Storage failure for single snippet'));
 
         const response = await request(app).get('/api/public/snippets/1');

server/routes.ts

@@ -132,18 +132,20 @@ export async function registerRoutes(app: Express): Promise<Server> {
   // ─── 3.2) Snippets endpoints ─────────────────────────────────────
   // ────────────────────────────────────────────────────────────────
 
-  // GET all snippets (public access)
-  app.get("/api/snippets", async (req, res) => {
+  // GET all snippets (requires authentication)
+  app.get("/api/snippets", authMiddleware, async (req, res) => {
     try {
       console.log("[GET_ALL] Get all snippets request received");
+      const userId = (req as any).user.id;
 
-      const filters: any = {};
+      const filters: any = { userId }; // Add userId to filters
       if (req.query.search) filters.search = String(req.query.search);
       if (req.query.language) filters.language = req.query.language;
       if (req.query.tag) filters.tag = req.query.tag;
       if (req.query.favorites === "true") filters.favorites = true;
 
       try {
+        // Assuming simpleStorage.getSnippets is updated or we prioritize storage
         const list = await simpleStorage.getSnippets(filters);
         console.log(`[GET_ALL] Found ${list.length} snippets using simpleStorage`);
         return res.json(list);
@@ -163,9 +165,9 @@ export async function registerRoutes(app: Express): Promise<Server> {
               SELECT id, title, code, language, description, tags, userid, createdat, updatedat, 
                     isfavorite, ispublic, shareid, viewcount 
               FROM snippets 
-              WHERE 1=1
+              WHERE userid = $1
             `;
-            const params: any[] = [];
+            const params: any[] = [userId];
 
             if (filters.search) {
               query += ` AND (title ILIKE $${params.length + 1} OR description ILIKE $${params.length + 1})`;

server/simple-storage.ts

@@ -5,9 +5,29 @@ class SimpleStorage {
     try {
       console.log('SimpleStorage: Getting snippets with filters:', filters);
 
-      // Simple implementation without filters for debugging
-      const result = await query('SELECT * FROM snippets ORDER BY id DESC', []);
-      console.log(`SimpleStorage: Found ${result.rows.length} snippets`);
+      let sqlQuery = 'SELECT * FROM snippets';
+      const params: any[] = [];
+      let whereClauseAdded = false;
+
+      if (filters?.userId) {
+        sqlQuery += ' WHERE userid = $1';
+        params.push(filters.userId);
+        whereClauseAdded = true;
+      }
+
+      // Add other potential filters here if simpleStorage is ever expanded
+      // For example:
+      // if (filters?.language) {
+      //   sqlQuery += whereClauseAdded ? ' AND' : ' WHERE';
+      //   sqlQuery += ` language = $${params.length + 1}`;
+      //   params.push(filters.language);
+      //   whereClauseAdded = true;
+      // }
+
+      sqlQuery += ' ORDER BY updatedat DESC'; // Changed to updatedat
+      
+      const result = await query(sqlQuery, params);
+      console.log(`SimpleStorage: Found ${result.rows.length} snippets with query: ${sqlQuery}`);
 
       return result.rows;
     } catch (error) {