Task 12: Create MaterialsToDeleteDAO with SQL injection protection
Implement MaterialsToDeleteDAO class with secure parameterized queries following the pattern established in Task 11 (commit 5aafb23). Features: - getMaterialsToDeleteByManagers(): Query materials_to_delete table with optional filtering by manager_names using parameterized queries - getAllMaterialsToDelete(): Wrapper to retrieve all materials without filtering Security: - Uses parameterized queries (@param0, @param1, etc.) to prevent SQL injection - Parameters passed separately from query string via executeSQLServerQuery() - Input validation for empty arrays - Comprehensive test coverage including SQL injection attempt scenarios Testing: - 11 comprehensive unit tests covering all methods and edge cases - Tests verify parameterized query pattern prevents SQL injection - All tests passing (12/12 including existing tests) Files: - src/main/dao/materials-to-delete.dao.ts: DAO implementation - tests/unit/dao/materials-to-delete.dao.test.ts: Unit tests Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
61
src/main/dao/materials-to-delete.dao.ts
Normal file
61
src/main/dao/materials-to-delete.dao.ts
Normal file
@@ -0,0 +1,61 @@
|
||||
import { DatabaseService } from '../services/database.service';
|
||||
import { LoggerService } from '../services/logger.service';
|
||||
|
||||
export class MaterialsToDeleteDAO {
|
||||
constructor(private dbService: DatabaseService) {}
|
||||
|
||||
/**
|
||||
* Query materials_to_delete table and return distinct material codes
|
||||
* @param managerNames - Optional array of manager names to filter by. Pass null to get all materials.
|
||||
* @returns Promise<string[]> - Array of distinct material codes
|
||||
*/
|
||||
async getMaterialsToDeleteByManagers(
|
||||
managerNames: string[] | null
|
||||
): Promise<string[]> {
|
||||
try {
|
||||
// Input validation
|
||||
if (managerNames && managerNames.length === 0) {
|
||||
LoggerService.warn('Empty manager names array provided, returning empty result');
|
||||
return [];
|
||||
}
|
||||
|
||||
LoggerService.info(
|
||||
`Querying materials_to_delete${managerNames ? ` for ${managerNames.length} managers` : ' (all managers)'}`
|
||||
);
|
||||
|
||||
// Build base query
|
||||
let query = `
|
||||
SELECT DISTINCT material_code
|
||||
FROM materials_to_delete
|
||||
`;
|
||||
|
||||
let params: string[] | undefined;
|
||||
|
||||
// Add WHERE clause with parameterized query if manager names provided
|
||||
// SECURITY: Use parameterized queries to prevent SQL injection
|
||||
if (managerNames && managerNames.length > 0) {
|
||||
const placeholders = managerNames.map((_, i) => `@param${i}`).join(',');
|
||||
query += ` WHERE manager_name IN (${placeholders})`;
|
||||
params = managerNames;
|
||||
}
|
||||
|
||||
// Execute query with parameters (if any)
|
||||
const result = await this.dbService.executeSQLServerQuery(query, params);
|
||||
const materials = result.rows.map((row: any) => row.material_code);
|
||||
|
||||
LoggerService.info(`Found ${materials.length} materials to delete`);
|
||||
return materials;
|
||||
} catch (error) {
|
||||
LoggerService.error('Failed to query materials_to_delete table', error);
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Get all materials from materials_to_delete table without filtering
|
||||
* @returns Promise<string[]> - Array of all distinct material codes
|
||||
*/
|
||||
async getAllMaterialsToDelete(): Promise<string[]> {
|
||||
return this.getMaterialsToDeleteByManagers(null);
|
||||
}
|
||||
}
|
||||
232
tests/unit/dao/materials-to-delete.dao.test.ts
Normal file
232
tests/unit/dao/materials-to-delete.dao.test.ts
Normal file
@@ -0,0 +1,232 @@
|
||||
import { MaterialsToDeleteDAO } from '../../../src/main/dao/materials-to-delete.dao';
|
||||
import { DatabaseService } from '../../../src/main/services/database.service';
|
||||
|
||||
// Mock LoggerService to avoid Electron app dependency
|
||||
jest.mock('../../../src/main/services/logger.service', () => ({
|
||||
LoggerService: {
|
||||
info: jest.fn(),
|
||||
warn: jest.fn(),
|
||||
error: jest.fn(),
|
||||
debug: jest.fn(),
|
||||
},
|
||||
}));
|
||||
|
||||
describe('MaterialsToDeleteDAO', () => {
|
||||
let dao: MaterialsToDeleteDAO;
|
||||
let mockDbService: jest.Mocked<DatabaseService>;
|
||||
|
||||
beforeEach(() => {
|
||||
// Clear all mocks before each test
|
||||
jest.clearAllMocks();
|
||||
|
||||
// Create a mock DatabaseService
|
||||
mockDbService = {
|
||||
executeSQLServerQuery: jest.fn(),
|
||||
} as any;
|
||||
|
||||
dao = new MaterialsToDeleteDAO(mockDbService);
|
||||
});
|
||||
|
||||
describe('getMaterialsToDeleteByManagers', () => {
|
||||
it('should return all materials when managerNames is null', async () => {
|
||||
const mockResult = {
|
||||
rows: [
|
||||
{ material_code: 'MAT001' },
|
||||
{ material_code: 'MAT002' },
|
||||
{ material_code: 'MAT003' },
|
||||
],
|
||||
rowCount: 3,
|
||||
};
|
||||
|
||||
mockDbService.executeSQLServerQuery.mockResolvedValue(mockResult);
|
||||
|
||||
const result = await dao.getMaterialsToDeleteByManagers(null);
|
||||
|
||||
expect(result).toEqual(['MAT001', 'MAT002', 'MAT003']);
|
||||
expect(mockDbService.executeSQLServerQuery).toHaveBeenCalledTimes(1);
|
||||
|
||||
// Verify the query does not contain WHERE clause
|
||||
const queryCall = mockDbService.executeSQLServerQuery.mock.calls[0];
|
||||
const query = queryCall[0];
|
||||
expect(query).not.toContain('WHERE');
|
||||
expect(query).toContain('SELECT DISTINCT material_code');
|
||||
expect(query).toContain('FROM materials_to_delete');
|
||||
});
|
||||
|
||||
it('should return materials filtered by manager names using parameterized query', async () => {
|
||||
const managerNames = ['Manager1', 'Manager2'];
|
||||
const mockResult = {
|
||||
rows: [
|
||||
{ material_code: 'MAT001' },
|
||||
{ material_code: 'MAT002' },
|
||||
],
|
||||
rowCount: 2,
|
||||
};
|
||||
|
||||
mockDbService.executeSQLServerQuery.mockResolvedValue(mockResult);
|
||||
|
||||
const result = await dao.getMaterialsToDeleteByManagers(managerNames);
|
||||
|
||||
expect(result).toEqual(['MAT001', 'MAT002']);
|
||||
expect(mockDbService.executeSQLServerQuery).toHaveBeenCalledTimes(1);
|
||||
|
||||
// Verify parameterized query is used (SQL injection protection)
|
||||
const queryCall = mockDbService.executeSQLServerQuery.mock.calls[0];
|
||||
const query = queryCall[0];
|
||||
const params = queryCall[1];
|
||||
|
||||
expect(query).toContain('WHERE manager_name IN');
|
||||
expect(query).toContain('@param0');
|
||||
expect(query).toContain('@param1');
|
||||
|
||||
// Verify parameters are passed separately (not concatenated in query)
|
||||
expect(params).toEqual(managerNames);
|
||||
|
||||
// Ensure no string concatenation of values in query
|
||||
expect(query).not.toContain("'Manager1'");
|
||||
expect(query).not.toContain("'Manager2'");
|
||||
});
|
||||
|
||||
it('should return empty array when managerNames is empty', async () => {
|
||||
const result = await dao.getMaterialsToDeleteByManagers([]);
|
||||
|
||||
expect(result).toEqual([]);
|
||||
expect(mockDbService.executeSQLServerQuery).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should handle single manager name', async () => {
|
||||
const managerNames = ['Manager1'];
|
||||
const mockResult = {
|
||||
rows: [{ material_code: 'MAT001' }],
|
||||
rowCount: 1,
|
||||
};
|
||||
|
||||
mockDbService.executeSQLServerQuery.mockResolvedValue(mockResult);
|
||||
|
||||
const result = await dao.getMaterialsToDeleteByManagers(managerNames);
|
||||
|
||||
expect(result).toEqual(['MAT001']);
|
||||
expect(mockDbService.executeSQLServerQuery).toHaveBeenCalledTimes(1);
|
||||
|
||||
const queryCall = mockDbService.executeSQLServerQuery.mock.calls[0];
|
||||
expect(queryCall[0]).toContain('@param0');
|
||||
expect(queryCall[1]).toEqual(['Manager1']);
|
||||
});
|
||||
|
||||
it('should return empty array when no materials found', async () => {
|
||||
const mockResult = {
|
||||
rows: [],
|
||||
rowCount: 0,
|
||||
};
|
||||
|
||||
mockDbService.executeSQLServerQuery.mockResolvedValue(mockResult);
|
||||
|
||||
const result = await dao.getMaterialsToDeleteByManagers(['Manager1']);
|
||||
|
||||
expect(result).toEqual([]);
|
||||
});
|
||||
|
||||
it('should handle SQL injection attempts via parameterized query', async () => {
|
||||
const maliciousInput = [
|
||||
"Manager1'; DROP TABLE materials_to_delete; --",
|
||||
"Manager2' OR '1'='1",
|
||||
];
|
||||
|
||||
const mockResult = {
|
||||
rows: [],
|
||||
rowCount: 0,
|
||||
};
|
||||
|
||||
mockDbService.executeSQLServerQuery.mockResolvedValue(mockResult);
|
||||
|
||||
await dao.getMaterialsToDeleteByManagers(maliciousInput);
|
||||
|
||||
// Verify parameters are passed as values, not concatenated
|
||||
const queryCall = mockDbService.executeSQLServerQuery.mock.calls[0];
|
||||
const query = queryCall[0];
|
||||
const params = queryCall[1];
|
||||
|
||||
// The malicious strings should be in params, not in query
|
||||
expect(params).toEqual(maliciousInput);
|
||||
|
||||
// Query should only contain placeholders, not actual values
|
||||
expect(query).not.toContain('DROP TABLE');
|
||||
expect(query).not.toContain('OR 1=1');
|
||||
expect(query).toMatch(/@param\d+/);
|
||||
});
|
||||
|
||||
it('should throw error when database query fails', async () => {
|
||||
const dbError = new Error('Database connection failed');
|
||||
mockDbService.executeSQLServerQuery.mockRejectedValue(dbError);
|
||||
|
||||
await expect(
|
||||
dao.getMaterialsToDeleteByManagers(['Manager1'])
|
||||
).rejects.toThrow('Database connection failed');
|
||||
});
|
||||
|
||||
it('should handle large number of manager names', async () => {
|
||||
const managerNames = Array.from({ length: 100 }, (_, i) => `Manager${i}`);
|
||||
const mockResult = {
|
||||
rows: [{ material_code: 'MAT001' }],
|
||||
rowCount: 1,
|
||||
};
|
||||
|
||||
mockDbService.executeSQLServerQuery.mockResolvedValue(mockResult);
|
||||
|
||||
const result = await dao.getMaterialsToDeleteByManagers(managerNames);
|
||||
|
||||
expect(result).toEqual(['MAT001']);
|
||||
|
||||
// Verify all parameters are passed
|
||||
const queryCall = mockDbService.executeSQLServerQuery.mock.calls[0];
|
||||
expect(queryCall[1]).toEqual(managerNames);
|
||||
expect(queryCall[1]?.length).toBe(100);
|
||||
});
|
||||
});
|
||||
|
||||
describe('getAllMaterialsToDelete', () => {
|
||||
it('should return all materials by calling getMaterialsToDeleteByManagers with null', async () => {
|
||||
const mockResult = {
|
||||
rows: [
|
||||
{ material_code: 'MAT001' },
|
||||
{ material_code: 'MAT002' },
|
||||
{ material_code: 'MAT003' },
|
||||
],
|
||||
rowCount: 3,
|
||||
};
|
||||
|
||||
mockDbService.executeSQLServerQuery.mockResolvedValue(mockResult);
|
||||
|
||||
const result = await dao.getAllMaterialsToDelete();
|
||||
|
||||
expect(result).toEqual(['MAT001', 'MAT002', 'MAT003']);
|
||||
expect(mockDbService.executeSQLServerQuery).toHaveBeenCalledTimes(1);
|
||||
|
||||
// Verify query without filter
|
||||
const queryCall = mockDbService.executeSQLServerQuery.mock.calls[0];
|
||||
expect(queryCall[0]).not.toContain('WHERE');
|
||||
});
|
||||
|
||||
it('should propagate errors from getMaterialsToDeleteByManagers', async () => {
|
||||
const dbError = new Error('Database error');
|
||||
mockDbService.executeSQLServerQuery.mockRejectedValue(dbError);
|
||||
|
||||
await expect(dao.getAllMaterialsToDelete()).rejects.toThrow(
|
||||
'Database error'
|
||||
);
|
||||
});
|
||||
|
||||
it('should return empty array when no materials exist', async () => {
|
||||
const mockResult = {
|
||||
rows: [],
|
||||
rowCount: 0,
|
||||
};
|
||||
|
||||
mockDbService.executeSQLServerQuery.mockResolvedValue(mockResult);
|
||||
|
||||
const result = await dao.getAllMaterialsToDelete();
|
||||
|
||||
expect(result).toEqual([]);
|
||||
});
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user