diff --git a/package-lock.json b/package-lock.json index 35aae6a..97dd532 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13,6 +13,7 @@ "cors": "^2.8.5", "dotenv": "^17.0.1", "express": "^4.18.2", + "express-rate-limit": "^7.5.1", "node-cron": "^3.0.3", "sqlite3": "^5.1.6", "swagger-jsdoc": "^6.2.8", @@ -3674,6 +3675,21 @@ "url": "https://opencollective.com/express" } }, + "node_modules/express-rate-limit": { + "version": "7.5.1", + "resolved": "https://registry.npmjs.org/express-rate-limit/-/express-rate-limit-7.5.1.tgz", + "integrity": "sha512-7iN8iPMDzOMHPUYllBEsQdWVB6fPDMPqwjBaFrgr4Jgr/+okjvzAy+UHlYYL/Vs0OsOrMkwS6PJDkFlJwoxUnw==", + "license": "MIT", + "engines": { + "node": ">= 16" + }, + "funding": { + "url": "https://github.com/sponsors/express-rate-limit" + }, + "peerDependencies": { + "express": ">= 4.11" + } + }, "node_modules/fast-json-stable-stringify": { "version": "2.1.0", "resolved": "https://registry.npmjs.org/fast-json-stable-stringify/-/fast-json-stable-stringify-2.1.0.tgz", diff --git a/package.json b/package.json index 89d4d29..b665cc1 100644 --- a/package.json +++ b/package.json @@ -23,6 +23,7 @@ "cors": "^2.8.5", "dotenv": "^17.0.1", "express": "^4.18.2", + "express-rate-limit": "^7.5.1", "node-cron": "^3.0.3", "sqlite3": "^5.1.6", "swagger-jsdoc": "^6.2.8", diff --git a/src/routes/locations.ts b/src/routes/locations.ts index a345ca0..980d652 100644 --- a/src/routes/locations.ts +++ b/src/routes/locations.ts @@ -1,4 +1,5 @@ import express, { Request, Response, Router } from 'express'; +import rateLimit from 'express-rate-limit'; import Location from '../models/Location'; import ProfanityFilterService from '../services/ProfanityFilterService'; import { LocationSubmission } from '../types'; @@ -13,15 +14,25 @@ interface LocationPostRequest extends Request { }; } -interface LocationDeleteRequest extends Request { - params: { - id: string; - }; -} export default (locationModel: Location, profanityFilter: ProfanityFilterService | any): Router => { const router = express.Router(); + // Rate limiting for location submissions to prevent abuse + const submitLocationLimiter = rateLimit({ + windowMs: 15 * 60 * 1000, // 15 minutes + limit: 10, // Limit each IP to 10 location submissions per 15 minutes + message: { + error: 'Too many location reports submitted', + message: 'You can submit up to 10 location reports every 15 minutes. Please wait before submitting more.', + retryAfter: '15 minutes' + }, + standardHeaders: true, // Return rate limit info in the `RateLimit-*` headers + legacyHeaders: false, // Disable the `X-RateLimit-*` headers + // Skip rate limiting in test environment + skip: (req) => process.env.NODE_ENV === 'test' + }); + /** * @swagger * /api/locations: @@ -154,16 +165,34 @@ export default (locationModel: Location, profanityFilter: ProfanityFilterService * schema: * $ref: '#/components/schemas/Error' */ - router.post('/', async (req: LocationPostRequest, res: Response): Promise => { + router.post('/', submitLocationLimiter, async (req: LocationPostRequest, res: Response): Promise => { const { address, latitude, longitude } = req.body; let { description } = req.body; console.log(`Attempt to add new location: ${address}`); + // Input validation for security if (!address) { console.warn('Failed to add location: Address is required'); res.status(400).json({ error: 'Address is required' }); return; } + + if (typeof address !== 'string' || address.length > 500) { + console.warn(`Failed to add location: Invalid address length (${address?.length || 0})`); + res.status(400).json({ error: 'Address must be a string with maximum 500 characters' }); + return; + } + + if (description && (typeof description !== 'string' || description.length > 1000)) { + console.warn(`Failed to add location: Invalid description length (${description?.length || 0})`); + res.status(400).json({ error: 'Description must be a string with maximum 1000 characters' }); + return; + } + + // Log suspicious activity + if (address.length > 200 || (description && description.length > 500)) { + console.warn(`Unusually long submission from IP: ${req.ip} - Address: ${address.length} chars, Description: ${description?.length || 0} chars`); + } // Check for profanity in description and reject if any is found if (description && profanityFilter) { @@ -212,24 +241,8 @@ export default (locationModel: Location, profanityFilter: ProfanityFilterService } }); - // Legacy delete route (keeping for backwards compatibility) - router.delete('/:id', async (req: LocationDeleteRequest, res: Response): Promise => { - const { id } = req.params; - - try { - const result = await locationModel.delete(parseInt(id, 10)); - - if (result.changes === 0) { - res.status(404).json({ error: 'Location not found' }); - return; - } - - res.json({ message: 'Location deleted successfully' }); - } catch (err) { - console.error('Error deleting location:', err); - res.status(500).json({ error: 'Internal server error' }); - } - }); + // DELETE functionality has been moved to admin-only routes for security. + // Use /api/admin/locations/:id (with authentication) for location deletion. return router; }; \ No newline at end of file diff --git a/tests/integration/routes/public.test.ts b/tests/integration/routes/public.test.ts index c1be07f..58d6faf 100644 --- a/tests/integration/routes/public.test.ts +++ b/tests/integration/routes/public.test.ts @@ -313,15 +313,42 @@ describe('Public API Routes', () => { }); describe('Request validation', () => { - it('should handle very long addresses', async () => { - const longAddress = 'A'.repeat(1000); + it('should reject overly long addresses for security', async () => { + const longAddress = 'A'.repeat(1000); // Over 500 character limit const response = await request(app) .post('/api/locations') .send({ address: longAddress }) + .expect(400); + + expect(response.body).toHaveProperty('error'); + expect(response.body.error).toBe('Address must be a string with maximum 500 characters'); + }); + + it('should accept addresses within the limit', async () => { + const validLongAddress = 'A'.repeat(400); // Under 500 character limit + + const response = await request(app) + .post('/api/locations') + .send({ address: validLongAddress }) .expect(200); - expect(response.body.address).toBe(longAddress); + expect(response.body.address).toBe(validLongAddress); + }); + + it('should reject overly long descriptions for security', async () => { + const longDescription = 'B'.repeat(1500); // Over 1000 character limit + + const response = await request(app) + .post('/api/locations') + .send({ + address: 'Test Address', + description: longDescription + }) + .expect(400); + + expect(response.body).toHaveProperty('error'); + expect(response.body.error).toBe('Description must be a string with maximum 1000 characters'); }); it('should handle special characters in address', async () => { diff --git a/tests/setup.ts b/tests/setup.ts index 8b13142..3c9fcc3 100644 --- a/tests/setup.ts +++ b/tests/setup.ts @@ -70,14 +70,9 @@ export const createTestProfanityDatabase = (): Promise => { }); }; -// Cleanup function for tests +// Cleanup function for tests (in-memory databases don't need file cleanup) export const cleanupTestDatabases = () => { - // Clean up any test database files - [TEST_DB_PATH, TEST_PROFANITY_DB_PATH].forEach(dbPath => { - if (fs.existsSync(dbPath)) { - fs.unlinkSync(dbPath); - } - }); + // Using in-memory databases (:memory:) - no file cleanup needed }; // Global test cleanup