🧪 [testing improvement] Add unit tests for backend authentication middleware#4
Conversation
…dleware - Installed vitest in the backend - Added "test" script to backend/package.json - Created backend/src/middleware/__tests__/auth.test.ts - Implemented tests for authenticateToken and generateToken - Verified tests pass and catch regressions Co-authored-by: davidraehles <6085055+davidraehles@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive unit tests for the backend authentication middleware, addressing a critical testing gap in the security layer. The tests use Vitest as the testing framework and cover both the authenticateToken middleware and the generateToken utility function.
Changes:
- Added unit tests for JWT authentication middleware covering 5 key scenarios: missing header, missing token, invalid token, invalid payload, and valid token
- Added test for token generation functionality
- Integrated Vitest testing framework with version 4.0.18 and added test script to package.json
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| backend/src/middleware/tests/auth.test.ts | New test file with comprehensive coverage of authentication middleware including edge cases and success scenarios |
| backend/package.json | Added vitest dependency and test script for running unit tests |
| backend/pnpm-lock.yaml | Lockfile update reflecting new vitest dependency and its transitive dependencies |
Files not reviewed (1)
- backend/pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| describe('authenticateToken middleware', () => { | ||
| let req: Partial<AuthRequest>; | ||
| let res: Partial<Response>; | ||
| let next: NextFunction; | ||
|
|
||
| beforeEach(() => { | ||
| req = { | ||
| headers: {}, | ||
| }; | ||
| res = { | ||
| status: vi.fn().mockReturnThis(), | ||
| json: vi.fn().mockReturnThis(), | ||
| }; | ||
| next = vi.fn(); | ||
| vi.clearAllMocks(); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| vi.restoreAllMocks(); | ||
| }); | ||
|
|
||
| it('should return 401 if no authorization header is present', () => { | ||
| authenticateToken(req as AuthRequest, res as Response, next); | ||
| expect(res.status).toHaveBeenCalledWith(401); | ||
| expect(res.json).toHaveBeenCalledWith({ message: 'Authentication required' }); | ||
| expect(next).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('should return 401 if authorization header is present but no token', () => { | ||
| req.headers!['authorization'] = 'Bearer '; | ||
| authenticateToken(req as AuthRequest, res as Response, next); | ||
| expect(res.status).toHaveBeenCalledWith(401); | ||
| expect(res.json).toHaveBeenCalledWith({ message: 'Authentication required' }); | ||
| }); | ||
|
|
||
| it('should return 403 if token is invalid', () => { | ||
| req.headers!['authorization'] = 'Bearer invalid-token'; | ||
|
|
||
| vi.spyOn(jwt, 'verify').mockImplementation((token, secret, callback: any) => { | ||
| callback(new Error('Invalid token'), null); | ||
| }); | ||
|
|
||
| authenticateToken(req as AuthRequest, res as Response, next); | ||
|
|
||
| expect(res.status).toHaveBeenCalledWith(403); | ||
| expect(res.json).toHaveBeenCalledWith({ message: 'Invalid or expired token' }); | ||
| expect(next).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('should return 403 if token payload is invalid (missing userId)', () => { | ||
| req.headers!['authorization'] = 'Bearer valid-token'; | ||
|
|
||
| vi.spyOn(jwt, 'verify').mockImplementation((token, secret, callback: any) => { | ||
| callback(null, {}); // Decoded object missing userId | ||
| }); | ||
|
|
||
| authenticateToken(req as AuthRequest, res as Response, next); | ||
|
|
||
| expect(res.status).toHaveBeenCalledWith(403); | ||
| expect(res.json).toHaveBeenCalledWith({ message: 'Invalid token payload' }); | ||
| expect(next).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('should call next and set req.userId if token is valid', () => { | ||
| req.headers!['authorization'] = 'Bearer valid-token'; | ||
| const userId = 'user-123'; | ||
|
|
||
| vi.spyOn(jwt, 'verify').mockImplementation((token, secret, callback: any) => { | ||
| callback(null, { userId }); | ||
| }); | ||
|
|
||
| authenticateToken(req as AuthRequest, res as Response, next); | ||
|
|
||
| expect(req.userId).toBe(userId); | ||
| expect(next).toHaveBeenCalled(); | ||
| expect(res.status).not.toHaveBeenCalled(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
The test suite doesn't cover the edge case where the authorization header doesn't follow the "Bearer TOKEN" format. For example, if a request sends just a token without the "Bearer " prefix (e.g., authorization: "some-token"), the current implementation would try to extract the token from split(' ')[1], which would be undefined. Consider adding a test case for malformed authorization headers that don't contain a space.
There was a problem hiding this comment.
Added test case for malformed authorization header without Bearer prefix. Verifies that authHeader.split(' ')[1] returns undefined when there's no space, triggering the 401 response.
There was a problem hiding this comment.
Added the requested test case for malformed authorization headers without the 'Bearer ' prefix. It verifies that such headers trigger a 401 response.
| expect(typeof token).toBe('string'); | ||
|
|
||
| const decoded = jwt.decode(token) as any; | ||
| expect(decoded.userId).toBe(userId); |
There was a problem hiding this comment.
The test for generateToken doesn't verify that the token includes the expected expiration time. According to the implementation in auth.ts, tokens should have a 7-day expiration (expiresIn: '7d'). Consider adding an assertion to verify the decoded token contains the 'exp' field and that it represents approximately 7 days from now.
| expect(decoded.userId).toBe(userId); | |
| expect(decoded.userId).toBe(userId); | |
| // Verify token has an expiration approximately 7 days from now | |
| expect(decoded.exp).toBeDefined(); | |
| expect(typeof decoded.exp).toBe('number'); | |
| const nowInSeconds = Math.floor(Date.now() / 1000); | |
| const sevenDaysInSeconds = 7 * 24 * 60 * 60; | |
| const toleranceInSeconds = 10; | |
| const expectedMinExp = nowInSeconds + sevenDaysInSeconds - toleranceInSeconds; | |
| const expectedMaxExp = nowInSeconds + sevenDaysInSeconds + toleranceInSeconds; | |
| expect(decoded.exp).toBeGreaterThanOrEqual(expectedMinExp); | |
| expect(decoded.exp).toBeLessThanOrEqual(expectedMaxExp); |
There was a problem hiding this comment.
Added expiration assertions to the generateToken test. Now verifies that decoded.exp is defined and falls within a 10-second window of the expected 7-day expiry.
There was a problem hiding this comment.
Added expiration assertions to the generateToken test as requested. It now confirms the token has a defined 'exp' claim and that it falls within the expected 7-day window.
…re tests - Added test case for malformed authorization header (no Bearer prefix) - Added expiration assertions to generateToken test Co-authored-by: davidraehles <6085055+davidraehles@users.noreply.github.com>
🎯 What: The testing gap addressed was the lack of unit tests for the backend authentication middleware.
📊 Coverage: The new tests cover the following scenarios for the
authenticateTokenmiddleware:userId(403 response)req.userIdand callsnext())Additionally, tests for
generateTokenwere added to ensure correct token creation.✨ Result: Improved test coverage for a critical security component, ensuring that authentication logic remains robust and reliable.
PR created automatically by Jules for task 7652412391144490455 started by @davidraehles