Skip to content

Conversation

@Jihyun3478
Copy link
Contributor

@Jihyun3478 Jihyun3478 commented Apr 20, 2025

🔥 변경 사항

  • qrcode app & QRCode API 구현
  • 비즈니스 로직 흐름 재반영
  • 이에 따른 테스트코드 업데이트

🔥 진행 중인 사항

  • qrcode repo integration test
  • 큐알코드 라우터 / 서비스 / 레포 user 쪽으로 합치는 부분

🏷 관련 이슈

  • 관련 이슈: #148

📸 스크린샷 (UI 변경 시 필수)

(변경 사항이 UI와 관련된 경우 스크린샷을 첨부해주세요.)

📌 체크리스트

  • 기능이 정상적으로 동작하는지 테스트 완료
  • 코드 스타일 가이드 준수 여부 확인
  • 관련 문서 업데이트 완료 (필요 시)

Summary by CodeRabbit

Summary by CodeRabbit

  • 신규 기능

    • QR 로그인 토큰 생성 및 자동 로그인 기능이 추가되었습니다. 사용자는 QR 코드를 통해 간편하게 로그인할 수 있습니다.
    • QR 로그인 토큰 발급(POST /api/qr-login) 및 토큰 사용(GET /qr-login?token=...) API가 추가되었습니다.
    • 클라이언트 IP 주소를 정확히 인식하기 위한 프록시 신뢰 설정이 추가되었습니다.
  • 테스트

    • QR 로그인 토큰 관련 서비스, 저장소의 단위 및 통합 테스트가 추가되었습니다.
  • 문서화

    • QR 로그인 관련 API에 대한 Swagger 문서가 추가되었습니다.
  • 유틸리티

    • 랜덤 토큰 생성 유틸리티가 추가되었습니다.
@Jihyun3478 Jihyun3478 self-assigned this Apr 20, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Apr 20, 2025

"""

Walkthrough

이 변경 사항들은 QR 로그인 토큰 기능의 도입과 관련된 코드 추가 및 확장을 포함합니다. UserService와 UserRepository에 QR 로그인 토큰 생성, 조회, 사용 메서드가 추가되었으며, UserController에는 토큰 생성 및 사용을 처리하는 새로운 요청 핸들러가 도입되었습니다. QR 로그인 관련 API 엔드포인트가 user.router에 추가되었고, 이에 따른 DTO, 타입, 유틸리티 함수도 신설되었습니다. 또한, 서비스와 레포지토리의 동작을 검증하는 단위 및 통합 테스트가 새롭게 작성되었습니다. 기존 엔티티의 선언은 변경되지 않았습니다.

Changes

파일/경로 요약 변경 요약
src/repositories/test/qr.repo.test.ts
src/services/test/qr.service.test.ts
src/repositories/test/qr.repo.integration.test.ts
QR 로그인 토큰 레포지토리 및 서비스에 대한 단위/통합 테스트 신규 작성
src/controllers/user.controller.ts QR 로그인 토큰 생성 및 사용을 위한 createToken, getToken 요청 핸들러 추가, 관련 타입 및 예외 처리 도입
src/repositories/user.repository.ts QR 로그인 토큰 생성, 조회, 사용(mark as used) 메서드 추가 및 에러 처리 강화
src/services/user.service.ts QR 로그인 토큰 생성, 조회, 사용 메서드 및 getDecryptedTokens 공개 메서드 추가
src/routes/user.router.ts /api/qr-login POST(토큰 생성), GET(토큰 사용) 라우트 및 Swagger 문서 추가
src/types/dto/responses/qrResponse.type.ts QRLoginTokenResponseDto, Token10 등 QR 토큰 응답용 DTO 및 타입 신규 추가
src/types/models/QRLoginToken.type.ts QRLoginToken 인터페이스 신규 정의
src/utils/generateRandomToken.util.ts 랜덤 알파벳/숫자 토큰 생성 유틸리티 함수 추가
src/routes/index.ts 파일 끝에 개행 추가(기능적 변화 없음)
src/app.ts Express 앱에 'trust proxy' 설정 추가

Sequence Diagram(s)

sequenceDiagram participant Client participant Router participant UserController participant UserService participant UserRepository participant DB Client->>Router: POST /api/qr-login (토큰 생성 요청) Router->>UserController: createToken(req) UserController->>UserService: create(userId, ip, userAgent) UserService->>UserRepository: createQRLoginToken(token, userId, ip, userAgent) UserRepository->>DB: INSERT QRLoginToken DB-->>UserRepository: OK UserRepository-->>UserService: 완료 UserService-->>UserController: token 반환 UserController-->>Client: QRLoginTokenResponseDto 반환 Client->>Router: GET /api/qr-login?token=... Router->>UserController: getToken(req) UserController->>UserService: useToken(token) UserService->>UserRepository: findQRLoginToken(token) UserRepository->>DB: SELECT QRLoginToken DB-->>UserRepository: Token or null UserRepository-->>UserService: Token or null alt 토큰 있음 UserService->>UserRepository: markTokenUsed(token) UserRepository->>DB: UPDATE is_used=true DB-->>UserRepository: OK UserRepository-->>UserService: 완료 UserService-->>UserController: token 반환 UserController->>UserService: getDecryptedTokens(...) UserController->>Client: 쿠키 설정 후 /main으로 리다이렉트 else 토큰 없음/만료 UserService-->>UserController: null 반환 UserController-->>Client: 404/400 에러 반환 end 
Loading

Suggested labels

bug, documentation

Suggested reviewers

  • six-standard
  • Nuung

Poem

QR 토큰이 반짝, 토끼 귀가 쫑긋!
새로운 로그인 길이 열렸네,
랜덤 토큰 춤추고,
DB 속 토큰도 깨어나네.
테스트 토끼도 신나서 깡총!
QR 한 번에 로그인,
🥕 오늘도 개발은 즐거워!
"""


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 21764ee and c2dd34a.

📒 Files selected for processing (1)
  • src/controllers/user.controller.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/controllers/user.controller.ts
⏰ Context from checks skipped due to timeout of 90000ms (4)
  • GitHub Check: build-and-test (20)
  • GitHub Check: build-and-test (23)
  • GitHub Check: build-and-test (22)
  • GitHub Check: build-and-test (21)
✨ Finishing Touches
  • 📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🧹 Nitpick comments (11)
src/types/models/QRLoginToken.type.ts (1)

1-9: QRLoginToken 인터페이스 개선 제안

인터페이스 설계는 전반적으로 잘 되어 있으나, 몇 가지 개선점이 있습니다:

  1. user 필드의 이름은 user_id로 변경하는 것이 목적을 더 명확히 할 수 있습니다.
  2. 각 필드에 대한 JSDoc 주석을 추가하여 목적과 용도를 명확히 하면 좋겠습니다.
