-
Notifications
You must be signed in to change notification settings - Fork 0
[25.04.21 / TASK-148] Feature - qrcode app & QRCode API #26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
""" Walkthrough이 변경 사항들은 QR 로그인 토큰 기능의 도입과 관련된 코드 추가 및 확장을 포함합니다. UserService와 UserRepository에 QR 로그인 토큰 생성, 조회, 사용 메서드가 추가되었으며, UserController에는 토큰 생성 및 사용을 처리하는 새로운 요청 핸들러가 도입되었습니다. QR 로그인 관련 API 엔드포인트가 user.router에 추가되었고, 이에 따른 DTO, 타입, 유틸리티 함수도 신설되었습니다. 또한, 서비스와 레포지토리의 동작을 검증하는 단위 및 통합 테스트가 새롭게 작성되었습니다. 기존 엔티티의 선언은 변경되지 않았습니다. Changes
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
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (4)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed 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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this 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 인터페이스 개선 제안인터페이스 설계는 전반적으로 잘 되어 있으나, 몇 가지 개선점이 있습니다:
user
필드의 이름은user_id
로 변경하는 것이 목적을 더 명확히 할 수 있습니다.- 각 필드에 대한 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
⛔ 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
latest
가11.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를 던지는 방식이 좋습니다.
src/controllers/qr.controller.ts
Outdated
res.status(404).json({ success: false, message: '유효하지 않거나 만료된 토큰입니다.' }); | ||
} | ||
|
||
res.status(200).json({ success: true, message: '유효한 QR 토큰입니다.', token: found }); |
There was a problem hiding this comment.
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.
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 | |
} | |
}); |
There was a problem hiding this 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
📒 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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 잘 읽었습니다!
제가 남겨드린 질문 하나 외에 큰 궁금증이나 단점은 없어보이네요~
좋았던 점
- 세세한 디테일들이 잘 되어있습니다. (?? 문법, undefined 값 처리 등)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수정 중이신 것 같아 업데이트 되면 다시 보겠습니다!
다른 분들이 리뷰 잘 해주셔서, 전 사소한 부분에 대해서만 몇 마디 얹어봤습니다... 😅
고생하셨습니다! 👍 🔥
좋았던 점
- 테스트 코드가 굉장히 깔끔해서 좋았습니다.
- 특히 uuid를 고정값으로 모킹하는 대신 정규표현식 +
toMatch()
로 확인하신게 인상 깊었어요!
비즈니스 로직 흐름에 맞게 수정 - 쿠키 값 확인 및 유효성 체크 - 쿠키 설정 및 자동 로그인 - 메인 페이지 리다이렉션
There was a problem hiding this 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
📒 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하여 명확하게 흐름을 제어하고, 에러 타입도 구체적으로 지정하여 클라이언트에게 적절한 응답을 제공할 수 있게 했습니다. 로깅도 체계적으로 구현되어 있습니다.
There was a problem hiding this 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
📒 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 로그인 기능에 필요한 것으로 보이지만, 접근 제어 메커니즘을 추가하여 인증된 컨텍스트에서만 사용되도록 하는 것이 좋습니다.
이 메서드가 호출되는 컨트롤러 코드를 검토하여 적절한 인증 및 권한 검사가 이루어지는지 확인하세요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
좋았던 점
- 지현님 리얼 고생 많으셨어요. 그리고, 잘하셨어요!! node 처음 이신데 사실 여기까지 하고 계신 것만으로도 완전 박수 👏👏
- 테스트 케이스도 좋고, 비즈니스 로직에 대한 고민의 흔적, 컨트롤러에서 검증, 그리고 얘기 나눴던 user flow chart 흐름대로 바뀐 전체적인 구조, user domain 으로 취합 다 너무 좋았어요!!
아쉬운 점
- 아무래도 이건 node 가 익숙하지 않기때문에 발생하는 것들이 좀 있는 것 같아요. 아무래도 디테일한 부분이니까요 ㅎㅎ
- 그래서 저는 최대한 node 외에서도, 어떤 언어에서든 프로그래밍을 할 때 중요한 리뷰잉을 위주로 코멘트를 달아봤어요!!
꼼꼼한 리뷰 감사드려요! 스웨거 관련 수정 완료했습니다! |
There was a problem hiding this 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 nameCHARSET_LENGTH
must match one of the following formats: camelCase
[warning] 3-3:
Variable nameCHARSET
must match one of the following formats: camelCase🪛 GitHub Check: build-and-test (21)
[warning] 4-4:
Variable nameCHARSET_LENGTH
must match one of the following formats: camelCase
[warning] 3-3:
Variable nameCHARSET
must match one of the following formats: camelCase🪛 GitHub Check: build-and-test (23)
[warning] 4-4:
Variable nameCHARSET_LENGTH
must match one of the following formats: camelCase
[warning] 3-3:
Variable nameCHARSET
must match one of the following formats: camelCase🪛 GitHub Check: build-and-test (20)
[warning] 4-4:
Variable nameCHARSET_LENGTH
must match one of the following formats: camelCase
[warning] 3-3:
Variable nameCHARSET
must match one of the following formats: camelCasesrc/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
📒 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
: 토큰 생성 테스트가 포괄적으로 구현되었습니다.토큰 생성과 유효성 검사, 에러 처리가 적절히 테스트되고 있습니다. 특히 정규식 패턴을 사용하여 토큰 형식을 검증한 부분이 잘 구현되었습니다.
리뷰 감사합니다!! user 도메인과 합치는 부분에서 추가적으로 발생할 수 있는 이슈, 의도치 않았지만 운영 DB에 테스트 데이터가 삽입된 이슈(😭😱)까지 더 꼼꼼하고 확실하게 고려해야 한다는 점을 배운 것 같습니다. 항상 느끼지만 개선 방향성이 명확할 수 있도록 리뷰해주셔서 갈팡질팡 하지 않았던 것 같습니다. PS. 어느덧 conversation이 90개에 가까워졌네요,, 의도치 않게 PR이 무거워졌습니다...! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
추가로 코멘트 남겨드린 것만 수정되면 될 것 같아요!
고생하셨어요~!!! 🔥 👍
There was a problem hiding this 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
📒 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
메서드에서 발생하는 예외가 적절히 전파되는지 테스트하는 케이스를 포함한 것이 좋습니다. 이는 에러 처리 로직이 올바르게 동작하는지 확인하는 중요한 부분입니다.
There was a problem hiding this 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
📒 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 컨텍스트를 포함하여 디버깅을 용이하게 한 점이 좋습니다.
🔥 변경 사항
🔥 진행 중인 사항
🏷 관련 이슈
#148
📸 스크린샷 (UI 변경 시 필수)
(변경 사항이 UI와 관련된 경우 스크린샷을 첨부해주세요.)
📌 체크리스트
Summary by CodeRabbit
Summary by CodeRabbit
신규 기능
테스트
문서화
유틸리티