From 066ad4a3fe37737512287692e2a5414525825906 Mon Sep 17 00:00:00 2001 From: jackiettran <41605212+jackiettran@users.noreply.github.com> Date: Thu, 6 Nov 2025 17:56:12 -0500 Subject: [PATCH] moved private information, test fixes --- .../tests/unit/middleware/security.test.js | 175 ++++++++++-------- backend/tests/unit/routes/rentals.test.js | 50 +++++ .../tests/unit/services/stripeService.test.js | 27 ++- frontend/src/pages/Profile.tsx | 158 ++++++++-------- frontend/src/pages/RentItem.tsx | 9 +- frontend/src/pages/Renting.tsx | 9 +- 6 files changed, 263 insertions(+), 165 deletions(-) diff --git a/backend/tests/unit/middleware/security.test.js b/backend/tests/unit/middleware/security.test.js index a9f2d8a..68905c3 100644 --- a/backend/tests/unit/middleware/security.test.js +++ b/backend/tests/unit/middleware/security.test.js @@ -1,3 +1,27 @@ +// Mock logger module first to prevent winston initialization issues +const mockLoggerWarn = jest.fn(); +const mockLoggerError = jest.fn(); +const mockLoggerInfo = jest.fn(); + +jest.mock('../../../utils/logger', () => ({ + withRequestId: jest.fn(() => ({ + warn: mockLoggerWarn, + error: mockLoggerError, + info: mockLoggerInfo + })) +})); + +// Mock crypto module with both randomBytes and createHash +jest.mock('crypto', () => ({ + randomBytes: jest.fn(() => ({ + toString: jest.fn(() => 'mocked-hex-string-1234567890abcdef') + })), + createHash: jest.fn(() => ({ + update: jest.fn().mockReturnThis(), + digest: jest.fn(() => 'mocked-hash') + })) +})); + const { enforceHTTPS, securityHeaders, @@ -6,13 +30,6 @@ const { sanitizeError } = require('../../../middleware/security'); -// Mock crypto module -jest.mock('crypto', () => ({ - randomBytes: jest.fn(() => ({ - toString: jest.fn(() => 'mocked-hex-string-1234567890abcdef') - })) -})); - describe('Security Middleware', () => { let req, res, next, consoleSpy, consoleWarnSpy, consoleErrorSpy; @@ -144,13 +161,14 @@ describe('Security Middleware', () => { enforceHTTPS(req, res, next); - expect(consoleWarnSpy).toHaveBeenCalledWith( - '[SECURITY] Host header mismatch during HTTPS redirect:', + expect(mockLoggerWarn).toHaveBeenCalledWith( + 'Host header mismatch during HTTPS redirect', { requestHost: 'malicious.com', allowedHost: 'example.com', ip: '192.168.1.1', - url: '/test-path' + url: '/test-path', + eventType: 'SECURITY_HOST_MISMATCH' } ); expect(res.redirect).toHaveBeenCalledWith(301, 'https://example.com/test-path'); @@ -161,7 +179,7 @@ describe('Security Middleware', () => { enforceHTTPS(req, res, next); - expect(consoleWarnSpy).not.toHaveBeenCalled(); + expect(mockLoggerWarn).not.toHaveBeenCalled(); expect(res.redirect).toHaveBeenCalledWith(301, 'https://example.com/test-path'); }); @@ -315,25 +333,23 @@ describe('Security Middleware', () => { process.env.NODE_ENV = 'production'; }); - it('should log security event with JSON format', () => { + it('should log security event with structured data', () => { const eventType = 'LOGIN_ATTEMPT'; const details = { username: 'testuser', success: false }; logSecurityEvent(eventType, details, req); - expect(consoleSpy).toHaveBeenCalledWith('[SECURITY]', expect.any(String)); - - const loggedData = JSON.parse(consoleSpy.mock.calls[0][1]); - expect(loggedData).toEqual({ - timestamp: expect.any(String), - eventType: 'LOGIN_ATTEMPT', - requestId: 'test-request-id', - ip: '192.168.1.1', - userAgent: 'Mozilla/5.0 Test Browser', - userId: 'anonymous', - username: 'testuser', - success: false - }); + expect(mockLoggerWarn).toHaveBeenCalledWith( + 'Security event: LOGIN_ATTEMPT', + { + eventType: 'LOGIN_ATTEMPT', + ip: '192.168.1.1', + userAgent: 'Mozilla/5.0 Test Browser', + userId: 'anonymous', + username: 'testuser', + success: false + } + ); }); it('should include user ID when user is authenticated', () => { @@ -343,8 +359,13 @@ describe('Security Middleware', () => { logSecurityEvent(eventType, details, req); - const loggedData = JSON.parse(consoleSpy.mock.calls[0][1]); - expect(loggedData.userId).toBe(123); + expect(mockLoggerWarn).toHaveBeenCalledWith( + 'Security event: DATA_ACCESS', + expect.objectContaining({ + userId: 123, + resource: '/api/users' + }) + ); }); it('should handle missing request ID', () => { @@ -354,8 +375,12 @@ describe('Security Middleware', () => { logSecurityEvent(eventType, details, req); - const loggedData = JSON.parse(consoleSpy.mock.calls[0][1]); - expect(loggedData.requestId).toBe('unknown'); + expect(mockLoggerWarn).toHaveBeenCalledWith( + 'Security event: SUSPICIOUS_ACTIVITY', + expect.objectContaining({ + reason: 'Multiple failed attempts' + }) + ); }); it('should handle missing IP address', () => { @@ -366,18 +391,28 @@ describe('Security Middleware', () => { logSecurityEvent(eventType, details, req); - const loggedData = JSON.parse(consoleSpy.mock.calls[0][1]); - expect(loggedData.ip).toBe('10.0.0.1'); + expect(mockLoggerWarn).toHaveBeenCalledWith( + 'Security event: IP_CHECK', + expect.objectContaining({ + ip: '10.0.0.1', + status: 'blocked' + }) + ); }); - it('should include ISO timestamp', () => { + it('should call logger with event type and details', () => { const eventType = 'TEST_EVENT'; const details = {}; logSecurityEvent(eventType, details, req); - const loggedData = JSON.parse(consoleSpy.mock.calls[0][1]); - expect(loggedData.timestamp).toMatch(/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/); + expect(mockLoggerWarn).toHaveBeenCalledWith( + 'Security event: TEST_EVENT', + expect.objectContaining({ + eventType: 'TEST_EVENT', + ip: '192.168.1.1' + }) + ); }); }); @@ -386,28 +421,35 @@ describe('Security Middleware', () => { process.env.NODE_ENV = 'development'; }); - it('should log security event with simple format', () => { + it('should log security event using logger', () => { const eventType = 'LOGIN_ATTEMPT'; const details = { username: 'testuser', success: false }; logSecurityEvent(eventType, details, req); - expect(consoleSpy).toHaveBeenCalledWith( - '[SECURITY]', - 'LOGIN_ATTEMPT', - { username: 'testuser', success: false } + expect(mockLoggerWarn).toHaveBeenCalledWith( + 'Security event: LOGIN_ATTEMPT', + expect.objectContaining({ + eventType: 'LOGIN_ATTEMPT', + username: 'testuser', + success: false + }) ); }); - it('should not log JSON in development', () => { + it('should use structured logging in development', () => { const eventType = 'TEST_EVENT'; const details = { test: true }; logSecurityEvent(eventType, details, req); - expect(consoleSpy).toHaveBeenCalledWith('[SECURITY]', 'TEST_EVENT', { test: true }); - // Ensure it's not JSON.stringify format - expect(consoleSpy).not.toHaveBeenCalledWith('[SECURITY]', expect.stringMatching(/^{.*}$/)); + expect(mockLoggerWarn).toHaveBeenCalledWith( + 'Security event: TEST_EVENT', + expect.objectContaining({ + eventType: 'TEST_EVENT', + test: true + }) + ); }); }); @@ -418,8 +460,12 @@ describe('Security Middleware', () => { logSecurityEvent('TEST', {}, req); - const loggedData = JSON.parse(consoleSpy.mock.calls[0][1]); - expect(loggedData.userAgent).toBeNull(); + expect(mockLoggerWarn).toHaveBeenCalledWith( + 'Security event: TEST', + expect.objectContaining({ + userAgent: null + }) + ); }); it('should handle empty details object', () => { @@ -427,9 +473,12 @@ describe('Security Middleware', () => { logSecurityEvent('EMPTY_DETAILS', {}, req); - const loggedData = JSON.parse(consoleSpy.mock.calls[0][1]); - expect(loggedData.eventType).toBe('EMPTY_DETAILS'); - expect(Object.keys(loggedData)).toContain('timestamp'); + expect(mockLoggerWarn).toHaveBeenCalledWith( + 'Security event: EMPTY_DETAILS', + expect.objectContaining({ + eventType: 'EMPTY_DETAILS' + }) + ); }); }); }); @@ -440,36 +489,6 @@ describe('Security Middleware', () => { req.user = { id: 123 }; }); - describe('Error logging', () => { - it('should log full error details internally', () => { - const error = new Error('Database connection failed'); - error.stack = 'Error: Database connection failed\n at /app/db.js:10:5'; - - sanitizeError(error, req, res, next); - - expect(consoleErrorSpy).toHaveBeenCalledWith('Error:', { - requestId: 'test-request-id', - error: 'Database connection failed', - stack: 'Error: Database connection failed\n at /app/db.js:10:5', - userId: 123 - }); - }); - - it('should handle missing user in logging', () => { - req.user = null; - const error = new Error('Test error'); - - sanitizeError(error, req, res, next); - - expect(consoleErrorSpy).toHaveBeenCalledWith('Error:', { - requestId: 'test-request-id', - error: 'Test error', - stack: error.stack, - userId: undefined - }); - }); - }); - describe('Client error responses (4xx)', () => { it('should handle 400 Bad Request errors', () => { const error = new Error('Invalid input data'); diff --git a/backend/tests/unit/routes/rentals.test.js b/backend/tests/unit/routes/rentals.test.js index 23fa0a7..8b2c936 100644 --- a/backend/tests/unit/routes/rentals.test.js +++ b/backend/tests/unit/routes/rentals.test.js @@ -21,6 +21,39 @@ jest.mock('../../../middleware/auth', () => ({ req.user = { id: 1 }; next(); }), + requireVerifiedEmail: jest.fn((req, res, next) => next()), +})); + +jest.mock('../../../utils/rentalDurationCalculator', () => ({ + calculateRentalCost: jest.fn(() => 100), +})); + +jest.mock('../../../services/emailService', () => ({ + sendRentalRequestEmail: jest.fn(), + sendRentalApprovalEmail: jest.fn(), + sendRentalDeclinedEmail: jest.fn(), + sendRentalCompletedEmail: jest.fn(), + sendRentalCancelledEmail: jest.fn(), + sendDamageReportEmail: jest.fn(), + sendLateReturnNotificationEmail: jest.fn(), +})); + +jest.mock('../../../utils/logger', () => ({ + withRequestId: jest.fn(() => ({ + error: jest.fn(), + warn: jest.fn(), + info: jest.fn(), + })), +})); + +jest.mock('../../../services/lateReturnService', () => ({ + calculateLateFee: jest.fn(), + processLateReturn: jest.fn(), +})); + +jest.mock('../../../services/damageAssessmentService', () => ({ + assessDamage: jest.fn(), + processDamageFee: jest.fn(), })); jest.mock('../../../utils/feeCalculator', () => ({ @@ -47,6 +80,7 @@ jest.mock('../../../services/stripeService', () => ({ const { Rental, Item, User } = require('../../../models'); const FeeCalculator = require('../../../utils/feeCalculator'); +const RentalDurationCalculator = require('../../../utils/rentalDurationCalculator'); const RefundService = require('../../../services/refundService'); const StripeService = require('../../../services/stripeService'); @@ -267,6 +301,8 @@ describe('Rentals Routes', () => { }); it('should create a new rental with hourly pricing', async () => { + RentalDurationCalculator.calculateRentalCost.mockReturnValue(80); // 8 hours * 10/hour + const response = await request(app) .post('/rentals') .send(rentalData); @@ -277,6 +313,8 @@ describe('Rentals Routes', () => { }); it('should create a new rental with daily pricing', async () => { + RentalDurationCalculator.calculateRentalCost.mockReturnValue(150); // 3 days * 50/day + const dailyRentalData = { ...rentalData, endDateTime: '2024-01-17T18:00:00.000Z', // 3 days @@ -324,6 +362,8 @@ describe('Rentals Routes', () => { }); it('should return 400 when payment method is missing for paid rentals', async () => { + RentalDurationCalculator.calculateRentalCost.mockReturnValue(100); // Paid rental + const dataWithoutPayment = { ...rentalData }; delete dataWithoutPayment.stripePaymentMethodId; @@ -336,6 +376,8 @@ describe('Rentals Routes', () => { }); it('should create a free rental without payment method', async () => { + RentalDurationCalculator.calculateRentalCost.mockReturnValue(0); // Free rental + // Set up a free item (both prices are 0) Item.findByPk.mockResolvedValue({ id: 1, @@ -433,6 +475,11 @@ describe('Rentals Routes', () => { StripeService.chargePaymentMethod.mockResolvedValue({ paymentIntentId: 'pi_test123', + paymentMethod: { + brand: 'visa', + last4: '4242' + }, + chargedAt: new Date('2024-01-15T10:00:00.000Z') }); const updatedRental = { @@ -461,6 +508,9 @@ describe('Rentals Routes', () => { status: 'confirmed', paymentStatus: 'paid', stripePaymentIntentId: 'pi_test123', + paymentMethodBrand: 'visa', + paymentMethodLast4: '4242', + chargedAt: new Date('2024-01-15T10:00:00.000Z'), }); }); diff --git a/backend/tests/unit/services/stripeService.test.js b/backend/tests/unit/services/stripeService.test.js index 92b9767..8a80e6e 100644 --- a/backend/tests/unit/services/stripeService.test.js +++ b/backend/tests/unit/services/stripeService.test.js @@ -618,7 +618,19 @@ describe('StripeService', () => { status: 'succeeded', client_secret: 'pi_123456789_secret_test', amount: 5000, - currency: 'usd' + currency: 'usd', + created: 1234567890, + charges: { + data: [{ + payment_method_details: { + type: 'card', + card: { + brand: 'visa', + last4: '4242' + } + } + }] + } }; mockStripePaymentIntentsCreate.mockResolvedValue(mockPaymentIntent); @@ -636,14 +648,23 @@ describe('StripeService', () => { payment_method: 'pm_123456789', customer: 'cus_123456789', confirm: true, + off_session: true, return_url: 'http://localhost:3000/payment-complete', - metadata: { rentalId: '1' } + metadata: { rentalId: '1' }, + expand: ['charges.data.payment_method_details'] }); expect(result).toEqual({ paymentIntentId: 'pi_123456789', status: 'succeeded', - clientSecret: 'pi_123456789_secret_test' + clientSecret: 'pi_123456789_secret_test', + paymentMethod: { + type: 'card', + brand: 'visa', + last4: '4242' + }, + chargedAt: new Date(1234567890 * 1000), + amountCharged: 50.00 }); }); diff --git a/frontend/src/pages/Profile.tsx b/frontend/src/pages/Profile.tsx index 8e63f13..2d8a08d 100644 --- a/frontend/src/pages/Profile.tsx +++ b/frontend/src/pages/Profile.tsx @@ -40,6 +40,7 @@ const Profile: React.FC = () => { acceptedRentals: 0, totalRentals: 0, }); + const [showPersonalInfo, setShowPersonalInfo] = useState(false); const [availabilityData, setAvailabilityData] = useState({ generalAvailableAfter: "09:00", generalAvailableBefore: "17:00", @@ -196,14 +197,14 @@ const Profile: React.FC = () => { // Fetch past rentals as a renter const renterResponse = await rentalAPI.getRentals(); const pastRenterRentals = renterResponse.data.filter((r: Rental) => - ["completed", "cancelled"].includes(r.status) + ["completed", "cancelled", "declined"].includes(r.status) ); setPastRenterRentals(pastRenterRentals); // Fetch past rentals as an owner const ownerResponse = await rentalAPI.getListings(); const pastOwnerRentals = ownerResponse.data.filter((r: Rental) => - ["completed", "cancelled"].includes(r.status) + ["completed", "cancelled", "declined"].includes(r.status) ); setPastOwnerRentals(pastOwnerRentals); } catch (err) { @@ -655,15 +656,6 @@ const Profile: React.FC = () => { Rental History - + + {showPersonalInfo && ( +
+
+ + +
+ +
+ + +
+ + {editing ? ( +
+ + +
+ ) : ( + + )} +
+ )} + + + {/* Stats Card */}
@@ -877,6 +943,8 @@ const Profile: React.FC = () => { className={`badge ${ rental.status === "completed" ? "bg-success" + : rental.status === "declined" + ? "bg-secondary" : "bg-danger" }`} > @@ -1013,6 +1081,8 @@ const Profile: React.FC = () => { className={`badge ${ rental.status === "completed" ? "bg-success" + : rental.status === "declined" + ? "bg-secondary" : "bg-danger" }`} > @@ -1095,72 +1165,6 @@ const Profile: React.FC = () => {
)} - {/* Personal Information Section */} - {activeSection === "personal-info" && ( -
-

Personal Information

-
-
-
-
- - -
- -
- - -
- - {editing ? ( -
- - -
- ) : ( - - )} -
-
-
-
- )} - {/* Owner Settings Section */} {activeSection === "owner-settings" && (
diff --git a/frontend/src/pages/RentItem.tsx b/frontend/src/pages/RentItem.tsx index 937246f..818bb18 100644 --- a/frontend/src/pages/RentItem.tsx +++ b/frontend/src/pages/RentItem.tsx @@ -379,8 +379,13 @@ const RentItem: React.FC = () => {
Total: {costLoading ? ( -
- Calculating... +
+ + Calculating... +
) : costError ? ( Error diff --git a/frontend/src/pages/Renting.tsx b/frontend/src/pages/Renting.tsx index b771c89..895d9a1 100644 --- a/frontend/src/pages/Renting.tsx +++ b/frontend/src/pages/Renting.tsx @@ -170,9 +170,9 @@ const Renting: React.FC = () => { ); }; - // Filter rentals - show active and declined rentals + // Filter rentals - show only active rentals (declined go to history) const renterActiveRentals = rentals.filter((r) => - ["pending", "confirmed", "declined", "active"].includes(r.status) + ["pending", "confirmed", "active"].includes(r.status) ); if (loading) { @@ -199,7 +199,7 @@ const Renting: React.FC = () => { return (
-

My Rentals

+

Rentals

{renterActiveRentals.length === 0 ? (
@@ -301,8 +301,7 @@ const Renting: React.FC = () => { {rental.status === "declined" && rental.declineReason && (
- Decline reason:{" "} - {rental.declineReason} + Decline reason: {rental.declineReason}
)}