-
Notifications
You must be signed in to change notification settings - Fork 0
[25.05.05 / TASK-182] Feature - middleware 인가 로직 전체 리펙토링, 그에 따른 대응 개발 #30
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이 변경사항은 Velog 인증 및 QR 로그인 토큰 처리 전반에 걸쳐 대대적인 리팩토링과 타입 정비를 수행합니다. Velog API와의 연동을 위한 별도 모듈이 도입되었고, 인증 미들웨어에서는 외부 API 호출을 제거하고 JWT 페이로드 기반의 검증으로 변경되었습니다. QR 로그인 토큰의 생성, 사용, 만료 처리 방식이 명확히 분리되었으며, 관련 예외 클래스와 테스트 코드가 추가 및 강화되었습니다. 또한, User, QRLoginToken 등 주요 타입의 명세가 구체화되고, 관련 서비스, 레포지토리, 컨트롤러, 라우터의 시그니처와 흐름이 일관되게 정비되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant UserController
participant VelogAPI
participant UserService
participant UserRepository
Client->>UserController: POST /user/login (accessToken, refreshToken)
UserController->>VelogAPI: fetchVelogApi(accessToken, refreshToken)
VelogAPI-->>UserController: VelogUserCurrentResponse
UserController->>UserService: handleUserTokensByVelogUUID(userData, accessToken, refreshToken)
UserService->>UserRepository: findByVelogUUID(uuid)
alt user exists
UserService->>UserRepository: updateUserTokens(uuid, email, encryptedTokens)
else user not exists
UserService->>UserRepository: createUser(uuid, email, tokens)
end
UserService-->>UserController: User
UserController-->>Client: User 정보 반환
sequenceDiagram
participant Client
participant UserController
participant UserService
participant UserRepository
Client->>UserController: POST /user/qr-login (token)
UserController->>UserService: useToken(token)
UserService->>UserRepository: findQRLoginToken(token)
alt token exists
UserService->>UserRepository: findByUserId(user_id)
UserService->>UserRepository: updateQRLoginTokenToUse(user_id)
UserService-->>UserController: { decryptedAccessToken, decryptedRefreshToken }
else token not exists
UserService-->>UserController: null
end
UserController-->>Client: 토큰 결과 반환
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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: 1
🔭 Outside diff range comments (1)
src/services/user.service.ts (1)
53-87
: 🛠️ Refactor suggestion신규 유저 처리 시 두 번의 DB 쓰기 발생
신규 가입 로직에서
- 평문 토큰으로
createUser
실행- 바로 이어서 암호화 토큰으로
updateUserTokens
실행두 번의 INSERT/UPDATE가 연달아 일어나면서 트래픽이 불필요하게 증가합니다.
평문으로 저장했다가 곧바로 덮어쓰는 대신,createUser
에서 group Id를 미리 생성한 뒤 암호화된 토큰으로 단 한 번에 저장하거나, 내부 트랜잭션으로 묶어 원자적으로 처리하는 방법을 고려해 보세요.
🧹 Nitpick comments (10)
src/repositories/__test__/qr.repo.integration.test.ts (1)
56-63
: afterAll 정리 코드의 가독성이 개선되었습니다.테스트 종료 후 정리 코드에서 불필요한 빈 줄을 제거하여 코드 가독성이 향상되었습니다.
src/modules/velog/velog.api.ts (1)
15-58
: 새로운 Velog API 모듈 구현 잘했습니다.이 모듈이 Velog API와의 통신을 전담하도록 분리한 접근 방식이 좋습니다. 특히 오류 처리 로직과 결과 정규화가 잘 되어 있습니다.
아래 몇 가지 개선 사항을 고려해보세요:
- 헤더 값을 상수로 분리하는 것이 좋을 것 같습니다.
- 토큰 쿠키 설정 방식을 검토해 보세요.
- 현재 email이 undefined인 경우 null로 변환하는 로직이 있는데, VelogUserCurrentResponse 타입에 맞게 처리됐는지 확인하세요.
- return { - ...currentUser, - email: currentUser.email ?? null - }; + // VelogUserCurrentResponse 타입에 맞게 변환 + return { + ...currentUser, + email: currentUser.email ?? null + };헤더 값을 상수로 분리하는 예시:
// velog.constans.ts에 추가할 수 있는 상수 export const VELOG_API_HEADERS = { AUTHORITY: 'v3.velog.io', ORIGIN: 'https://velog.io', CONTENT_TYPE: 'application/json', };src/modules/__test__/velog.api.test.ts (1)
15-70
: 성공 케이스 테스트가 잘 작성되었습니다.API 호출 결과 및 axios 호출 파라미터 검증을 포함한 성공 케이스 테스트가 잘 구현되었습니다. 테스트 구조와 검증 방식이 명확합니다.
한 가지 제안으로는 반복되는 모의 응답 데이터를 별도의 변수나 팩토리 함수로 추출하여 테스트 코드를 더 간결하게 만들 수 있을 것 같습니다.
src/middlewares/auth.middleware.ts (1)
49-56
: 토큰 검증 로직 개선이 잘 되었습니다.외부 API 호출 대신 JWT 페이로드를 직접 추출하고 검증하는 방식으로 변경한 것이 좋습니다. 특히:
- payload.user_id가 존재하는지 확인하고 UUID 형식인지 검증
- 데이터베이스에서 해당 UUID로 사용자를 조회
- 사용자가 존재하지 않을 경우 적절한 오류 발생
이러한 접근 방식은 성능을 향상시키고 의존성을 줄입니다.
토큰 만료 검증을 추가하면 더 완벽할 것 같습니다.
다음과 같이 토큰 만료 검증을 추가할 수 있습니다:
const payload = extractPayload(accessToken); if (!payload.user_id || !isUUID(payload.user_id)) { throw new InvalidTokenError('유효하지 않은 토큰 페이로드 입니다.'); } + + // 토큰 만료 시간 검증 + if (payload.exp && payload.exp < Math.floor(Date.now() / 1000)) { + throw new InvalidTokenError('만료된 토큰입니다.'); + } const user = (await pool.query('SELECT * FROM "users_user" WHERE velog_uuid = $1', [payload.user_id])).rows[0] as User; if (!user) throw new DBError('사용자를 찾을 수 없습니다.');src/services/__test__/qr.service.test.ts (4)
8-20
: AES Encryption 모킹 범위가 다소 제한적입니다
encrypt
/decrypt
를 모두 동일한 문자열만 반환하도록 지정하면, 암·복호화가 제대로 실행되는지(특히 key mismatch, 예외 처리 등) 테스트-커버리지가 부족해질 수 있습니다.
토큰이 입력값에 따라 달라지는지까지 검증하려면 입력값 → 출력값 매핑을 테스트마다 지정하거나,encrypt
는 간단히 Base64 인코딩 수준으로 동작하도록 모킹해 기대값을 역산할 수 있습니다.
27-43
: Repository 클래스 모킹 방식 개선 권장
jest.mock('@/repositories/user.repository')
만으로는 생성자 인자를 무시하고 빈 객체를 반환하는 mock 함수가 생성됩니다.
그 결과new UserRepository(mockPool)
호출 시 내부에서mockPool
을 사용하는 로직이 전혀 실행되지 않습니다. 실제 구현이 커넥션 풀을 요구한다면, 테스트-시나리오와 실제 동작이 괴리될 가능성이 있습니다.-jest.mock('@/repositories/user.repository'); +jest.mock('@/repositories/user.repository', () => { + return { + // 생성자 모킹 + UserRepository: jest.fn().mockImplementation(() => ({ + createQRLoginToken: jest.fn(), + findQRLoginToken: jest.fn(), + findByUserId: jest.fn(), + updateQRLoginTokenToUse: jest.fn(), + })), + }; +});이처럼 명시적으로 메서드를 정의하면 타입 안정성과 가독성이 높아집니다.
49-61
: 무작위 토큰 검증 테스트가 플레이키(Flaky)할 수 있습니다
generateRandomToken(10)
이 허용되지 않은 특수문자를 뽑을 경우 정규식 테스트가 실패할 여지가 있습니다.
안정성을 높이려면generateRandomToken
을 모킹하여 고정 토큰을 반환하거나, 토큰 생성 로직을 독립 테스트(유닛)로 분리하고 이곳에서는 “토큰이 저장된다”만 검증하는 편이 안전합니다.
95-107
: 추가 검증 포인트 제안
현재 성공 시나리오에서updateQRLoginTokenToUse
가 호출만 되었는지 확인하지만, 호출 순서(find → update → decrypt)까지 보장되지는 않습니다.jest
의toHaveBeenNthCalledWith
를 활용해 의도한 흐름을 명시적으로 검증하면 리팩터링-시 실수로 순서가 바뀌어도 테스트가 잡아낼 수 있습니다.src/services/user.service.ts (2)
128-133
: QR 토큰 충돌 가능성 검토
10 자리 랜덤 토큰은 고속 발급/사용 환경에서 충돌 확률이 의외로 높을 수 있습니다.
- 길이를 늘리거나
crypto.randomUUID()
등 충돌 가능성이 훨씬 낮은 식별자를 사용- DB UNIQUE INDEX로 중복을 강제 차단
중 최소 한 가지는 적용해 두는 편이 안전합니다.
134-150
: 토큰 소모(update)와 복호화 동시 실패 시 일관성 문제
updateQRLoginTokenToUse
호출 후decryptTokens
단계에서 예외가 발생하면,
- 토큰은 이미 ‘사용됨’으로 변경
- 클라이언트는 500 오류
라는 불일치가 생길 수 있습니다.트랜잭션으로 묶거나, 최소한
updateQRLoginTokenToUse
이전에 복호화가 성공하는지 확인한 뒤 마지막 단계에서 토큰을 소모하도록 순서를 조정해 보세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
src/controllers/post.controller.ts
(0 hunks)src/controllers/user.controller.ts
(5 hunks)src/exception/index.ts
(1 hunks)src/exception/token.exception.ts
(2 hunks)src/middlewares/__test__/auth.middleware.test.ts
(1 hunks)src/middlewares/auth.middleware.ts
(4 hunks)src/modules/__test__/velog.api.test.ts
(1 hunks)src/modules/token_encryption/aes_encryption.ts
(1 hunks)src/modules/velog/velog.api.ts
(1 hunks)src/modules/velog/velog.type.ts
(1 hunks)src/repositories/__test__/qr.repo.integration.test.ts
(5 hunks)src/repositories/__test__/qr.repo.test.ts
(3 hunks)src/repositories/user.repository.ts
(3 hunks)src/routes/leaderboard.router.ts
(3 hunks)src/routes/noti.router.ts
(1 hunks)src/routes/user.router.ts
(4 hunks)src/services/__test__/qr.service.test.ts
(1 hunks)src/services/user.service.ts
(5 hunks)src/types/dto/userWithToken.type.ts
(2 hunks)src/types/express.d.ts
(1 hunks)src/types/index.ts
(1 hunks)src/types/models/QRLoginToken.type.ts
(1 hunks)src/types/models/User.type.ts
(1 hunks)src/types/velog.type.ts
(0 hunks)
💤 Files with no reviewable changes (2)
- src/controllers/post.controller.ts
- src/types/velog.type.ts
🧰 Additional context used
🧠 Learnings (5)
src/types/express.d.ts (2)
Learnt from: HA0N1
PR: Check-Data-Out/velog-dashboard-v2-api#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` 속성을 추가하였습니다.
Learnt from: HA0N1
PR: Check-Data-Out/velog-dashboard-v2-api#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`을 가진 객체입니다.
src/middlewares/auth.middleware.ts (2)
Learnt from: HA0N1
PR: Check-Data-Out/velog-dashboard-v2-api#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` 속성을 추가하였습니다.
Learnt from: HA0N1
PR: Check-Data-Out/velog-dashboard-v2-api#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`을 가진 객체입니다.
src/controllers/user.controller.ts (1)
Learnt from: HA0N1
PR: Check-Data-Out/velog-dashboard-v2-api#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`을 가진 객체입니다.
src/modules/velog/velog.type.ts (1)
Learnt from: HA0N1
PR: Check-Data-Out/velog-dashboard-v2-api#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`을 가진 객체입니다.
src/repositories/user.repository.ts (1)
Learnt from: HA0N1
PR: Check-Data-Out/velog-dashboard-v2-api#6
File: src/reposities/user.repository.ts:7-15
Timestamp: 2024-12-05T17:21:07.909Z
Learning: Function `findByUserEmail` in `src/reposities/user.repository.ts` is scheduled to be deleted and should be ignored in future code reviews.
🧬 Code Graph Analysis (10)
src/routes/noti.router.ts (1)
src/middlewares/auth.middleware.ts (1)
authMiddleware
(71-73)
src/routes/leaderboard.router.ts (1)
src/middlewares/auth.middleware.ts (1)
authMiddleware
(71-73)
src/types/express.d.ts (1)
src/types/models/User.type.ts (1)
User
(1-11)
src/routes/user.router.ts (4)
src/middlewares/validation.middleware.ts (1)
validateRequestDto
(8-38)src/types/index.ts (1)
LoginRequestDto
(9-9)src/types/dto/requests/loginRequest.type.ts (1)
LoginRequestDto
(20-33)src/middlewares/auth.middleware.ts (1)
authMiddleware
(71-73)
src/exception/token.exception.ts (2)
src/exception/index.ts (3)
QRTokenExpiredError
(3-3)BadRequestError
(5-5)QRTokenInvalidError
(3-3)src/exception/badRequest.exception.ts (1)
BadRequestError
(3-7)
src/modules/velog/velog.api.ts (3)
src/modules/velog/velog.type.ts (1)
VelogUserCurrentResponse
(17-24)src/modules/velog/velog.constans.ts (2)
VELOG_QUERIES
(3-14)VELOG_API_URL
(2-2)src/exception/token.exception.ts (1)
InvalidTokenError
(17-21)
src/repositories/__test__/qr.repo.test.ts (2)
src/types/models/QRLoginToken.type.ts (1)
QRLoginToken
(1-10)src/exception/index.ts (1)
DBError
(2-2)
src/middlewares/__test__/auth.middleware.test.ts (1)
src/middlewares/auth.middleware.ts (1)
authMiddleware
(71-73)
src/services/__test__/qr.service.test.ts (6)
src/services/user.service.ts (1)
UserService
(12-151)src/repositories/user.repository.ts (1)
UserRepository
(7-146)src/exception/index.ts (1)
DBError
(2-2)src/types/index.ts (1)
User
(1-1)src/types/models/User.type.ts (1)
User
(1-11)src/types/models/QRLoginToken.type.ts (1)
QRLoginToken
(1-10)
src/repositories/user.repository.ts (2)
src/types/models/User.type.ts (1)
User
(1-11)src/exception/db.exception.ts (1)
DBError
(3-7)
⏰ 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 (20)
- GitHub Check: build-and-test (21)
🔇 Additional comments (65)
src/routes/noti.router.ts (1)
42-42
: 인증 미들웨어 변경 적용 완료
authMiddleware.verify
로의 변경은 인증 미들웨어 리팩토링과 일치합니다. 외부 Velog API 호출을 제거하고 JWT 토큰을 로컬에서 검증하는 방식으로 전환함으로써 인증 과정이 간소화되었습니다.src/types/express.d.ts (1)
1-6
: Request.user 타입 변경 적절히 적용됨Express의
Request
인터페이스에서user
속성의 타입을VelogUserLoginResponse
에서User
로 변경한 것은 전체 코드베이스의 타입 시스템 일관성 향상에 기여합니다. 이 변경은 인증된 사용자 정보를 처리하는 방식을 통일하고, 미들웨어와 컨트롤러 간의 타입 안정성을 강화합니다.src/types/models/User.type.ts (1)
7-7
: email 필드 타입 개선
email?: string
)에서 명시적인 nullable 타입(email: string | null
)으로 변경한 것은 좋은 개선입니다. 이 변경으로:
- 이메일이 항상 존재하되 값이 없을 수 있음을 명확히 표현
- 코드베이스 전반에 걸쳐 일관된 이메일 처리 가능
- 사용자 데이터 관련 타입 안정성 향상
타입 시스템을 통해 의도를 더 명확히 표현하는 접근 방식이 코드 품질을 향상시킵니다.
src/routes/leaderboard.router.ts (3)
3-3
: 인증 미들웨어 import 추가리더보드 라우터에 인증 기능을 추가하기 위한 import 구문이 적절하게 추가되었습니다.
48-48
: 사용자 리더보드에 인증 미들웨어 적용사용자 리더보드 엔드포인트에
authMiddleware.verify
미들웨어를 추가한 것은 적절합니다. 이로써 인증된 사용자만 리더보드 데이터에 접근할 수 있게 되었습니다. 전체 인증 로직 리팩토링 방향과 일치하는 변경입니다.
85-85
: 게시물 리더보드에 인증 미들웨어 적용게시물 리더보드 엔드포인트에도
authMiddleware.verify
미들웨어를 추가한 것은 일관된 접근 방식입니다. 사용자 인증이 필요한 모든 엔드포인트에 동일한 인증 미들웨어를 적용하여 보안성과 일관성을 향상시켰습니다.src/types/models/QRLoginToken.type.ts (3)
2-2
:id
필드가 추가됨인터페이스에
id
필드가 추가되어 데이터베이스 모델과의 일관성이 향상되었습니다.
4-4
:user
필드가user_id
로 명확하게 변경됨
user
에서user_id
로의 이름 변경은 필드의 역할을 더 명확히 표현하며, 데이터베이스 스키마와의 일관성을 유지합니다.
9-9
: 필드 순서 재배치
created_at
필드가expires_at
뒤로 이동되었습니다. 이는 필드 순서의 일관성을 유지하는 데 도움이 됩니다.src/types/index.ts (2)
5-5
: Velog 관련 타입 모듈화 및 개선기존
VelogUserLoginResponse
대신VelogJWTPayload
와VelogUserCurrentResponse
를 새로운 모듈 경로에서 불러오도록 변경되었습니다. 이는 Velog API 관련 로직의 모듈화와 JWT 기반 인증으로의 전환을 보여줍니다.
9-9
: 로그인 요청 DTO 추가
LoginRequestDto
가 새롭게 추가되었습니다. 이는 로그인 요청 검증 로직이 개선되었음을 의미합니다.src/exception/index.ts (1)
3-3
: QR 토큰 관련 예외 클래스 추가
QRTokenExpiredError
와QRTokenInvalidError
두 개의 새로운 QR 토큰 관련 예외 클래스가 추가되었습니다. 이는 QR 토큰 처리에 대한 더 세분화된 오류 처리를 가능하게 합니다.src/modules/token_encryption/aes_encryption.ts (2)
3-6
: 토큰 암호화 서비스 인터페이스 도입
TokenEncryptionService
인터페이스가 새롭게 추가되었습니다. 이 인터페이스는 암호화와 복호화를 위한 표준 계약을 정의합니다. 이는 추후 다양한 암호화 전략을 구현할 수 있는 확장성을 제공합니다.
12-12
: 인터페이스 구현 명시
AESEncryption
클래스가TokenEncryptionService
인터페이스를 명시적으로 구현하도록 변경되었습니다. 이는 클래스의 역할과 책임을 더 명확히 하며 타입 안전성을 향상시킵니다.src/exception/token.exception.ts (2)
2-2
: import 구문 추가가 적절합니다.BadRequestError를 상속하는 새로운 QR 토큰 관련 에러 클래스를 위해 필요한 import 구문이 추가되었습니다.
22-37
: QR 토큰 전용 에러 클래스 구현이 잘 되었습니다.QR 토큰 관련 에러를 명확히 구분하여 관리할 수 있도록 두 개의 새로운 클래스가 추가되었습니다:
QRTokenExpiredError
: QR 토큰 만료 에러QRTokenInvalidError
: 유효하지 않은 QR 토큰 에러기존의 일반 토큰 에러들과 달리
UnauthorizedError
(401) 대신BadRequestError
(400)를 상속하여 클라이언트 오류임을 명확히 구분한 점이 좋습니다.src/modules/velog/velog.type.ts (2)
1-11
: Velog JWT 페이로드 타입 정의가 잘 되었습니다.Velog 인증 토큰의 payload 구조를 명확한 인터페이스로 정의했습니다. 각 필드에 대한 주석 설명이 잘 되어 있어 이해하기 쉽습니다. 특히
user_id
가 UUID 값이며 키 이름을 변경할 수 없다는 주석은 유용한 정보입니다.표준 JWT 필드(iat, exp, iss, sub)도 포함되어 있어 완전한 타입 정의가 이루어졌습니다.
13-24
: Velog 사용자 응답 타입 정의가 적절합니다.Velog API의 currentUser 엔드포인트 응답 구조를 타입으로 정의했습니다. 필요한 필드들(id, username, email, profile.thumbnail)이 적절히 포함되어 있으며, email이 null일 수 있다는 점도 정확하게 타입에 반영되었습니다.
이 인터페이스 정의로 Velog API와의 통신 결과를 타입 안전하게 처리할 수 있게 되었습니다.
src/routes/user.router.ts (4)
7-8
: import 구문 수정이 적절합니다.토큰 검증 로직 리팩토링에 따라 필요한
validateRequestDto
미들웨어와LoginRequestDto
타입을 import하도록 수정되었습니다. 이전에 사용하던 주석 처리된 import 구문을 대체합니다.
49-49
: 로그인 라우트의 미들웨어 변경이 적절합니다.로그인 라우트에서 기존의
authMiddleware.login
미들웨어 대신validateRequestDto(LoginRequestDto, 'body')
미들웨어를 사용하도록 변경되었습니다. 이는 PR 목표인 인가 로직 리팩토링에 부합하며, 요청 본문 검증을 명시적으로 수행합니다.이 변경으로 인해 토큰 검증 로직이 미들웨어에서 컨트롤러로 이동되었을 가능성이 있습니다.
115-115
: 현재 사용자 조회 라우트의 미들웨어 변경이 적절합니다.
/me
라우트에서 기존의authMiddleware.login
대신authMiddleware.verify
를 사용하도록 변경되었습니다. 이는 인증 방식의 일관성을 높이고, 토큰 검증 방식을 단일화하는 리팩토링에 부합합니다.
129-129
: QR 로그인 토큰 생성 라우트의 미들웨어 변경이 적절합니다.QR 로그인 토큰 생성 라우트에서도
authMiddleware.login
대신authMiddleware.verify
를 사용하도록 변경되었습니다. 이는 인증 미들웨어 사용의 일관성을 높이고, 인가 로직 리팩토링의 목표에 부합합니다.src/repositories/__test__/qr.repo.integration.test.ts (3)
11-11
: 테스트 스위트 이름 변경이 적절합니다.테스트 스위트 이름이 "QRLoginTokenRepository 통합 테스트"에서 "UserRepository QR 토큰 통합 테스트"로 변경되었습니다. 이는 QR 토큰 기능이 UserRepository로 통합되었음을 반영하며, 리팩토링된 구조에 맞게 적절히 수정되었습니다.
82-87
: 토큰 속성 검증이 강화되었습니다.QR 토큰 생성 및 조회 테스트에서 추가적인 속성 검증 로직이 구현되었습니다. 이전에는 토큰 문자열과 사용 여부만 검증했으나, 이제는 user_id, ip_address, user_agent 등 더 많은 속성을 검증하여 테스트 커버리지가 향상되었습니다.
107-115
: QR 토큰 사용 처리 테스트 로직이 개선되었습니다.QR 토큰 사용 처리 테스트가 다음과 같이 리팩토링되었습니다:
- 이전: 토큰 문자열로 직접 사용 처리(
markTokenUsed(token)
)- 현재: 사용자 ID로 사용 처리(
updateQRLoginTokenToUse(TEST_DATA.USER_ID)
)이 변경은 리포지토리 메서드가 토큰 문자열이 아닌 사용자 ID를 기반으로 토큰을 사용 처리하도록 변경된 것을 반영합니다. 또한, 토큰을 찾아 존재 여부를 확인한 후 사용 처리하고, 사용 처리 후에는 토큰이 조회되지 않는지 검증하는 흐름으로 개선되었습니다.
테스트 흐름을 설명하는 주석이 추가되어 코드의 이해도가 향상되었습니다.
src/modules/__test__/velog.api.test.ts (3)
6-13
: 모킹 설정이 잘 구현되었습니다.axios와 logger를 모킹하여 테스트를 외부 의존성과 분리한 접근 방식이 좋습니다. 이는 테스트의 안정성과 예측 가능성을 높입니다.
72-102
: 이메일 필드 처리 테스트가 적절합니다.이메일이 없는 사용자 정보 처리 테스트는 중요한 엣지 케이스를 커버하고 있습니다. fetchVelogApi 함수가 이메일 필드를 적절히 null로 정규화하는지 검증하는 좋은 테스트입니다.
104-154
: 오류 케이스 테스트가 포괄적입니다.여러 오류 시나리오에 대한 테스트가 잘 작성되었습니다:
- GraphQL 오류 응답
- CurrentUser가 null인 경우
- 네트워크 오류
각 테스트는 명확한 오류 메시지와 함께 적절한 InvalidTokenError가 발생하는지 검증합니다.
src/types/dto/userWithToken.type.ts (3)
1-1
: class-validator 임포트 업데이트가 적절합니다.IsOptional 데코레이터를 추가한 임포트 변경이 잘 되었습니다.
6-6
: id에서 uuid로의 필드명 변경 및 이메일 타입 변경이 적절합니다.필드명을 id에서 uuid로 변경하고 이메일 타입을
string | null
로 변경한 것은 일관된 ID 처리와 이메일의 선택적 특성을 명확히 합니다.추가된 주석은 이 변경의 목적을 명확히 설명하고 있습니다.
Also applies to: 8-10
20-21
: 생성자 매개변수 업데이트가 적절합니다.생성자 매개변수를 필드명 변경에 맞춰 업데이트한 것이 좋습니다. 이는 타입 안전성을 유지하는 데 중요합니다.
src/middlewares/auth.middleware.ts (5)
6-6
: 타입 임포트 변경이 적절합니다.
VelogJWTPayload
와User
타입 임포트를 통합한 것이 좋습니다. 이는 미들웨어에서 필요한 타입을 명확히 하고 가독성을 높입니다.
24-27
: 함수 주석 업데이트가 적절합니다."이건 진짜 velog 에서 사용하는 걸 그대로 가져온 함수임!" 주석은 코드의 출처를 명확히 하여 유지보수와 이해에 도움이 됩니다. 반환 타입이
VelogJWTPayload
로 명확히 문서화된 것도 좋습니다.
35-35
: JWT 페이로드 추출 함수 타입 명시가 적절합니다.extractPayload` 함수의 반환 타입을 `VelogJWTPayload`로 명시한 것이 타입 안전성을 높이고 함수의 목적을 명확히 합니다. --- `38-40`: **함수 주석 업데이트가 간결하고 명확합니다.** `verifyBearerTokens` 함수의 목적을 명확히 설명하는 주석으로 업데이트되었습니다. --- `69-73`: **`authMiddleware` 객체 간소화가 적절합니다.** `authMiddleware` 객체에서 `login` 미들웨어를 제거하고 `verify` 미들웨어만 남긴 것이 좋습니다. 이는 미들웨어의 역할을 명확히 하고 불필요한 노출을 줄입니다. </details> <details> <summary>src/repositories/__test__/qr.repo.test.ts (10)</summary> `4-4`: **`QRLoginToken` 타입 임포트 추가로 타입 안전성 개선.** 임포트한 `QRLoginToken` 타입을 사용하여 테스트 데이터의 타입 안전성이 향상되었습니다. --- `10-10`: **테스트 스위트 명칭 변경으로 명확성 향상.** 테스트 스위트 명칭이 `UserRepository - QR Login Token`으로 변경되어 테스트 대상 기능을 더 명확하게 나타냅니다. --- `29-34`: **테스트 검증 포인트 개선.** 쿼리 호출 횟수와 파라미터를 명시적으로 검증하는 내용이 추가되어 테스트의 정확성이 향상되었습니다. --- `47-57`: **모의 토큰 데이터 타입화로 테스트 품질 향상.** `QRLoginToken` 타입을 활용한 완전한 형태의 모의 데이터를 생성하여 테스트의 정확성과 타입 안전성을 개선했습니다. --- `63-71`: **결과 및 쿼리 검증 주석 추가로 테스트 의도 명확화.** 결과값 검증과 쿼리 호출 검증 부분에 주석이 추가되어 테스트의 의도와 중요 검증 포인트를 명확히 표현했습니다. --- `79-87`: **null 반환 시나리오 검증 개선.** 토큰이 없을 때 null 반환 검증과 함께 쿼리 호출 검증을 추가하여 테스트의 완성도가 향상되었습니다. --- `94-99`: **예외 처리 테스트 개선.** 예외 발생 시 쿼리 호출 검증 로직 추가로 에러 케이스에 대한 테스트 커버리지가 향상되었습니다. --- `103-104`: **메서드명 변경 적용으로 코드 일관성 유지.** `markTokenUsed`에서 `updateQRLoginTokenToUse`로 메서드명 변경이 테스트에도 일관되게 적용되었습니다. --- `107-113`: **토큰 대신 유저 ID 사용으로 매개변수 변경 적용.** 토큰 문자열 대신 유저 ID를 사용하도록 테스트가 수정되어 리포지토리 메서드의 변경 사항과 일관성을 유지합니다. --- `119-121`: **예외 테스트 개선으로 테스트 완성도 향상.** 예외 발생 테스트에도 쿼리 호출 횟수 검증 로직이 추가되어 테스트 커버리지가 강화되었습니다. </details> <details> <summary>src/middlewares/__test__/auth.middleware.test.ts (9)</summary> `1-15`: **인증 미들웨어 테스트 구성 적절.** DB 및 로거에 대한 모킹이 잘 구성되어 있으며, 필요한 모듈을 올바르게 임포트하고 있습니다. --- `16-38`: **테스트 픽스처 설정 우수.** 각 테스트 전후에 필요한 상태를 초기화하고 정리하는 로직이 잘 구성되어 있어 테스트 격리를 보장합니다. --- `39-81`: **유효한 토큰 검증 테스트 철저.** 유효한 토큰으로 사용자 인증 시나리오에 대한 테스트가 매우 철저하게 구현되어 있습니다: - 쿠키에서 토큰 추출 - DB 쿼리 및 결과 처리 - 요청 객체에 사용자 정보와 토큰 연결 - next() 함수 호출 검증 - 올바른 DB 쿼리 파라미터 확인 이러한 상세한 검증은 미들웨어의 핵심 기능이 의도대로 작동하는지 확인하는 데 매우 중요합니다. --- `83-102`: **토큰 부재 시나리오 테스트 적절.** 토큰이 없는 경우 적절한 오류가 발생하는지 확인하는 테스트가 잘 구현되어 있습니다. --- `104-123`: **유효하지 않은 토큰 테스트 정확.** 잘못된 형식의 토큰이 제공될 때 적절한 오류 처리가 이루어지는지 검증하는 테스트가 올바르게 구현되어 있습니다. --- `124-148`: **페이로드 검증 테스트 철저.** UUID가 없는 토큰 페이로드에 대한 오류 처리를 검증하는 테스트가 매우 상세하게 구현되어 있습니다. --- `150-172`: **사용자 미발견 시나리오 테스트 적절.** 유효한 토큰이지만 DB에 사용자가 없는 경우를 처리하는 로직을 검증하는 테스트가 포함되어 있어 엣지 케이스에 대한 커버리지가 좋습니다. --- `174-206`: **요청 본문에서 토큰 추출 테스트 철저.** 쿠키 대신 요청 본문에서 토큰을 추출하는 시나리오를 검증하는 테스트가 잘 구현되어 있습니다. --- `207-238`: **헤더에서 토큰 추출 테스트 철저.** 쿠키와 본문 대신 헤더에서 토큰을 추출하는 시나리오를 검증하는 테스트가 잘 구현되어 있어 다양한 토큰 소스에 대한 유연성을 보장합니다. </details> <details> <summary>src/controllers/user.controller.ts (6)</summary> `3-7`: **필요한 모듈 임포트 업데이트 적절.** QR 토큰 관련 예외 클래스와 Velog API 함수를 적절히 임포트하여 외부 API 호출과 오류 처리 개선이 이루어졌습니다. --- `35-41`: **로그인 로직 리팩토링으로 책임 분리 개선.** 로그인 로직이 다음과 같이 명확하게 개선되었습니다: 1. 외부 API(Velog) 호출을 `fetchVelogApi`로 분리 2. 토큰 및 사용자 검증을 명확히 구분 3. DB 존재 여부 확인 및 사용자 처리 단계 분리 이러한 변경으로 코드의 책임 분리가 향상되었습니다. --- `125-125`: **메서드명 변경으로 기능 명확화.** `createToken`에서 `createUserQRToken`으로 메서드명을 변경하여 QR 토큰 생성 목적을 더 명확히 표현했습니다. --- `145-146`: **QR 토큰 특화 예외 클래스 사용으로 오류 처리 개선.** 일반 오류 대신 `QRTokenInvalidError`를 사용하여 더 명확한 오류 분류와 처리가 가능해졌습니다. --- `149-151`: **토큰 만료 예외 처리 개선.** 토큰이 유효하지 않을 경우 `QRTokenExpiredError`를 발생시켜 클라이언트에 더 명확한 오류 정보를 제공합니다. --- `156-157`: **토큰 처리 방식 개선.** `userLoginToken` 객체에서 직접 복호화된 토큰을 사용하여 쿠키를 설정하는 방식으로 변경되어 코드 흐름이 단순화되었습니다. </details> <details> <summary>src/repositories/user.repository.ts (3)</summary> `10-19`: **사용자 ID로 조회하는 메서드 추가로 기능 확장.** `findByUserId` 메서드가 추가되어 DB에서 사용자 ID로 사용자를 조회할 수 있는 기능이 추가되었습니다. 이 메서드는 다른 finder 메서드와 일관된 패턴으로 구현되었으며, 적절한 오류 처리와 로깅을 포함합니다. --- `71-71`: **이메일 파라미터 타입 변경으로 타입 정확성 향상.** `createUser` 메서드의 `email` 파라미터 타입이 `string | undefined`에서 `string | null`로 변경되어 DB 스키마 및 타입 정의와 더 일치하게 되었습니다. 이는 선택적 이메일 필드의 표현을 더 정확하게 합니다. --- `135-141`: **QR 토큰 업데이트 메서드 개선.** 다음과 같은 개선이 이루어졌습니다: 1. 메서드명이 `markTokenUsed`에서 `updateQRLoginTokenToUse`로 변경되어 목적이 더 명확해짐 2. 토큰 문자열 대신 사용자 ID를 매개변수로 사용하여 더 일관된 접근 방식 제공 3. SQL 쿼리가 토큰 대신 사용자 ID로 매칭되도록 수정 이러한 변경으로 QR 토큰 업데이트 로직이 더 명확하고 일관되게 되었습니다. </details> <details> <summary>src/services/__test__/qr.service.test.ts (1)</summary> `120-128`: **예외 시나리오 분리 테스트 👍** `findByUserId` 실패·`updateQRLoginTokenToUse` 실패 두 가지 분기 모두를 분리해 검증하신 점이 인상적입니다. 예외 메시지 매핑도 명확하여 유지보수성이 좋습니다. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
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: 0
🧹 Nitpick comments (1)
.github/workflows/test-ci.yaml (1)
55-55
: CI 환경에 Slack Webhook URL 시크릿 주입 확인
Create .env file
스텝에SLACK_WEBHOOK_URL
시크릿이 정상적으로 추가되어 Slack 알림 로직이 CI에서 동작할 수 있습니다.
- 저장소 시크릿에
SLACK_WEBHOOK_URL
이 설정되어 있는지 확인해주세요.- 테스트/빌드 과정에서 실제 Slack API 호출이 발생하지 않도록 모킹(mock) 또는 테스트용 폴백(fallback) 처리가 되어 있는지 검토가 필요합니다.
필요시 특수문자나 개행 이슈를 방지하기 위해
printf
를 사용하여 안전하게 환경 변수를 기록할 수 있습니다:- echo "SLACK_WEBHOOK_URL=${{ secrets.SLACK_WEBHOOK_URL }}" >> .env + printf 'SLACK_WEBHOOK_URL=%s\n' "${{ secrets.SLACK_WEBHOOK_URL }}" >> .env
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/test-ci.yaml
(1 hunks)src/repositories/user.repository.ts
(3 hunks)src/services/user.service.ts
(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/repositories/user.repository.ts
- src/services/user.service.ts
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.
코드 전부 잘 읽었습니다!
대공사라 그런가 바뀐 파일 수가 엄청나네요;;
좋았던 점
- 코드에서 쉽게 혼동할만한 부분이 사라진 것 같습니다
- 특히 token 네이밍에서 QR과 JWT가 혼동되는 경우가 많았는데, 이 부분이 훨씬 개선됬네요
- UUID 네이밍도 비교적 확실해졌고요 (이름은 id인데 UUID가 튀어나오는 등의 상황이 사라짐)
- 주석이 늘어나면서 굳이 테스트코드를 읽지 않고도 코드의 의도를 알기 쉬워졌네요
이전에 비해 인증 / 인가의 구분이 확실하게 구분된 것 같아 보기 좋네요!
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.
코드 잘 봤습니다!
코멘트는 가볍게 읽어주세요. 고생 많으셨습니다! 👍 🔥
좋았던 점
- 자주 사용되는 모킹 객체를 상위로 빼서 재사용할 수 있게 만드신 부분이 깔끔하고 좋았습니다.
- 실패 케이스 시점에 따라 후속 함수들은
not.toHaveBennCalledWith()
를 통해 올바른 위치에서 실패했는지 확인하는 부분이 좋았습니다! 테스트 신뢰성이 매우 올라갈 것 같아요! - 벨로그 관련 부분을 아예 모듈로 빼신 게 너무 깔끔하고 좋네요! 그에 따른 테스트 코드 작성까지 굿입니다 👍 👍 (인가 미들웨어도요!)
- 제가 리더보드에서 빼먹은 부분도 감사해요~!
it('사용자를 찾을 수 없으면 next를 호출해야 한다', async () => { | ||
// 유효한 토큰 준비 | ||
mockRequest.cookies = { | ||
'access_token': validToken, | ||
'refresh_token': 'refresh-token' | ||
}; | ||
|
||
// 사용자가 없음 모킹 | ||
(pool.query as jest.Mock).mockResolvedValueOnce({ | ||
rows: [] | ||
}); | ||
|
||
// 미들웨어 실행 | ||
await authMiddleware.verify( | ||
mockRequest as Request, | ||
mockResponse as Response, | ||
nextFunction | ||
); | ||
|
||
// 검증 | ||
expect(nextFunction).toHaveBeenCalledTimes(1); | ||
expect(mockRequest.user).toBeUndefined(); | ||
}); |
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.
미들웨어 로직상 DB에 사용자가 없으면 DB Error를 던지고 next를 호출하는데, 그 부분이 포함되면 더 좋을 것 같네요~!
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.
좋은 포인트 지적 너무 감사해요! 테스트 구성을 아예 바꿀게요! 생각해보니까 에러 여부 자체 테스팅 보다
- next callback 호출하는지 2) 동시에 callback 인자에 error instance 가 인자로 주어지는지 로 모두 맞출게요!
it('유저 ID로 토큰을 사용 처리해야 한다', async () => { | ||
(mockPool.query as jest.Mock).mockResolvedValueOnce(undefined); | ||
|
||
await expect(repo.markTokenUsed('token')).resolves.not.toThrow(); | ||
await expect(repo.updateQRLoginTokenToUse(1)).resolves.not.toThrow(); | ||
|
||
expect(mockPool.query).toHaveBeenCalledTimes(1); | ||
expect(mockPool.query).toHaveBeenCalledWith( | ||
expect.stringContaining('UPDATE users_qrlogintoken SET is_used = true'), | ||
['token'] | ||
expect.stringContaining('UPDATE users_qrlogintoken'), | ||
[1] |
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.
좋아요~! targetUserId
로 바꿔 볼게요!
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.
좋았던 점
- QRToken에 대한 CustomException을 따로 정의하는 부분을 놓쳤었는데, 명확히 짚어주셔서 배울 수 있었습니다!
- auth.middleware에서는 인가만 관리하도록 Velog 관련 모듈을 독립시키신 부분이 인상깊었습니다.
PS. 이번 UUID → bigint ID 관련 오류는 제가 먼저 캐치하고 수정했어야 했는데, 휴일 중에도 FU 해주셔서 감사합니다! 당장 눈 앞의 오류를 해결하기에만 집중했는데, 조금 더 넓게 바라봐야한다는 점을 배웠던 것 같습니다. 이번 PR을 통해서 인증/인가 구조의 근본적인 방향성 및 기존 코드에 대한 명확한 이해의 중요성도 다시 한번 상기되었네요!
constructor(message = '유효하지 않은 QR 토큰입니다') { | ||
super(message, 'INVALID_TOKEN'); | ||
} | ||
} |
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.
QR 토큰에 대한 에러는 UnauthorizedError가 아닌 BadRequestError가 확실히 더 적절하겠네요!
디테일한 부분을 놓쳤는데 수정해주셔서 감사합니다!👍
throw new TokenExpiredError(); | ||
const userLoginToken = await this.userService.useToken(token); | ||
if (!userLoginToken) { | ||
throw new QRTokenExpiredError(); |
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.
found가 아닌 userLoginToken으로 명시하니 코드가 의미하는 바가 명확해진 것 같습니다!
이 역시 디테일한 부분을 놓쳤었던 것 같네요🥲
try { | ||
const query = ` | ||
UPDATE users_qrlogintoken SET is_used = true WHERE token = $1; | ||
UPDATE users_qrlogintoken SET is_used = true WHERE user_id = $1; |
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.
유저 ID 기반으로 모든 토큰을 비활성화시켜야 한 번 로그인했을 때 이전에 생성된 QR 토큰은 모두 무효화가 되겠네요,,!!
이 부분 역시 디테일하게 수정해주셔서 감사합니다🙏
🔥 변경 사항
🏷 관련 이슈
📸 스크린샷 (UI 변경 시 필수)
📌 체크리스트
Summary by CodeRabbit
신규 기능
기능 개선
버그 수정
테스트
기타