more fixin tests
Some checks failed
Deploy to Test Environment / deploy-to-test (push) Has been cancelled

This commit is contained in:
2025-12-22 08:47:18 -08:00
parent fee55b0afd
commit ed857f588a
5 changed files with 1581 additions and 900 deletions

View File

@@ -4,7 +4,10 @@ import * as bcrypt from 'bcrypt';
import { Request, Response, NextFunction } from 'express';
// Define a type for the JWT verify callback function for type safety.
type VerifyCallback = (payload: { user_id: string }, done: (error: Error | null, user?: object | false) => void) => Promise<void>;
type VerifyCallback = (
payload: { user_id: string },
done: (error: Error | null, user?: object | false) => void,
) => Promise<void>;
// FIX: Use vi.hoisted to declare variables that need to be accessed inside vi.mock
const { verifyCallbackWrapper } = vi.hoisted(() => {
@@ -12,15 +15,15 @@ const { verifyCallbackWrapper } = vi.hoisted(() => {
// We use a wrapper object to hold the callback reference
// Initialize with a more specific type instead of `any`.
verifyCallbackWrapper: {
callback: null as VerifyCallback | null
}
callback: null as VerifyCallback | null,
},
};
});
// Mock the 'passport-jwt' module to capture the verify callback.
vi.mock('passport-jwt', () => ({
// The Strategy constructor is mocked to capture its second argument
Strategy: vi.fn(function(options, verify) {
Strategy: vi.fn(function (options, verify) {
// FIX: Assign to the hoisted wrapper object
verifyCallbackWrapper.callback = verify;
return { name: 'jwt', authenticate: vi.fn() };
@@ -30,21 +33,29 @@ vi.mock('passport-jwt', () => ({
// FIX: Add a similar mock for 'passport-local' to capture its verify callback.
const { localStrategyCallbackWrapper } = vi.hoisted(() => {
type LocalVerifyCallback = (req: Request, email: string, pass: string, done: (error: Error | null, user?: object | false, options?: { message: string }) => void) => Promise<void>;
type LocalVerifyCallback = (
req: Request,
email: string,
pass: string,
done: (error: Error | null, user?: object | false, options?: { message: string }) => void,
) => Promise<void>;
return {
localStrategyCallbackWrapper: { callback: null as LocalVerifyCallback | null }
localStrategyCallbackWrapper: { callback: null as LocalVerifyCallback | null },
};
});
vi.mock('passport-local', () => ({
Strategy: vi.fn(function(options, verify) {
Strategy: vi.fn(function (options, verify) {
localStrategyCallbackWrapper.callback = verify;
}),
}));
import * as db from '../services/db/index.db';
import { UserProfile } from '../types';
import { createMockUserProfile, createMockUserWithPasswordHash } from '../tests/utils/mockFactories';
import {
createMockUserProfile,
createMockUserWithPasswordHash,
} from '../tests/utils/mockFactories';
import { mockLogger } from '../tests/utils/mockLogger';
// Mock dependencies before importing the passport configuration
@@ -118,7 +129,9 @@ describe('Passport Configuration', () => {
}),
refresh_token: 'mock-refresh-token',
};
vi.mocked(mockedDb.userRepo.findUserWithProfileByEmail).mockResolvedValue(mockAuthableProfile);
vi.mocked(mockedDb.userRepo.findUserWithProfileByEmail).mockResolvedValue(
mockAuthableProfile,
);
vi.mocked(bcrypt.compare).mockResolvedValue(true as never);
// Act
@@ -127,11 +140,24 @@ describe('Passport Configuration', () => {
}
// Assert
expect(mockedDb.userRepo.findUserWithProfileByEmail).toHaveBeenCalledWith('test@test.com', logger);
expect(mockedDb.userRepo.findUserWithProfileByEmail).toHaveBeenCalledWith(
'test@test.com',
logger,
);
expect(bcrypt.compare).toHaveBeenCalledWith('password', 'hashed_password');
expect(mockedDb.adminRepo.resetFailedLoginAttempts).toHaveBeenCalledWith(mockAuthableProfile.user.user_id, '127.0.0.1', logger);
expect(mockedDb.adminRepo.resetFailedLoginAttempts).toHaveBeenCalledWith(
mockAuthableProfile.user.user_id,
'127.0.0.1',
logger,
);
// The strategy now just strips auth fields.
const { password_hash, failed_login_attempts, last_failed_login, last_login_ip, refresh_token, ...expectedUserProfile } = mockAuthableProfile;
const {
password_hash,
failed_login_attempts,
last_failed_login,
refresh_token,
...expectedUserProfile
} = mockAuthableProfile;
expect(done).toHaveBeenCalledWith(null, expectedUserProfile);
});
@@ -165,14 +191,25 @@ describe('Passport Configuration', () => {
vi.mocked(mockedDb.adminRepo.incrementFailedLoginAttempts).mockResolvedValue(2);
if (localStrategyCallbackWrapper.callback) {
await localStrategyCallbackWrapper.callback(mockReq, 'test@test.com', 'wrong_password', done);
await localStrategyCallbackWrapper.callback(
mockReq,
'test@test.com',
'wrong_password',
done,
);
}
expect(mockedDb.adminRepo.incrementFailedLoginAttempts).toHaveBeenCalledWith(mockUser.user.user_id, logger);
expect(mockedDb.adminRepo.logActivity).toHaveBeenCalledWith(expect.objectContaining({
action: 'login_failed_password',
details: { source_ip: '127.0.0.1', new_attempt_count: 2 },
}), logger);
expect(mockedDb.adminRepo.incrementFailedLoginAttempts).toHaveBeenCalledWith(
mockUser.user.user_id,
logger,
);
expect(mockedDb.adminRepo.logActivity).toHaveBeenCalledWith(
expect.objectContaining({
action: 'login_failed_password',
details: { source_ip: '127.0.0.1', new_attempt_count: 2 },
}),
logger,
);
expect(done).toHaveBeenCalledWith(null, false, { message: 'Incorrect email or password.' });
});
@@ -196,12 +233,22 @@ describe('Passport Configuration', () => {
vi.mocked(mockedDb.adminRepo.incrementFailedLoginAttempts).mockResolvedValue(5);
if (localStrategyCallbackWrapper.callback) {
await localStrategyCallbackWrapper.callback(mockReq, 'test@test.com', 'wrong_password', done);
await localStrategyCallbackWrapper.callback(
mockReq,
'test@test.com',
'wrong_password',
done,
);
}
expect(mockedDb.adminRepo.incrementFailedLoginAttempts).toHaveBeenCalledWith(mockUser.user.user_id, logger);
expect(mockedDb.adminRepo.incrementFailedLoginAttempts).toHaveBeenCalledWith(
mockUser.user.user_id,
logger,
);
// It should now return the lockout message, not the generic "incorrect password"
expect(done).toHaveBeenCalledWith(null, false, { message: expect.stringContaining('Account is temporarily locked') });
expect(done).toHaveBeenCalledWith(null, false, {
message: expect.stringContaining('Account is temporarily locked'),
});
});
it('should call done(null, false) for an OAuth user (no password hash)', async () => {
@@ -221,10 +268,18 @@ describe('Passport Configuration', () => {
vi.mocked(mockedDb.userRepo.findUserWithProfileByEmail).mockResolvedValue(mockUser);
if (localStrategyCallbackWrapper.callback) {
await localStrategyCallbackWrapper.callback(mockReq, 'oauth@test.com', 'any_password', done);
await localStrategyCallbackWrapper.callback(
mockReq,
'oauth@test.com',
'any_password',
done,
);
}
expect(done).toHaveBeenCalledWith(null, false, { message: 'This account was created using a social login. Please use Google or GitHub to sign in.' });
expect(done).toHaveBeenCalledWith(null, false, {
message:
'This account was created using a social login. Please use Google or GitHub to sign in.',
});
});
it('should call done(null, false) if account is locked', async () => {
@@ -245,10 +300,17 @@ describe('Passport Configuration', () => {
vi.mocked(mockedDb.userRepo.findUserWithProfileByEmail).mockResolvedValue(mockUser);
if (localStrategyCallbackWrapper.callback) {
await localStrategyCallbackWrapper.callback(mockReq, 'locked@test.com', 'any_password', done);
await localStrategyCallbackWrapper.callback(
mockReq,
'locked@test.com',
'any_password',
done,
);
}
expect(done).toHaveBeenCalledWith(null, false, { message: 'Account is temporarily locked. Please try again in 15 minutes.' });
expect(done).toHaveBeenCalledWith(null, false, {
message: 'Account is temporarily locked. Please try again in 15 minutes.',
});
});
it('should allow login if lockout period has expired', async () => {
@@ -270,7 +332,12 @@ describe('Passport Configuration', () => {
vi.mocked(bcrypt.compare).mockResolvedValue(true as never); // Correct password
if (localStrategyCallbackWrapper.callback) {
await localStrategyCallbackWrapper.callback(mockReq, 'expired@test.com', 'correct_password', done);
await localStrategyCallbackWrapper.callback(
mockReq,
'expired@test.com',
'correct_password',
done,
);
}
// Should proceed to successful login
@@ -293,8 +360,12 @@ describe('Passport Configuration', () => {
describe('JwtStrategy (Isolated Callback Logic)', () => {
it('should call done(null, userProfile) on successful authentication', async () => {
// Arrange
const jwtPayload = { user_id: 'user-123' };
const mockProfile = { role: 'user', points: 100, user: { user_id: 'user-123', email: 'test@test.com' } } as UserProfile;
const jwtPayload = { user_id: 'user-123' };
const mockProfile = {
role: 'user',
points: 100,
user: { user_id: 'user-123', email: 'test@test.com' },
} as UserProfile;
vi.mocked(mockedDb.userRepo.findUserProfileById).mockResolvedValue(mockProfile);
const done = vi.fn();
@@ -360,7 +431,10 @@ describe('Passport Configuration', () => {
it('should call next() if user has "admin" role', () => {
// Arrange
const mockReq: Partial<Request> = {
user: createMockUserProfile({ role: 'admin', user: { user_id: 'admin-id', email: 'admin@test.com' } }),
user: createMockUserProfile({
role: 'admin',
user: { user_id: 'admin-id', email: 'admin@test.com' },
}),
};
// Act
@@ -374,7 +448,10 @@ describe('Passport Configuration', () => {
it('should return 403 Forbidden if user does not have "admin" role', () => {
// Arrange
const mockReq: Partial<Request> = {
user: createMockUserProfile({ role: 'user', user: { user_id: 'user-id', email: 'user@test.com' } }),
user: createMockUserProfile({
role: 'user',
user: { user_id: 'user-id', email: 'user@test.com' },
}),
};
// Act
@@ -383,7 +460,9 @@ describe('Passport Configuration', () => {
// Assert
expect(mockNext).not.toHaveBeenCalled(); // This was a duplicate, fixed.
expect(mockRes.status).toHaveBeenCalledWith(403);
expect(mockRes.json).toHaveBeenCalledWith({ message: 'Forbidden: Administrator access required.' });
expect(mockRes.json).toHaveBeenCalledWith({
message: 'Forbidden: Administrator access required.',
});
});
it('should return 403 Forbidden if req.user is missing', () => {
@@ -413,7 +492,9 @@ describe('Passport Configuration', () => {
// Assert
expect(mockNext).not.toHaveBeenCalled(); // This was a duplicate, fixed.
expect(mockRes.status).toHaveBeenCalledWith(403);
expect(mockRes.json).toHaveBeenCalledWith({ message: 'Forbidden: Administrator access required.' });
expect(mockRes.json).toHaveBeenCalledWith({
message: 'Forbidden: Administrator access required.',
});
});
});
@@ -428,10 +509,13 @@ describe('Passport Configuration', () => {
it('should populate req.user and call next() if authentication succeeds', () => {
// Arrange
const mockReq = {} as Request;
const mockUser = createMockUserProfile({ role: 'admin', user: { user_id: 'admin-id', email: 'admin@test.com' } });
const mockUser = createMockUserProfile({
role: 'admin',
user: { user_id: 'admin-id', email: 'admin@test.com' },
});
// Mock passport.authenticate to call its callback with a user
vi.mocked(passport.authenticate).mockImplementation(
(_strategy, _options, callback) => () => callback?.(null, mockUser, undefined)
(_strategy, _options, callback) => () => callback?.(null, mockUser, undefined),
);
// Act
@@ -446,7 +530,7 @@ describe('Passport Configuration', () => {
// Arrange
const mockReq = {} as Request;
vi.mocked(passport.authenticate).mockImplementation(
(_strategy, _options, callback) => () => callback?.(null, false, undefined)
(_strategy, _options, callback) => () => callback?.(null, false, undefined),
);
optionalAuth(mockReq, mockRes as Response, mockNext);
@@ -461,7 +545,7 @@ describe('Passport Configuration', () => {
const mockInfo = { message: 'Token expired' };
// Mock passport.authenticate to call its callback with an info object
vi.mocked(passport.authenticate).mockImplementation(
(_strategy, _options, callback) => () => callback?.(null, false, mockInfo)
(_strategy, _options, callback) => () => callback?.(null, false, mockInfo),
);
// Act
@@ -479,7 +563,7 @@ describe('Passport Configuration', () => {
const mockInfoError = new Error('Token is malformed');
// Mock passport.authenticate to call its callback with an info object
vi.mocked(passport.authenticate).mockImplementation(
(_strategy, _options, callback) => () => callback?.(null, false, mockInfoError)
(_strategy, _options, callback) => () => callback?.(null, false, mockInfoError),
);
// Act
@@ -487,7 +571,10 @@ describe('Passport Configuration', () => {
// Assert
// info.message is 'Token is malformed'
expect(logger.info).toHaveBeenCalledWith({ info: 'Token is malformed' }, 'Optional auth info:');
expect(logger.info).toHaveBeenCalledWith(
{ info: 'Token is malformed' },
'Optional auth info:',
);
expect(mockNext).toHaveBeenCalledTimes(1);
});
@@ -497,14 +584,17 @@ describe('Passport Configuration', () => {
const mockInfo = { custom: 'some info' };
// Mock passport.authenticate to call its callback with a custom info object
vi.mocked(passport.authenticate).mockImplementation(
(_strategy, _options, callback) => () => callback?.(null, false, mockInfo as any)
(_strategy, _options, callback) => () => callback?.(null, false, mockInfo as any),
);
// Act
optionalAuth(mockReq, mockRes as Response, mockNext);
// Assert
expect(logger.info).toHaveBeenCalledWith({ info: mockInfo.toString() }, 'Optional auth info:');
expect(logger.info).toHaveBeenCalledWith(
{ info: mockInfo.toString() },
'Optional auth info:',
);
expect(mockNext).toHaveBeenCalledTimes(1);
});
@@ -514,7 +604,7 @@ describe('Passport Configuration', () => {
const authError = new Error('Malformed token');
// Mock passport.authenticate to call its callback with an error
vi.mocked(passport.authenticate).mockImplementation(
(_strategy, _options, callback) => () => callback?.(authError, false, undefined)
(_strategy, _options, callback) => () => callback?.(authError, false, undefined),
);
// Act
@@ -567,4 +657,4 @@ describe('Passport Configuration', () => {
expect(mockNext).toHaveBeenCalledTimes(1);
});
});
});
});