Code Review Agent

주요 평가는 다음과 같습니다. 현재 구현은 비밀번호 재설정 기능으로 사용하기에 보안상 치명적인 문제가 있습니다.

주요 문제점

1. 예측 가능한 토큰 생성

const token = Math.random().toString(36).substring(2);

Math.random()은 암호학적으로 안전하지 않아 토큰을 추측하거나 재현할 수 있습니다.

crypto.randomBytes() 또는 randomUUID()를 사용해야 합니다.

const token = randomBytes(32).toString('hex');

2. 토큰 만료 시간을 검증하지 않음

발급 시 만료 시간을 저장하지만 검증할 때 확인하지 않습니다.

const user = await this.userRepo.findOne({
  where: {
    resetToken: token,
    resetTokenExpiresAt: MoreThan(new Date()),
  },
});

만료 시간은 일반적으로 15~60분 정도로 짧게 설정하는 것이 안전합니다.

3. 비밀번호를 평문으로 저장

user.password = newPassword;

비밀번호는 반드시 Argon2id 또는 bcrypt 같은 전용 해시 알고리즘으로 저장해야 합니다.

user.password = await argon2.hash(newPassword);

비밀번호 재설정 시에도 일반 로그인과 동일한 비밀번호 정책을 적용해야 합니다.

4. 토큰을 DB에 평문 저장

DB가 유출되면 아직 사용되지 않은 모든 재설정 링크가 즉시 탈취됩니다.

원본 토큰은 이메일로만 보내고, DB에는 해시값만 저장하는 편이 좋습니다.

const token = randomBytes(32).toString('hex');
const tokenHash = createHash('sha256').update(token).digest('hex');

user.resetTokenHash = tokenHash;

검증할 때도 입력 토큰을 해시한 뒤 조회합니다.

5. 토큰을 사용 후 폐기하지 않음

현재는 비밀번호를 변경한 뒤에도 resetToken이 남아 있어 동일 토큰을 재사용할 수 있습니다.

user.resetTokenHash = null;
user.resetTokenExpiresAt = null;

가능하면 비밀번호 변경 시 모든 기존 세션과 refresh token도 폐기해야 합니다.

6. 원자적 검증 및 소비가 아님

동시에 여러 요청이 들어오면 같은 토큰이 중복 사용될 수 있습니다. 토큰 검증과 비밀번호 변경을 트랜잭션 또는 조건부 업데이트로 처리해야 합니다.

예를 들어 다음 조건을 포함한 단일 업데이트를 사용할 수 있습니다.

UPDATE users
SET password_hash = :hash,
    reset_token_hash = NULL,
    reset_token_expires_at = NULL
WHERE reset_token_hash = :tokenHash
  AND reset_token_expires_at > NOW();

업데이트된 행이 0개이면 토큰이 유효하지 않거나 이미 사용된 것으로 처리합니다.

7. 계정 존재 여부 노출 가능성

현재 코드는 반환값만 보면 계정 존재 여부를 직접 노출하지 않지만, 응답 시간이나 이메일 발송 동작 차이로 계정 존재 여부를 추측할 수 있습니다.

항상 다음과 같이 동일한 일반 메시지를 반환하는 API 계층을 두는 것이 좋습니다.

이메일 주소가 등록되어 있다면 비밀번호 재설정 안내를 발송했습니다.

또한 IP와 이메일 기준의 rate limit, CAPTCHA 또는 abuse 방지 정책이 필요합니다.

8. 이메일 발송 실패 처리 부족

DB 저장 후 이메일 발송이 실패하면 토큰은 발급됐지만 사용자는 링크를 받지 못합니다. 반대로 재시도 시 기존 토큰을 덮어쓸 수도 있습니다.

운영 환경에서는 다음 중 하나를 고려해야 합니다.

• 발송 작업을 큐에 넣고 재시도
• Outbox 패턴 사용
• 메일 발송 상태 및 만료 정책 관리
• 요청 횟수 제한

9. 입력값 검증 부족

이메일과 비밀번호에 대한 검증이 없습니다.

• 이메일 trim 및 정규화
• 비밀번호 길이 및 복잡도 정책
• 비밀번호가 이메일이나 기존 비밀번호와 지나치게 유사한지 검사
• 토큰 형식 및 최대 길이 제한

NestJS에서는 DTO와 class-validator를 사용하는 것이 적절합니다.

개선 예시

import { createHash, randomBytes } from 'node:crypto';
import * as argon2 from 'argon2';

@Injectable()
export class PasswordResetService {
  constructor(
    private readonly userRepo: UserRepository,
    private readonly mailer: MailerService,
  ) {}

  async requestReset(email: string): Promise<void> {
    const normalizedEmail = email.trim().toLowerCase();
    const user = await this.userRepo.findOne({
      where: { email: normalizedEmail },
    });

    // 호출자에게는 항상 동일한 응답을 반환하도록 API 계층에서 처리
    if (!user) return;

    const token = randomBytes(32).toString('hex');
    const tokenHash = createHash('sha256')
      .update(token)
      .digest('hex');

    user.resetTokenHash = tokenHash;
    user.resetTokenExpiresAt = new Date(Date.now() + 30 * 60 * 1000);

    await this.userRepo.save(user);

    await this.mailer.send(normalizedEmail, 'reset', { token });
  }

  async resetPassword(token: string, newPassword: string): Promise<void> {
    const tokenHash = createHash('sha256')
      .update(token)
      .digest('hex');

    const user = await this.userRepo.findOne({
      where: {
        resetTokenHash: tokenHash,
        resetTokenExpiresAt: MoreThan(new Date()),
      },
    });

    if (!user) {
      throw new BadRequestException('Invalid or expired token');
    }

    user.password = await argon2.hash(newPassword);
    user.resetTokenHash = null;
    user.resetTokenExpiresAt = null;

    await this.userRepo.save(user);

    // 필요하다면 기존 세션/refresh token도 모두 폐기
  }
}

단, 위 예시도 동시 요청에 대한 완전한 방어를 위해서는 트랜잭션이나 조건부 업데이트가 필요합니다.

우선순위

가장 먼저 수정해야 할 항목은 다음 네 가지입니다.

1. Math.random() 제거 및 암호학적 난수 사용
2. 토큰 만료 시간 검증
3. 비밀번호 해시 저장
4. 사용 후 토큰 즉시 폐기 및 원자적 처리

현재 상태로 배포하면 토큰 탈취, 토큰 재사용, 비밀번호 평문 저장으로 인해 심각한 계정 탈취 및 개인정보 유출 위험이 있습니다.