Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ on:
branches: [main, master, develop]
pull_request:
branches: [main, master, develop]
workflow_dispatch:

env:
NODE_VERSION: '20'
Expand Down
12 changes: 12 additions & 0 deletions apps/web/src/app/api/auth/__tests__/login-redirect.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,18 @@ vi.mock('@pagespace/lib/server', () => ({
logSecurityEvent: vi.fn(),
}));

vi.mock('@pagespace/lib/audit', () => ({
securityAudit: {
logEvent: vi.fn().mockResolvedValue(undefined),
},
maskEmail: vi.fn((email: string) => {
const [local, domain] = email.split('@');
if (!local || !domain) return '***@***';
const visibleChars = Math.min(2, local.length);
return `${local.slice(0, visibleChars)}***@${domain}`;
}),
}));

vi.mock('@pagespace/lib/security', () => ({
checkDistributedRateLimit: vi.fn(),
resetDistributedRateLimit: vi.fn(),
Expand Down
6 changes: 6 additions & 0 deletions apps/web/src/app/api/auth/__tests__/login.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,12 @@ vi.mock('@pagespace/lib/server', () => ({
},
logAuthEvent: vi.fn(),
logSecurityEvent: vi.fn(),
securityAudit: {
logAuthSuccess: vi.fn().mockResolvedValue(undefined),
logAuthFailure: vi.fn().mockResolvedValue(undefined),
logTokenCreated: vi.fn().mockResolvedValue(undefined),
logAccessDenied: vi.fn().mockResolvedValue(undefined),
},
}));

// Mock distributed rate limiting (P1-T5)
Expand Down
4 changes: 4 additions & 0 deletions apps/web/src/app/api/auth/__tests__/logout.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ vi.mock('@pagespace/lib/server', () => ({
},
},
logAuthEvent: vi.fn(),
securityAudit: {
logLogout: vi.fn().mockResolvedValue(undefined),
logTokenRevoked: vi.fn().mockResolvedValue(undefined),
},
}));

