diff --git a/backend/config/imageLimits.js b/backend/config/imageLimits.js new file mode 100644 index 0000000..7feff25 --- /dev/null +++ b/backend/config/imageLimits.js @@ -0,0 +1,13 @@ +/** + * Image upload limits configuration + * Keep in sync with frontend/src/config/imageLimits.ts + */ +const IMAGE_LIMITS = { + items: 10, + forum: 10, + conditionChecks: 10, + profile: 1, + messages: 1, +}; + +module.exports = { IMAGE_LIMITS }; diff --git a/backend/middleware/upload.js b/backend/middleware/upload.js deleted file mode 100644 index e7a7a0d..0000000 --- a/backend/middleware/upload.js +++ /dev/null @@ -1,94 +0,0 @@ -const multer = require('multer'); -const path = require('path'); -const { v4: uuidv4 } = require('uuid'); - -// Configure storage for profile images -const profileImageStorage = multer.diskStorage({ - destination: function (req, file, cb) { - cb(null, path.join(__dirname, '../uploads/profiles')); - }, - filename: function (req, file, cb) { - // Generate unique filename: uuid + original extension - const uniqueId = uuidv4(); - const ext = path.extname(file.originalname); - cb(null, `${uniqueId}${ext}`); - } -}); - -// File filter to accept only images -const imageFileFilter = (req, file, cb) => { - // Accept images only - const allowedMimes = ['image/jpeg', 'image/jpg', 'image/png', 'image/gif', 'image/webp']; - if (allowedMimes.includes(file.mimetype)) { - cb(null, true); - } else { - cb(new Error('Invalid file type. Only JPEG, PNG, GIF and WebP images are allowed.'), false); - } -}; - -// Create multer upload middleware for profile images -const uploadProfileImage = multer({ - storage: profileImageStorage, - fileFilter: imageFileFilter, - limits: { - fileSize: 5 * 1024 * 1024 // 5MB limit - } -}).single('profileImage'); - -// Configure storage for message images -const messageImageStorage = multer.diskStorage({ - destination: function (req, file, cb) { - cb(null, path.join(__dirname, '../uploads/messages')); - }, - filename: function (req, file, cb) { - // Generate unique filename: uuid + original extension - const uniqueId = uuidv4(); - const ext = path.extname(file.originalname); - cb(null, `${uniqueId}${ext}`); - } -}); - -// Create multer upload middleware for message images -const uploadMessageImage = multer({ - storage: messageImageStorage, - fileFilter: imageFileFilter, - limits: { - fileSize: 5 * 1024 * 1024 // 5MB limit - } -}).single('image'); - -// Configure storage for forum images -const forumImageStorage = multer.diskStorage({ - destination: function (req, file, cb) { - cb(null, path.join(__dirname, '../uploads/forum')); - }, - filename: function (req, file, cb) { - const uniqueId = uuidv4(); - const ext = path.extname(file.originalname); - cb(null, `${uniqueId}${ext}`); - } -}); - -// Factory function to create forum image upload middleware -const createForumImageUpload = (maxFiles) => { - return multer({ - storage: forumImageStorage, - fileFilter: imageFileFilter, - limits: { - fileSize: 5 * 1024 * 1024 // 5MB limit per file - } - }).array('images', maxFiles); -}; - -// Create multer upload middleware for forum post images (up to 5 images) -const uploadForumPostImages = createForumImageUpload(5); - -// Create multer upload middleware for forum comment images (up to 3 images) -const uploadForumCommentImages = createForumImageUpload(3); - -module.exports = { - uploadProfileImage, - uploadMessageImage, - uploadForumPostImages, - uploadForumCommentImages -}; \ No newline at end of file diff --git a/backend/package-lock.json b/backend/package-lock.json index a7fed21..aec86fa 100644 --- a/backend/package-lock.json +++ b/backend/package-lock.json @@ -29,7 +29,6 @@ "jsdom": "^27.0.0", "jsonwebtoken": "^9.0.2", "morgan": "^1.10.1", - "multer": "^2.0.2", "node-cron": "^3.0.3", "pg": "^8.16.3", "sequelize": "^6.37.7", @@ -5160,12 +5159,6 @@ "node": ">= 8" } }, - "node_modules/append-field": { - "version": "1.0.0", - "resolved": "https://registry.npmjs.org/append-field/-/append-field-1.0.0.tgz", - "integrity": "sha512-klpgFSWLW1ZEs8svjfb7g4qWY0YS5imI82dTg+QahUvJ8YqAY0P10Uk8tTyh9ZGuYEZEMaeJYCF5BFuX552hsw==", - "license": "MIT" - }, "node_modules/argparse": { "version": "1.0.10", "resolved": "https://registry.npmjs.org/argparse/-/argparse-1.0.10.tgz", @@ -5539,19 +5532,9 @@ "version": "1.1.2", "resolved": "https://registry.npmjs.org/buffer-from/-/buffer-from-1.1.2.tgz", "integrity": "sha512-E+XQCRwSbaaiChtv6k6Dwgc+bx+Bs6vuKJHHl5kox/BaKbhiXzqQOwK4cO22yElGp2OCmjwVhT3HmxgyPGnJfQ==", + "dev": true, "license": "MIT" }, - "node_modules/busboy": { - "version": "1.6.0", - "resolved": "https://registry.npmjs.org/busboy/-/busboy-1.6.0.tgz", - "integrity": "sha512-8SFQbg/0hQ9xy3UNTB0YEnsNBbWfhf7RtnzpL7TkBiTBRfrQ9Fxcnz7VJsleJpyp6rVLvXiuORqjlHi5q+PYuA==", - "dependencies": { - "streamsearch": "^1.1.0" - }, - "engines": { - "node": ">=10.16.0" - } - }, "node_modules/bytes": { "version": "3.1.2", "resolved": "https://registry.npmjs.org/bytes/-/bytes-3.1.2.tgz", @@ -5934,21 +5917,6 @@ "integrity": "sha512-/Srv4dswyQNBfohGpz9o6Yb3Gz3SrUDqBH5rTuhGR7ahtlbYKnVxw2bCFMRljaA7EXHaXZ8wsHdodFvbkhKmqg==", "dev": true }, - "node_modules/concat-stream": { - "version": "2.0.0", - "resolved": "https://registry.npmjs.org/concat-stream/-/concat-stream-2.0.0.tgz", - "integrity": "sha512-MWufYdFw53ccGjCA+Ol7XJYpAlW6/prSMzuPOTRnJGcGzuhLn4Scrz7qf6o8bROZ514ltazcIFJZevcfbo0x7A==", - "engines": [ - "node >= 6.0" - ], - "license": "MIT", - "dependencies": { - "buffer-from": "^1.0.0", - "inherits": "^2.0.3", - "readable-stream": "^3.0.2", - "typedarray": "^0.0.6" - } - }, "node_modules/config-chain": { "version": "1.1.13", "resolved": "https://registry.npmjs.org/config-chain/-/config-chain-1.1.13.tgz", @@ -8900,15 +8868,6 @@ "url": "https://github.com/sponsors/isaacs" } }, - "node_modules/minimist": { - "version": "1.2.8", - "resolved": "https://registry.npmjs.org/minimist/-/minimist-1.2.8.tgz", - "integrity": "sha512-2yyAR8qBkN3YuheJanUpWC5U3bb5osDywNB8RzDVlDwDHbocAJveqqj1u8+SVD7jkWT4yvsHCpWqqWqAxb0zCA==", - "license": "MIT", - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, "node_modules/minipass": { "version": "7.1.2", "resolved": "https://registry.npmjs.org/minipass/-/minipass-7.1.2.tgz", @@ -8917,18 +8876,6 @@ "node": ">=16 || 14 >=14.17" } }, - "node_modules/mkdirp": { - "version": "0.5.6", - "resolved": "https://registry.npmjs.org/mkdirp/-/mkdirp-0.5.6.tgz", - "integrity": "sha512-FP+p8RB8OWpF3YZBCrP5gtADmtXApB5AMLn+vdyA+PyxCjrCs00mjyUozssO33cwDeT3wNGdLxJ5M//YqtHAJw==", - "license": "MIT", - "dependencies": { - "minimist": "^1.2.6" - }, - "bin": { - "mkdirp": "bin/cmd.js" - } - }, "node_modules/moment": { "version": "2.30.1", "resolved": "https://registry.npmjs.org/moment/-/moment-2.30.1.tgz", @@ -8996,67 +8943,6 @@ "resolved": "https://registry.npmjs.org/ms/-/ms-2.1.3.tgz", "integrity": "sha512-6FlzubTLZG3J2a/NVCAleEhjzq5oxgHyaCU9yYXvcLsvoVaHJq/s5xXI6/XXP6tz7R9xAOtHnSO/tXtF3WRTlA==" }, - "node_modules/multer": { - "version": "2.0.2", - "resolved": "https://registry.npmjs.org/multer/-/multer-2.0.2.tgz", - "integrity": "sha512-u7f2xaZ/UG8oLXHvtF/oWTRvT44p9ecwBBqTwgJVq0+4BW1g8OW01TyMEGWBHbyMOYVHXslaut7qEQ1meATXgw==", - "license": "MIT", - "dependencies": { - "append-field": "^1.0.0", - "busboy": "^1.6.0", - "concat-stream": "^2.0.0", - "mkdirp": "^0.5.6", - "object-assign": "^4.1.1", - "type-is": "^1.6.18", - "xtend": "^4.0.2" - }, - "engines": { - "node": ">= 10.16.0" - } - }, - "node_modules/multer/node_modules/media-typer": { - "version": "0.3.0", - "resolved": "https://registry.npmjs.org/media-typer/-/media-typer-0.3.0.tgz", - "integrity": "sha512-dq+qelQ9akHpcOl/gUVRTxVIOkAJ1wR3QAvb4RsVjS8oVoFjDGTc679wJYmUmknUF5HwMLOgb5O+a3KxfWapPQ==", - "license": "MIT", - "engines": { - "node": ">= 0.6" - } - }, - "node_modules/multer/node_modules/mime-db": { - "version": "1.52.0", - "resolved": "https://registry.npmjs.org/mime-db/-/mime-db-1.52.0.tgz", - "integrity": "sha512-sPU4uV7dYlvtWJxwwxHD0PuihVNiE7TyAbQ5SWxDCB9mUYvOgroQOwYQQOKPJ8CIbE+1ETVlOoK1UC2nU3gYvg==", - "license": "MIT", - "engines": { - "node": ">= 0.6" - } - }, - "node_modules/multer/node_modules/mime-types": { - "version": "2.1.35", - "resolved": "https://registry.npmjs.org/mime-types/-/mime-types-2.1.35.tgz", - "integrity": "sha512-ZDY+bPm5zTTF+YpCrAU9nK0UgICYPT0QtT1NZWFv4s++TNkcgVaT0g6+4R2uI4MjQjzysHB1zxuWL50hzaeXiw==", - "license": "MIT", - "dependencies": { - "mime-db": "1.52.0" - }, - "engines": { - "node": ">= 0.6" - } - }, - "node_modules/multer/node_modules/type-is": { - "version": "1.6.18", - "resolved": "https://registry.npmjs.org/type-is/-/type-is-1.6.18.tgz", - "integrity": "sha512-TkRKr9sUTxEH8MdfuCSP7VizJyzRNMjj2J2do2Jr3Kym598JVdEksuzPQCnlFPW4ky9Q+iA+ma9BGm06XQBy8g==", - "license": "MIT", - "dependencies": { - "media-typer": "0.3.0", - "mime-types": "~2.1.24" - }, - "engines": { - "node": ">= 0.6" - } - }, "node_modules/nanoid": { "version": "3.3.11", "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.11.tgz", @@ -10637,14 +10523,6 @@ "node": ">= 0.8" } }, - "node_modules/streamsearch": { - "version": "1.1.0", - "resolved": "https://registry.npmjs.org/streamsearch/-/streamsearch-1.1.0.tgz", - "integrity": "sha512-Mcc5wHehp9aXz1ax6bZUyY5afg9u2rv5cqQI3mRrYkGC8rW2hM02jWuwjtL++LS5qinSyhj2QfLyNsuc+VsExg==", - "engines": { - "node": ">=10.0.0" - } - }, "node_modules/strict-uri-encode": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/strict-uri-encode/-/strict-uri-encode-2.0.0.tgz", @@ -11143,12 +11021,6 @@ "node": ">= 0.6" } }, - "node_modules/typedarray": { - "version": "0.0.6", - "resolved": "https://registry.npmjs.org/typedarray/-/typedarray-0.0.6.tgz", - "integrity": "sha512-/aCDEGatGvZ2BIk+HmLf4ifCJFwvKFNb9/JeZPMulfgFracn9QFcAf5GO8B/mweUjSoblS5In0cWhqpfs/5PQA==", - "license": "MIT" - }, "node_modules/uid-safe": { "version": "2.1.5", "resolved": "https://registry.npmjs.org/uid-safe/-/uid-safe-2.1.5.tgz", diff --git a/backend/package.json b/backend/package.json index 4296e94..0645fe7 100644 --- a/backend/package.json +++ b/backend/package.json @@ -54,7 +54,6 @@ "jsdom": "^27.0.0", "jsonwebtoken": "^9.0.2", "morgan": "^1.10.1", - "multer": "^2.0.2", "node-cron": "^3.0.3", "pg": "^8.16.3", "sequelize": "^6.37.7", diff --git a/backend/routes/conditionChecks.js b/backend/routes/conditionChecks.js index 9800655..bfce062 100644 --- a/backend/routes/conditionChecks.js +++ b/backend/routes/conditionChecks.js @@ -2,6 +2,8 @@ const express = require("express"); const { authenticateToken } = require("../middleware/auth"); const ConditionCheckService = require("../services/conditionCheckService"); const logger = require("../utils/logger"); +const { validateS3Keys } = require("../utils/s3KeyValidator"); +const { IMAGE_LIMITS } = require("../config/imageLimits"); const router = express.Router(); @@ -13,10 +15,24 @@ router.post("/:rentalId", authenticateToken, async (req, res) => { const userId = req.user.id; // Ensure imageFilenames is an array (S3 keys) - const imageFilenames = Array.isArray(rawImageFilenames) + const imageFilenamesArray = Array.isArray(rawImageFilenames) ? rawImageFilenames : []; + // Validate S3 keys format and folder + const keyValidation = validateS3Keys(imageFilenamesArray, "condition-checks", { + maxKeys: IMAGE_LIMITS.conditionChecks, + }); + if (!keyValidation.valid) { + return res.status(400).json({ + success: false, + error: keyValidation.error, + details: keyValidation.invalidKeys, + }); + } + + const imageFilenames = imageFilenamesArray; + const conditionCheck = await ConditionCheckService.submitConditionCheck( rentalId, checkType, diff --git a/backend/routes/forum.js b/backend/routes/forum.js index e59a1ab..ee078cc 100644 --- a/backend/routes/forum.js +++ b/backend/routes/forum.js @@ -6,6 +6,8 @@ const logger = require('../utils/logger'); const emailServices = require('../services/email'); const googleMapsService = require('../services/googleMapsService'); const locationService = require('../services/locationService'); +const { validateS3Keys } = require('../utils/s3KeyValidator'); +const { IMAGE_LIMITS } = require('../config/imageLimits'); const router = express.Router(); // Helper function to build nested comment tree @@ -239,10 +241,20 @@ router.get('/posts/:id', optionalAuth, async (req, res, next) => { // POST /api/forum/posts - Create new post router.post('/posts', authenticateToken, async (req, res, next) => { try { - let { title, content, category, tags, zipCode, latitude: providedLat, longitude: providedLng, imageFilenames } = req.body; + let { title, content, category, tags, zipCode, latitude: providedLat, longitude: providedLng, imageFilenames: rawImageFilenames } = req.body; - // Ensure imageFilenames is an array - imageFilenames = Array.isArray(imageFilenames) ? imageFilenames : []; + // Ensure imageFilenames is an array and validate S3 keys + const imageFilenamesArray = Array.isArray(rawImageFilenames) ? rawImageFilenames : []; + + const keyValidation = validateS3Keys(imageFilenamesArray, 'forum', { maxKeys: IMAGE_LIMITS.forum }); + if (!keyValidation.valid) { + return res.status(400).json({ + error: keyValidation.error, + details: keyValidation.invalidKeys + }); + } + + const imageFilenames = imageFilenamesArray; // Initialize location fields let latitude = null; @@ -488,9 +500,26 @@ router.put('/posts/:id', authenticateToken, async (req, res, next) => { return res.status(403).json({ error: 'Unauthorized' }); } - const { title, content, category, tags } = req.body; + const { title, content, category, tags, imageFilenames: rawImageFilenames } = req.body; - await post.update({ title, content, category }); + // Build update object + const updateData = { title, content, category }; + + // Handle imageFilenames if provided + if (rawImageFilenames !== undefined) { + const imageFilenamesArray = Array.isArray(rawImageFilenames) ? rawImageFilenames : []; + + const keyValidation = validateS3Keys(imageFilenamesArray, 'forum', { maxKeys: IMAGE_LIMITS.forum }); + if (!keyValidation.valid) { + return res.status(400).json({ + error: keyValidation.error, + details: keyValidation.invalidKeys + }); + } + updateData.imageFilenames = imageFilenamesArray; + } + + await post.update(updateData); // Update tags if provided if (tags !== undefined) { @@ -927,8 +956,18 @@ router.post('/posts/:id/comments', authenticateToken, async (req, res, next) => } } - // Ensure imageFilenames is an array - const imageFilenames = Array.isArray(rawImageFilenames) ? rawImageFilenames : []; + // Ensure imageFilenames is an array and validate S3 keys + const imageFilenamesArray = Array.isArray(rawImageFilenames) ? rawImageFilenames : []; + + const keyValidation = validateS3Keys(imageFilenamesArray, 'forum', { maxKeys: IMAGE_LIMITS.forum }); + if (!keyValidation.valid) { + return res.status(400).json({ + error: keyValidation.error, + details: keyValidation.invalidKeys + }); + } + + const imageFilenames = imageFilenamesArray; const comment = await ForumComment.create({ postId: req.params.id, diff --git a/backend/routes/items.js b/backend/routes/items.js index 432dcc9..c4f49a3 100644 --- a/backend/routes/items.js +++ b/backend/routes/items.js @@ -3,8 +3,56 @@ const { Op } = require("sequelize"); const { Item, User, Rental } = require("../models"); // Import from models/index.js to get models with associations const { authenticateToken, requireVerifiedEmail, requireAdmin, optionalAuth } = require("../middleware/auth"); const logger = require("../utils/logger"); +const { validateS3Keys } = require("../utils/s3KeyValidator"); +const { IMAGE_LIMITS } = require("../config/imageLimits"); const router = express.Router(); +// Allowed fields for item create/update (prevents mass assignment) +const ALLOWED_ITEM_FIELDS = [ + 'name', + 'description', + 'pickUpAvailable', + 'localDeliveryAvailable', + 'localDeliveryRadius', + 'shippingAvailable', + 'inPlaceUseAvailable', + 'pricePerHour', + 'pricePerDay', + 'pricePerWeek', + 'pricePerMonth', + 'replacementCost', + 'address1', + 'address2', + 'city', + 'state', + 'zipCode', + 'country', + 'latitude', + 'longitude', + 'imageFilenames', + 'isAvailable', + 'rules', + 'availableAfter', + 'availableBefore', + 'specifyTimesPerDay', + 'weeklyTimes', +]; + +/** + * Extract only allowed fields from request body + * @param {Object} body - Request body + * @returns {Object} - Object with only allowed fields + */ +function extractAllowedFields(body) { + const result = {}; + for (const field of ALLOWED_ITEM_FIELDS) { + if (body[field] !== undefined) { + result[field] = body[field]; + } + } + return result; +} + router.get("/", async (req, res, next) => { try { const { @@ -232,8 +280,27 @@ router.get("/:id", optionalAuth, async (req, res, next) => { router.post("/", authenticateToken, requireVerifiedEmail, async (req, res, next) => { try { + // Extract only allowed fields (prevents mass assignment) + const allowedData = extractAllowedFields(req.body); + + // Validate imageFilenames if provided + if (allowedData.imageFilenames) { + const imageFilenames = Array.isArray(allowedData.imageFilenames) + ? allowedData.imageFilenames + : []; + + const keyValidation = validateS3Keys(imageFilenames, 'items', { maxKeys: IMAGE_LIMITS.items }); + if (!keyValidation.valid) { + return res.status(400).json({ + error: keyValidation.error, + details: keyValidation.invalidKeys + }); + } + allowedData.imageFilenames = imageFilenames; + } + const item = await Item.create({ - ...req.body, + ...allowedData, ownerId: req.user.id, }); @@ -300,7 +367,26 @@ router.put("/:id", authenticateToken, async (req, res, next) => { return res.status(403).json({ error: "Unauthorized" }); } - await item.update(req.body); + // Extract only allowed fields (prevents mass assignment) + const allowedData = extractAllowedFields(req.body); + + // Validate imageFilenames if provided + if (allowedData.imageFilenames !== undefined) { + const imageFilenames = Array.isArray(allowedData.imageFilenames) + ? allowedData.imageFilenames + : []; + + const keyValidation = validateS3Keys(imageFilenames, 'items', { maxKeys: IMAGE_LIMITS.items }); + if (!keyValidation.valid) { + return res.status(400).json({ + error: keyValidation.error, + details: keyValidation.invalidKeys + }); + } + allowedData.imageFilenames = imageFilenames; + } + + await item.update(allowedData); const updatedItem = await Item.findByPk(item.id, { include: [ diff --git a/backend/routes/messages.js b/backend/routes/messages.js index 544aec9..c7a49bc 100644 --- a/backend/routes/messages.js +++ b/backend/routes/messages.js @@ -8,6 +8,8 @@ const { Op } = require('sequelize'); const emailServices = require('../services/email'); const fs = require('fs'); const path = require('path'); +const { validateS3Keys } = require('../utils/s3KeyValidator'); +const { IMAGE_LIMITS } = require('../config/imageLimits'); const router = express.Router(); // Get all messages for the current user (inbox) @@ -240,6 +242,17 @@ router.post('/', authenticateToken, async (req, res, next) => { try { const { receiverId, content, imageFilename } = req.body; + // Validate imageFilename if provided + if (imageFilename) { + const keyValidation = validateS3Keys([imageFilename], 'messages', { maxKeys: IMAGE_LIMITS.messages }); + if (!keyValidation.valid) { + return res.status(400).json({ + error: keyValidation.error, + details: keyValidation.invalidKeys + }); + } + } + // Check if receiver exists const receiver = await User.findByPk(receiverId); if (!receiver) { diff --git a/backend/routes/users.js b/backend/routes/users.js index b64a0d4..b2609dd 100644 --- a/backend/routes/users.js +++ b/backend/routes/users.js @@ -1,13 +1,41 @@ const express = require('express'); const { User, UserAddress } = require('../models'); // Import from models/index.js to get models with associations const { authenticateToken } = require('../middleware/auth'); -const { uploadProfileImage } = require('../middleware/upload'); const logger = require('../utils/logger'); const userService = require('../services/UserService'); -const fs = require('fs').promises; -const path = require('path'); +const { validateS3Keys } = require('../utils/s3KeyValidator'); +const { IMAGE_LIMITS } = require('../config/imageLimits'); const router = express.Router(); +// Allowed fields for profile update (prevents mass assignment) +const ALLOWED_PROFILE_FIELDS = [ + 'firstName', + 'lastName', + 'email', + 'phone', + 'address1', + 'address2', + 'city', + 'state', + 'zipCode', + 'country', + 'imageFilename', + 'itemRequestNotificationRadius', +]; + +/** + * Extract only allowed fields from request body + */ +function extractAllowedProfileFields(body) { + const result = {}; + for (const field of ALLOWED_PROFILE_FIELDS) { + if (body[field] !== undefined) { + result[field] = body[field]; + } + } + return result; +} + router.get('/profile', authenticateToken, async (req, res, next) => { try { const user = await User.findByPk(req.user.id, { @@ -182,8 +210,22 @@ router.get('/:id', async (req, res, next) => { router.put('/profile', authenticateToken, async (req, res, next) => { try { + // Extract only allowed fields (prevents mass assignment) + const allowedData = extractAllowedProfileFields(req.body); + + // Validate imageFilename if provided + if (allowedData.imageFilename !== undefined && allowedData.imageFilename !== null) { + const keyValidation = validateS3Keys([allowedData.imageFilename], 'profiles', { maxKeys: IMAGE_LIMITS.profile }); + if (!keyValidation.valid) { + return res.status(400).json({ + error: keyValidation.error, + details: keyValidation.invalidKeys + }); + } + } + // Use UserService to handle update and email notification - const updatedUser = await userService.updateProfile(req.user.id, req.body); + const updatedUser = await userService.updateProfile(req.user.id, allowedData); res.json(updatedUser); } catch (error) { @@ -192,65 +234,4 @@ router.put('/profile', authenticateToken, async (req, res, next) => { } }); -// Upload profile image endpoint -router.post('/profile/image', authenticateToken, (req, res) => { - uploadProfileImage(req, res, async (err) => { - if (err) { - const reqLogger = logger.withRequestId(req.id); - reqLogger.error("Profile image upload error", { - error: err.message, - userId: req.user.id - }); - return res.status(400).json({ error: err.message }); - } - - if (!req.file) { - return res.status(400).json({ error: 'No file uploaded' }); - } - - try { - // Delete old profile image if exists - const user = await User.findByPk(req.user.id); - if (user.imageFilename) { - const oldImagePath = path.join(__dirname, '../uploads/profiles', user.imageFilename); - try { - await fs.unlink(oldImagePath); - } catch (unlinkErr) { - const reqLogger = logger.withRequestId(req.id); - reqLogger.warn("Error deleting old profile image", { - error: unlinkErr.message, - userId: req.user.id, - oldImagePath - }); - } - } - - // Update user with new image filename - await user.update({ - imageFilename: req.file.filename - }); - - const reqLogger = logger.withRequestId(req.id); - reqLogger.info("Profile image uploaded successfully", { - userId: req.user.id, - filename: req.file.filename - }); - - res.json({ - message: 'Profile image uploaded successfully', - filename: req.file.filename, - imageUrl: `/uploads/profiles/${req.file.filename}` - }); - } catch (error) { - const reqLogger = logger.withRequestId(req.id); - reqLogger.error("Profile image database update failed", { - error: error.message, - stack: error.stack, - userId: req.user.id - }); - res.status(500).json({ error: 'Failed to update profile image' }); - } - }); -}); - module.exports = router; \ No newline at end of file diff --git a/backend/tests/unit/routes/users.test.js b/backend/tests/unit/routes/users.test.js index a93899e..58542a4 100644 --- a/backend/tests/unit/routes/users.test.js +++ b/backend/tests/unit/routes/users.test.js @@ -1,9 +1,9 @@ -const request = require('supertest'); -const express = require('express'); -const usersRouter = require('../../../routes/users'); +const request = require("supertest"); +const express = require("express"); +const usersRouter = require("../../../routes/users"); // Mock all dependencies -jest.mock('../../../models', () => ({ +jest.mock("../../../models", () => ({ User: { findByPk: jest.fn(), update: jest.fn(), @@ -15,41 +15,22 @@ jest.mock('../../../models', () => ({ }, })); -jest.mock('../../../middleware/auth', () => ({ +jest.mock("../../../middleware/auth", () => ({ authenticateToken: jest.fn((req, res, next) => { req.user = { id: 1, - update: jest.fn() + update: jest.fn(), }; next(); }), })); -jest.mock('../../../middleware/upload', () => ({ - uploadProfileImage: jest.fn((req, res, callback) => { - // Mock successful upload - req.file = { - filename: 'test-profile.jpg' - }; - callback(null); - }), -})); - -jest.mock('fs', () => ({ - promises: { - unlink: jest.fn(), - }, -})); - -jest.mock('path'); -const { User, UserAddress } = require('../../../models'); -const { uploadProfileImage } = require('../../../middleware/upload'); -const fs = require('fs').promises; +const { User, UserAddress } = require("../../../models"); // Create express app with the router const app = express(); app.use(express.json()); -app.use('/users', usersRouter); +app.use("/users", usersRouter); // Mock models const mockUserFindByPk = User.findByPk; @@ -58,97 +39,96 @@ const mockUserAddressFindAll = UserAddress.findAll; const mockUserAddressFindByPk = UserAddress.findByPk; const mockUserAddressCreate = UserAddress.create; -describe('Users Routes', () => { +describe("Users Routes", () => { beforeEach(() => { jest.clearAllMocks(); }); - describe('GET /profile', () => { - it('should get user profile for authenticated user', async () => { + describe("GET /profile", () => { + it("should get user profile for authenticated user", async () => { const mockUser = { id: 1, - firstName: 'John', - lastName: 'Doe', - email: 'john@example.com', - phone: '555-1234', - imageFilename: 'profile.jpg', + firstName: "John", + lastName: "Doe", + email: "john@example.com", + phone: "555-1234", + imageFilename: "profile.jpg", }; mockUserFindByPk.mockResolvedValue(mockUser); - const response = await request(app) - .get('/users/profile'); + const response = await request(app).get("/users/profile"); expect(response.status).toBe(200); expect(response.body).toEqual(mockUser); expect(mockUserFindByPk).toHaveBeenCalledWith(1, { - attributes: { exclude: ['password'] } + attributes: { exclude: ["password"] }, }); }); - it('should handle database errors', async () => { - mockUserFindByPk.mockRejectedValue(new Error('Database error')); + it("should handle database errors", async () => { + mockUserFindByPk.mockRejectedValue(new Error("Database error")); - const response = await request(app) - .get('/users/profile'); + const response = await request(app).get("/users/profile"); expect(response.status).toBe(500); - expect(response.body).toEqual({ error: 'Database error' }); + expect(response.body).toEqual({ error: "Database error" }); }); }); - describe('GET /addresses', () => { - it('should get user addresses', async () => { + describe("GET /addresses", () => { + it("should get user addresses", async () => { const mockAddresses = [ { id: 1, userId: 1, - address1: '123 Main St', - city: 'New York', + address1: "123 Main St", + city: "New York", isPrimary: true, }, { id: 2, userId: 1, - address1: '456 Oak Ave', - city: 'Boston', + address1: "456 Oak Ave", + city: "Boston", isPrimary: false, }, ]; mockUserAddressFindAll.mockResolvedValue(mockAddresses); - const response = await request(app) - .get('/users/addresses'); + const response = await request(app).get("/users/addresses"); expect(response.status).toBe(200); expect(response.body).toEqual(mockAddresses); expect(mockUserAddressFindAll).toHaveBeenCalledWith({ where: { userId: 1 }, - order: [['isPrimary', 'DESC'], ['createdAt', 'ASC']] + order: [ + ["isPrimary", "DESC"], + ["createdAt", "ASC"], + ], }); }); - it('should handle database errors', async () => { - mockUserAddressFindAll.mockRejectedValue(new Error('Database error')); + it("should handle database errors", async () => { + mockUserAddressFindAll.mockRejectedValue(new Error("Database error")); - const response = await request(app) - .get('/users/addresses'); + const response = await request(app).get("/users/addresses"); expect(response.status).toBe(500); - expect(response.body).toEqual({ error: 'Database error' }); + expect(response.body).toEqual({ error: "Database error" }); }); }); - describe('POST /addresses', () => { - it('should create a new address', async () => { + describe("POST /addresses", () => { + it("should create a new address", async () => { const addressData = { - address1: '789 Pine St', - address2: 'Apt 4B', - city: 'Chicago', - state: 'IL', - zipCode: '60601', - country: 'USA', + address1: "789 Pine St", + address2: "Apt 4B", + city: "Chicago", + state: "IL", + zipCode: "60601", + country: "USA", isPrimary: false, }; @@ -161,38 +141,36 @@ describe('Users Routes', () => { mockUserAddressCreate.mockResolvedValue(mockCreatedAddress); const response = await request(app) - .post('/users/addresses') + .post("/users/addresses") .send(addressData); expect(response.status).toBe(201); expect(response.body).toEqual(mockCreatedAddress); expect(mockUserAddressCreate).toHaveBeenCalledWith({ ...addressData, - userId: 1 + userId: 1, }); }); - it('should handle database errors during creation', async () => { - mockUserAddressCreate.mockRejectedValue(new Error('Database error')); + it("should handle database errors during creation", async () => { + mockUserAddressCreate.mockRejectedValue(new Error("Database error")); - const response = await request(app) - .post('/users/addresses') - .send({ - address1: '789 Pine St', - city: 'Chicago', - }); + const response = await request(app).post("/users/addresses").send({ + address1: "789 Pine St", + city: "Chicago", + }); expect(response.status).toBe(500); - expect(response.body).toEqual({ error: 'Database error' }); + expect(response.body).toEqual({ error: "Database error" }); }); }); - describe('PUT /addresses/:id', () => { + describe("PUT /addresses/:id", () => { const mockAddress = { id: 1, userId: 1, - address1: '123 Main St', - city: 'New York', + address1: "123 Main St", + city: "New York", update: jest.fn(), }; @@ -200,68 +178,68 @@ describe('Users Routes', () => { mockUserAddressFindByPk.mockResolvedValue(mockAddress); }); - it('should update user address', async () => { + it("should update user address", async () => { const updateData = { - address1: '123 Updated St', - city: 'Updated City', + address1: "123 Updated St", + city: "Updated City", }; mockAddress.update.mockResolvedValue(); const response = await request(app) - .put('/users/addresses/1') + .put("/users/addresses/1") .send(updateData); expect(response.status).toBe(200); expect(response.body).toEqual({ id: 1, userId: 1, - address1: '123 Main St', - city: 'New York', + address1: "123 Main St", + city: "New York", }); expect(mockAddress.update).toHaveBeenCalledWith(updateData); }); - it('should return 404 for non-existent address', async () => { + it("should return 404 for non-existent address", async () => { mockUserAddressFindByPk.mockResolvedValue(null); const response = await request(app) - .put('/users/addresses/999') - .send({ address1: 'Updated St' }); + .put("/users/addresses/999") + .send({ address1: "Updated St" }); expect(response.status).toBe(404); - expect(response.body).toEqual({ error: 'Address not found' }); + expect(response.body).toEqual({ error: "Address not found" }); }); - it('should return 403 for unauthorized user', async () => { + it("should return 403 for unauthorized user", async () => { const unauthorizedAddress = { ...mockAddress, userId: 2 }; mockUserAddressFindByPk.mockResolvedValue(unauthorizedAddress); const response = await request(app) - .put('/users/addresses/1') - .send({ address1: 'Updated St' }); + .put("/users/addresses/1") + .send({ address1: "Updated St" }); expect(response.status).toBe(403); - expect(response.body).toEqual({ error: 'Unauthorized' }); + expect(response.body).toEqual({ error: "Unauthorized" }); }); - it('should handle database errors', async () => { - mockUserAddressFindByPk.mockRejectedValue(new Error('Database error')); + it("should handle database errors", async () => { + mockUserAddressFindByPk.mockRejectedValue(new Error("Database error")); const response = await request(app) - .put('/users/addresses/1') - .send({ address1: 'Updated St' }); + .put("/users/addresses/1") + .send({ address1: "Updated St" }); expect(response.status).toBe(500); - expect(response.body).toEqual({ error: 'Database error' }); + expect(response.body).toEqual({ error: "Database error" }); }); }); - describe('DELETE /addresses/:id', () => { + describe("DELETE /addresses/:id", () => { const mockAddress = { id: 1, userId: 1, - address1: '123 Main St', + address1: "123 Main St", destroy: jest.fn(), }; @@ -269,211 +247,210 @@ describe('Users Routes', () => { mockUserAddressFindByPk.mockResolvedValue(mockAddress); }); - it('should delete user address', async () => { + it("should delete user address", async () => { mockAddress.destroy.mockResolvedValue(); - const response = await request(app) - .delete('/users/addresses/1'); + const response = await request(app).delete("/users/addresses/1"); expect(response.status).toBe(204); expect(mockAddress.destroy).toHaveBeenCalled(); }); - it('should return 404 for non-existent address', async () => { + it("should return 404 for non-existent address", async () => { mockUserAddressFindByPk.mockResolvedValue(null); - const response = await request(app) - .delete('/users/addresses/999'); + const response = await request(app).delete("/users/addresses/999"); expect(response.status).toBe(404); - expect(response.body).toEqual({ error: 'Address not found' }); + expect(response.body).toEqual({ error: "Address not found" }); }); - it('should return 403 for unauthorized user', async () => { + it("should return 403 for unauthorized user", async () => { const unauthorizedAddress = { ...mockAddress, userId: 2 }; mockUserAddressFindByPk.mockResolvedValue(unauthorizedAddress); - const response = await request(app) - .delete('/users/addresses/1'); + const response = await request(app).delete("/users/addresses/1"); expect(response.status).toBe(403); - expect(response.body).toEqual({ error: 'Unauthorized' }); + expect(response.body).toEqual({ error: "Unauthorized" }); }); - it('should handle database errors', async () => { - mockUserAddressFindByPk.mockRejectedValue(new Error('Database error')); + it("should handle database errors", async () => { + mockUserAddressFindByPk.mockRejectedValue(new Error("Database error")); - const response = await request(app) - .delete('/users/addresses/1'); + const response = await request(app).delete("/users/addresses/1"); expect(response.status).toBe(500); - expect(response.body).toEqual({ error: 'Database error' }); + expect(response.body).toEqual({ error: "Database error" }); }); }); - describe('GET /availability', () => { - it('should get user availability settings', async () => { + describe("GET /availability", () => { + it("should get user availability settings", async () => { const mockUser = { - defaultAvailableAfter: '09:00', - defaultAvailableBefore: '17:00', + defaultAvailableAfter: "09:00", + defaultAvailableBefore: "17:00", defaultSpecifyTimesPerDay: true, - defaultWeeklyTimes: { monday: '09:00-17:00', tuesday: '10:00-16:00' }, + defaultWeeklyTimes: { monday: "09:00-17:00", tuesday: "10:00-16:00" }, }; mockUserFindByPk.mockResolvedValue(mockUser); - const response = await request(app) - .get('/users/availability'); + const response = await request(app).get("/users/availability"); expect(response.status).toBe(200); expect(response.body).toEqual({ - generalAvailableAfter: '09:00', - generalAvailableBefore: '17:00', + generalAvailableAfter: "09:00", + generalAvailableBefore: "17:00", specifyTimesPerDay: true, - weeklyTimes: { monday: '09:00-17:00', tuesday: '10:00-16:00' }, + weeklyTimes: { monday: "09:00-17:00", tuesday: "10:00-16:00" }, }); expect(mockUserFindByPk).toHaveBeenCalledWith(1, { - attributes: ['defaultAvailableAfter', 'defaultAvailableBefore', 'defaultSpecifyTimesPerDay', 'defaultWeeklyTimes'] + attributes: [ + "defaultAvailableAfter", + "defaultAvailableBefore", + "defaultSpecifyTimesPerDay", + "defaultWeeklyTimes", + ], }); }); - it('should handle database errors', async () => { - mockUserFindByPk.mockRejectedValue(new Error('Database error')); + it("should handle database errors", async () => { + mockUserFindByPk.mockRejectedValue(new Error("Database error")); - const response = await request(app) - .get('/users/availability'); + const response = await request(app).get("/users/availability"); expect(response.status).toBe(500); - expect(response.body).toEqual({ error: 'Database error' }); + expect(response.body).toEqual({ error: "Database error" }); }); }); - describe('PUT /availability', () => { - it('should update user availability settings', async () => { + describe("PUT /availability", () => { + it("should update user availability settings", async () => { const availabilityData = { - generalAvailableAfter: '08:00', - generalAvailableBefore: '18:00', + generalAvailableAfter: "08:00", + generalAvailableBefore: "18:00", specifyTimesPerDay: false, - weeklyTimes: { monday: '08:00-18:00' }, + weeklyTimes: { monday: "08:00-18:00" }, }; mockUserUpdate.mockResolvedValue([1]); const response = await request(app) - .put('/users/availability') + .put("/users/availability") .send(availabilityData); expect(response.status).toBe(200); - expect(response.body).toEqual({ message: 'Availability updated successfully' }); - expect(mockUserUpdate).toHaveBeenCalledWith({ - defaultAvailableAfter: '08:00', - defaultAvailableBefore: '18:00', - defaultSpecifyTimesPerDay: false, - defaultWeeklyTimes: { monday: '08:00-18:00' }, - }, { - where: { id: 1 } + expect(response.body).toEqual({ + message: "Availability updated successfully", }); + expect(mockUserUpdate).toHaveBeenCalledWith( + { + defaultAvailableAfter: "08:00", + defaultAvailableBefore: "18:00", + defaultSpecifyTimesPerDay: false, + defaultWeeklyTimes: { monday: "08:00-18:00" }, + }, + { + where: { id: 1 }, + } + ); }); - it('should handle database errors', async () => { - mockUserUpdate.mockRejectedValue(new Error('Database error')); + it("should handle database errors", async () => { + mockUserUpdate.mockRejectedValue(new Error("Database error")); - const response = await request(app) - .put('/users/availability') - .send({ - generalAvailableAfter: '08:00', - generalAvailableBefore: '18:00', - }); + const response = await request(app).put("/users/availability").send({ + generalAvailableAfter: "08:00", + generalAvailableBefore: "18:00", + }); expect(response.status).toBe(500); - expect(response.body).toEqual({ error: 'Database error' }); + expect(response.body).toEqual({ error: "Database error" }); }); }); - describe('GET /:id', () => { - it('should get public user profile by ID', async () => { + describe("GET /:id", () => { + it("should get public user profile by ID", async () => { const mockUser = { id: 2, - firstName: 'Jane', - lastName: 'Smith', - username: 'janesmith', - imageFilename: 'jane.jpg', + firstName: "Jane", + lastName: "Smith", + username: "janesmith", + imageFilename: "jane.jpg", }; mockUserFindByPk.mockResolvedValue(mockUser); - const response = await request(app) - .get('/users/2'); + const response = await request(app).get("/users/2"); expect(response.status).toBe(200); expect(response.body).toEqual(mockUser); - expect(mockUserFindByPk).toHaveBeenCalledWith('2', { - attributes: { exclude: ['password', 'email', 'phone', 'address'] } + expect(mockUserFindByPk).toHaveBeenCalledWith("2", { + attributes: { exclude: ["password", "email", "phone", "address"] }, }); }); - it('should return 404 for non-existent user', async () => { + it("should return 404 for non-existent user", async () => { mockUserFindByPk.mockResolvedValue(null); - const response = await request(app) - .get('/users/999'); + const response = await request(app).get("/users/999"); expect(response.status).toBe(404); - expect(response.body).toEqual({ error: 'User not found' }); + expect(response.body).toEqual({ error: "User not found" }); }); - it('should handle database errors', async () => { - mockUserFindByPk.mockRejectedValue(new Error('Database error')); + it("should handle database errors", async () => { + mockUserFindByPk.mockRejectedValue(new Error("Database error")); - const response = await request(app) - .get('/users/2'); + const response = await request(app).get("/users/2"); expect(response.status).toBe(500); - expect(response.body).toEqual({ error: 'Database error' }); + expect(response.body).toEqual({ error: "Database error" }); }); }); - describe('PUT /profile', () => { + describe("PUT /profile", () => { const mockUpdatedUser = { id: 1, - firstName: 'Updated', - lastName: 'User', - email: 'updated@example.com', - phone: '555-9999', + firstName: "Updated", + lastName: "User", + email: "updated@example.com", + phone: "555-9999", }; beforeEach(() => { mockUserFindByPk.mockResolvedValue(mockUpdatedUser); }); - it('should update user profile', async () => { + it("should update user profile", async () => { const profileData = { - firstName: 'Updated', - lastName: 'User', - email: 'updated@example.com', - phone: '555-9999', - address1: '123 New St', - city: 'New City', + firstName: "Updated", + lastName: "User", + email: "updated@example.com", + phone: "555-9999", + address1: "123 New St", + city: "New City", }; const response = await request(app) - .put('/users/profile') + .put("/users/profile") .send(profileData); expect(response.status).toBe(200); expect(response.body).toEqual(mockUpdatedUser); }); - it('should exclude empty email from update', async () => { + it("should exclude empty email from update", async () => { const profileData = { - firstName: 'Updated', - lastName: 'User', - email: '', - phone: '555-9999', + firstName: "Updated", + lastName: "User", + email: "", + phone: "555-9999", }; const response = await request(app) - .put('/users/profile') + .put("/users/profile") .send(profileData); expect(response.status).toBe(200); @@ -481,178 +458,51 @@ describe('Users Routes', () => { // (This would need to check the actual update call if we spy on req.user.update) }); - it('should handle validation errors', async () => { - const mockValidationError = new Error('Validation error'); + it("should handle validation errors", async () => { + const mockValidationError = new Error("Validation error"); mockValidationError.errors = [ - { path: 'email', message: 'Invalid email format' } + { path: "email", message: "Invalid email format" }, ]; // Mock req.user.update to throw validation error - const { authenticateToken } = require('../../../middleware/auth'); + const { authenticateToken } = require("../../../middleware/auth"); authenticateToken.mockImplementation((req, res, next) => { req.user = { id: 1, - update: jest.fn().mockRejectedValue(mockValidationError) + update: jest.fn().mockRejectedValue(mockValidationError), }; next(); }); - const response = await request(app) - .put('/users/profile') - .send({ - firstName: 'Test', - email: 'invalid-email', - }); + const response = await request(app).put("/users/profile").send({ + firstName: "Test", + email: "invalid-email", + }); expect(response.status).toBe(500); expect(response.body).toEqual({ - error: 'Validation error', - details: [{ field: 'email', message: 'Invalid email format' }] + error: "Validation error", + details: [{ field: "email", message: "Invalid email format" }], }); }); - it('should handle general database errors', async () => { + it("should handle general database errors", async () => { // Reset the authenticateToken mock to use default user - const { authenticateToken } = require('../../../middleware/auth'); + const { authenticateToken } = require("../../../middleware/auth"); authenticateToken.mockImplementation((req, res, next) => { req.user = { id: 1, - update: jest.fn().mockRejectedValue(new Error('Database error')) + update: jest.fn().mockRejectedValue(new Error("Database error")), }; next(); }); - const response = await request(app) - .put('/users/profile') - .send({ - firstName: 'Test', - }); + const response = await request(app).put("/users/profile").send({ + firstName: "Test", + }); expect(response.status).toBe(500); - expect(response.body).toEqual({ error: 'Database error' }); + expect(response.body).toEqual({ error: "Database error" }); }); }); - - describe('POST /profile/image', () => { - const mockUser = { - id: 1, - imageFilename: 'old-image.jpg', - update: jest.fn(), - }; - - beforeEach(() => { - mockUserFindByPk.mockResolvedValue(mockUser); - }); - - it('should upload profile image successfully', async () => { - mockUser.update.mockResolvedValue(); - fs.unlink.mockResolvedValue(); - - const response = await request(app) - .post('/users/profile/image'); - - expect(response.status).toBe(200); - expect(response.body).toEqual({ - message: 'Profile image uploaded successfully', - filename: 'test-profile.jpg', - imageUrl: '/uploads/profiles/test-profile.jpg' - }); - expect(fs.unlink).toHaveBeenCalled(); // Old image deleted - expect(mockUser.update).toHaveBeenCalledWith({ - imageFilename: 'test-profile.jpg' - }); - }); - - it('should handle upload errors', async () => { - uploadProfileImage.mockImplementation((req, res, callback) => { - callback(new Error('File too large')); - }); - - const response = await request(app) - .post('/users/profile/image'); - - expect(response.status).toBe(400); - expect(response.body).toEqual({ error: 'File too large' }); - }); - - it('should handle missing file', async () => { - uploadProfileImage.mockImplementation((req, res, callback) => { - req.file = null; - callback(null); - }); - - const response = await request(app) - .post('/users/profile/image'); - - expect(response.status).toBe(400); - expect(response.body).toEqual({ error: 'No file uploaded' }); - }); - - it('should handle database update errors', async () => { - // Mock upload to succeed but database update to fail - uploadProfileImage.mockImplementation((req, res, callback) => { - req.file = { filename: 'test-profile.jpg' }; - callback(null); - }); - - const userWithError = { - ...mockUser, - update: jest.fn().mockRejectedValue(new Error('Database error')) - }; - mockUserFindByPk.mockResolvedValue(userWithError); - - const response = await request(app) - .post('/users/profile/image'); - - expect(response.status).toBe(500); - expect(response.body).toEqual({ error: 'Failed to update profile image' }); - }); - - it('should handle case when user has no existing profile image', async () => { - // Mock upload to succeed - uploadProfileImage.mockImplementation((req, res, callback) => { - req.file = { filename: 'test-profile.jpg' }; - callback(null); - }); - - const userWithoutImage = { - id: 1, - imageFilename: null, - update: jest.fn().mockResolvedValue() - }; - mockUserFindByPk.mockResolvedValue(userWithoutImage); - - const response = await request(app) - .post('/users/profile/image'); - - expect(response.status).toBe(200); - expect(fs.unlink).not.toHaveBeenCalled(); // No old image to delete - }); - - it('should continue if old image deletion fails', async () => { - // Mock upload to succeed - uploadProfileImage.mockImplementation((req, res, callback) => { - req.file = { filename: 'test-profile.jpg' }; - callback(null); - }); - - const userWithImage = { - id: 1, - imageFilename: 'old-image.jpg', - update: jest.fn().mockResolvedValue() - }; - mockUserFindByPk.mockResolvedValue(userWithImage); - fs.unlink.mockRejectedValue(new Error('File not found')); - - const response = await request(app) - .post('/users/profile/image'); - - expect(response.status).toBe(200); - expect(response.body).toEqual({ - message: 'Profile image uploaded successfully', - filename: 'test-profile.jpg', - imageUrl: '/uploads/profiles/test-profile.jpg' - }); - }); - }); -}); \ No newline at end of file +}); diff --git a/backend/tests/unit/utils/s3KeyValidator.test.js b/backend/tests/unit/utils/s3KeyValidator.test.js new file mode 100644 index 0000000..46a334f --- /dev/null +++ b/backend/tests/unit/utils/s3KeyValidator.test.js @@ -0,0 +1,112 @@ +const { validateS3Keys } = require('../../../utils/s3KeyValidator'); + +describe('S3 Key Validator', () => { + describe('validateS3Keys', () => { + const validUuid1 = '550e8400-e29b-41d4-a716-446655440000'; + const validUuid2 = '660e8400-e29b-41d4-a716-446655440001'; + + test('should accept valid arrays of keys', () => { + const keys = [ + `items/${validUuid1}.jpg`, + `items/${validUuid2}.png`, + ]; + expect(validateS3Keys(keys, 'items')).toEqual({ valid: true }); + }); + + test('should accept valid keys for each folder', () => { + expect(validateS3Keys([`profiles/${validUuid1}.jpg`], 'profiles')).toEqual({ valid: true }); + expect(validateS3Keys([`items/${validUuid1}.png`], 'items')).toEqual({ valid: true }); + expect(validateS3Keys([`messages/${validUuid1}.gif`], 'messages')).toEqual({ valid: true }); + expect(validateS3Keys([`forum/${validUuid1}.webp`], 'forum')).toEqual({ valid: true }); + expect(validateS3Keys([`condition-checks/${validUuid1}.jpeg`], 'condition-checks')).toEqual({ valid: true }); + }); + + test('should accept different valid extensions', () => { + const extensions = ['jpg', 'jpeg', 'png', 'gif', 'webp']; + for (const ext of extensions) { + expect(validateS3Keys([`items/${validUuid1}.${ext}`], 'items')).toEqual({ valid: true }); + } + }); + + test('should be case-insensitive for extensions', () => { + expect(validateS3Keys([`items/${validUuid1}.JPG`], 'items')).toEqual({ valid: true }); + expect(validateS3Keys([`items/${validUuid1}.PNG`], 'items')).toEqual({ valid: true }); + }); + + test('should accept empty arrays', () => { + expect(validateS3Keys([], 'items')).toEqual({ valid: true }); + }); + + test('should reject non-array input', () => { + const result = validateS3Keys('not-an-array', 'items'); + expect(result.valid).toBe(false); + expect(result.error).toBe('Keys must be an array'); + }); + + test('should reject arrays exceeding maxKeys', () => { + const keys = Array(6).fill(`items/${validUuid1}.jpg`); + const result = validateS3Keys(keys, 'items', { maxKeys: 5 }); + expect(result.valid).toBe(false); + expect(result.error).toContain('Maximum 5 keys allowed'); + }); + + test('should reject duplicate keys', () => { + const keys = [ + `items/${validUuid1}.jpg`, + `items/${validUuid1}.jpg`, + ]; + const result = validateS3Keys(keys, 'items'); + expect(result.valid).toBe(false); + expect(result.error).toBe('Duplicate keys not allowed'); + }); + + test('should reject keys with wrong folder prefix', () => { + const result = validateS3Keys([`profiles/${validUuid1}.jpg`], 'items'); + expect(result.valid).toBe(false); + expect(result.invalidKeys).toHaveLength(1); + }); + + test('should reject invalid UUID format', () => { + const result = validateS3Keys(['items/invalid-uuid.jpg'], 'items'); + expect(result.valid).toBe(false); + }); + + test('should reject non-image extensions', () => { + const result = validateS3Keys([`items/${validUuid1}.exe`], 'items'); + expect(result.valid).toBe(false); + }); + + test('should reject path traversal attempts', () => { + const result = validateS3Keys([`../items/${validUuid1}.jpg`], 'items'); + expect(result.valid).toBe(false); + }); + + test('should reject empty or null keys in array', () => { + expect(validateS3Keys([''], 'items').valid).toBe(false); + expect(validateS3Keys([null], 'items').valid).toBe(false); + }); + + test('should reject invalid folder names', () => { + const result = validateS3Keys([`items/${validUuid1}.jpg`], 'invalid-folder'); + expect(result.valid).toBe(false); + }); + + test('should return invalid keys in error response', () => { + const keys = [ + `items/${validUuid1}.jpg`, + 'invalid-key.jpg', + `items/${validUuid2}.exe`, + ]; + const result = validateS3Keys(keys, 'items'); + expect(result.valid).toBe(false); + expect(result.invalidKeys).toHaveLength(2); + }); + + test('should use default maxKeys of 20', () => { + const keys = Array(20).fill(0).map((_, i) => + `items/550e8400-e29b-41d4-a716-44665544${String(i).padStart(4, '0')}.jpg` + ); + expect(validateS3Keys(keys, 'items')).toEqual({ valid: true }); + }); + }); +}); diff --git a/backend/utils/s3KeyValidator.js b/backend/utils/s3KeyValidator.js new file mode 100644 index 0000000..dc2f719 --- /dev/null +++ b/backend/utils/s3KeyValidator.js @@ -0,0 +1,102 @@ +/** + * S3 Key Validation Utility + * Validates that user-supplied S3 keys match expected formats + * to prevent IDOR and arbitrary data injection attacks + */ + +// UUID v4 regex pattern +const UUID_PATTERN = + "[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}"; + +// Allowed image extensions +const ALLOWED_EXTENSIONS = ["jpg", "jpeg", "png", "gif", "webp"]; + +// Valid S3 folders +const VALID_FOLDERS = [ + "profiles", + "items", + "messages", + "forum", + "condition-checks", +]; + +/** + * Build regex pattern for a specific folder + * @param {string} folder - The S3 folder name + * @returns {RegExp} + */ +function buildKeyPattern(folder) { + const extPattern = ALLOWED_EXTENSIONS.join("|"); + return new RegExp(`^${folder}/${UUID_PATTERN}\\.(${extPattern})$`, "i"); +} + +/** + * Validate a single S3 key + * @param {string} key - The S3 key to validate + * @param {string} folder - Expected folder (profiles, items, messages, forum, condition-checks) + * @returns {{ valid: boolean, error?: string }} + */ +function validateS3Key(key, folder) { + if (!key || typeof key !== "string") { + return { valid: false, error: "Key must be a non-empty string" }; + } + + if (!VALID_FOLDERS.includes(folder)) { + return { valid: false, error: `Invalid folder` }; + } + + const pattern = buildKeyPattern(folder); + if (!pattern.test(key)) { + return { + valid: false, + error: `Invalid key format`, + }; + } + + return { valid: true }; +} + +/** + * Validate an array of S3 keys + * @param {Array} keys - Array of S3 keys to validate + * @param {string} folder - Expected folder (profiles, items, messages, forum, condition-checks) + * @param {Object} options - Validation options + * @param {number} options.maxKeys - Maximum number of keys allowed + * @returns {{ valid: boolean, error?: string, invalidKeys?: Array }} + */ +function validateS3Keys(keys, folder, options = {}) { + const { maxKeys = 20 } = options; + + if (!Array.isArray(keys)) { + return { valid: false, error: "Keys must be an array" }; + } + + if (keys.length > maxKeys) { + return { valid: false, error: `Maximum ${maxKeys} keys allowed` }; + } + + if (keys.length === 0) { + return { valid: true }; + } + + const uniqueKeys = new Set(keys); + if (uniqueKeys.size !== keys.length) { + return { valid: false, error: "Duplicate keys not allowed" }; + } + + const invalidKeys = []; + for (const key of keys) { + const result = validateS3Key(key, folder); + if (!result.valid) { + invalidKeys.push({ key, error: result.error }); + } + } + + if (invalidKeys.length > 0) { + return { valid: false, error: "Invalid S3 key format", invalidKeys }; + } + + return { valid: true }; +} + +module.exports = { validateS3Keys }; diff --git a/frontend/src/components/CommentForm.tsx b/frontend/src/components/CommentForm.tsx index 46aaa9b..176a33b 100644 --- a/frontend/src/components/CommentForm.tsx +++ b/frontend/src/components/CommentForm.tsx @@ -87,7 +87,6 @@ const CommentForm: React.FC = ({ imagePreviews={imagePreviews} onImageChange={handleImageChange} onRemoveImage={handleRemoveImage} - maxImages={3} compact={true} /> )} diff --git a/frontend/src/components/ConditionCheckModal.tsx b/frontend/src/components/ConditionCheckModal.tsx index 6279a72..91a1cc8 100644 --- a/frontend/src/components/ConditionCheckModal.tsx +++ b/frontend/src/components/ConditionCheckModal.tsx @@ -1,5 +1,6 @@ import React, { useState } from "react"; import { conditionCheckAPI } from "../services/api"; +import { IMAGE_LIMITS } from "../config/imageLimits"; interface ConditionCheckModalProps { show: boolean; @@ -59,8 +60,8 @@ const ConditionCheckModal: React.FC = ({ const handleFileChange = (e: React.ChangeEvent) => { if (e.target.files) { const selectedFiles = Array.from(e.target.files); - if (selectedFiles.length + photos.length > 20) { - setError("Maximum 20 photos allowed"); + if (selectedFiles.length + photos.length > IMAGE_LIMITS.conditionChecks) { + setError(`Maximum ${IMAGE_LIMITS.conditionChecks} photos allowed`); return; } setPhotos((prev) => [...prev, ...selectedFiles]); @@ -153,7 +154,7 @@ const ConditionCheckModal: React.FC = ({
) => void; onRemoveImage: (index: number) => void; - maxImages?: number; compact?: boolean; } @@ -14,9 +14,10 @@ const ForumImageUpload: React.FC = ({ imagePreviews, onImageChange, onRemoveImage, - maxImages = 5, compact = false }) => { + const maxImages = IMAGE_LIMITS.forum; + return (