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
64 changes: 52 additions & 12 deletions src/server/server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,15 @@ const { mockDb, mockDbQueryBuilder } = vi.hoisted(() => {
builder.where = vi.fn(() => builder);
builder.limit = vi.fn(() => Promise.resolve(returnValue));
builder.values = vi.fn(() => Promise.resolve());
builder.set = vi.fn(() => builder);
return builder;
};

const mockDb = {
select: vi.fn(),
delete: vi.fn(),
insert: vi.fn(),
update: vi.fn(),
};

return { mockDb, mockDbQueryBuilder };
Expand All @@ -31,7 +33,10 @@ const { mockDb, mockDbQueryBuilder } = vi.hoisted(() => {

vi.mock('@/lib/auth', () => ({
auth: {
api: { getSession: vi.fn() },
api: {
getSession: vi.fn(),
setPassword: vi.fn(),
},
handler: vi.fn(),
},
}));
Expand All @@ -41,6 +46,7 @@ vi.mock('@/db', () => ({ db: mockDb }));
vi.mock('@/db/schema', () => ({
authSession: { token: 'token', id: 'id', userId: 'userId' },
authAuditLog: {},
users: { email: 'email' },
}));

vi.mock('@/lib/logger', () => ({
Expand Down Expand Up @@ -97,13 +103,14 @@ describe('server Route Handlers', () => {
// ─── POST /api/auth/password/set ───────────────────────────────────────────

describe('pOST /api/auth/password/set', () => {
it('should proxy to auth.handler with path rewritten to /api/auth/set-password', async () => {
(auth.handler as any).mockResolvedValue(
new Response(JSON.stringify({ status: true }), {
status: 200,
headers: { 'Content-Type': 'application/json' },
})
);
it('should call auth.api.setPassword, fetch session, and update user status to verified', async () => {
(auth.api.setPassword as any).mockResolvedValue(undefined);
(auth.api.getSession as any).mockResolvedValue({
user: { email: 'test@example.com' },
});

const mockUpdateBuilder = mockDbQueryBuilder([]);
mockDb.update.mockReturnValue(mockUpdateBuilder);

const res = await server.request('/api/auth/password/set', {
method: 'POST',
Expand All @@ -112,11 +119,44 @@ describe('server Route Handlers', () => {
});

expect(res.status).toBe(200);
expect(auth.handler).toHaveBeenCalledOnce();
expect(await res.json()).toEqual({ success: true });
expect(auth.api.setPassword).toHaveBeenCalledWith({
body: { newPassword: 'hunter2' },
headers: expect.any(Headers),
});
expect(auth.api.getSession).toHaveBeenCalledWith({
headers: expect.any(Headers),
});
expect(mockDb.update).toHaveBeenCalled();
expect(mockUpdateBuilder.set).toHaveBeenCalledWith(
expect.objectContaining({ status: 'verified' })
);
});

const proxiedRequest: Request = (auth.handler as any).mock.calls[0][0];
expect(new URL(proxiedRequest.url).pathname).toBe('/api/auth/set-password');
expect(proxiedRequest.headers.get('Authorization')).toBe('Bearer test-token');
it('should return 400 when JSON body is malformed', async () => {
const res = await server.request('/api/auth/password/set', {
method: 'POST',
headers: { 'Content-Type': 'application/json', Authorization: 'Bearer test-token' },
body: 'invalid-json-body',
});

expect(res.status).toBe(400);
const json = await res.json();
expect(json.message).toBeDefined();
});
Comment on lines +136 to +146

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Strengthen malformed-JSON test with a no-side-effect assertion.

After Line 145, assert auth.api.setPassword was not called to lock in short-circuit behavior on parse failure.

Proposed test addition
     expect(res.status).toBe(400);
     const json = await res.json();
     expect(json.message).toBeDefined();
+    expect(auth.api.setPassword).not.toHaveBeenCalled();
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
it('should return 400 when JSON body is malformed', async () => {
const res = await server.request('/api/auth/password/set', {
method: 'POST',
headers: { 'Content-Type': 'application/json', Authorization: 'Bearer test-token' },
body: 'invalid-json-body',
});
expect(res.status).toBe(400);
const json = await res.json();
expect(json.message).toBeDefined();
});
it('should return 400 when JSON body is malformed', async () => {
const res = await server.request('/api/auth/password/set', {
method: 'POST',
headers: { 'Content-Type': 'application/json', Authorization: 'Bearer test-token' },
body: 'invalid-json-body',
});
expect(res.status).toBe(400);
const json = await res.json();
expect(json.message).toBeDefined();
expect(auth.api.setPassword).not.toHaveBeenCalled();
});
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/server/server.test.ts` around lines 136 - 146, The malformed-JSON test in
the 'should return 400 when JSON body is malformed' test case needs to verify
that the setPassword operation was not executed when parsing fails. After the
expect(json.message).toBeDefined() assertion, add a verification that
auth.api.setPassword was not called to confirm the request handler properly
short-circuits on JSON parse failure without proceeding to business logic
execution.


it('should return 400 when auth.api.setPassword throws an error', async () => {
(auth.api.setPassword as any).mockRejectedValue(new Error('Invalid password strength'));

const res = await server.request('/api/auth/password/set', {
method: 'POST',
headers: { 'Content-Type': 'application/json', Authorization: 'Bearer test-token' },
body: JSON.stringify({ newPassword: '123' }),
});

expect(res.status).toBe(400);
const json = await res.json();
expect(json.message).toBe('Invalid password strength');
});
});

Expand Down
34 changes: 33 additions & 1 deletion src/server/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,39 @@ export function createServer() {
return auth.handler(new Request(url, c.req.raw));
}

app.post('/api/auth/password/set', (c) => proxyToAuth(c, '/api/auth/set-password'));
app.post('/api/auth/password/set', async (c) => {
try {
Comment thread
coderabbitai[bot] marked this conversation as resolved.
const { newPassword } = await c.req.json();
await auth.api.setPassword({
body: { newPassword },
headers: c.req.raw.headers,
});

const session = await auth.api.getSession({
headers: c.req.raw.headers,
});

if (session?.user?.email) {
await db
.update(schema.users)
.set({ status: 'verified', updatedAt: new Date() })
.where(eq(schema.users.email, session.user.email));
logger.info(`User status updated to verified for email: ${session.user.email}`);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Remove raw email from success logs.

Line 111 logs a full email address, which introduces avoidable PII exposure in application logs.

Proposed fix
-        logger.info(`User status updated to verified for email: ${session.user.email}`);
+        logger.info('User status updated to verified');
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
logger.info(`User status updated to verified for email: ${session.user.email}`);
logger.info('User status updated to verified');
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/server/server.ts` at line 111, The logger.info call that logs user status
updates contains a raw email address which is personally identifiable
information (PII) and should not be exposed in logs. Modify the success log
message in the logger.info call to remove the ${session.user.email} reference
entirely. Either replace it with a generic message like "User status updated to
verified" or use a non-PII identifier such as a user ID if available from the
session object, to maintain the ability to track the action without exposing
sensitive email data.

} else {
logger.warn(
'Password set succeeded, but session or user email is missing. Skipped updating user status to verified.'
);
}

return c.json({ success: true });
Comment thread
Joel-Joseph-George marked this conversation as resolved.
} catch (err) {
logger.error('Failed to set password:', err);
return c.json(
{ message: err instanceof Error ? err.message : 'Failed to set password' },
400
);
}
});

// POST /api/auth/forget-password
app.post('/api/auth/forget-password', (c) => proxyToAuth(c, '/api/auth/request-password-reset'));
Expand Down
Loading