vi.mock('@pagespace/lib/activity-tracker', () => ({
Expand Down
8 changes: 8 additions & 0 deletions apps/web/src/app/api/auth/login/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ export async function POST(req: Request) {
const reason = !user ? 'invalid_email' : 'invalid_password';
logAuthEvent('failed', user?.id, email, clientIP, reason === 'invalid_email' ? 'Invalid email' : 'Invalid password');
trackAuthEvent(user?.id, 'failed_login', { reason, email, ip: clientIP });
securityAudit.logAuthFailure(email, clientIP, reason).catch(() => {});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Find the security audit service implementation
find . -type f -name "*security*audit*" -o -name "*audit*" | grep -E "\.(ts|tsx)$" | head -20

Repository: 2witstudios/PageSpace

Length of output: 591


🏁 Script executed:

# Search for logAuthFailure import and implementation
rg -n "logAuthFailure" --type ts -A 2

Repository: 2witstudios/PageSpace

Length of output: 2312


🏁 Script executed:

# Check the `@pagespace/lib/server` imports to find where securityAudit comes from
rg -n "from.*@pagespace/lib/server" --type ts

Repository: 2witstudios/PageSpace

Length of output: 45704


🏁 Script executed:

# Read the logAuthFailure implementation from security-audit.ts
sed -n '223,260p' packages/lib/src/audit/security-audit.ts

Repository: 2witstudios/PageSpace

Length of output: 807


🏁 Script executed:

# Search for email masking logic in the security audit service
rg -n "mask.*email|email.*mask|slice.*replace" packages/lib/src/audit/security-audit.ts

Repository: 2witstudios/PageSpace

Length of output: 47


🏁 Script executed:

# Check the full logAuthFailure method and surrounding context
sed -n '220,280p' packages/lib/src/audit/security-audit.ts

Repository: 2witstudios/PageSpace

Length of output: 1262


🏁 Script executed:

# Find the logEvent implementation
sed -n '100,200p' packages/lib/src/audit/security-audit.ts | head -60

Repository: 2witstudios/PageSpace

Length of output: 1969


🏁 Script executed:

# Search for any email masking logic in the entire security-audit.ts file
rg -n "slice|mask|replace|PII" packages/lib/src/audit/security-audit.ts | head -20

Repository: 2witstudios/PageSpace

Length of output: 47


🏁 Script executed:

# Check how other routes handle email masking before calling securityAudit methods
rg -B3 "securityAudit.logAuth" apps/web/src/app/api/auth/ --type ts | head -40

Repository: 2witstudios/PageSpace

Length of output: 853


🏁 Script executed:

# Check logAuthSuccess implementation and how email is handled there
sed -n '200,250p' packages/lib/src/audit/security-audit.ts

Repository: 2witstudios/PageSpace

Length of output: 1119


🏁 Script executed:

# Check if there's any email masking done in the login route before the calls
sed -n '140,160p' apps/web/src/app/api/auth/login/route.ts

Repository: 2witstudios/PageSpace

Length of output: 1049


🏁 Script executed:

# Compare with other auth routes to see the masking pattern
rg -B5 "securityAudit\.|trackAuthEvent" apps/web/src/app/api/auth/signup-passkey/route.ts | head -30

Repository: 2witstudios/PageSpace

Length of output: 810


Mask email before passing to securityAudit.logAuthFailure to comply with PII retention policies.

The email is passed directly to logAuthFailure without masking. Unlike the signup-passkey route which masks PII before logging (using the pattern email.substring(0, 3) + '***@' + domain), the login route passes the full email unmasked. This is inconsistent and fails to comply with data retention policies.

Apply the same masking pattern used in signup-passkey/route.ts before the call:

const maskedEmail = email.substring(0, 3) + '***@' + (email.split('@')[1] || '***');
securityAudit.logAuthFailure(maskedEmail, clientIP, reason).catch(() => {});

Note: Also review logAuthEvent and trackAuthEvent calls on the same line block, as they also receive the unmasked email.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/src/app/api/auth/login/route.ts` at line 152, The login route is
sending raw PII to audit/tracking calls; before calling
securityAudit.logAuthFailure, logAuthEvent, and trackAuthEvent replace email
with a masked version using the same pattern as signup-passkey (e.g., take
email.substring(0,3) + '***@' + (email.split('@')[1] || '***') into a
maskedEmail variable) and pass maskedEmail instead of email to
securityAudit.logAuthFailure, logAuthEvent and trackAuthEvent to comply with PII
retention policies.


// Record failed attempt for lockout tracking (only for existing users to avoid info leakage)
if (user) {
Expand Down Expand Up @@ -217,6 +218,13 @@ export async function POST(req: Request) {
ip: clientIP,
userAgent: req.headers.get('user-agent')
});
securityAudit.logAuthSuccess(
user.id,
sessionClaims.sessionId,
clientIP,
req.headers.get('user-agent') || 'unknown'
).catch(() => {});
securityAudit.logTokenCreated(user.id, 'session', clientIP).catch(() => {});

const headers = new Headers();
appendSessionCookie(headers, sessionToken);
Expand Down
4 changes: 3 additions & 1 deletion apps/web/src/app/api/auth/logout/route.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { sessionService } from '@pagespace/lib/auth';
import { loggers, logAuthEvent } from '@pagespace/lib/server';
import { loggers, logAuthEvent, securityAudit } from '@pagespace/lib/server';
import { trackAuthEvent } from '@pagespace/lib/activity-tracker';
import { getClientIP } from '@/lib/auth';
import { getSessionFromCookies, appendClearCookies } from '@/lib/auth/cookie-config';
Expand Down Expand Up @@ -38,6 +38,8 @@ export async function POST(req: Request) {
ip: clientIP,
userAgent: req.headers.get('user-agent')
});
securityAudit.logLogout(userId, sessionClaims?.sessionId ?? 'unknown', clientIP).catch(() => {});
securityAudit.logTokenRevoked(userId, 'session', 'user_logout').catch(() => {});
}

const headers = new Headers();
Expand Down
3 changes: 3 additions & 0 deletions apps/web/src/lib/auth/__tests__/auth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ vi.mock('../csrf-validation', () => ({
// Mock security event logging
vi.mock('@pagespace/lib/server', () => ({
logSecurityEvent: vi.fn(),
securityAudit: {
logAccessDenied: vi.fn().mockResolvedValue(undefined),
},
}));

import { authenticateSessionRequest } from '../index';
Expand Down
10 changes: 8 additions & 2 deletions apps/web/src/lib/auth/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ import { NextResponse } from 'next/server';
import { authenticateSessionRequest, isAuthError } from './index';
import { validateAdminAccess, type AdminValidationResult } from './admin-role';
import { validateCSRF } from './csrf-validation';
import { logSecurityEvent } from '@pagespace/lib/server';
import { securityAudit } from '@pagespace/lib/audit/security-audit';
import { logSecurityEvent, securityAudit } from '@pagespace/lib/server';

export interface VerifiedUser {
id: string;
Expand Down Expand Up @@ -131,6 +130,7 @@ export async function verifyAdminAuth(request: Request): Promise<VerifiedUser |
authType: 'session',
action: 'deny_access',
});
securityAudit.logAccessDenied(user.id, 'admin_route', method, 'csrf_validation_failed').catch(() => {});
// Return the CSRF error response directly to preserve error codes
return csrfError;
}
Expand All @@ -152,6 +152,12 @@ export async function verifyAdminAuth(request: Request): Promise<VerifiedUser |
authType: 'session',
action: 'deny_access',
});
securityAudit.logAccessDenied(
user.id,
'admin_route',
'admin_access',
validationResult.reason ?? 'admin_role_version_validation_failed'
).catch(() => {});
return NextResponse.json(
{ error: 'Forbidden: Admin access required' },
{ status: 403 }
Expand Down
160 changes: 160 additions & 0 deletions packages/lib/src/audit/__tests__/security-audit-adapter.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
import { describe, it, expect, vi, beforeEach } from 'vitest';

vi.mock('../security-audit', () => ({
securityAudit: {
logEvent: vi.fn().mockResolvedValue(undefined),
logAuthSuccess: vi.fn().mockResolvedValue(undefined),
logAuthFailure: vi.fn().mockResolvedValue(undefined),
logAccessDenied: vi.fn().mockResolvedValue(undefined),
logTokenCreated: vi.fn().mockResolvedValue(undefined),
logTokenRevoked: vi.fn().mockResolvedValue(undefined),
},
}));

vi.mock('@pagespace/db', () => ({
securityAuditLog: {},
}));

import { auditAuthEvent, auditSecurityEvent } from '../security-audit-adapter';
import { securityAudit } from '../security-audit';

describe('Security Audit Adapter', () => {
beforeEach(() => {
vi.clearAllMocks();
});

describe('auditAuthEvent', () => {
it('dispatches login success to securityAudit.logEvent', () => {
auditAuthEvent('login', 'user-1', 'test@example.com', '1.2.3.4');

expect(securityAudit.logEvent).toHaveBeenCalledWith(
expect.objectContaining({
eventType: 'auth.login.success',
userId: 'user-1',
ipAddress: '1.2.3.4',
})
);
});

it('dispatches login failure with risk score', () => {
auditAuthEvent('failed', undefined, 'bad@example.com', '1.2.3.4', 'invalid_password');

expect(securityAudit.logEvent).toHaveBeenCalledWith(
expect.objectContaining({
eventType: 'auth.login.failure',
riskScore: 0.3,
details: expect.objectContaining({
reason: 'invalid_password',
}),
})
);
});

it('dispatches logout event', () => {
auditAuthEvent('logout', 'user-1', undefined, '1.2.3.4');

expect(securityAudit.logEvent).toHaveBeenCalledWith(
expect.objectContaining({
eventType: 'auth.logout',
userId: 'user-1',
})
);
});

it('dispatches token refresh event', () => {
auditAuthEvent('refresh', 'user-1', undefined, '1.2.3.4');

expect(securityAudit.logEvent).toHaveBeenCalledWith(
expect.objectContaining({
eventType: 'auth.token.created',
userId: 'user-1',
})
);
});

it('masks email in audit details', () => {
auditAuthEvent('login', 'user-1', 'longuser@example.com', '1.2.3.4');

expect(securityAudit.logEvent).toHaveBeenCalledWith(
expect.objectContaining({
details: expect.objectContaining({
email: 'lo***@example.com',
}),
})
);
});

it('does not throw when securityAudit.logEvent rejects', () => {
vi.mocked(securityAudit.logEvent).mockRejectedValueOnce(new Error('DB down'));

expect(() => {
auditAuthEvent('login', 'user-1', 'test@example.com', '1.2.3.4');
}).not.toThrow();
});
});

describe('auditSecurityEvent', () => {
it('dispatches unauthorized as access denied', () => {
auditSecurityEvent('unauthorized', {
userId: 'user-1',
ip: '1.2.3.4',
reason: 'csrf_failed',
});

expect(securityAudit.logEvent).toHaveBeenCalledWith(
expect.objectContaining({
eventType: 'authz.access.denied',
userId: 'user-1',
ipAddress: '1.2.3.4',
details: expect.objectContaining({
reason: 'csrf_failed',
originalEvent: 'unauthorized',
}),
})
);
});

it('dispatches rate_limit event', () => {
auditSecurityEvent('rate_limit', {
ip: '1.2.3.4',
endpoint: '/api/auth/login',
});

expect(securityAudit.logEvent).toHaveBeenCalledWith(
expect.objectContaining({
eventType: 'security.rate.limited',
ipAddress: '1.2.3.4',
riskScore: 0.4,
})
);
});

it('dispatches account_locked_login_attempt with high risk score', () => {
auditSecurityEvent('account_locked_login_attempt', {
email: 'locked@test.com',
ip: '1.2.3.4',
});

expect(securityAudit.logEvent).toHaveBeenCalledWith(
expect.objectContaining({
eventType: 'security.brute.force.detected',
riskScore: 0.8,
})
);
});

it('skips unmapped security events without calling logEvent', () => {
auditSecurityEvent('login_csrf_missing', { ip: '1.2.3.4' });

expect(securityAudit.logEvent).not.toHaveBeenCalled();
});

it('does not throw when securityAudit.logEvent rejects', () => {
vi.mocked(securityAudit.logEvent).mockRejectedValueOnce(new Error('DB down'));

expect(() => {
auditSecurityEvent('unauthorized', { userId: 'user-1' });
}).not.toThrow();
});
});
});
5 changes: 5 additions & 0 deletions packages/lib/src/audit/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ export {
type QueryEventsOptions,
} from './security-audit';

export {
auditAuthEvent,
auditSecurityEvent,
} from './security-audit-adapter';

/** Mask email to prevent PII in audit logs (e.g., john@example.com -> jo***@example.com) */
export function maskEmail(email: string): string {
const [local, domain] = email.split('@');
Expand Down
Loading
Loading