[26.01.13 / TASK-275] Feature - brute force secure#49
Conversation
WalkthroughAuthRateLimitService를 추가하고 인증 및 오류 처리 미들웨어를 팩토리화하여 캐시 기반 rate-limit 주입을 가능하게 했으며, JWT 유효성 검사 유틸리티와 앱 팩토리(createApp(cache))로 애플리케이션 초기화를 변경했습니다. (≤50단어) Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AuthMW as Auth Middleware
participant RateLimit as AuthRateLimitService
participant Cache
participant ErrorMW as Error Handler
Client->>AuthMW: 요청 (Authorization: Bearer <token>)
AuthMW->>AuthMW: isValidJwtFormat / safeExtractPayload
AuthMW->>RateLimit: isIpBlocked(ip)
RateLimit->>Cache: get(key)
Cache-->>RateLimit: 기록 반환
alt IP 차단됨
RateLimit-->>AuthMW: blocked
AuthMW-->>Client: 429 Too Many Requests (Retry-After)
else 차단 아님
AuthMW->>AuthMW: 토큰 검증 로직
alt 토큰 유효
AuthMW-->>Client: 요청 진행 (인증 성공)
else 토큰 무효
AuthMW->>RateLimit: trackAuthFailure(ip)
RateLimit->>Cache: set/update(key, count, ttl)
Cache-->>RateLimit: 저장 완료
AuthMW->>ErrorMW: throw InvalidTokenError
ErrorMW->>RateLimit: (옵션) trackAuthFailure 호출
ErrorMW-->>Client: 401 Unauthorized
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60분 Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @src/app.ts:
- Around line 25-27: The app creates AuthRateLimitService before the cache is
ready because initCache() is called without await; change initializeServices()
(or wherever initCache() is invoked) to await initCache() so Redis is fully
initialized before constructing AuthRateLimitService and calling
setRateLimitService(authRateLimitService), ensuring the AuthRateLimitService
receives a ready cache instance.
In @src/utils/jwt.util.ts:
- Around line 26-35: Add an explicit Node.js engine requirement to package.json
to guarantee support for Buffer.from(..., 'base64url'): update or add the
"engines" field with a minimum version of ">=15.7.0" (or higher if desired) so
consumers and CI know the runtime requirement; reference the safeExtractPayload
function in src/utils/jwt.util.ts which relies on Buffer.from(..., 'base64url')
to ensure this runtime is available.
🧹 Nitpick comments (6)
.gitignore (1)
1-6: LGTM! 개발 과정에서 생성된 AI/문서 파일들을 적절하게 제외하고 있습니다.추가된 패턴들은 개발 과정에서 생성되는 LLM 관련 파일과 설계 문서를 git에서 제외하는 합리적인 변경입니다.
선택적 개선사항: 패턴 명확성 향상
디렉토리를 의도한 경우 trailing slash를 추가하고, 하위 디렉토리에서도 매칭이 필요한 경우 와일드카드를 고려할 수 있습니다:
# LLM 관련 파일들 -.serena -.claude -CLAUDE.md -SYSTEM_DESIGN.md -plan.md +.serena/ +.claude/ +**/CLAUDE.md +**/SYSTEM_DESIGN.md +**/plan.md현재 패턴도 동작하지만, 위와 같이 수정하면:
.serena/,.claude/: 디렉토리만 명확하게 매칭**/파일명.md: 저장소의 모든 레벨에서 해당 파일 매칭src/services/__test__/authRateLimit.service.test.ts (2)
75-88: 윈도우 시간 테스트에서 CONFIG 값과의 결합도를 확인하세요.테스트 주석에 "5분 = 300초"로 명시되어 있으나, 실제
CONFIG.WINDOW_SECONDS가 변경되면 이 테스트가 의도와 다르게 동작할 수 있습니다. 현재는 6분(360초)으로 설정되어 올바르게 동작하지만, CONFIG 상수를 export하여 테스트에서 참조하는 방식을 고려해 볼 수 있습니다.
90-102: TTL 값 900이 하드코딩되어 있습니다.
CONFIG.LOCKOUT_SECONDS가 변경되면 이 테스트가 실패할 수 있습니다. 서비스 코드의 CONFIG를 export하거나, 테스트 내에서 상수로 정의하여 유지보수성을 높일 수 있습니다.src/services/authRateLimit.service.ts (1)
9-14: CONFIG 상수에 대한 문서화를 고려해주세요.
WINDOW_SECONDS(5분)와LOCKOUT_SECONDS(15분)의 차이가 의도적인 설계인지 명확하지 않습니다. 윈도우 시간 내 실패 횟수를 누적하고, 임계값 도달 시 더 긴 잠금 시간을 적용하는 것으로 보입니다. 간단한 주석으로 이 설계 의도를 명시하면 유지보수에 도움이 됩니다.📝 설계 의도 주석 추가 제안
const CONFIG = { KEY_PREFIX: 'auth-failure:', - FAILURE_THRESHOLD: 5, - WINDOW_SECONDS: 5 * 60, - LOCKOUT_SECONDS: 15 * 60, + FAILURE_THRESHOLD: 5, // 차단 임계값 + WINDOW_SECONDS: 5 * 60, // 실패 횟수 누적 윈도우 (5분) + LOCKOUT_SECONDS: 15 * 60, // 임계값 도달 후 차단 유지 시간 (15분) };src/middlewares/errorHandling.middleware.ts (1)
11-14:trackAuthFailure호출의 에러 처리가 누락되었습니다.
trackAuthFailure가 실패할 경우(예: Redis 연결 오류) 에러가 catch되지 않고 전파될 수 있습니다.AuthRateLimitService.trackAuthFailure내부에서 에러를 catch하고 있지만, 미들웨어 레벨에서도 방어적으로 처리하는 것이 좋습니다.♻️ 권장 수정안
// InvalidTokenError 발생 시 인증 실패 추적 if (err instanceof InvalidTokenError && authRateLimitService) { const ip = req.ip || req.socket?.remoteAddress || 'unknown'; - await authRateLimitService.trackAuthFailure(ip); + try { + await authRateLimitService.trackAuthFailure(ip); + } catch (trackError) { + // fail-open: 추적 실패해도 에러 응답은 계속 진행 + // trackAuthFailure 내부에서 이미 로깅하므로 추가 로깅 불필요 + } }src/middlewares/auth.middleware.ts (1)
11-11:LOCKOUT_SECONDS상수가 중복 정의되어 있습니다.
auth.middleware.ts의LOCKOUT_SECONDS와authRateLimit.service.ts의CONFIG.LOCKOUT_SECONDS가 별도로 정의되어 있습니다. 현재는 두 값이 동기화되어 있지만, 향후 유지보수 과정에서 한쪽만 수정될 위험이 있습니다.♻️ 권장 수정안
공통 설정 파일에서 상수를 관리하거나,
authRateLimit.service.ts에서 설정 값을 export하여 중복을 제거할 수 있습니다:// 옵션 1: authRateLimit.service.ts에서 export export const AUTH_RATE_LIMIT_CONFIG = { LOCKOUT_SECONDS: 15 * 60, // ... }; // 옵션 2: 공통 설정 파일 생성 // src/configs/authRateLimit.config.ts export const LOCKOUT_SECONDS = 15 * 60;
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.gitignoresrc/app.tssrc/middlewares/__test__/auth.middleware.test.tssrc/middlewares/__test__/errorHandling.middleware.test.tssrc/middlewares/auth.middleware.tssrc/middlewares/errorHandling.middleware.tssrc/services/__test__/authRateLimit.service.test.tssrc/services/authRateLimit.service.tssrc/utils/__test__/jwt.util.test.tssrc/utils/jwt.util.ts
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2024-12-04T14:05:58.537Z
Learnt from: HA0N1
Repo: Check-Data-Out/velog-dashboard-v2-api PR: 5
File: src/middlewares/error-handling.middleware.ts:1-2
Timestamp: 2024-12-04T14:05:58.537Z
Learning: `src/middlewares/error-handling.middleware.ts` 파일의 에러 핸들링 미들웨어에서 `NextFunction`을 사용하지 않으며, `err`은 커스텀 에러로 사용되므로 `NextFunction`과 `ErrorRequestHandler`를 임포트할 필요가 없습니다.
Applied to files:
src/middlewares/errorHandling.middleware.tssrc/middlewares/__test__/errorHandling.middleware.test.tssrc/app.ts
📚 Learning: 2024-12-04T13:59:57.198Z
Learnt from: HA0N1
Repo: Check-Data-Out/velog-dashboard-v2-api PR: 5
File: src/middlewares/error-handling.middleware.ts:4-7
Timestamp: 2024-12-04T13:59:57.198Z
Learning: 프로젝트에서 `Express` 네임스페이스에 `CustomError` 타입이 정의되어 있습니다.
Applied to files:
src/middlewares/errorHandling.middleware.tssrc/middlewares/__test__/errorHandling.middleware.test.ts
📚 Learning: 2024-12-04T13:28:34.692Z
Learnt from: HA0N1
Repo: Check-Data-Out/velog-dashboard-v2-api PR: 5
File: src/types/dto/user-with-token.dto.ts:3-26
Timestamp: 2024-12-04T13:28:34.692Z
Learning: In `src/types/dto/user-with-token.dto.ts`, since the project is using Express instead of NestJS, `nestjs/swagger` cannot be used. Documentation will be implemented later using an Express-specific Swagger library.
Applied to files:
src/middlewares/__test__/auth.middleware.test.ts
📚 Learning: 2024-12-04T13:26:58.075Z
Learnt from: HA0N1
Repo: Check-Data-Out/velog-dashboard-v2-api PR: 5
File: src/middlewares/auth.middleware.ts:116-117
Timestamp: 2024-12-04T13:26:58.075Z
Learning: 'velog-dashboard-v2-api' 코드베이스에서는 `src/types/express.d.ts` 파일에서 Express의 `Request` 인터페이스를 확장하여 `user`와 `tokens` 속성을 추가하였습니다.
Applied to files:
src/middlewares/__test__/auth.middleware.test.tssrc/middlewares/auth.middleware.ts
📚 Learning: 2024-12-06T14:29:50.385Z
Learnt from: HA0N1
Repo: Check-Data-Out/velog-dashboard-v2-api PR: 6
File: src/controllers/user.controller.ts:11-12
Timestamp: 2024-12-06T14:29:50.385Z
Learning: TypeScript Express 프로젝트에서, `Express.Request` 인터페이스는 전역으로 확장되어 `user`와 `tokens` 프로퍼티를 포함합니다. `user` 프로퍼티는 `VelogUserLoginResponse | VelogUserVerifyResponse` 타입이고, `tokens`는 `accessToken`과 `refreshToken`을 가진 객체입니다.
Applied to files:
src/middlewares/__test__/auth.middleware.test.tssrc/middlewares/auth.middleware.ts
📚 Learning: 2024-11-29T14:21:32.376Z
Learnt from: HA0N1
Repo: Check-Data-Out/velog-dashboard-v2-api PR: 4
File: src/middlewares/auth.middleware.ts:48-49
Timestamp: 2024-11-29T14:21:32.376Z
Learning: `auth.middleware.ts` 파일에서 `VELOG_API_URL`은 개발 및 배포 환경 모두에서 고정된 값으로, 환경 변수로 이동하지 않고 상수로 설정하여 가독성을 높이는 것이 좋습니다.
Applied to files:
src/app.tssrc/middlewares/auth.middleware.ts
📚 Learning: 2024-12-06T12:07:05.887Z
Learnt from: HA0N1
Repo: Check-Data-Out/velog-dashboard-v2-api PR: 6
File: src/reposities/user.repository.ts:0-0
Timestamp: 2024-12-06T12:07:05.887Z
Learning: `src/services/user.service.ts`에서 `VelogUserLoginDto`를 사용하여 이미 유효성 검증을 진행하므로, `src/repositories/user.repository.ts`의 `createUser` 함수에서는 추가적인 유효성 검증이 필요하지 않다.
Applied to files:
src/middlewares/auth.middleware.ts
🧬 Code graph analysis (7)
src/services/__test__/authRateLimit.service.test.ts (2)
src/services/authRateLimit.service.ts (1)
AuthRateLimitService(16-61)src/modules/cache/cache.type.ts (1)
ICache(23-80)
src/services/authRateLimit.service.ts (2)
src/configs/cache.config.ts (1)
cache(17-17)src/modules/cache/cache.type.ts (1)
ICache(23-80)
src/middlewares/errorHandling.middleware.ts (5)
src/services/authRateLimit.service.ts (1)
AuthRateLimitService(16-61)src/exception/index.ts (2)
InvalidTokenError(6-6)CustomError(1-1)src/exception/token.exception.ts (1)
InvalidTokenError(17-21)src/exception/custom.exception.ts (1)
CustomError(1-10)src/utils/logging.util.ts (1)
logError(51-85)
src/utils/__test__/jwt.util.test.ts (1)
src/utils/jwt.util.ts (2)
isValidJwtFormat(9-18)safeExtractPayload(26-35)
src/middlewares/__test__/errorHandling.middleware.test.ts (2)
src/services/authRateLimit.service.ts (1)
AuthRateLimitService(16-61)src/middlewares/errorHandling.middleware.ts (1)
createErrorHandlingMiddleware(8-40)
src/middlewares/__test__/auth.middleware.test.ts (3)
src/middlewares/auth.middleware.ts (2)
authMiddleware(154-159)createAuthMiddleware(133-138)src/services/authRateLimit.service.ts (1)
AuthRateLimitService(16-61)src/utils/fixtures.ts (1)
mockUser(28-41)
src/middlewares/auth.middleware.ts (4)
src/services/authRateLimit.service.ts (1)
AuthRateLimitService(16-61)src/exception/token.exception.ts (1)
InvalidTokenError(17-21)src/utils/jwt.util.ts (2)
isValidJwtFormat(9-18)safeExtractPayload(26-35)src/types/index.ts (1)
VelogJWTPayload(3-3)
🪛 Gitleaks (8.30.0)
src/middlewares/__test__/auth.middleware.test.ts
[high] 316-316: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build-and-test (21)
- GitHub Check: build-and-test (23)
- GitHub Check: build-and-test (20)
- GitHub Check: build-and-test (22)
🔇 Additional comments (26)
src/services/__test__/authRateLimit.service.test.ts (4)
1-12: LGTM! 모킹 설정이 적절합니다.로거 모킹이
warn,error,info메서드를 모두 포함하고 있어 테스트 격리가 잘 되어 있습니다.
18-34: LGTM! ICache 인터페이스 전체를 모킹하여 테스트 격리가 잘 되어 있습니다.
beforeEach에서 각 테스트마다 새로운 mock을 생성하고,afterEach에서jest.clearAllMocks()로 정리하여 테스트 간 간섭을 방지합니다.
119-125: Fail-open 동작 테스트가 잘 작성되었습니다.캐시 에러 발생 시 예외를 던지지 않고 정상 종료하는 fail-open 패턴을 검증하여 가용성을 보장합니다. 보안 기능이 캐시 장애로 인해 서비스 전체를 중단시키지 않도록 하는 올바른 설계입니다.
161-167: Fail-open 시 false 반환 검증이 적절합니다.
isIpBlocked가 캐시 에러 시false를 반환하여 정상 사용자의 접근을 차단하지 않습니다. 보안과 가용성 간의 적절한 균형입니다.src/services/authRateLimit.service.ts (3)
19-40: Fail-open 패턴이 잘 구현되었습니다.캐시 오류 발생 시 예외를 던지지 않고 로깅만 수행하여 서비스 가용성을 보장합니다. 보안 기능이 인프라 장애로 전체 서비스를 중단시키지 않도록 하는 올바른 설계입니다.
42-51: LGTM! isIpBlocked 구현이 명확합니다.
record !== null && record.count >= CONFIG.FAILURE_THRESHOLD조건으로 명확하게 차단 여부를 판단하며, 에러 시false반환으로 fail-open 동작을 보장합니다.
53-60: LGTM! clearFailures 구현이 적절합니다.성공적인 인증 후 실패 기록을 초기화하는 데 사용될 수 있습니다. 에러 처리도 일관되게 적용되어 있습니다.
src/utils/jwt.util.ts (2)
1-1: LGTM! Base64URL 정규식이 RFC 4648 표준에 부합합니다.
/^[A-Za-z0-9_-]+$/패턴이 Base64URL 인코딩의 허용 문자를 정확하게 검증합니다.
9-18: Fail-fast 검증이 잘 구현되었습니다.DB 쿼리 전 빠른 형식 검증으로 잘못된 토큰을 조기에 거부합니다. 각 검증 단계가 명확하고 순차적으로 구성되어 있습니다.
src/utils/__test__/jwt.util.test.ts (2)
1-55: LGTM! isValidJwtFormat 테스트가 포괄적으로 작성되었습니다.null, undefined, 빈 문자열, 비문자열 타입, 점 개수, 빈 파트, Base64URL 문자셋 검증 등 모든 경계 조건을 테스트합니다.
57-102: LGTM! safeExtractPayload 테스트가 잘 구성되었습니다.유효한 페이로드 추출, 형식 오류, 손상된 Base64, 잘못된 JSON, 제네릭 타입 지정 등 다양한 시나리오를 검증합니다. 특히 Line 86-101의 제네릭 타입 테스트는 TypeScript 타입 안전성을 확인하는 좋은 예시입니다.
src/middlewares/__test__/errorHandling.middleware.test.ts (5)
1-23: LGTM! 모킹 설정이 적절합니다.logger, logError 유틸리티, Sentry를 모킹하여 테스트 격리를 보장합니다.
55-84: LGTM! 팩토리 함수의 선택적 인자 처리가 잘 테스트되었습니다.
AuthRateLimitService없이도 기존 동작이 유지되는지 검증하여 하위 호환성을 보장합니다.
87-131: 인증 실패 추적 연동 테스트가 포괄적입니다.
InvalidTokenError에 대해서만trackAuthFailure가 호출되고, 다른 에러나 서비스 미주입 시에는 호출되지 않는 것을 검증합니다. 이는 PR 목표인 brute force 방어 로직과 정확히 일치합니다.
133-161: Sentry 연동 테스트가 올바르게 구현되었습니다.500 에러(일반 Error)는 Sentry로 전송하고, CustomError(400번대)는 클라이언트 에러이므로 Sentry로 전송하지 않는 것이 적절합니다. 불필요한 Sentry 노이즈를 방지합니다.
163-211: 응답 형식 테스트가 잘 작성되었습니다.상태 코드(404, 500)와 응답 본문 구조(
success,message,error)를 명확하게 검증합니다.src/middlewares/__test__/auth.middleware.test.ts (3)
42-42: 테스트용 JWT 토큰 사용은 허용됩니다.Gitleaks가 JWT 토큰을 감지했지만, 이 토큰들은 테스트 목적으로만 사용되는 더미 토큰입니다. 페이로드에 민감한 정보가 포함되어 있지 않고, 테스트 환경에서만 사용되므로 보안 위험이 없습니다. 이 토큰들이 실제 프로덕션 환경의 시크릿이 아님을 확인하시기 바랍니다.
Also applies to: 316-316
207-306: JWT 형식 검증 테스트가 잘 구성되어 있습니다.fail-first 패턴에 대한 테스트가 포괄적으로 작성되었습니다:
- JWT 형식이 아닌 토큰
- 손상된 Base64 페이로드
- 잘못된 Base64URL 문자
- 유효하지 않은 UUID 형식
모든 테스트에서
pool.query가 호출되지 않음을 검증하여 DB 부하 방지 목표를 잘 테스트하고 있습니다.
421-441: fail-open 동작 테스트가 중요한 가용성 보장을 검증합니다.Redis 오류 발생 시에도 인증이 계속 진행되는 fail-open 동작을 테스트하는 것은 매우 중요합니다. 이로써 rate limit 서비스 장애가 전체 인증 시스템을 다운시키지 않도록 보장합니다.
src/app.ts (1)
96-96: 에러 핸들링 미들웨어 팩토리 패턴이 적절하게 적용되었습니다.동일한
authRateLimitService인스턴스가 인증 미들웨어와 에러 핸들링 미들웨어에 공유되어 일관된 실패 추적이 가능합니다.src/middlewares/errorHandling.middleware.ts (2)
16-22: CustomError 처리 흐름이 올바르게 구현되었습니다.
res.status()와res.json()이 분리되어 있고,logError가 적절한 시점에 호출됩니다.return문으로 명시적 종료가 보장됩니다.
42-43: 하위 호환성이 잘 유지되었습니다.기존
errorHandlingMiddlewareexport를 유지하여 기존 코드에서 수정 없이 사용할 수 있습니다. 새로운 기능이 필요한 경우에만 팩토리 함수를 사용하면 됩니다.src/middlewares/auth.middleware.ts (4)
39-56: Rate limit 체크 로직이 적절하게 구현되었습니다.
- IP 추출 시 폴백 체인(
req.ip→req.socket?.remoteAddress→'unknown')이 적절합니다.- fail-open 패턴으로 Redis 장애 시에도 인증이 계속 진행됩니다.
- 429 응답에
retryAfter정보를 포함하여 클라이언트에 유용한 정보를 제공합니다.
64-77: fail-first JWT 검증 패턴이 올바르게 구현되었습니다.검증 순서가 적절합니다:
- JWT 형식 검증 (
isValidJwtFormat)- 안전한 페이로드 추출 (
safeExtractPayload)- UUID 형식 검증 (
isUUID)- DB 조회 (모든 검증 통과 후에만)
이 패턴으로 유효하지 않은 토큰으로 인한 불필요한 DB 부하를 효과적으로 방지합니다.
133-138: 팩토리 함수가 깔끔하게 구현되었습니다.테스트에서 명시적 의존성 주입이 가능하고, 전역 상태에 의존하지 않는 미들웨어 생성이 가능합니다.
verifySignature는 rate limiting과 무관하므로 별도로 처리하는 것이 적절합니다.
154-157:verifygetter는 라우터 설정 시점에만 호출되므로 실제 성능 영향은 미미합니다.
authMiddleware.verify는 라우터 초기화 중 각 경로마다 한 번만 접근됩니다. Express는 등록된 미들웨어 함수 참조를 캐시하므로 이후 요청들은 동일한 함수 인스턴스를 재사용합니다. 따라서 반복되는 함수 생성은 실제로 발생하지 않습니다.현재 구현의 getter 패턴은
setRateLimitService호출 후 최신 서비스를 주입할 수 있도록 하는 의도된 설계이며, 성능상 문제가 없으므로 이대로 유지하시기 바랍니다.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/app.ts:
- Around line 37-38: The current use of app.set('trust proxy',
process.env.NODE_ENV === 'production') sets trust proxy to boolean true in
production which trusts all X-Forwarded-* headers; change it to a more
restrictive value by setting app.set('trust proxy', process.env.NODE_ENV ===
'production' ? 1 : 0) so only one proxy hop is trusted in typical nginx setups,
or alternatively supply an explicit array or CIDR list when using multiple known
proxies (refer to the app.set('trust proxy', ...) call to locate the change).
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.gitignorepackage.jsonrun.shsrc/app.tssrc/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- .gitignore
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-12-04T13:26:58.075Z
Learnt from: HA0N1
Repo: Check-Data-Out/velog-dashboard-v2-api PR: 5
File: src/middlewares/auth.middleware.ts:116-117
Timestamp: 2024-12-04T13:26:58.075Z
Learning: 'velog-dashboard-v2-api' 코드베이스에서는 `src/types/express.d.ts` 파일에서 Express의 `Request` 인터페이스를 확장하여 `user`와 `tokens` 속성을 추가하였습니다.
Applied to files:
src/app.ts
📚 Learning: 2024-11-29T14:21:32.376Z
Learnt from: HA0N1
Repo: Check-Data-Out/velog-dashboard-v2-api PR: 4
File: src/middlewares/auth.middleware.ts:48-49
Timestamp: 2024-11-29T14:21:32.376Z
Learning: `auth.middleware.ts` 파일에서 `VELOG_API_URL`은 개발 및 배포 환경 모두에서 고정된 값으로, 환경 변수로 이동하지 않고 상수로 설정하여 가독성을 높이는 것이 좋습니다.
Applied to files:
src/app.ts
🧬 Code graph analysis (2)
src/index.ts (3)
src/configs/cache.config.ts (2)
initCache(23-34)cache(17-17)src/configs/db.config.ts (1)
initializeDatabase(37-73)src/app.ts (1)
createApp(30-107)
src/app.ts (10)
src/configs/cache.config.ts (2)
cache(17-17)getCacheStatus(48-62)src/modules/cache/cache.type.ts (1)
ICache(23-80)src/services/authRateLimit.service.ts (1)
AuthRateLimitService(16-61)src/middlewares/auth.middleware.ts (1)
setRateLimitService(144-146)src/configs/swagger.config.ts (1)
options(1-38)src/middlewares/accessLog.middleware.ts (1)
accessLogMiddleware(8-21)src/configs/sentry.config.ts (1)
getSentryStatus(39-41)src/exception/index.ts (1)
NotFoundError(12-12)src/exception/notFound.exception.ts (1)
NotFoundError(3-7)src/middlewares/errorHandling.middleware.ts (1)
createErrorHandlingMiddleware(8-40)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build-and-test (21)
- GitHub Check: build-and-test (22)
- GitHub Check: build-and-test (23)
- GitHub Check: build-and-test (20)
🔇 Additional comments (12)
package.json (1)
37-39: LGTM! Node.js 버전 제약 조건 추가Node.js 20 LTS 버전 이상을 요구하는 것은 보안 기능과 최신 API 지원을 위해 적절한 선택입니다.
src/app.ts (4)
30-35: 팩토리 패턴과 Rate Limit 서비스 통합이 잘 구현되었습니다.캐시 주입을 통한
AuthRateLimitService생성과 전역 설정이 적절합니다. 이 패턴은 테스트 용이성을 높이고 분산 환경에서의 rate limiting을 지원합니다.
59-90: 헬스체크 엔드포인트 구현이 적절합니다.각 서비스 상태를 개별적으로 확인하고 오류 처리가 잘 되어 있습니다. 다만, 캐시 서비스가 다운된 상태에서도 200을 반환하므로, 로드 밸런서가 이 동작을 인식하고 있는지 확인하세요. Rate limiting이 Redis에 의존하므로 캐시 장애 시 보안 기능이 저하될 수 있습니다.
99-106: 404 핸들러와 에러 처리 미들웨어 통합이 올바릅니다.
next()를 사용한 에러 전파와authRateLimitService주입이 적절합니다.InvalidTokenError발생 시 인증 실패가 추적되어 brute force 방어가 동작합니다.
53-53: CORS 설정이 API의 실제 사용 메서드와 일치합니다.현재 API는 GET과 POST 메서드만 사용하므로 CORS 구성이 적절합니다. 모든 엔드포인트(사용자 인증, 통계 조회, 리더보드, 웹훅)가 이 두 메서드로 충분하며, 대시보드의 읽기 전용 특성과 일치합니다.
src/index.ts (3)
117-127: 초기화 순서가 올바르게 구성되었습니다.캐시 초기화 완료 후 Express 앱을 생성하는 순서가 적절합니다. 이로 인해
AuthRateLimitService가 항상 유효한 캐시 인스턴스를 받게 됩니다.
117-119: 캐시 연결 실패 시 보안 기능 저하에 대한 고려
initCache()는 연결 실패 시에도 예외를 던지지 않고 애플리케이션이 계속 실행됩니다(관련 코드cache.config.ts참조). 이 경우 rate limiting이 동작하지 않아 brute force 공격에 취약해집니다.가용성과 보안 간의 트레이드오프를 인지하고 계신다면 현재 구현도 괜찮습니다. 보안을 우선시한다면 캐시 연결 실패 시 서버 시작을 차단하는 것을 고려하세요.
133-150: 서버 생성 로직이 안전하게 구현되었습니다.앱 초기화 여부를 확인하는 가드 절이 있어 런타임 오류를 방지합니다.
run.sh (4)
4-8: Bash strict mode 적용이 좋습니다.
errexit,nounset,pipefail설정으로 스크립트의 안정성과 오류 감지가 향상됩니다.
81-109: Exponential backoff을 적용한 헬스체크 함수가 잘 구현되었습니다.재시도 로직, 최대 대기 시간 제한(30초), curl 타임아웃 설정이 적절합니다. 서비스 시작 시간이 불규칙한 컨테이너 환경에 적합한 패턴입니다.
111-145: 서비스 상태 확인 로직이 적절합니다.각 서비스에 대해 개별 헬스체크를 수행하고 종합 상태를 보고합니다. 헬스체크 실패 시에도 스크립트가 계속 진행되어 로그 모니터링으로 이어지는 것이 적절합니다.
148-171: 배포 스크립트의 실행 흐름이 명확합니다.단계별 실행과 최종 상태 출력이 잘 구성되어 있습니다.
main "$@"로 향후 CLI 인자 확장도 가능합니다.
| readonly GREEN='\033[0;32m' | ||
| readonly RED='\033[0;31m' | ||
| readonly NC='\033[0m' # No Color | ||
| readonly YELLOW='\033[1;33m' |
There was a problem hiding this comment.
여기서 수정된 readonly도 안정성에 영향을 끼치는 부분일까요??
There was a problem hiding this comment.
재할당(값 변경) 불가능! 상수화 하는 문법입니다! 임뮤터블하게 만들었어요!
ooheunda
left a comment
ooheunda
left a comment
There was a problem hiding this comment.
코드 잘 봤습니다. 코멘트는 참고만 해주시면 될 것 같습니다. 고생 많으셨습니다~!! 👍🔥
추가로 궁금한 것은, 이렇게 Redis에서 IP 블랙리스트?를 관리하는게 현업에서도 많이 쓰이는 방식일까요? Ops단에서 할 것 같기도 하고요... 어쨌든 새로운 걸 알아가는 것 같습니다! 노션에 써주신 부분으로 빠르게 이해할 수 있었습니다.
좋았던 점
- 인증 미들웨어 파일에 있는 Extract 코드가 옛날에 좀 거슬렸던 부분이었는데 이번에 안정성있게 리팩토링 되어서 좋습니다!
- 에러 핸들링 미들웨어를 수정하시면서 하위 호환성까지 고려하신 부분이 인상깊었습니다.
- 전체적으로
AuthRateLimitService의 초기화 시점을 고려하신 부분이 코드에 드러나서 좋았습니다.
| // Fail-Fast: JWT 형식 검증 | ||
| if (!isValidJwtFormat(accessToken)) { | ||
| throw new InvalidTokenError('유효하지 않은 JWT 형식입니다.'); | ||
| } | ||
|
|
||
| // 안전한 페이로드 추출 (JSON 파싱 실패 시 null 반환) | ||
| const payload = safeExtractPayload<VelogJWTPayload>(accessToken); | ||
| if (!payload) { | ||
| throw new InvalidTokenError('토큰 페이로드를 추출할 수 없습니다.'); | ||
| } |
There was a problem hiding this comment.
safeExtractPayload 안에서 isValidJwtFormat 을 호출하고 있어 JWT 형식에 있어서 중복 검증이 될 것 같은데 의도하신 부분일까요?
There was a problem hiding this comment.
@ooheunda 오 놓친 부분 발견해주셔서 감사해요!@#@!#@! 짱짱
첫 설계시 safeExtractPayload 를 isValidJwtFormat 를 포함하는 큰 집합으로 하려고 헀는데, 나중에는 safeExtractPayload 분리 시켰었거든요!! jwt.util 에서 이게 빠졌네요! 업데이트 갑니다!!
| } | ||
| } | ||
|
|
||
| async clearFailures(ip: string): Promise<void> { |
There was a problem hiding this comment.
저는 이 부분을 일정 횟수 실패하다가 성공했을 경우에 캐시를 제거하는 로직이나 단순 내부 테스트용 로직 정도로 이해했는데, 혹시 맞을까요??
There was a problem hiding this comment.
넵 기준님 얘기가 맞습니다. (1) 실패 반복 하다 성공 -> clear (2) E2E 테스팅 할때 강제 비움 목적입니다!
src/middlewares/auth.middleware.ts
Outdated
| /** | ||
| * 인증 미들웨어 팩토리 함수 | ||
| * @param rateLimitService - Rate limit 서비스 (선택적) | ||
| * @returns verify, verifySignature 미들웨어를 포함한 객체 | ||
| */ | ||
| export const createAuthMiddleware = (rateLimitService?: AuthRateLimitService) => { | ||
| return { | ||
| verify: verifyBearerTokens(rateLimitService), | ||
| verifySignature: verifySentrySignature(), | ||
| }; | ||
| }; |
There was a problem hiding this comment.
기존에 사용하던 authMiddleware 변수에 verify를 getter 형식으로 바꿔 rateLimitService를 넣어주는 수정사항과 이 함수의 역할이 겹치는 것처럼 보여서 어떤 케이스를 생각하고 넣으셨는지 궁금합니다!
There was a problem hiding this comment.
(export를 안하는) verifyBearerTokens 를 test 에서 mocking 없이 바로 사용하려고, 어답터(또는 팩토리) 처럼 쓰려고 넣었던 친구인데, 굳이 싶긴하네요! 테스트 먼저 작성하다가 생긴 혼돈 같아요! 빼는게 나을듯!! 예리한 리뷰 감사 ㅠㅜ 감격 ㅠ
There was a problem hiding this comment.
전체적으로 함수화된 부분이 많은 느낌인데, 혹시 왜 이렇게 바뀐걸까요??
테스트나 확장성을 용이하게 하려고 이렇게 바꾸신건지, 아니면 그 외의 용도가 있는지 궁금합니다!
There was a problem hiding this comment.
원래 cache 만 쓰던 redis 를 최적화 관점에서 필요할때만 init 해서 쓰게 lazy init + single tone 이었어요.
근데 이번에 redis 가 필수에 가깝게 되었고, lazy init 일 필요가 없어졌어요. 그러다보니 의존성이 있는 것에 injection 하기 위한 "순서"가 중요해졌어요. -> 참조 -> #49 (comment)
사실 lazy init 으로 둬도 이슈는 없었을텐데 cold start 에 딜레이가 있을 수 있기에 init 되는 순서를 바꿨어요.
There was a problem hiding this comment.
고생하셨습니다!!
브루트포스 공격은 말로만 들어보았는데, PR 리뷰하면서 해결 방법에 대한 시야를 얻어가는 것 같습니다 :)
좋았던 점
- JWT 형식 검증을 Fail-Fast로 DB 조회 전에 처리해서, 쓰레기 값 기반 요청에 대한 불필요한 DB 부하를 줄인 점이 인상 깊었습니다!
- Redis 기반으로 Rate Limiting을 구현하신 부분이 확장성 측면에서 좋았습니다.
- readonly 키워드로 불변임을 명시해 의도치 않은 재할당을 방지하신 점이 코드 안정성 측면에서 좋았습니다!
| FAILURE_THRESHOLD: 5, | ||
| WINDOW_SECONDS: 5 * 60, | ||
| LOCKOUT_SECONDS: 15 * 60, | ||
| }; |
There was a problem hiding this comment.
Rate Limit의 실패 횟수 임계값이나 블락 시간은 어떤 기준으로 설정하셨는지 궁금합니다!
There was a problem hiding this comment.
The best examples of rate limiting involve large services like Google Maps API (limiting geocoding requests per user) and X (Twitter) (limiting tweets/actions per timeframe per user/app)
의 기준이랄까요..?! 여기에 + 뇌피셜 정도
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/utils/jwt.util.ts (1)
27-39: 안전한 페이로드 추출 구현이 잘 되었습니다.예외 대신 null을 반환하여 호출부에서 안전하게 처리할 수 있습니다.
과거 리뷰에서 언급된
Buffer.from(..., 'base64url')의 Node.js 15.7.0+ 요구사항에 대해package.json의engines.node필드 추가가 필요합니다.
🧹 Nitpick comments (2)
src/types/dto/requests/getPostQuery.type.ts (1)
33-54: 날짜 검증 패턴 개선 권고 (선택사항)포매팅 변경은 가독성을 향상시킵니다. 다만
@Validate데코레이터에 인라인 함수를 직접 전달하는 패턴은 class-validator의 권장 API 사용법과 맞지 않습니다. class-validator 문서에 따르면 인라인 검증을 위해서는@ValidateBy()데코레이터나 커스텀ValidatorConstraint클래스를 사용하는 것이 권고됩니다.향후 리팩토링 시 다음 중 하나를 고려해 주세요:
@ValidateBy()데코레이터 사용@IsDateString()데코레이터로 간소화 (ISO 8601 형식)- 커스텀
ValidatorConstraint클래스 구현src/middlewares/errorHandling.middleware.ts (1)
8-14:trackAuthFailure호출 시await제거 고려
trackAuthFailure는 실패 추적/로깅 목적이므로 에러 응답을 지연시킬 필요가 없습니다. fire-and-forget 방식으로 변경하면 응답 속도가 개선됩니다.♻️ 제안 수정
// InvalidTokenError 발생 시 인증 실패 추적 if (err instanceof InvalidTokenError && authRateLimitService) { const ip = req.ip || req.socket?.remoteAddress || 'unknown'; - await authRateLimitService.trackAuthFailure(ip); + // fire-and-forget: 에러 응답 지연 방지 + authRateLimitService.trackAuthFailure(ip).catch((trackError) => { + // trackAuthFailure 내부에서 이미 에러 로깅하므로 추가 처리 불필요 + }); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (39)
.husky/pre-commitpackage.jsonsrc/controllers/__test__/user.controller.test.tssrc/controllers/__test__/webhook.controller.test.tssrc/middlewares/__test__/auth.middleware.test.tssrc/middlewares/__test__/errorHandling.middleware.test.tssrc/middlewares/auth.middleware.tssrc/middlewares/errorHandling.middleware.tssrc/modules/cache/cache.exception.tssrc/modules/cache/cache.type.tssrc/modules/slack/__test__/slack.notifier.test.tssrc/modules/slack/slack.notifier.tssrc/modules/velog/__test__/velog.api.test.tssrc/modules/velog/velog.api.tssrc/modules/velog/velog.constans.tssrc/modules/velog/velog.type.tssrc/repositories/__test__/integration/leaderboard.repo.integration.test.tssrc/repositories/__test__/integration/post.repo.integration.test.tssrc/repositories/__test__/integration/qr.repo.integration.test.tssrc/repositories/__test__/leaderboard.repo.test.tssrc/repositories/__test__/post.repo.test.tssrc/repositories/__test__/qr.repo.test.tssrc/services/__test__/authRateLimit.service.test.tssrc/services/__test__/post.service.test.tssrc/services/__test__/qr.service.test.tssrc/services/authRateLimit.service.tssrc/types/dto/requests/getPostQuery.type.tssrc/types/dto/requests/getTotalStatsQuery.type.tssrc/types/dto/responses/notiResponse.type.tssrc/types/dto/responses/totalStatsResponse.type.tssrc/types/dto/userWithToken.type.tssrc/types/models/NotiPost.type.tssrc/types/models/QRLoginToken.type.tssrc/types/models/Sentry.type.tssrc/types/models/User.type.tssrc/utils/__test__/date.util.test.tssrc/utils/__test__/jwt.util.test.tssrc/utils/__test__/logging.util.test.tssrc/utils/jwt.util.ts
💤 Files with no reviewable changes (2)
- src/types/models/User.type.ts
- src/modules/velog/velog.constans.ts
✅ Files skipped from review due to trivial changes (16)
- src/repositories/test/leaderboard.repo.test.ts
- src/controllers/test/user.controller.test.ts
- src/types/dto/responses/notiResponse.type.ts
- src/services/test/post.service.test.ts
- src/repositories/test/integration/qr.repo.integration.test.ts
- src/modules/cache/cache.type.ts
- src/repositories/test/integration/leaderboard.repo.integration.test.ts
- src/repositories/test/post.repo.test.ts
- src/modules/slack/test/slack.notifier.test.ts
- .husky/pre-commit
- src/utils/test/date.util.test.ts
- src/types/dto/requests/getTotalStatsQuery.type.ts
- src/services/test/qr.service.test.ts
- src/types/models/NotiPost.type.ts
- src/types/dto/userWithToken.type.ts
- src/modules/slack/slack.notifier.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/utils/test/jwt.util.test.ts
- src/services/test/authRateLimit.service.test.ts
- package.json
🧰 Additional context used
🧠 Learnings (11)
📚 Learning: 2024-12-04T13:26:58.075Z
Learnt from: HA0N1
Repo: Check-Data-Out/velog-dashboard-v2-api PR: 5
File: src/middlewares/auth.middleware.ts:116-117
Timestamp: 2024-12-04T13:26:58.075Z
Learning: 'velog-dashboard-v2-api' 코드베이스에서는 `src/types/express.d.ts` 파일에서 Express의 `Request` 인터페이스를 확장하여 `user`와 `tokens` 속성을 추가하였습니다.
Applied to files:
src/utils/__test__/logging.util.test.tssrc/middlewares/__test__/auth.middleware.test.tssrc/middlewares/auth.middleware.tssrc/repositories/__test__/qr.repo.test.tssrc/types/models/QRLoginToken.type.tssrc/modules/velog/velog.type.ts
📚 Learning: 2024-12-06T14:29:50.385Z
Learnt from: HA0N1
Repo: Check-Data-Out/velog-dashboard-v2-api PR: 6
File: src/controllers/user.controller.ts:11-12
Timestamp: 2024-12-06T14:29:50.385Z
Learning: TypeScript Express 프로젝트에서, `Express.Request` 인터페이스는 전역으로 확장되어 `user`와 `tokens` 프로퍼티를 포함합니다. `user` 프로퍼티는 `VelogUserLoginResponse | VelogUserVerifyResponse` 타입이고, `tokens`는 `accessToken`과 `refreshToken`을 가진 객체입니다.
Applied to files:
src/utils/__test__/logging.util.test.tssrc/middlewares/__test__/auth.middleware.test.tssrc/middlewares/auth.middleware.tssrc/types/models/QRLoginToken.type.tssrc/modules/velog/velog.type.ts
📚 Learning: 2024-11-29T14:21:32.376Z
Learnt from: HA0N1
Repo: Check-Data-Out/velog-dashboard-v2-api PR: 4
File: src/middlewares/auth.middleware.ts:48-49
Timestamp: 2024-11-29T14:21:32.376Z
Learning: `auth.middleware.ts` 파일에서 `VELOG_API_URL`은 개발 및 배포 환경 모두에서 고정된 값으로, 환경 변수로 이동하지 않고 상수로 설정하여 가독성을 높이는 것이 좋습니다.
Applied to files:
src/middlewares/auth.middleware.tssrc/modules/velog/velog.type.tssrc/modules/velog/velog.api.ts
📚 Learning: 2024-12-06T12:07:05.887Z
Learnt from: HA0N1
Repo: Check-Data-Out/velog-dashboard-v2-api PR: 6
File: src/reposities/user.repository.ts:0-0
Timestamp: 2024-12-06T12:07:05.887Z
Learning: `src/services/user.service.ts`에서 `VelogUserLoginDto`를 사용하여 이미 유효성 검증을 진행하므로, `src/repositories/user.repository.ts`의 `createUser` 함수에서는 추가적인 유효성 검증이 필요하지 않다.
Applied to files:
src/middlewares/auth.middleware.tssrc/modules/velog/velog.type.ts
📚 Learning: 2024-12-04T13:28:34.692Z
Learnt from: HA0N1
Repo: Check-Data-Out/velog-dashboard-v2-api PR: 5
File: src/types/dto/user-with-token.dto.ts:3-26
Timestamp: 2024-12-04T13:28:34.692Z
Learning: In `src/types/dto/user-with-token.dto.ts`, since the project is using Express instead of NestJS, `nestjs/swagger` cannot be used. Documentation will be implemented later using an Express-specific Swagger library.
Applied to files:
src/types/models/QRLoginToken.type.ts
📚 Learning: 2024-11-28T09:36:44.629Z
Learnt from: HA0N1
Repo: Check-Data-Out/velog-dashboard-v2-api PR: 3
File: src/types/User.type.ts:4-5
Timestamp: 2024-11-28T09:36:44.629Z
Learning: In `src/types/User.type.ts`, the `access_token` and `refresh_token` fields are stored encrypted.
Applied to files:
src/types/models/QRLoginToken.type.ts
📚 Learning: 2024-12-04T14:05:58.537Z
Learnt from: HA0N1
Repo: Check-Data-Out/velog-dashboard-v2-api PR: 5
File: src/middlewares/error-handling.middleware.ts:1-2
Timestamp: 2024-12-04T14:05:58.537Z
Learning: `src/middlewares/error-handling.middleware.ts` 파일의 에러 핸들링 미들웨어에서 `NextFunction`을 사용하지 않으며, `err`은 커스텀 에러로 사용되므로 `NextFunction`과 `ErrorRequestHandler`를 임포트할 필요가 없습니다.
Applied to files:
src/middlewares/__test__/errorHandling.middleware.test.tssrc/middlewares/errorHandling.middleware.ts
📚 Learning: 2024-12-04T13:59:57.198Z
Learnt from: HA0N1
Repo: Check-Data-Out/velog-dashboard-v2-api PR: 5
File: src/middlewares/error-handling.middleware.ts:4-7
Timestamp: 2024-12-04T13:59:57.198Z
Learning: 프로젝트에서 `Express` 네임스페이스에 `CustomError` 타입이 정의되어 있습니다.
Applied to files:
src/middlewares/__test__/errorHandling.middleware.test.tssrc/middlewares/errorHandling.middleware.ts
📚 Learning: 2024-11-28T09:42:54.348Z
Learnt from: HA0N1
Repo: Check-Data-Out/velog-dashboard-v2-api PR: 3
File: src/types/User.type.ts:2-3
Timestamp: 2024-11-28T09:42:54.348Z
Learning: `src/types/User.type.ts` 파일은 `velog_uuid` 컬럼을 응답값에서 그대로 저장하는 용도로 사용되므로, 해당 파일에 대한 코드 리뷰 제안을 하지 않습니다.
Applied to files:
src/modules/velog/velog.type.ts
📚 Learning: 2024-12-09T11:15:44.429Z
Learnt from: HA0N1
Repo: Check-Data-Out/velog-dashboard-v2-api PR: 6
File: src/types/dto/velog-user.dto.ts:23-30
Timestamp: 2024-12-09T11:15:44.429Z
Learning: `src/types/dto/velog-user.dto.ts` 파일에서 `ProfileDTO` 클래스는 외부에서 import될 필요가 없으므로 export하지 않아도 된다.
Applied to files:
src/modules/velog/velog.type.ts
📚 Learning: 2025-04-25T12:40:39.781Z
Learnt from: ooheunda
Repo: Check-Data-Out/velog-dashboard-v2-api PR: 27
File: src/controllers/leaderboard.controller.ts:20-23
Timestamp: 2025-04-25T12:40:39.781Z
Learning: 앱에서는 쿼리 파라미터 검증을 위해 class-transformer와 class-validator를 활용한 DTO 패턴을 사용하고 있으며, IsOptional, IsNumber, Transform, Min, Max 등의 데코레이터로 타입 변환 및 범위 검증을 처리합니다.
Applied to files:
src/types/dto/requests/getPostQuery.type.ts
🧬 Code graph analysis (9)
src/utils/__test__/logging.util.test.ts (3)
src/types/models/User.type.ts (1)
User(1-16)src/types/index.ts (1)
User(2-2)src/utils/logging.util.ts (2)
logError(51-85)logAccess(94-120)
src/modules/velog/__test__/velog.api.test.ts (1)
src/modules/velog/velog.api.ts (1)
fetchVelogApi(15-58)
src/middlewares/__test__/auth.middleware.test.ts (3)
src/utils/fixtures.ts (1)
mockUser(28-41)src/middlewares/auth.middleware.ts (2)
verifyBearerTokens(34-89)verify(141-143)src/services/authRateLimit.service.ts (1)
AuthRateLimitService(16-61)
src/middlewares/auth.middleware.ts (6)
src/services/authRateLimit.service.ts (2)
AuthRateLimitService(16-61)AUTH_RATE_LIMIT_CONFIG(9-14)src/exception/index.ts (1)
InvalidTokenError(6-6)src/exception/token.exception.ts (1)
InvalidTokenError(17-21)src/utils/jwt.util.ts (2)
isValidJwtFormat(9-18)safeExtractPayload(27-39)src/modules/velog/velog.type.ts (1)
VelogJWTPayload(4-10)src/types/index.ts (1)
VelogJWTPayload(3-3)
src/repositories/__test__/qr.repo.test.ts (2)
src/utils/fixtures.ts (1)
mockPool(17-21)src/exception/index.ts (1)
DBError(2-2)
src/middlewares/__test__/errorHandling.middleware.test.ts (2)
src/services/authRateLimit.service.ts (1)
AuthRateLimitService(16-61)src/middlewares/errorHandling.middleware.ts (1)
createErrorHandlingMiddleware(8-40)
src/middlewares/errorHandling.middleware.ts (4)
src/services/authRateLimit.service.ts (1)
AuthRateLimitService(16-61)src/exception/token.exception.ts (1)
InvalidTokenError(17-21)src/exception/custom.exception.ts (1)
CustomError(1-10)src/utils/logging.util.ts (1)
logError(51-85)
src/types/dto/responses/totalStatsResponse.type.ts (2)
src/types/index.ts (2)
BadgeDataResponseDto(46-46)BadgeData(41-41)src/types/dto/responses/baseResponse.type.ts (1)
BaseResponseDto(28-40)
src/services/authRateLimit.service.ts (2)
src/configs/cache.config.ts (1)
cache(17-17)src/modules/cache/cache.type.ts (1)
ICache(23-80)
🪛 Gitleaks (8.30.0)
src/middlewares/__test__/auth.middleware.test.ts
[high] 43-43: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
[high] 100-100: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
[high] 117-117: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
[high] 278-278: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (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 (30)
src/types/models/QRLoginToken.type.ts (1)
1-10: LGTM!인터페이스 속성들의 들여쓰기 정리만 수행된 포맷팅 변경입니다. 타입 정의에 문제가 없습니다.
src/repositories/__test__/qr.repo.test.ts (1)
1-112: LGTM!테스트 파일 전반에 걸친 포맷팅 정리입니다:
- Import 경로 따옴표 스타일 통일
- assertion을 간결한 단일 라인 형태로 정리
- 공백 및 trailing comma 조정
테스트 로직에는 변경이 없으며, 기존 검증 로직이 올바르게 유지됩니다.
src/types/dto/responses/totalStatsResponse.type.ts (1)
179-179: 좋습니다! 모든 호출부가 호환됩니다.명시적 생성자를 제거하고
BaseResponseDto의 생성자를 상속받도록 변경한 것은 동일 파일 내 다른 DTO 클래스들(TotalStatsResponseDto,StatsRefreshResponseDto)과 일관된 패턴입니다.
BadgeDataResponseDto인스턴스 생성 호출부(src/controllers/totalStats.controller.ts:48)에서 전달하는 4개의 파라미터(success, message, data, error)가BaseResponseDto의 생성자 시그니처와 완벽하게 일치하므로 변경이 안전합니다.src/types/models/Sentry.type.ts (1)
6-13: LGTM!유니온 타입을 여러 줄로 포맷팅한 것은 가독성을 높이고, 향후 값 추가 시 diff를 더 깔끔하게 만드는 좋은 개선입니다.
src/controllers/__test__/webhook.controller.test.ts (5)
47-66: LGTM!
slug필드 추가는SentryProject인터페이스의 필수 필드와 일치하며,count값을 문자열'5'로 변경한 것은SentryIssue타입의count: string정의와 올바르게 맞습니다.
74-79: 각 메시지 구성 요소를 개별적으로 검증하는 방식이 좋습니다.
stringContaining을 사용한 개별 assertion은 메시지 포맷이 변경되어도 테스트 유지보수가 용이합니다. 다만, 동일한mockSendSlackMessage호출에 대해 여러expect를 사용하고 있는데, 이는 해당 문자열들이 모두 같은 호출에 포함되어 있음을 검증합니다.
89-108: permalink fallback URL 패턴 확인 필요.
project.slug가'velog-dashboard'인데, 기대하는 fallback URL은'velog-dashboardv2.sentry.io'를 사용합니다. 이는 Sentry organization 이름이 프로젝트 slug와 다르기 때문에 의도된 동작일 수 있지만, 컨트롤러에서 해당 fallback 로직이 organization을 하드코딩하고 있는지 확인해 주세요.
110-224: LGTM!에러 케이스들에 대한 테스트 커버리지가 잘 구성되어 있습니다.
INVALID_SYNTAX코드를 일관되게 사용하고, 다양한 잘못된 입력 시나리오(빈 body, 잘못된 action, 필수 필드 누락 등)를 검증합니다.
227-266: LGTM!
formatSentryMessage통합 테스트가 완전한 데이터로 예상 메시지 형식을 올바르게 검증합니다.completeData에도slug필드가 추가되어 타입 정의와 일치합니다.src/utils/__test__/logging.util.test.ts (1)
1-156: LGTM!포맷팅 및 공백 조정 변경 사항입니다. 테스트 로직이나 어설션에는 기능적 변경이 없으며, 커밋 메시지에 언급된 lint/format 적용과 일치합니다.
src/modules/velog/velog.type.ts (1)
4-22: LGTM!인라인 주석 앞의 공백 정리만 포함된 포맷팅 변경입니다. 인터페이스 구조와 타입에는 변경이 없습니다.
src/modules/velog/velog.api.ts (1)
45-58: LGTM!trailing comma 추가 및 파일 끝 개행 정리만 포함된 포맷팅 변경입니다.
fetchVelogApi함수의 동작에는 영향이 없습니다.src/modules/velog/__test__/velog.api.test.ts (1)
1-158: LGTM!테스트 데이터 객체와 assertion에 대한 포맷팅 조정(trailing comma, 들여쓰기)입니다. 테스트 로직과 기대 동작에는 변경이 없으며, PR의 코드 스타일 일관성 적용과 일치합니다.
src/modules/cache/cache.exception.ts (1)
1-23: LGTM!생성자 매개변수의 멀티라인 포맷팅 및 파일 끝 개행 정리입니다.
CacheError클래스 계층 구조와 동작에는 변경이 없습니다.src/repositories/__test__/integration/post.repo.integration.test.ts (1)
1-518: LGTM! 포맷팅 및 스타일 개선테스트 로직이나 어설션에는 변경이 없으며, 코드 스타일이 일관되게 정리되었습니다. arrow function 파라미터 래핑, Promise 생성 시 공백 표준화, map 콜백 문법 등이 깔끔하게 정리되었습니다.
src/utils/jwt.util.ts (1)
9-18: LGTM! Fail-fast JWT 형식 검증 구현JWT 형식 검증이 잘 구현되었습니다:
- falsy 및 타입 체크로 방어적 프로그래밍
- 3-part 구조 검증
- Base64URL 패턴 검증으로 DB 쿼리 전 잘못된 토큰을 빠르게 거부
이 방식은 PR 목적에 맞게 불필요한 DB 부하를 줄이는 데 효과적입니다.
src/middlewares/__test__/errorHandling.middleware.test.ts (3)
24-53: LGTM! 테스트 설정이 잘 구성되었습니다.Mock 객체들이 적절하게 설정되어 있고,
beforeEach/afterEach에서 초기화 및 정리가 올바르게 수행됩니다.AuthRateLimitService모킹도 필요한 메서드들(trackAuthFailure,isIpBlocked,clearFailures)을 모두 포함하고 있습니다.
82-111: LGTM! 인증 실패 추적 테스트 커버리지가 좋습니다.세 가지 핵심 시나리오를 잘 커버합니다:
InvalidTokenError발생 시trackAuthFailure호출- 다른 에러 타입에서는 호출하지 않음
- 서비스 미주입 시에도 정상 동작
src/middlewares/errorHandling.middleware.ts의 구현과 일치합니다.
113-166: LGTM! Sentry 연동 및 응답 형식 테스트Sentry 캡처 로직(500 에러만 전송, 클라이언트 에러 제외)과 응답 형식 검증이 적절합니다.
src/services/authRateLimit.service.ts (3)
9-14: LGTM! Rate limit 설정이 적절합니다.5분(300초) 윈도우 내 5회 실패 시 15분(900초) 차단은 brute force 방지에 합리적인 값입니다. 상수로 분리하여 설정 변경이 용이합니다.
19-40: LGTM! trackAuthFailure 구현이 잘 되었습니다.Fail-open 패턴으로 캐시 에러 시에도 인증 플로우가 중단되지 않습니다. 윈도우 내 실패 누적 및 임계값 도달 시 로깅이 적절합니다.
한 가지 동작 확인:
WINDOW_SECONDS(5분) 내 실패가 누적되고, 레코드 TTL은LOCKOUT_SECONDS(15분)로 설정됩니다. 이로 인해 임계값 도달 후 15분간 차단이 유지됩니다.
42-60: LGTM! isIpBlocked와 clearFailures 구현
isIpBlocked의 fail-open 동작(return falseon error)은 보안 미들웨어에 적절한 패턴입니다. 캐시 장애 시에도 서비스 가용성을 유지합니다.
clearFailures는 과거 리뷰에서 논의된 대로 인증 성공 시 차단 해제 및 E2E 테스트 용도로 활용됩니다.src/middlewares/__test__/auth.middleware.test.ts (3)
42-43: 테스트용 JWT 토큰 - Gitleaks 경고는 false positive입니다.Gitleaks가 JWT를 감지했으나, 이 토큰들은 테스트 목적으로 의도적으로 생성된 더미 토큰입니다. 페이로드의
user_id가 테스트용 UUID이며 서명도 유효하지 않습니다. 실제 환경에서 사용되는 비밀이 아니므로 보안 위험이 없습니다.
184-267: LGTM! Fail-fast 시나리오 테스트 커버리지가 우수합니다.PR 목적에 맞게 다양한 잘못된 토큰 시나리오에서 DB 쿼리가 호출되지 않음을 검증합니다:
- JWT 형식이 아닌 토큰 (점 없음)
- 깨진 Base64 페이로드
- Base64URL 무효 문자
- UUID 형식이 아닌 user_id
expect(pool.query).not.toHaveBeenCalled()어설션으로 불필요한 DB 부하 방지를 확인합니다.
316-400: LGTM! Rate Limit 통합 테스트가 포괄적입니다.모든 핵심 시나리오를 커버합니다:
isIpBlocked우선 호출 확인- 차단된 IP에 429 응답 및
retryAfter포함- 차단되지 않은 경우 정상 토큰 검증 진행
- Redis 에러 시 fail-open 동작 (토큰 검증 계속)
- 서비스 미주입 시 rate limit 체크 생략
src/middlewares/auth.middleware.ts의verifyBearerTokens구현과 일치합니다.src/middlewares/auth.middleware.ts (3)
8-12: 전역 상태 패턴은 허용되지만 주의 필요전역
globalRateLimitService변수를 사용한 의존성 주입 패턴은 Express 애플리케이션에서 일반적으로 사용됩니다. 다만, 테스트 간 격리를 위해 리셋 함수 추가를 고려해볼 수 있습니다.
62-71: Fail-Fast JWT 검증 로직 LGTM
isValidJwtFormat으로 형식을 먼저 검증하고,safeExtractPayload로 페이로드를 추출하는 순서가 적절합니다. 이전 리뷰에서 언급된 중복 검증 문제가 해결된 것으로 보입니다.
140-144: 초기화 순서는 이미 올바르게 처리되고 있습니다.app.ts에서setRateLimitService()가 라우터 등록 전에 호출되므로globalRateLimitService는 미들웨어 사용 시점에 항상 설정되어 있습니다. Getter 패턴으로 인한 새 미들웨어 인스턴스 생성도 문제가 되지 않습니다.src/middlewares/errorHandling.middleware.ts (2)
16-38: 에러 처리 로직 LGTM
CustomError분기 처리와 Sentry 연동이 적절하게 구현되었습니다. 응답 형식도 일관성 있게 유지되고 있습니다.
8-40: 리뷰 코멘트가 부정확합니다.errorHandlingMiddleware는 코드베이스 어디에서도 사용되지 않으므로 하위 호환성 문제가 없습니다. 현재 설계는createErrorHandlingMiddleware팩토리 함수만 내보내는 것이 의도된 패턴입니다.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
4 participants
🔥 변경 사항
flowchart TD subgraph Request["HTTP Request"] A[요청 진입] end subgraph RateLimitMiddleware["authRateLimitMiddleware"] B[IP 추출<br />req.ip / socket.remoteAddress] C{Redis 조회<br />key: auth-failure:IP} D{record 존재 AND<br />count >= 5 ?} E[429 응답 반환<br />retryAfter: 900초] end subgraph AuthMiddleware["authMiddleware.verify"] F[토큰 추출<br />accessToken / refreshToken] G{토큰 존재 여부} H{JWT 형식 검증<br />isValidJwtFormat} I{페이로드 추출<br />safeExtractPayload} J{user_id UUID 검증} K[DB 조회<br />users_user 테이블] L{사용자 존재 여부} M[req.user 설정<br />next 호출] end subgraph ErrorHandling["errorHandlingMiddleware"] N{InvalidTokenError ?} O[trackAuthFailure 호출] P[Redis 조회<br />key: auth-failure:IP] Q{기존 레코드 존재 AND<br />현재시간 - firstFailure<br /> < 300초 ?} R[count++<br />firstFailure 유지] S[count = 1<br />firstFailure = now] T[Redis 저장<br />key: auth-failure:IP<br />TTL: 900초] U{count >= 5 ?} V[로그 경고<br />Auth brute force detected] W[에러 응답 반환] end subgraph Success["Success Response"] X[정상 처리 계속] end A --> B B --> C C --> D D -->|Yes| E D -->|No| F F --> G G -->|No| N G -->|Yes| H H -->|Invalid| N H -->|Valid| I I -->|null| N I -->|Valid| J J -->|Invalid| N J -->|Valid| K K --> L L -->|No| N L -->|Yes| M M --> X N -->|Yes| O N -->|No| W O --> P P --> Q Q -->|Yes| R Q -->|No| S R --> T S --> T T --> U U -->|Yes| V U -->|No| W V --> W🏷 관련 이슈
#123📸 스크린샷 (UI 변경 시 필수)
📌 체크리스트
Summary by CodeRabbit
새로운 기능
개선
기타
✏️ Tip: You can customize this high-level summary in your review settings.