+/** + * QR 로그인 토큰 정보를 나타내는 인터페이스 + */ export interface QRLoginToken { + /** 고유 식별 토큰 */ token: string; - user: number; + /** 토큰과 연결된 사용자 ID */ + user_id: number; + /** 토큰 생성 시간 */ created_at: Date; + /** 토큰 만료 시간 */ expires_at: Date; + /** 토큰 사용 여부 */ is_used: boolean; + /** 토큰 생성 요청 IP 주소 */ ip_address: string; + /** 토큰 생성 요청 User-Agent */ user_agent: string; }
src/types/dto/responses/qrResponse.type.ts (2)

3-5: 코드 들여쓰기 수정 필요

인터페이스 정의의 들여쓰기가 불일치합니다. 들여쓰기를 일관되게 조정해 주세요.

 interface QRLoginTokenResponseData { - token: string; - } + token: string; }

1-7: 응답 DTO 개선 제안

현재 응답 DTO는 토큰 값만 포함하고 있습니다. 사용자 경험 향상을 위해 유효 기간과 같은 추가 정보를 포함하는 것이 좋겠습니다.

 interface QRLoginTokenResponseData { token: string; + expires_at: string; // ISO 형식의 만료 시간 } -export class QRLoginTokenResponseDto extends BaseResponseDto<QRLoginTokenResponseData> { } +export class QRLoginTokenResponseDto extends BaseResponseDto<QRLoginTokenResponseData> { + constructor( + success: boolean, + message: string, + data: QRLoginTokenResponseData, + error: string | null = null + ) { + super(success, message, data, error); + } +}
src/services/qr.service.ts (1)

8-12: QR 토큰 생성 로직이 명확합니다.

UUID를 생성하고 저장소에 위임하는 방식이 깔끔합니다. 다만, 비동기 처리 중 에러가 발생할 경우에 대한 추가적인 처리(try-catch)를 고려해볼 수 있습니다.

async create(userId: number, ip: string, userAgent: string): Promise<string> { - const token = randomUUID(); - await this.qrRepo.createQRLoginToken(token, userId, ip, userAgent); - return token; + try { + const token = randomUUID(); + await this.qrRepo.createQRLoginToken(token, userId, ip, userAgent); + return token; + } catch (error) { + // 레포지토리에서 이미 에러를 처리하지만, 서비스 계층에서도 로깅이나 추가 처리를 할 수 있습니다 + throw error; + } }
src/repositories/qr.repository.ts (1)

11-15: SQL 쿼리가 적절히 구성되어 있습니다.

파라미터화된 쿼리를 사용하여 SQL 인젝션을 방지하고, 토큰의 유효기간을 5분으로 설정한 것이 좋습니다. 다만, 토큰 유효기간을 상수로 분리하면 코드 가독성과 유지보수성이 향상될 수 있습니다.

+ private readonly TOKEN_EXPIRY_MINUTES = 5; + async createQRLoginToken(token: String, userId: number, ip: string, userAgent: string): Promise<void> { try { const query = ` INSERT INTO qr_login_tokens (token, user_id, created_at, expires_at, is_used, ip_address, user_agent) - VALUES ($1, $2, NOW(), NOW() + INTERVAL '5 minutes', false, $3, $4); + VALUES ($1, $2, NOW(), NOW() + INTERVAL '${this.TOKEN_EXPIRY_MINUTES} minutes', false, $3, $4); `; await this.pool.query(query, [token, userId, ip, userAgent]);
src/services/__test__/qr.service.test.ts (4)

13-14: 명시적인 any 타입 사용을 피하세요.

repository를 모킹할 때 {} as any와 같은 명시적인 any 타입 사용은 타입 안전성을 저해합니다. 더 구체적인 모킹 방법을 사용하는 것이 좋습니다.

다음과 같이 개선할 수 있습니다:

- const repoInstance = new QRLoginTokenRepository({} as any) + const mockPool = { query: jest.fn() }; + const repoInstance = new QRLoginTokenRepository(mockPool as unknown as Pool);
🧰 Tools
🪛 ESLint

[error] 13-13: Unexpected any. Specify a different type.

(@typescript-eslint/no-explicit-any)


28-28: UUID 유효성 검사 개선.

UUID 정규식 패턴은 올바르지만, 유지보수성과 가독성을 위해 검증용 라이브러리나 함수를 사용하는 것이 좋습니다.

다음과 같이 개선할 수 있습니다:

- const uuidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i; + // UUID 유효성 검증 함수 + const isValidUUID = (uuid: string): boolean => { + return /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i.test(uuid); + };

그리고 테스트에서:

- expect(token).toMatch(uuidRegex); + expect(isValidUUID(token)).toBe(true);

35-43: 오류 테스트 강화.

현재 테스트는 특정 오류 메시지를 확인하지만, 예외 유형도 함께 확인하면 더 명확한 테스트가 될 수 있습니다. 또한 구체적인 예외 유형을 검증하면 테스트가 더 견고해집니다.

다음과 같이 개선할 수 있습니다:

- await expect(service.create(userId, ip, userAgent)).rejects.toThrow('생성 실패'); + await expect(service.create(userId, ip, userAgent)).rejects.toThrow(DBError); + await expect(service.create(userId, ip, userAgent)).rejects.toHaveProperty('message', '생성 실패');

76-81: 오류 테스트 강화.

getByToken 메서드의 오류 테스트도 create 메서드의 오류 테스트와 동일한 방식으로 개선할 수 있습니다.

다음과 같이 개선할 수 있습니다:

- await expect(service.getByToken(mockToken)).rejects.toThrow('조회 실패'); + await expect(service.getByToken(mockToken)).rejects.toThrow(DBError); + await expect(service.getByToken(mockToken)).rejects.toHaveProperty('message', '조회 실패');
src/controllers/qr.controller.ts (2)

34-34: 타입 명시가 필요합니다.

getToken 메서드의 매개변수에 타입 주석이 누락되어 있습니다. 이로 인해 타입 안전성이 저하될 수 있습니다.

다음과 같이 명시적인 타입을 추가하세요:

- getToken: RequestHandler = async (req, res, next) => { + getToken: RequestHandler = async ( + req: Request, + res: Response, + next: NextFunction, + ) => {

36-36: 토큰 유효성 검사를 추가하세요.

쿼리 파라미터에서 가져온 토큰 문자열에 대한 기본적인 유효성 검사가 없습니다. 이로 인해 잘못된 형식의 토큰이 서비스 레이어로 전달될 수 있습니다.

다음과 같이 기본적인 유효성 검사를 추가하세요:

 const token = req.query.token as string; + // UUID 형식 검증 + const uuidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i; + if (!token || !uuidRegex.test(token)) { + return res.status(400).json({ success: false, message: '유효하지 않은 토큰 형식입니다.' }); + }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5f14e86 and 6d3b0ec.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (10)
  • package.json (1 hunks)
  • src/controllers/qr.controller.ts (1 hunks)
  • src/repositories/__test__/qr.repo.test.ts (1 hunks)
  • src/repositories/qr.repository.ts (1 hunks)
  • src/routes/index.ts (2 hunks)
  • src/routes/qr.router.ts (1 hunks)
  • src/services/__test__/qr.service.test.ts (1 hunks)
  • src/services/qr.service.ts (1 hunks)
  • src/types/dto/responses/qrResponse.type.ts (1 hunks)
  • src/types/models/QRLoginToken.type.ts (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (6)
src/types/dto/responses/qrResponse.type.ts (1)
src/types/dto/responses/baseResponse.type.ts (1)
  • BaseResponseDto (28-40)
src/services/qr.service.ts (2)
src/repositories/qr.repository.ts (1)
  • QRLoginTokenRepository (6-36)
src/types/models/QRLoginToken.type.ts (1)
  • QRLoginToken (1-9)
src/repositories/qr.repository.ts (1)
src/types/models/QRLoginToken.type.ts (1)
  • QRLoginToken (1-9)
src/routes/qr.router.ts (4)
src/repositories/qr.repository.ts (1)
  • QRLoginTokenRepository (6-36)
src/services/qr.service.ts (1)
  • QRLoginTokenService (5-17)
src/controllers/qr.controller.ts (1)
  • QRLoginController (6-54)
src/middlewares/auth.middleware.ts (1)
  • authMiddleware (120-123)
src/controllers/qr.controller.ts (2)
src/services/qr.service.ts (1)
  • QRLoginTokenService (5-17)
src/types/dto/responses/qrResponse.type.ts (1)
  • QRLoginTokenResponseDto (7-7)
src/services/__test__/qr.service.test.ts (3)
src/services/qr.service.ts (1)
  • QRLoginTokenService (5-17)
src/repositories/qr.repository.ts (1)
  • QRLoginTokenRepository (6-36)
src/types/models/QRLoginToken.type.ts (1)
  • QRLoginToken (1-9)
🪛 Biome (1.9.4)
src/repositories/qr.repository.ts

[error] 9-9: Don't use 'String' as a type.

Use lowercase primitives for consistency.
Safe fix: Use 'string' instead

(lint/complexity/noBannedTypes)

🪛 GitHub Check: build-and-test (23)
src/repositories/qr.repository.ts

[failure] 9-9:
Prefer using the primitive string as a type name, rather than the upper-cased String

🪛 GitHub Check: build-and-test (20)
src/repositories/qr.repository.ts

[failure] 9-9:
Prefer using the primitive string as a type name, rather than the upper-cased String

🪛 GitHub Check: build-and-test (22)
src/repositories/qr.repository.ts

[failure] 9-9:
Prefer using the primitive string as a type name, rather than the upper-cased String

🪛 GitHub Check: build-and-test (21)
src/repositories/qr.repository.ts

[failure] 9-9:
Prefer using the primitive string as a type name, rather than the upper-cased String

🪛 ESLint
src/repositories/qr.repository.ts

[error] 9-9: Prefer using the primitive string as a type name, rather than the upper-cased String.

(@typescript-eslint/no-wrapper-object-types)

src/services/__test__/qr.service.test.ts

[error] 13-13: Unexpected any. Specify a different type.

(@typescript-eslint/no-explicit-any)

🪛 GitHub Actions: Test CI
src/repositories/qr.repository.ts

[error] 9-9: ESLint: Prefer using the primitive string as a type name, rather than the upper-cased String (@typescript-eslint/no-wrapper-object-types)

🔇 Additional comments (9)
package.json (1)

38-38:

✅ Verification successful

uuid 패키지 버전 확인 필요

현재 추가된 uuid 패키지의 버전(^11.1.0)이 최신 버전보다 높게 설정되어 있습니다. 이 버전이 실제로 존재하는지 확인하고, 호환성 문제가 없는지 검토해 주세요.


🏁 Script executed:

#!/bin/bash # Description: 최신 uuid 패키지 버전 확인 # npm 레지스트리에서 uuid 패키지의 최신 버전 정보 가져오기 curl -s https://registry.npmjs.org/uuid | jq '.["dist-tags"].latest' # 만약 11.1.0 버전이 존재하는지 확인 curl -s https://registry.npmjs.org/uuid/11.1.0 | jq '.version'

Length of output: 151


uuid 패키지 버전 정상 확인됨

npm 레지스트리에서 dist‑tag latest11.1.0으로 확인되었고, 11.1.0 버전도 정상 존재합니다.
따라서 package.json에 설정된 "uuid": "^11.1.0" 은 유효하며 추가 검토나 변경이 필요하지 않습니다.

src/routes/index.ts (1)

5-5: QR 라우터 통합 적절히 구현됨

기존의 라우터 구조 패턴과 일관되게 QR 라우터가 추가되었습니다. 다른 라우터들과 동일하게 루트 경로('/')에 마운트되어 있어 코드 스타일이 일관적으로 유지되고 있습니다.

Also applies to: 16-16

src/repositories/__test__/qr.repo.test.ts (1)

1-56: 테스트 케이스가 잘 구성되어 있습니다.

모든 성공 및 실패 경로를 포함한 철저한 테스트 케이스를 작성해 주셨네요. 특히 DB 오류 시의 예외 처리와 토큰이 존재하지 않는 경우의 처리가 잘 테스트되어 있습니다.

src/services/qr.service.ts (2)

3-3: crypto 모듈 사용이 적절합니다.

Node.js 내장 crypto 모듈의 randomUUID를 사용하여 안전한 토큰 생성을 구현하였습니다.


14-16: getByToken 메서드가 간결합니다.

레포지토리에 위임하는 방식이 적절합니다. 여기서도 에러 처리를 고려해볼 수 있으나, 이미 레포지토리에서 에러 처리를 하고 있으므로 선택 사항입니다.

src/routes/qr.router.ts (2)

11-13: 의존성 주입 패턴이 잘 적용되었습니다.

레포지토리, 서비스, 컨트롤러 인스턴스를 순차적으로 생성하여 의존성을 주입하는 방식이 좋습니다.


29-48: GET 엔드포인트와 Swagger 문서화가 적절합니다.

토큰 조회를 위한 엔드포인트와 파라미터 정의, 응답 코드가 잘 문서화되어 있습니다.

src/repositories/qr.repository.ts (2)

22-31: 토큰 조회 로직이 적절합니다.

만료되지 않고 사용되지 않은 토큰만 조회하도록 WHERE 조건이 잘 설정되어 있습니다. 또한 결과가 없을 경우 null을 반환하는 처리도 적절합니다.


31-34: 에러 처리와 로깅이 잘 구현되어 있습니다.

에러 발생 시 로깅하고 적절한 에러 메시지로 DBError를 던지는 방식이 좋습니다.

res.status(404).json({ success: false, message: '유효하지 않거나 만료된 토큰입니다.' });
}

res.status(200).json({ success: true, message: '유효한 QR 토큰입니다.', token: found });
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

보안을 위해 토큰 응답 데이터를 제한하세요.

현재 응답에는 전체 토큰 객체가 포함되어 있어 민감한 정보가 노출될 수 있습니다. 필요한 정보만 반환하도록 수정하는 것이 좋습니다.

다음과 같이 응답 데이터를 제한하세요:

- res.status(200).json({ success: true, message: '유효한 QR 토큰입니다.', token: found }); + res.status(200).json({ + success: true, + message: '유효한 QR 토큰입니다.', + data: { + token: found.token, + user: found.user, + expires_at: found.expires_at + } + });
📝 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
res.status(200).json({ success: true, message: '유효한 QR 토큰입니다.', token: found });
res.status(200).json({
success: true,
message: '유효한 QR 토큰입니다.',
data: {
token: found.token,
user: found.user,
expires_at: found.expires_at
}
});
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

🧹 Nitpick comments (1)
src/repositories/__test__/qr.repo.integration.test.ts (1)

16-26: DB 풀 구성의 효율성을 개선하세요.

테스트 풀 구성이 잘 이루어졌지만, 테스트에서는 데이터베이스 연결 값을 더 효율적으로 관리할 수 있습니다. 환경 변수 의존성을 줄이고 테스트 환경에 맞게 최적화하는 것이 좋습니다.

const testPoolConfig: pg.PoolConfig = { user: process.env.POSTGRES_USER, host: process.env.POSTGRES_HOST, database: process.env.DATABASE_NAME, password: process.env.POSTGRES_PASSWORD, port: Number(process.env.POSTGRES_PORT), max: 1, - idleTimeoutMillis: 30000, - connectionTimeoutMillis: 5000, - statement_timeout: 30000, + idleTimeoutMillis: 10000, + connectionTimeoutMillis: 2000, + statement_timeout: 10000, };
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6d3b0ec and 3b6adaa.

📒 Files selected for processing (4)
  • src/repositories/__test__/qr.repo.integration.test.ts (1 hunks)
  • src/repositories/qr.repository.ts (1 hunks)
  • src/routes/qr.router.ts (1 hunks)
  • src/types/dto/requests/createQrRequest.type.ts (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • src/types/dto/requests/createQrRequest.type.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/routes/qr.router.ts
  • src/repositories/qr.repository.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/repositories/__test__/qr.repo.integration.test.ts (1)
src/repositories/qr.repository.ts (1)
  • QRLoginTokenRepository (6-36)
🪛 GitHub Actions: Test CI
src/repositories/__test__/qr.repo.integration.test.ts

[error] 41-41: Integration test failed: 'qr_login_tokens 테이블이 존재하지 않습니다.' (qr_login_tokens table does not exist). This caused test failures in QRLoginTokenRepository integration tests.


[error] 51-51: Unhandled error event due to database termination (shutdown). This error caused the test suite to fail with an unhandled exception.

Copy link
Member

@six-standard six-standard left a comment

Choose a reason for hiding this comment

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

코드 잘 읽었습니다!
제가 남겨드린 질문 하나 외에 큰 궁금증이나 단점은 없어보이네요~

좋았던 점

  • 세세한 디테일들이 잘 되어있습니다. (?? 문법, undefined 값 처리 등)
@Nuung Nuung added the enhancement New feature or request label Apr 21, 2025
Copy link
Contributor

@ooheunda ooheunda left a comment

Choose a reason for hiding this comment

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

수정 중이신 것 같아 업데이트 되면 다시 보겠습니다!
다른 분들이 리뷰 잘 해주셔서, 전 사소한 부분에 대해서만 몇 마디 얹어봤습니다... 😅
고생하셨습니다! 👍 🔥

좋았던 점

  • 테스트 코드가 굉장히 깔끔해서 좋았습니다.
  • 특히 uuid를 고정값으로 모킹하는 대신 정규표현식 + toMatch()로 확인하신게 인상 깊었어요!
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (3)
src/services/qr.service.ts (1)

18-25: 불필요한 공백을 제거하세요.

코드에 불필요한 공백이 있습니다. 특히 useToken 메소드 내부에 과도한 공백이 있어 가독성을 떨어뜨립니다.

다음과 같이 불필요한 공백을 제거하세요:

async useToken(token: string): Promise<QRLoginToken | null> { const qrToken = await this.qrRepo.findQRLoginToken(token); -  if (!qrToken) { return null; } -  await this.qrRepo.markTokenUsed(token); return qrToken; }
src/repositories/qr.repository.ts (1)

37-47: markTokenUsed 메소드의 들여쓰기 일관성 개선

메소드 내의 들여쓰기가 다른 메소드와 일관되지 않습니다. 코드 스타일의 일관성을 유지하는 것이 중요합니다.

다음과 같이 들여쓰기를 수정하세요:

async markTokenUsed(token: string): Promise<void> { try { - const query = ` - UPDATE qr_login_tokens SET is_used = true WHERE token = $1; - `; - await this.pool.query(query, [token]); + const query = ` + UPDATE qr_login_tokens SET is_used = true WHERE token = $1; + `; + await this.pool.query(query, [token]); } catch (error) { - logger.error('QRLoginToken Repo mark as used Error : ', error); - throw new DBError('QR 코드 사용 처리 중 문제가 발생했습니다.'); + logger.error('QRLoginToken Repo mark as used Error : ', error); + throw new DBError('QR 코드 사용 처리 중 문제가 발생했습니다.'); } }
src/services/__test__/qr.service.test.ts (1)

12-16: any 타입 사용을 피하세요.

{} as any를 사용하여 Pool의 모의 객체를 생성하는 것보다 더 구체적인 타입을 사용하는 것이 좋습니다. 이는 타입 안전성을 향상시키고 오류를 줄이는 데 도움이 됩니다.

다음과 같이 모의 객체를 생성하세요:

- const repoInstance = new QRLoginTokenRepository({} as any); + // Pool의 필수 메소드만 모킹 + const mockPool = { + query: jest.fn(), + } as unknown as Pool; + const repoInstance = new QRLoginTokenRepository(mockPool);
🧰 Tools
🪛 ESLint

[error] 13-13: Unexpected any. Specify a different type.

(@typescript-eslint/no-explicit-any)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 065879c and a007870.

📒 Files selected for processing (6)
  • src/controllers/qr.controller.ts (1 hunks)
  • src/repositories/__test__/qr.repo.test.ts (1 hunks)
  • src/repositories/qr.repository.ts (1 hunks)
  • src/routes/qr.router.ts (1 hunks)
  • src/services/__test__/qr.service.test.ts (1 hunks)
  • src/services/qr.service.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/routes/qr.router.ts
  • src/repositories/test/qr.repo.test.ts
🧰 Additional context used
🧬 Code Graph Analysis (4)
src/services/__test__/qr.service.test.ts (3)
src/services/qr.service.ts (1)
  • QRLoginTokenService (5-28)
src/repositories/qr.repository.ts (1)
  • QRLoginTokenRepository (6-48)
src/types/models/QRLoginToken.type.ts (1)
  • QRLoginToken (1-9)
src/controllers/qr.controller.ts (3)
src/services/qr.service.ts (1)
  • QRLoginTokenService (5-28)
src/services/user.service.ts (1)
  • UserService (10-125)
src/types/dto/responses/qrResponse.type.ts (1)
  • QRLoginTokenResponseDto (9-9)
src/services/qr.service.ts (2)
src/repositories/qr.repository.ts (1)
  • QRLoginTokenRepository (6-48)
src/types/models/QRLoginToken.type.ts (1)
  • QRLoginToken (1-9)
src/repositories/qr.repository.ts (1)
src/types/models/QRLoginToken.type.ts (1)
  • QRLoginToken (1-9)
🪛 ESLint
src/services/__test__/qr.service.test.ts

[error] 13-13: Unexpected any. Specify a different type.

(@typescript-eslint/no-explicit-any)

🔇 Additional comments (9)
src/services/qr.service.ts (2)

8-12: 코드 구현이 잘 되었습니다.

UUID를 사용하여 토큰을 생성하고 저장하는 로직이 명확하게 구현되었습니다. 사용자 ID, IP 주소, 사용자 에이전트를 함께 저장하는 방식으로 보안성을 높였습니다.


14-16: getByToken 메소드가 간결하게 구현되었습니다.

리포지토리의 findQRLoginToken 메소드를 직접 호출하여 불필요한 중복 코드를 방지했습니다. 필요한 경우 나중에 비즈니스 로직을 추가할 수 있는 확장성도 갖추고 있습니다.

src/repositories/qr.repository.ts (2)

9-20: 안전한 SQL 쿼리 구현 👍

매개변수화된 쿼리를 사용하여 SQL 인젝션 공격을 방지하고, 토큰 만료 시간을 5분으로 설정하여 보안을 강화했습니다. 또한 적절한 예외 처리와 로깅을 구현했습니다.


22-35: 효과적인 토큰 검증 로직

만료되지 않고 사용되지 않은 토큰만 조회하도록 WHERE 절에 조건을 명시적으로 추가한 점이 좋습니다. null 처리도 명확하게 구현되었습니다.

src/services/__test__/qr.service.test.ts (3)

22-40: create 메소드에 대한 테스트가 철저하게 구현되었습니다.

성공 케이스와 실패 케이스 모두 테스트하고 있으며, UUID 형식에 대한 정규식 검증도 포함되어 있습니다. 이는 코드의 신뢰성을 높이는 좋은 방법입니다.


42-69: getByToken 메소드에 대한 테스트가 잘 작성되었습니다.

세 가지 시나리오(유효한 토큰, 존재하지 않는 토큰, 오류 발생)에 대한 테스트를 모두 포함하고 있습니다. 테스트 데이터도 적절하게 구성되어 있습니다.


71-111: useToken 메소드에 대한 테스트가 철저합니다.

토큰 사용 성공, 토큰 미발견, 오류 발생 시나리오를 모두 테스트하고 있습니다. 특히 markTokenUsed 호출 검증은 중요한 부분입니다.

src/controllers/qr.controller.ts (2)

19-35: cookieOption 메소드가 환경에 따라 적절히 구현되었습니다.

프로덕션 환경과 개발 환경에 따라 쿠키 옵션을 다르게 설정하는 것은 좋은 방식입니다. 특히 보안적인 측면(httpOnly, secure, sameSite)을 고려한 점이 좋습니다.


63-95: 에러 처리를 적절히 구현했습니다.

에러를 throw하여 명확하게 흐름을 제어하고, 에러 타입도 구체적으로 지정하여 클라이언트에게 적절한 응답을 제공할 수 있게 했습니다. 로깅도 체계적으로 구현되어 있습니다.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/services/user.service.ts (1)

126-128: 문서화 및 타입 명시 개선 필요

이 새로운 공개 메서드에는 JSDoc 문서화 및 명시적인 반환 타입이 누락되어 있습니다. 메서드의 용도, 매개변수 및 반환 값을 설명하는 문서를 추가하면 다른 개발자가 이 메서드를 올바르게 사용하는 데 도움이 됩니다.

다음과 같이 문서화와 타입을 개선할 수 있습니다:

+ /** + * 사용자의 액세스 토큰과 리프레시 토큰을 복호화합니다. + * @param userId 사용자 그룹 ID + * @param accessToken 암호화된 액세스 토큰 + * @param refreshToken 암호화된 리프레시 토큰 + * @returns 복호화된 액세스 토큰과 리프레시 토큰을 포함하는 객체 + * @throws {TokenError} 토큰 복호화 중 오류가 발생한 경우 + */ - public getDecryptedTokens(userId: number, accessToken: string, refreshToken: string) {  + public getDecryptedTokens(userId: number, accessToken: string, refreshToken: string): { decryptedAccessToken: string, decryptedRefreshToken: string } {  return this.decryptTokens(userId, accessToken, refreshToken); }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a007870 and a95635e.

📒 Files selected for processing (2)
  • src/controllers/qr.controller.ts (1 hunks)
  • src/services/user.service.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/controllers/qr.controller.ts
🔇 Additional comments (1)
src/services/user.service.ts (1)

126-128: 보안 고려사항

토큰 복호화 기능을 공개 메서드로 노출하면 보안 취약점이 생길 수 있습니다. 이 메서드가 QR 로그인 기능에 필요한 것으로 보이지만, 접근 제어 메커니즘을 추가하여 인증된 컨텍스트에서만 사용되도록 하는 것이 좋습니다.

이 메서드가 호출되는 컨트롤러 코드를 검토하여 적절한 인증 및 권한 검사가 이루어지는지 확인하세요.

Copy link
Member

@Nuung Nuung left a comment

Choose a reason for hiding this comment

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

좋았던 점

  1. 지현님 리얼 고생 많으셨어요. 그리고, 잘하셨어요!! node 처음 이신데 사실 여기까지 하고 계신 것만으로도 완전 박수 👏👏
  2. 테스트 케이스도 좋고, 비즈니스 로직에 대한 고민의 흔적, 컨트롤러에서 검증, 그리고 얘기 나눴던 user flow chart 흐름대로 바뀐 전체적인 구조, user domain 으로 취합 다 너무 좋았어요!!

아쉬운 점

  1. 아무래도 이건 node 가 익숙하지 않기때문에 발생하는 것들이 좀 있는 것 같아요. 아무래도 디테일한 부분이니까요 ㅎㅎ
  2. 그래서 저는 최대한 node 외에서도, 어떤 언어에서든 프로그래밍을 할 때 중요한 리뷰잉을 위주로 코멘트를 달아봤어요!!
@Jihyun3478
Copy link
Contributor Author

코드 잘 봤습니다! 코멘트 확인 부탁드릴게요! 대부분 자잘한 사항들인데, 스웨거 UI에서 테스트가 안 되어서 관련 부분은 수정 부탁드릴게요!

너무너무 고생하셨습니다!! 👍 🔥

좋았던 점

  • 브랜딩 타입을 통해 토큰 길이 체크를 하시는 부분이 새롭고 좋았어요!
  • generateRandomToken 유틸 함수 굿굿입니다 👍
  • 케이스 설명을 자세하고 명확하게 쓰셔서 테스트 코드가 눈에 잘 들어와요!
  • 리턴 타입 명시해주시는게 좋아요.

꼼꼼한 리뷰 감사드려요! 스웨거 관련 수정 완료했습니다!

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (2)
src/repositories/__test__/qr.repo.integration.test.ts (1)

33-35: 보안: SSL 인증서 검증 비활성화 문제

rejectUnauthorized: false 설정은 인증서 검증을 비활성화하여 중간자 공격(MITM)에 취약하게 만듭니다. 이는 테스트 환경에서만 사용되어야 하며, 프로덕션 환경에서는 절대 사용해서는 안 됩니다.

#!/bin/bash # 프로덕션 코드에서 SSL 인증서 검증 비활성화 설정 검사 # 보안 이슈가 프로덕션 코드에 존재하는지 확인 grep -r "rejectUnauthorized.*false" --include="*.ts" src/configs/
src/services/__test__/qr.service.test.ts (1)

25-25: TypeScript 타입 단언 방식 개선 필요

ESLint가 as 키워드 대신 타입 선언을 직접 사용하는 형태를 권장하고 있습니다.

- const mockPool = {} as jest.Mocked<Pool>; + const mockPool: jest.Mocked<Pool> = {};
🧰 Tools
🪛 ESLint

[error] 25-25: Always prefer const x: T = { ... }.

(@typescript-eslint/consistent-type-assertions)

🧹 Nitpick comments (6)
src/app.ts (2)

16-17: 보안 관련 고려사항 - 제한된 프록시 신뢰 설정 고려

현재 구현은 모든 프록시를 신뢰하도록 설정되어 있습니다(true). 프로덕션 환경에서는 특정 프록시만 신뢰하도록 설정하는 것이 더 안전할 수 있습니다.

- app.set('trust proxy', true); + // 신뢰할 수 있는 프록시 IP 주소 또는 서브넷 지정 + app.set('trust proxy', process.env.NODE_ENV === 'production' ? process.env.TRUSTED_PROXIES || '127.0.0.1' : true);

16-17: 환경별 설정 분리 제안

QR 코드 기능과 관련해 환경에 따라 다른 proxy 설정이 필요할 수 있습니다. 환경 변수를 통해 이 설정을 제어하는 것을 고려해보세요.

- // 실제 클라이언트 IP를 알기 위한 trust proxy 설정 - app.set('trust proxy', true); + // 실제 클라이언트 IP를 알기 위한 trust proxy 설정 + // 개발 환경에서는 단순히 true로, 프로덕션에서는 환경 변수로 제어 + app.set('trust proxy', process.env.TRUST_PROXY || true);
src/utils/generateRandomToken.util.ts (1)

3-4: 명명 규칙 준수를 위한 리팩토링이 필요합니다.

상수명이 camelCase 형식을 따르지 않아 정적 분석 도구에서 경고가 발생하고 있습니다. 프로젝트의 일관성을 위해 변수명을 camelCase로 변경하는 것이 좋겠습니다.

-const CHARSET = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_.~!'; -const CHARSET_LENGTH = CHARSET.length; +const charset = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_.~!'; +const charsetLength = charset.length;

11번 줄과 같은 사용 위치도 업데이트해야 합니다:

- result[i] = CHARSET[randomBytes[i] % CHARSET_LENGTH]; + result[i] = charset[randomBytes[i] % charsetLength];
🧰 Tools
🪛 GitHub Check: build-and-test (22)

[warning] 4-4:
Variable name CHARSET_LENGTH must match one of the following formats: camelCase


[warning] 3-3:
Variable name CHARSET must match one of the following formats: camelCase

🪛 GitHub Check: build-and-test (21)

[warning] 4-4:
Variable name CHARSET_LENGTH must match one of the following formats: camelCase


[warning] 3-3:
Variable name CHARSET must match one of the following formats: camelCase

🪛 GitHub Check: build-and-test (23)

[warning] 4-4:
Variable name CHARSET_LENGTH must match one of the following formats: camelCase


[warning] 3-3:
Variable name CHARSET must match one of the following formats: camelCase

🪛 GitHub Check: build-and-test (20)

[warning] 4-4:
Variable name CHARSET_LENGTH must match one of the following formats: camelCase


[warning] 3-3:
Variable name CHARSET must match one of the following formats: camelCase

src/repositories/__test__/qr.repo.integration.test.ts (2)

57-63: 지연 사용보다 비동기 작업 처리 개선 필요

setTimeout을 사용한 지연은 비동기 작업 관리의 안티패턴이며, 일관되지 않은 결과를 초래할 수 있습니다. 데이터베이스 작업 완료를 보장하기 위한 더 견고한 방법을 사용하세요.

- await new Promise(resolve => setTimeout(resolve, 1000)); -  - if (testPool) { - await testPool.end(); - } -  - await new Promise(resolve => setTimeout(resolve, 1000)); + if (testPool) { + await testPool.end(); + }

인위적인 지연이 필요하다면 코드에 그 이유를 명확히 주석으로 남기세요.


45-68: 테스트 데이터 정리 로직 개선

테스트 데이터 정리와 연결 종료 로직이 잘 구현되어 있지만, 추가적인 개선이 가능합니다. 현재 구현은 이전 리뷰 피드백을 반영해 테스트 데이터를 식별 가능한 필드로 정리하고 있습니다.

에러 처리를 더 구체적으로 하여 문제 디버깅을 용이하게 할 수 있습니다:

 } catch (error) { - logger.error('테스트 종료 중 오류:', error); + const errorMessage = error instanceof Error ? error.message : String(error); + logger.error(`테스트 종료 중 오류: ${errorMessage}`); + logger.debug(error); }
src/services/__test__/qr.service.test.ts (1)

85-102: 테스트 케이스가 두 가지 동작을 함께 검증합니다.

현재 테스트에서는 토큰 반환과 markTokenUsed 메서드 호출을 동시에 검증하고 있습니다. 단일 책임 원칙에 따라 테스트를 분리하면 테스트의 의도가 더 명확해집니다.

 it('유효한 토큰 사용 처리 후 반환', async () => { const mockToken: QRLoginToken = { token: 'token', user: 1, created_at: new Date(), expires_at: new Date(), is_used: false, ip_address: '127.0.0.1', user_agent: 'Chrome', }; repo.findQRLoginToken.mockResolvedValue(mockToken); const result = await service.useToken('token'); expect(result).toEqual(mockToken); expect(repo.markTokenUsed).toHaveBeenCalledWith('token'); }); + +it('토큰 사용 처리 시 markTokenUsed를 호출해야 한다', async () => { + const mockToken: QRLoginToken = { + token: 'specificToken', + user: 1, + created_at: new Date(), + expires_at: new Date(), + is_used: false, + ip_address: '127.0.0.1', + user_agent: 'Chrome', + }; + repo.findQRLoginToken.mockResolvedValue(mockToken); + + await service.useToken('specificToken'); + + expect(repo.markTokenUsed).toHaveBeenCalledWith('specificToken'); +});
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5613dbb and 5c8f0a9.

📒 Files selected for processing (7)
  • src/app.ts (1 hunks)
  • src/controllers/user.controller.ts (2 hunks)
  • src/repositories/__test__/qr.repo.integration.test.ts (1 hunks)
  • src/routes/user.router.ts (1 hunks)
  • src/services/__test__/qr.service.test.ts (1 hunks)
  • src/types/dto/responses/qrResponse.type.ts (1 hunks)
  • src/utils/generateRandomToken.util.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/routes/user.router.ts
  • src/types/dto/responses/qrResponse.type.ts
  • src/controllers/user.controller.ts
🧰 Additional context used
🪛 ESLint
src/repositories/__test__/qr.repo.integration.test.ts

[error] 83-83: Avoid calling expect conditionally`

(jest/no-conditional-expect)

src/services/__test__/qr.service.test.ts

[error] 25-25: Always prefer const x: T = { ... }.

(@typescript-eslint/consistent-type-assertions)

🪛 GitHub Check: build-and-test (22)
src/utils/generateRandomToken.util.ts

[warning] 4-4:
Variable name CHARSET_LENGTH must match one of the following formats: camelCase


[warning] 3-3:
Variable name CHARSET must match one of the following formats: camelCase

🪛 GitHub Check: build-and-test (21)
src/utils/generateRandomToken.util.ts

[warning] 4-4:
Variable name CHARSET_LENGTH must match one of the following formats: camelCase


[warning] 3-3:
Variable name CHARSET must match one of the following formats: camelCase

🪛 GitHub Check: build-and-test (23)
src/utils/generateRandomToken.util.ts

[warning] 4-4:
Variable name CHARSET_LENGTH must match one of the following formats: camelCase


[warning] 3-3:
Variable name CHARSET must match one of the following formats: camelCase

🪛 GitHub Check: build-and-test (20)
src/utils/generateRandomToken.util.ts

[warning] 4-4:
Variable name CHARSET_LENGTH must match one of the following formats: camelCase


[warning] 3-3:
Variable name CHARSET must match one of the following formats: camelCase

🔇 Additional comments (5)
src/app.ts (1)

16-17: 프록시 설정이 적절하게 추가되었습니다.

'trust proxy' 설정을 추가한 것은 프록시나 로드 밸런서 뒤에서 실행될 때 클라이언트의 실제 IP 주소를 정확하게 파악하기 위한 좋은 접근입니다. QR 로그인 토큰 기능에서 클라이언트 IP를 추적하는 데 필요한 설정입니다.

src/utils/generateRandomToken.util.ts (1)

6-15: 안전하고 효율적인 토큰 생성 구현입니다.

암호학적으로 안전한 crypto.randomBytes()를 사용하고, 문자열 연산보다 효율적인 배열 조작을 활용한 좋은 구현입니다. 또한 URL에서 안전하게 사용 가능한 특수 문자(-_.~!)를 포함시켜 보안성을 강화했습니다.

src/services/__test__/qr.service.test.ts (3)

20-33: 테스트 설정이 잘 구현되었습니다.

Mock 객체를 세팅하고 각 테스트 후에 정리하는 구조가 잘 구현되어 있습니다. 테스트 간 독립성이 유지되어 안정적인 테스트 수행이 가능합니다.

🧰 Tools
🪛 ESLint

[error] 25-25: Always prefer const x: T = { ... }.

(@typescript-eslint/consistent-type-assertions)


8-16: Mock 환경 설정이 적절히 구현되었습니다.

테스트에 필요한 모듈을 적절히 모킹하여 외부 의존성을 제거했습니다. 특히 key.util 모듈의 모킹은 테스트 실패 원인이었던 환경 변수 의존성 문제를 해결했습니다.


35-54: 토큰 생성 테스트가 포괄적으로 구현되었습니다.

토큰 생성과 유효성 검사, 에러 처리가 적절히 테스트되고 있습니다. 특히 정규식 패턴을 사용하여 토큰 형식을 검증한 부분이 잘 구현되었습니다.

@Jihyun3478
Copy link
Contributor Author

좋았던 점

  1. 지현님 리얼 고생 많으셨어요. 그리고, 잘하셨어요!! node 처음 이신데 사실 여기까지 하고 계신 것만으로도 완전 박수 👏👏
  2. 테스트 케이스도 좋고, 비즈니스 로직에 대한 고민의 흔적, 컨트롤러에서 검증, 그리고 얘기 나눴던 user flow chart 흐름대로 바뀐 전체적인 구조, user domain 으로 취합 다 너무 좋았어요!!

아쉬운 점

  1. 아무래도 이건 node 가 익숙하지 않기때문에 발생하는 것들이 좀 있는 것 같아요. 아무래도 디테일한 부분이니까요 ㅎㅎ
  2. 그래서 저는 최대한 node 외에서도, 어떤 언어에서든 프로그래밍을 할 때 중요한 리뷰잉을 위주로 코멘트를 달아봤어요!!

리뷰 감사합니다!! user 도메인과 합치는 부분에서 추가적으로 발생할 수 있는 이슈, 의도치 않았지만 운영 DB에 테스트 데이터가 삽입된 이슈(😭😱)까지 더 꼼꼼하고 확실하게 고려해야 한다는 점을 배운 것 같습니다. 항상 느끼지만 개선 방향성이 명확할 수 있도록 리뷰해주셔서 갈팡질팡 하지 않았던 것 같습니다.

PS. 어느덧 conversation이 90개에 가까워졌네요,, 의도치 않게 PR이 무거워졌습니다...!

Copy link
Contributor

@ooheunda ooheunda left a comment

Choose a reason for hiding this comment

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

추가로 코멘트 남겨드린 것만 수정되면 될 것 같아요!
고생하셨어요~!!! 🔥 👍

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (1)
src/services/__test__/qr.service.test.ts (1)

25-25: 타입스크립트 타입 단언 방법 수정 필요

as 문법 대신 명시적인 타입 선언을 사용하는 것이 좋습니다.

- const mockPool = {} as jest.Mocked<Pool>; + const mockPool: jest.Mocked<Pool> = {};
🧰 Tools
🪛 ESLint

[error] 25-25: Always prefer const x: T = { ... }.

(@typescript-eslint/consistent-type-assertions)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0769497 and c20e48e.

📒 Files selected for processing (3)
  • src/routes/user.router.ts (1 hunks)
  • src/services/__test__/qr.service.test.ts (1 hunks)
  • src/services/user.service.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/routes/user.router.ts
  • src/services/user.service.ts
🧰 Additional context used
🪛 ESLint
src/services/__test__/qr.service.test.ts

[error] 25-25: Always prefer const x: T = { ... }.

(@typescript-eslint/consistent-type-assertions)

⏰ Context from checks skipped due to timeout of 90000ms (4)
  • GitHub Check: build-and-test (23)
  • GitHub Check: build-and-test (22)
  • GitHub Check: build-and-test (21)
  • GitHub Check: build-and-test (20)
🔇 Additional comments (5)
src/services/__test__/qr.service.test.ts (5)

20-20: 테스트 설명과 파일명 일치에 관한 이전 피드백을 적용했네요!

이전 리뷰에서 언급된 것처럼 테스트 설명을 "UserService 의 QRService"로 명명하여 파일명과 테스트 목적의 관계를 명확히 했습니다. 이는 코드를 살펴보는 다른 개발자들이 직관적으로 역할을 이해하는 데 도움이 됩니다.


8-12: 환경 변수 관련 모킹 방식이 개선되었습니다

이전 리뷰에서 지적된 "Key must be 256 bits (32 bytes)" 에러 해결을 위해 암호화 키를 올바르게 모킹했습니다. crypto.randomBytes(32)를 사용하여 적절한 길이의 키를 생성하고, key.util을 모킹하여 이 키를 반환하도록 처리한 방식이 좋습니다.


14-16: 슬랙 알림 기능 모킹 개선

이전 리뷰에서 지적된 SLACK_WEBHOOK_URL 관련 문제를 해결하기 위해 슬랙 모듈을 적절히 모킹했습니다. 이렇게 하면 테스트가 실제 슬랙 서비스에 의존하지 않고 독립적으로 실행될 수 있습니다.


43-46: 토큰 생성 테스트가 잘 구현되었습니다

토큰의 타입, 길이, 포맷을 모두 검증하고 있으며, 레포지토리 호출이 올바른 인자로 이루어지는지 확인하는 방식이 좋습니다. 특히 정규표현식을 사용하여 토큰 형식을 검증하는 것은 훌륭한 접근입니다.


110-124: 에러 처리 테스트가 잘 구현되었습니다

markTokenUsed 메서드에서 발생하는 예외가 적절히 전파되는지 테스트하는 케이스를 포함한 것이 좋습니다. 이는 에러 처리 로직이 올바르게 동작하는지 확인하는 중요한 부분입니다.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

♻️ Duplicate comments (2)
src/services/__test__/qr.service.test.ts (2)

20-29: ESLint 경고: 타입 단언 방식 수정 필요

타입 단언 방식이 프로젝트의 ESLint 규칙에 맞지 않습니다. as 구문 대신 타입 선언을 사용하는 것이 좋습니다.

- const mockPool = {} as jest.Mocked<Pool>; + const mockPool: jest.Mocked<Pool> = {};
🧰 Tools
🪛 ESLint

[error] 25-25: Always prefer const x: T = { ... }.

(@typescript-eslint/consistent-type-assertions)


75-79: 🛠️ Refactor suggestion

서비스 계층 테스트로 수정 필요

이 테스트는 서비스 메서드 대신 저장소 메서드를 직접 테스트하고 있습니다. 서비스 계층 테스트의 일관성을 위해 service.useToken을 호출하도록 수정해야 합니다.

 repo.findQRLoginToken.mockResolvedValue(null); - const result = await service.useToken('token'); + const result = await service.useToken('token'); expect(result).toBeNull();
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c20e48e and 21764ee.

📒 Files selected for processing (3)
  • src/controllers/user.controller.ts (2 hunks)
  • src/services/__test__/qr.service.test.ts (1 hunks)
  • src/services/user.service.ts (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/services/user.service.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/controllers/user.controller.ts (3)
src/types/dto/responses/qrResponse.type.ts (1)
  • QRLoginTokenResponseDto (35-35)
src/exception/index.ts (2)
  • InvalidTokenError (3-3)
  • TokenExpiredError (3-3)
src/exception/token.exception.ts (2)
  • InvalidTokenError (16-20)
  • TokenExpiredError (10-14)
🪛 GitHub Check: build-and-test (22)
src/controllers/user.controller.ts

[failure] 7-7:
'NotFoundError' is defined but never used


[failure] 150-150:
'user' is assigned a value but never used

🪛 GitHub Check: build-and-test (23)
src/controllers/user.controller.ts

[failure] 7-7:
'NotFoundError' is defined but never used


[failure] 150-150:
'user' is assigned a value but never used

🪛 GitHub Check: build-and-test (21)
src/controllers/user.controller.ts

[failure] 7-7:
'NotFoundError' is defined but never used


[failure] 150-150:
'user' is assigned a value but never used

🪛 GitHub Check: build-and-test (20)
src/controllers/user.controller.ts

[failure] 7-7:
'NotFoundError' is defined but never used


[failure] 150-150:
'user' is assigned a value but never used

🪛 ESLint
src/controllers/user.controller.ts

[error] 7-7: 'NotFoundError' is defined but never used.

(@typescript-eslint/no-unused-vars)


[error] 150-150: 'user' is assigned a value but never used.

(@typescript-eslint/no-unused-vars)

src/services/__test__/qr.service.test.ts

[error] 25-25: Always prefer const x: T = { ... }.

(@typescript-eslint/consistent-type-assertions)

🪛 GitHub Actions: Test CI
src/controllers/user.controller.ts

[error] 7-7: ESLint: 'NotFoundError' is defined but never used. (@typescript-eslint/no-unused-vars)

🔇 Additional comments (7)
src/services/__test__/qr.service.test.ts (6)

1-19: 임포트 구문 및 모킹 설정이 적절하게 구성되어 있습니다

테스트에 필요한 모듈이 잘 임포트되어 있고, 암호화에 필요한 키 생성, 외부 의존성 모킹(key.util, slack.notifier, UserRepository)이 적절하게 설정되어 있습니다. 환경 변수 의존성을 제거하여 테스트 안정성을 높인 부분이 좋습니다.


31-33: 테스트 정리 로직이 올바르게 구현되어 있습니다

afterEach를 사용하여 각 테스트 후 모든 모킹을 정리하는 것은 테스트 간 상태 누수를 방지하는 좋은 습관입니다.


35-47: QR 토큰 생성 테스트가 잘 구현되어 있습니다

토큰 생성 로직을 테스트할 때 토큰 길이, 문자 패턴, 저장소 메서드 호출 여부를 모두 확인하는 점이 훌륭합니다. 정규식으로 토큰 형식을 검증하는 방식은 매우 효과적입니다.


49-53: 오류 처리 테스트가 적절히 구현되어 있습니다

서비스 계층에서 저장소의 예외가 올바르게 전파되는지 확인하는 테스트가 포함되어 있어 좋습니다.


57-73: 토큰 사용 처리 테스트가 잘 구현되어 있습니다

유효한 토큰의 사용 처리와 마킹 테스트가 잘 작성되어 있습니다. 모의 객체를 활용하여 QRLoginToken의 형태와 동작을 명확하게 테스트하고 있습니다.


81-95: 에러 전파 테스트가 적절하게 구현되어 있습니다

토큰 사용 처리 중 발생하는 예외가 적절히 전파되는지 테스트하고 있습니다. 실제 프로덕션 환경에서 발생할 수 있는 에러 시나리오를 포함하는 것은 좋은 테스트 관행입니다.

src/controllers/user.controller.ts (1)

112-136: QR 토큰 생성 처리가 적절히 구현되어 있습니다

클라이언트 IP 주소를 정확하게 추출하기 위해 x-forwarded-for 헤더를 올바르게 처리하고 있습니다. 또한 오류 로깅시 사용자 ID 컨텍스트를 포함하여 디버깅을 용이하게 한 점이 좋습니다.

@Jihyun3478 Jihyun3478 merged commit 408bfcf into main May 3, 2025
5 checks passed
@Jihyun3478 Jihyun3478 deleted the feature/qrcode-app branch May 3, 2025 05:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

5 participants