test: cover the auth paths the design decisions depend on

src/auth/strategies/jwt.strategy.spec.ts

@@ -0,0 +1,89 @@
+import { Test, TestingModule } from '@nestjs/testing';
+import { ConfigService } from '@nestjs/config';
+import { getRepositoryToken } from '@nestjs/typeorm';
+import { UnauthorizedException } from '@nestjs/common';
+import { JwtStrategy } from './jwt.strategy';
+import { User, UserRole } from '../entities/user.entity';
+import { JwtPayload } from '../jwt-payload.interface';
+
+describe('JwtStrategy', () => {
+  let strategy: JwtStrategy;
+  let userRepo: jest.Mocked<any>;
+
+  const payload: JwtPayload = {
+    sub: 'user-1',
+    email: 'viewer@example.com',
+    role: UserRole.VIEWER,
+  };
+
+  beforeEach(async () => {
+    userRepo = { findOne: jest.fn() };
+
+    const module: TestingModule = await Test.createTestingModule({
+      providers: [
+        JwtStrategy,
+        {
+          provide: ConfigService,
+          useValue: { get: () => 'test-secret-at-least-16-characters' },
+        },
+        { provide: getRepositoryToken(User), useValue: userRepo },
+      ],
+    }).compile();
+
+    strategy = module.get(JwtStrategy);
+  });
+
+  it('looks the user up by the token subject', async () => {
+    userRepo.findOne.mockResolvedValue({
+      id: 'user-1',
+      email: 'viewer@example.com',
+      role: UserRole.VIEWER,
+    });
+
+    await strategy.validate(payload);
+
+    expect(userRepo.findOne).toHaveBeenCalledWith({
+      where: { id: payload.sub },
+    });
+  });
+
+  it('returns the shape that becomes request.user', async () => {
+    userRepo.findOne.mockResolvedValue({
+      id: 'user-1',
+      email: 'viewer@example.com',
+      role: UserRole.VIEWER,
+      passwordHash: 'should-not-be-returned',
+    });
+
+    const result = await strategy.validate(payload);
+
+    expect(result).toEqual({
+      id: 'user-1',
+      email: 'viewer@example.com',
+      role: UserRole.VIEWER,
+    });
+    expect(result).not.toHaveProperty('passwordHash');
+  });
+
+  it('trusts the database role over the token claim', async () => {
+    // The token was issued while this user was a viewer; they are now an admin.
+    userRepo.findOne.mockResolvedValue({
+      id: 'user-1',
+      email: 'viewer@example.com',
+      role: UserRole.ADMIN,
+    });
+
+    const result = await strategy.validate(payload);
+
+    expect(payload.role).toBe(UserRole.VIEWER);
+    expect(result.role).toBe(UserRole.ADMIN);
+  });
+
+  it('rejects a token whose user no longer exists', async () => {
+    userRepo.findOne.mockResolvedValue(null);
+
+    await expect(strategy.validate(payload)).rejects.toBeInstanceOf(
+      UnauthorizedException,
+    );
+  });
+});

src/services/services.service.spec.ts

@@ -243,17 +243,25 @@ describe('ServicesService', () => {
 
   describe('create', () => {
     it('translates a unique violation into 409 Conflict', async () => {
-      // Reachable only via duplicate version names in the payload: service
-      // names carry no unique constraint.
+      // The only unique constraint a create can trip is (service_id, name) on
+      // versions -- service names carry none -- so the payload that provokes
+      // it is one repeating a version name.
       const uniqueViolation = Object.assign(
         new QueryFailedError('INSERT', [], new Error('duplicate key')),
         { code: '23505' },
       );
       serviceRepo.save.mockRejectedValue(uniqueViolation);
 
-      await expect(
-        service.create({ name: 'Payment Gateway', description: 'x' }),
-      ).rejects.toBeInstanceOf(ConflictException);
+      const attempt = service.create({
+        name: 'Any Service',
+        description: 'x',
+        versions: [{ name: 'v1.0.0' }, { name: 'v1.0.0' }],
+      });
+
+      await expect(attempt).rejects.toBeInstanceOf(ConflictException);
+      await expect(attempt).rejects.toThrow(
+        'The submitted versions contain duplicate version names',
+      );
     });
 
     it('rethrows unrelated database errors untouched', async () => {
@@ -264,7 +272,11 @@ describe('ServicesService', () => {
       serviceRepo.save.mockRejectedValue(other);
 
       await expect(
-        service.create({ name: 'Payment Gateway', description: 'x' }),
+        service.create({
+          name: 'Any Service',
+          description: 'x',
+          versions: [{ name: 'v1.0.0' }],
+        }),
       ).rejects.toBe(other);
     });
 

test/services.e2e-spec.ts

@@ -95,6 +95,22 @@ describe('Services API (e2e)', () => {
         .expect(400));
   });
 
+  describe('GET /api/auth/me', () => {
+    it('returns the caller identified by the token', async () => {
+      const res = await asViewer(api().get('/api/auth/me')).expect(200);
+
+      expect(res.body).toEqual({
+        id: expect.any(String),
+        email: TEST_VIEWER.email,
+        role: 'viewer',
+      });
+      expect(res.body).not.toHaveProperty('passwordHash');
+    });
+
+    it('rejects an unauthenticated caller', () =>
+      api().get('/api/auth/me').expect(401));
+  });
+
   describe('authorization', () => {
     it('rejects an unauthenticated read', () =>
       api().get('/api/services').expect(401));
@@ -117,6 +133,57 @@ describe('Services API (e2e)', () => {
       asAdmin(api().post('/api/services'))
         .send({ name: 'Allowed', description: 'created by admin' })
         .expect(201));
+
+    // The reason JwtStrategy.validate() re-reads the user rather than trusting
+    // the token claims: deleting a user must take effect on the next request,
+    // not whenever their token happens to expire.
+    it('rejects a still-unexpired token whose user has been deleted', async () => {
+      const token = await login(TEST_VIEWER);
+
+      await api()
+        .get('/api/services')
+        .set('Authorization', `Bearer ${token}`)
+        .expect(200);
+
+      await dataSource.query('DELETE FROM users WHERE email = $1', [
+        TEST_VIEWER.email,
+      ]);
+
+      // Same token, still cryptographically valid and well inside its expiry.
+      await api()
+        .get('/api/services')
+        .set('Authorization', `Bearer ${token}`)
+        .expect(401);
+    });
+
+    it('reflects a role change without re-issuing the token', async () => {
+      const token = await login(TEST_VIEWER);
+
+      await api()
+        .post('/api/services')
+        .set('Authorization', `Bearer ${token}`)
+        .send({ name: 'Before promotion', description: 'x' })
+        .expect(403);
+
+      await dataSource.query('UPDATE users SET role = $1 WHERE email = $2', [
+        'admin',
+        TEST_VIEWER.email,
+      ]);
+
+      // Prove the premise rather than asserting it in a comment: the token is
+      // unchanged and still claims "viewer", so a 201 can only mean the guard
+      // used the freshly-read database role.
+      const claims = JSON.parse(
+        Buffer.from(token.split('.')[1], 'base64url').toString(),
+      );
+      expect(claims.role).toBe('viewer');
+
+      await api()
+        .post('/api/services')
+        .set('Authorization', `Bearer ${token}`)
+        .send({ name: 'After promotion', description: 'x' })
+        .expect(201);
+    });
   });
 
   describe('GET /api/services', () => {