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
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
CREATE TABLE "PasswordResetToken" (
"id" TEXT NOT NULL PRIMARY KEY,
"userId" TEXT NOT NULL,
"tokenHash" TEXT NOT NULL,
"expiresAt" DATETIME NOT NULL,
"usedAt" DATETIME,
"createdAt" DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP,
CONSTRAINT "PasswordResetToken_userId_fkey" FOREIGN KEY ("userId") REFERENCES "User" ("id") ON DELETE CASCADE ON UPDATE CASCADE
);

CREATE UNIQUE INDEX "PasswordResetToken_tokenHash_key" ON "PasswordResetToken"("tokenHash");
CREATE INDEX "PasswordResetToken_userId_idx" ON "PasswordResetToken"("userId");
13 changes: 13 additions & 0 deletions apps/api/prisma/schema.prisma
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ model User {
emailVerificationTokens EmailVerificationToken[]
sessions Session[]
loginEvents LoginEvent[]
passwordResetTokens PasswordResetToken[]
}

model LoginEvent {
Expand Down Expand Up @@ -63,6 +64,18 @@ model EmailVerificationToken {
@@index([userId])
}

model PasswordResetToken {
id String @id
userId String
tokenHash String @unique
expiresAt DateTime
usedAt DateTime?
createdAt DateTime @default(now())
user User @relation(fields: [userId], references: [id], onDelete: Cascade)

@@index([userId])
Comment on lines +67 to +76

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== locate prisma schema and reset token related files =="
git ls-files | rg '(^|/)schema\.prisma$|reset|password|auth|otp|token' | head -200

echo
echo "== schema excerpt around PasswordResetToken =="
if [ -f apps/api/prisma/schema.prisma ]; then
  nl -ba apps/api/prisma/schema.prisma | sed -n '1,130p' | rg -n -C 8 'model PasswordResetToken|PasswordResetToken|Token' || true
fi

echo
echo "== search relevant symbols =="
rg -n "PasswordResetToken|resetPassword|markUsed|usedAt|resetPasswordToken|createPasswordReset|usePasswordReset|PasswordReset" -S .

Repository: mankatcheung/job-finder

Length of output: 26895


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== ResetPasswordUseCase implementation =="
cat -n apps/api/src/use-cases/auth/ResetPasswordUseCase.ts

echo
echo "== PasswordResetTokenRepository implementation =="
cat -n apps/api/src/infrastructure/db/repositories/PrismaPasswordResetTokenRepository.ts

echo
echo "== PasswordResetToken interface =="
cat -n apps/api/src/domain/passwordResetToken/PasswordResetToken.ts

echo
echo "== IPasswordResetTokenRepository port =="
cat -n apps/api/src/use-cases/ports/IPasswordResetTokenRepository.ts

echo
echo "== AuthResolver resetPassword request flow =="
cat -n apps/api/src/interface-adapters/resolvers/AuthResolver.ts | sed -n '55,70p'

echo
echo "== package versions relevant to Prisma =="
cat package.json | rg -i 'prisma|`@prisma/client`|client' || true
if [ -f apps/api/package.json ]; then cat apps/api/package.json | rg -i 'prisma|`@prisma/client`|client' || true; fi

Repository: mankatcheung/job-finder

Length of output: 6232


Make reset-token claims atomic.

ResetPasswordUseCase.execute() reads the token, then separately calls markUsed(), while the repository only does update({ id }, { usedAt }) with no where: { usedAt: null }. Concurrent reset requests can both pass the usedAt guard and complete updates. Change markUsed() into a conditional update that updates only when usedAt IS NULL, throw if only one row is claimed, and wrap the password update/session revocation in the same transaction.

🤖 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 `@apps/api/prisma/schema.prisma` around lines 67 - 76, The reset-token claim
flow must be atomic against concurrent requests. Update the repository’s
markUsed() implementation to conditionally update only records whose usedAt is
null, and throw when the conditional update does not claim exactly one row; then
change ResetPasswordUseCase.execute() to perform the password update and session
revocation within the same database transaction as the token claim.

}

model ApiToken {
id String @id
userId String
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
import { describe, it, expect, vi, beforeEach } from 'vitest';
import { RequestPasswordResetUseCase } from '@/use-cases/auth/RequestPasswordResetUseCase.js';
import {
makeUserRepository,
makePasswordResetTokenRepository,
makeUser,
makeRateLimiter,
} from '@/__tests__/helpers/mocks.js';
import type { IEmailService } from '@/use-cases/ports/IEmailService.js';

const makeEmailService = (overrides?: Partial<IEmailService>): IEmailService => ({
sendFollowUpReminder: vi.fn().mockResolvedValue(undefined),
sendWeeklyDigest: vi.fn().mockResolvedValue(undefined),
sendPasswordReset: vi.fn().mockResolvedValue(undefined),
sendEmailVerification: vi.fn().mockResolvedValue(undefined),
...overrides,
});

const makeDeps = (overrides?: object) => ({
userRepository: makeUserRepository(),
passwordResetTokenRepository: makePasswordResetTokenRepository(),
emailService: makeEmailService(),
passwordResetRateLimiter: makeRateLimiter(),
generateId: vi.fn().mockReturnValue('generated-id'),
webAppOrigin: 'http://localhost:3000',
...overrides,
});

describe('RequestPasswordResetUseCase', () => {
beforeEach(() => {
vi.clearAllMocks();
});

it('silently no-ops when the email does not match a user', async () => {
const userRepository = makeUserRepository({ findByEmail: vi.fn().mockResolvedValue(null) });
const passwordResetTokenRepository = makePasswordResetTokenRepository();
const emailService = makeEmailService();

await new RequestPasswordResetUseCase(
makeDeps({ userRepository, passwordResetTokenRepository, emailService }),
).execute({ email: 'nobody@example.com', ipAddress: '127.0.0.1' });

expect(passwordResetTokenRepository.create).not.toHaveBeenCalled();
expect(emailService.sendPasswordReset).not.toHaveBeenCalled();
});

it('deletes existing tokens, creates a new one, and emails a reset link', async () => {
const user = makeUser({ id: 'user-1', email: 'test@example.com' });
const userRepository = makeUserRepository({ findByEmail: vi.fn().mockResolvedValue(user) });
const passwordResetTokenRepository = makePasswordResetTokenRepository();
const emailService = makeEmailService();

await new RequestPasswordResetUseCase(
makeDeps({
userRepository,
passwordResetTokenRepository,
emailService,
webAppOrigin: 'https://app.jobfinder.com',
}),
).execute({ email: 'test@example.com', ipAddress: '127.0.0.1' });

expect(passwordResetTokenRepository.deleteAllForUser).toHaveBeenCalledWith('user-1');
expect(passwordResetTokenRepository.create).toHaveBeenCalledWith(
expect.objectContaining({ id: 'generated-id', userId: 'user-1' }),
);
expect(emailService.sendPasswordReset).toHaveBeenCalledWith(
'test@example.com',
expect.stringMatching(/^https:\/\/app\.jobfinder\.com\/reset-password\?token=[a-f0-9]+$/),
);
});

it('sets an expiry roughly one hour in the future', async () => {
const user = makeUser({ id: 'user-1' });
const userRepository = makeUserRepository({ findByEmail: vi.fn().mockResolvedValue(user) });
const passwordResetTokenRepository = makePasswordResetTokenRepository();

const before = Date.now();
await new RequestPasswordResetUseCase(
makeDeps({ userRepository, passwordResetTokenRepository }),
).execute({ email: 'test@example.com', ipAddress: '127.0.0.1' });
const after = Date.now();

const createCall = vi.mocked(passwordResetTokenRepository.create).mock.calls[0][0];
const expiresAtMs = createCall.expiresAt.getTime();
expect(expiresAtMs).toBeGreaterThanOrEqual(before + 60 * 60 * 1000 - 1000);
expect(expiresAtMs).toBeLessThanOrEqual(after + 60 * 60 * 1000 + 1000);
});

it('does not let an email-provider failure surface differently than the unknown-email no-op', async () => {
const user = makeUser({ id: 'user-1', email: 'test@example.com' });
const userRepository = makeUserRepository({ findByEmail: vi.fn().mockResolvedValue(user) });
const emailService = makeEmailService({
sendPasswordReset: vi.fn().mockRejectedValue(new Error('Brevo API error 500')),
});

await expect(
new RequestPasswordResetUseCase(makeDeps({ userRepository, emailService })).execute({
email: 'test@example.com',
ipAddress: '127.0.0.1',
}),
).resolves.toBeUndefined();
});

it('rate-limits by email address regardless of whether the account exists', async () => {
const userRepository = makeUserRepository({ findByEmail: vi.fn().mockResolvedValue(null) });
const rateLimiter = makeRateLimiter({ consume: vi.fn().mockReturnValue(false) });

const err = await new RequestPasswordResetUseCase(
makeDeps({ userRepository, passwordResetRateLimiter: rateLimiter }),
)
.execute({ email: 'nobody@example.com', ipAddress: '127.0.0.1' })
.catch((e) => e);

expect((err as { code: string }).code).toBe('RATE_LIMITED');
expect(rateLimiter.consume).toHaveBeenCalledWith('password-reset:email:nobody@example.com');
expect(userRepository.findByEmail).not.toHaveBeenCalled();
});

it('rate-limits by IP address in addition to email', async () => {
const rateLimiter = makeRateLimiter({
consume: vi.fn().mockImplementation((key: string) => !key.startsWith('password-reset:ip:')),
});

const err = await new RequestPasswordResetUseCase(
makeDeps({ passwordResetRateLimiter: rateLimiter }),
)
.execute({ email: 'test@example.com', ipAddress: '203.0.113.5' })
.catch((e) => e);

expect((err as { code: string }).code).toBe('RATE_LIMITED');
expect(rateLimiter.consume).toHaveBeenCalledWith('password-reset:ip:203.0.113.5');
});

it('lowercases the email when building the rate-limit key', async () => {
const rateLimiter = makeRateLimiter();
const userRepository = makeUserRepository({ findByEmail: vi.fn().mockResolvedValue(null) });

await new RequestPasswordResetUseCase(
makeDeps({ userRepository, passwordResetRateLimiter: rateLimiter }),
).execute({
email: 'Test@Example.com',
ipAddress: null,
});

expect(rateLimiter.consume).toHaveBeenCalledWith('password-reset:email:test@example.com');
});
});
129 changes: 129 additions & 0 deletions apps/api/src/__tests__/application/auth/ResetPasswordUseCase.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
import { createHash } from 'crypto';
import { describe, it, expect, vi, beforeEach } from 'vitest';
import bcrypt from 'bcryptjs';
import { ResetPasswordUseCase } from '@/use-cases/auth/ResetPasswordUseCase.js';
import {
makeUserRepository,
makePasswordResetTokenRepository,
makePasswordResetToken,
makeSessionRepository,
} from '@/__tests__/helpers/mocks.js';

vi.mock('bcryptjs', () => ({
default: {
hash: vi.fn(),
compare: vi.fn(),
},
}));

const RAW_TOKEN = 'raw-reset-token';
const TOKEN_HASH = createHash('sha256').update(RAW_TOKEN).digest('hex');

const makeDeps = (overrides?: object) => ({
userRepository: makeUserRepository(),
passwordResetTokenRepository: makePasswordResetTokenRepository(),
sessionRepository: makeSessionRepository(),
...overrides,
});

describe('ResetPasswordUseCase', () => {
beforeEach(() => {
vi.clearAllMocks();
});

it('throws VALIDATION when the new password is too short', async () => {
const userRepository = makeUserRepository();

const err = await new ResetPasswordUseCase(makeDeps({ userRepository }))
.execute({ token: RAW_TOKEN, newPassword: 'short1' })
.catch((e) => e);

expect((err as { code: string }).code).toBe('VALIDATION');
expect(userRepository.update).not.toHaveBeenCalled();
});

it('throws UNAUTHORIZED when no token matches the hash', async () => {
const passwordResetTokenRepository = makePasswordResetTokenRepository({
findByTokenHash: vi.fn().mockResolvedValue(null),
});
const userRepository = makeUserRepository();

const err = await new ResetPasswordUseCase(
makeDeps({ userRepository, passwordResetTokenRepository }),
)
.execute({ token: RAW_TOKEN, newPassword: 'newPassword123' })
.catch((e) => e);

expect((err as { code: string }).code).toBe('UNAUTHORIZED');
expect(userRepository.update).not.toHaveBeenCalled();
});

it('throws UNAUTHORIZED when the token was already used', async () => {
const resetToken = makePasswordResetToken({
tokenHash: TOKEN_HASH,
usedAt: new Date('2024-01-01T00:30:00.000Z'),
});
const passwordResetTokenRepository = makePasswordResetTokenRepository({
findByTokenHash: vi.fn().mockResolvedValue(resetToken),
});
const userRepository = makeUserRepository();

const err = await new ResetPasswordUseCase(
makeDeps({ userRepository, passwordResetTokenRepository }),
)
.execute({ token: RAW_TOKEN, newPassword: 'newPassword123' })
.catch((e) => e);

expect((err as { code: string }).code).toBe('UNAUTHORIZED');
expect(userRepository.update).not.toHaveBeenCalled();
});

it('throws UNAUTHORIZED when the token has expired', async () => {
const resetToken = makePasswordResetToken({
tokenHash: TOKEN_HASH,
expiresAt: new Date(Date.now() - 1000),
});
const passwordResetTokenRepository = makePasswordResetTokenRepository({
findByTokenHash: vi.fn().mockResolvedValue(resetToken),
});
const userRepository = makeUserRepository();

const err = await new ResetPasswordUseCase(
makeDeps({ userRepository, passwordResetTokenRepository }),
)
.execute({ token: RAW_TOKEN, newPassword: 'newPassword123' })
.catch((e) => e);

expect((err as { code: string }).code).toBe('UNAUTHORIZED');
expect(userRepository.update).not.toHaveBeenCalled();
});

it('updates the password, marks the token used, and revokes all sessions for a valid token', async () => {
const resetToken = makePasswordResetToken({
id: 'reset-1',
userId: 'user-1',
tokenHash: TOKEN_HASH,
expiresAt: new Date(Date.now() + 60 * 60 * 1000),
});
const passwordResetTokenRepository = makePasswordResetTokenRepository({
findByTokenHash: vi.fn().mockResolvedValue(resetToken),
});
const userRepository = makeUserRepository();
const sessionRepository = makeSessionRepository();
vi.mocked(bcrypt.hash).mockResolvedValue('new-hashed-password' as never);

await new ResetPasswordUseCase(
makeDeps({ userRepository, passwordResetTokenRepository, sessionRepository }),
).execute({
token: RAW_TOKEN,
newPassword: 'newPassword123',
});

expect(bcrypt.hash).toHaveBeenCalledWith('newPassword123', 12);
expect(userRepository.update).toHaveBeenCalledWith('user-1', {
passwordHash: 'new-hashed-password',
});
expect(passwordResetTokenRepository.markUsed).toHaveBeenCalledWith('reset-1');
expect(sessionRepository.revokeAllForUser).toHaveBeenCalledWith('user-1');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const makeEmailService = (overrides?: Partial<IEmailService>): IEmailService =>
sendFollowUpReminder: vi.fn().mockResolvedValue(undefined),
sendWeeklyDigest: vi.fn().mockResolvedValue(undefined),
sendEmailVerification: vi.fn().mockResolvedValue(undefined),
sendPasswordReset: vi.fn().mockResolvedValue(undefined),
...overrides,
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type { IEmailService } from '@/use-cases/ports/IEmailService.js';
const makeEmailService = (overrides?: Partial<IEmailService>): IEmailService => ({
sendFollowUpReminder: vi.fn().mockResolvedValue(undefined),
sendWeeklyDigest: vi.fn().mockResolvedValue(undefined),
sendPasswordReset: vi.fn().mockResolvedValue(undefined),
sendEmailVerification: vi.fn().mockResolvedValue(undefined),
...overrides,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ function makeEmailService(): IEmailService {
return {
sendFollowUpReminder: vi.fn().mockResolvedValue(undefined),
sendWeeklyDigest: vi.fn().mockResolvedValue(undefined),
sendPasswordReset: vi.fn().mockResolvedValue(undefined),
sendEmailVerification: vi.fn().mockResolvedValue(undefined),
};
}
Expand Down
10 changes: 10 additions & 0 deletions apps/api/src/__tests__/helpers/createTestDb.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,16 @@ const SCHEMA_STATEMENTS = [
FOREIGN KEY ("userId") REFERENCES "User"("id") ON DELETE CASCADE
)`,
`CREATE INDEX "EmailVerificationToken_userId_idx" ON "EmailVerificationToken"("userId")`,
`CREATE TABLE "PasswordResetToken" (
"id" TEXT PRIMARY KEY,
"userId" TEXT NOT NULL,
"tokenHash" TEXT NOT NULL UNIQUE,
"expiresAt" DATETIME NOT NULL,
"usedAt" DATETIME,
"createdAt" DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP,
FOREIGN KEY ("userId") REFERENCES "User"("id") ON DELETE CASCADE
)`,
`CREATE INDEX "PasswordResetToken_userId_idx" ON "PasswordResetToken"("userId")`,
];

export interface TestDb {
Expand Down
Loading
Loading