Conversation
|
Note
|
| 코호트 / 파일 | 변경 사항 요약 |
|---|---|
인증 전략 인터페이스 src/main/java/OneQ/OnSurvey/global/auth/application/strategy/AuthStrategy.java |
authenticate(HttpServletRequest) 메서드를 선언하는 새로운 공개 인터페이스 추가 |
로컬 인증 구현 src/main/java/OneQ/OnSurvey/global/auth/application/strategy/LocalAuthStrategy.java |
AuthStrategy 구현 클래스 추가. local, docker-local 프로필에서 하드코딩된 Member(id=1, userKey=1234567890, role=MEMBER, status=ACTIVE)를 반환 |
Toss 인증 구현 src/main/java/OneQ/OnSurvey/global/auth/application/strategy/TossAuthStrategy.java |
AuthStrategy 구현 클래스 추가. AuthUseCase와 MemberRepository를 주입받아 authenticateWithToss 결과로부터 Member를 조회 |
인증 필터 리팩토링 src/main/java/OneQ/OnSurvey/global/auth/filter/AuthFilter.java |
TossAuthFilter에서 AuthFilter로 클래스명 변경. AuthStrategy 의존성으로 변경하여 전략 기반 인증 흐름 구현. MemberRepository, LoginMeResponse 의존성 제거 |
보안 설정 업데이트 src/main/java/OneQ/OnSurvey/global/common/config/SecurityConfig.java |
TossAuthFilter 빈과 관련 의존성(memberRepository, authUseCase) 제거. 필터 체인에서 AuthFilter 사용으로 변경 |
Sequence Diagram(s)
sequenceDiagram
participant Client
participant AuthFilter
participant AuthStrategy
participant AuthUseCase
participant MemberRepository
participant SecurityContext
rect rgb(200, 220, 240)
note over Client,SecurityContext: 기존 흐름 (TossAuthFilter)
Client->>AuthFilter: HTTP 요청
AuthFilter->>AuthUseCase: authenticateWithToss(request)
AuthUseCase-->>AuthFilter: userDetails
AuthFilter->>MemberRepository: findByUserKey(userKey)
MemberRepository-->>AuthFilter: Member
AuthFilter->>SecurityContext: CustomUserDetails 등록
end
rect rgb(220, 240, 200)
note over Client,SecurityContext: 신규 흐름 (AuthFilter + AuthStrategy)
Client->>AuthFilter: HTTP 요청
AuthFilter->>AuthStrategy: authenticate(request)
alt Local Profile
AuthStrategy-->>AuthFilter: 하드코딩된 Member 반환
else Production Profile
AuthStrategy->>AuthUseCase: authenticateWithToss(request)
AuthUseCase-->>AuthStrategy: userDetails
AuthStrategy->>MemberRepository: findByUserKey(userKey)
MemberRepository-->>AuthStrategy: Member
AuthStrategy-->>AuthFilter: Member
end
AuthFilter->>SecurityContext: CustomUserDetails 등록
end
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | PR 제목이 변경 사항의 핵심을 명확하게 요약하고 있습니다. 프로파일별 인증로직 분리라는 주요 목표가 명확히 반영되어 있습니다. |
| Description check | ✅ Passed | PR 설명이 저장소의 템플릿을 따르고 있으며, 관련 이슈, 완료된 작업 세부사항이 모두 포함되어 있습니다. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing touches
- 📝 Generate docstrings
Comment @coderabbitai help to get the list of available commands and usage tips.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In
@src/main/java/OneQ/OnSurvey/global/auth/application/strategy/AuthStrategy.java:
- Around line 6-8: Add Javadoc to the AuthStrategy interface's
authenticate(HttpServletRequest request) method describing its contract and
failure behavior: explain it returns a Member on successful authentication and
document that implementations should throw appropriate runtime exceptions (e.g.,
BadCredentialsException or a custom AuthenticationException) when authentication
fails or the request is invalid, and note any nullability guarantees; reference
the authenticate method, the Member return type, and HttpServletRequest as part
of the doc so implementers know what to expect and which exceptions to handle.
In
@src/main/java/OneQ/OnSurvey/global/auth/application/strategy/LocalAuthStrategy.java:
- Around line 18-20: The static block in LocalAuthStrategy uses Lombok's
@Slf4j-generated log (an instance field) which causes a compile error and emits
"authenticate called" at class-load time; remove the static block and move the
logging into the instance method that performs authentication (e.g.,
authenticate(...) in LocalAuthStrategy) or replace with a static logger if you
truly need class-load logging; ensure the log message is emitted at the actual
authentication call and reference LocalAuthStrategy and the authenticate method
when making this change.
- Around line 15-16: Remove the unnecessary Lombok no-arg constructor annotation
from the LocalAuthStrategy class: delete the @NoArgsConstructor annotation above
the class declaration (LocalAuthStrategy implements AuthStrategy) and also
remove the corresponding import (lombok.NoArgsConstructor) if it becomes unused;
the class has no fields so the Java compiler provides the default constructor
and Spring can instantiate the bean without an explicit no-arg constructor.
In
@src/main/java/OneQ/OnSurvey/global/auth/application/strategy/TossAuthStrategy.java:
- Around line 23-25: The static initializer in TossAuthStrategy is using the
Lombok-generated instance-level log (from @Slf4j), causing a compile error; also
the message wrongly references "ProdAuthStrategy". Fix by either removing the
static block or declaring a static logger (e.g., a private static final Logger
LOGGER = LoggerFactory.getLogger(TossAuthStrategy.class)) and using that in the
static block, and update the message to "TossAuthStrategy" (or simply move the
info log into an instance initializer/constructor to use the Lombok log).
- Around line 14-18: The project currently only provides TossAuthStrategy (class
TossAuthStrategy, annotated @Profile("prod") and implementing AuthStrategy),
which can cause NoSuchBeanDefinitionException for other profiles; add a fallback
implementation (e.g., class DefaultAuthStrategy implements AuthStrategy) and
register it as a Spring bean (either with @Component and @Primary or with
@Profile("!prod") to cover non-prod profiles) so an AuthStrategy is always
available, or alternatively implement a startup validator bean that checks
active profiles and fails fast with a clear message; reference TossAuthStrategy
and AuthStrategy when adding the fallback or validator.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
src/main/java/OneQ/OnSurvey/global/auth/application/strategy/AuthStrategy.javasrc/main/java/OneQ/OnSurvey/global/auth/application/strategy/LocalAuthStrategy.javasrc/main/java/OneQ/OnSurvey/global/auth/application/strategy/TossAuthStrategy.javasrc/main/java/OneQ/OnSurvey/global/auth/filter/AuthFilter.javasrc/main/java/OneQ/OnSurvey/global/common/config/SecurityConfig.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/OneQ/OnSurvey/global/auth/application/strategy/LocalAuthStrategy.java (3)
src/main/java/OneQ/OnSurvey/global/common/entity/BaseEntity.java (1)
NoArgsConstructor(16-37)src/main/java/OneQ/OnSurvey/global/auth/application/strategy/TossAuthStrategy.java (1)
Slf4j(14-33)src/main/java/OneQ/OnSurvey/global/auth/filter/AuthFilter.java (1)
Component(24-73)
⏰ 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). (1)
- GitHub Check: build-and-push
🔇 Additional comments (5)
src/main/java/OneQ/OnSurvey/global/auth/filter/AuthFilter.java (2)
27-31: 전략 패턴을 통한 깔끔한 리팩토링 👍
AuthFilter가AuthStrategy에 의존하도록 변경하여 인증 로직을 효과적으로 분리했습니다. 이제 필터는 인증의 구체적인 구현 방식(Toss vs Local)을 알 필요 없이 전략 인터페이스에만 의존합니다.
53-72: 인증 흐름이 적절하게 구현되었습니다.전략을 통한 인증 → 상태 확인 → SecurityContext 설정의 흐름이 명확합니다. 예외 발생 시 SecurityContext를 정리하는 처리도 적절합니다.
src/main/java/OneQ/OnSurvey/global/common/config/SecurityConfig.java (1)
51-66: SecurityConfig가 AuthFilter를 올바르게 주입받습니다.Spring이 활성 프로파일에 따라 적절한
AuthStrategy구현체를AuthFilter에 주입하므로, 설정 변경 없이 환경별 인증 로직이 적용됩니다.src/main/java/OneQ/OnSurvey/global/auth/application/strategy/TossAuthStrategy.java (1)
27-32: 인증 로직 구현이 올바릅니다.Toss 인증 후 userKey로 Member를 조회하는 흐름이 적절합니다. Member를 찾지 못할 때
BadCredentialsException을 던지는 것도 적합합니다.src/main/java/OneQ/OnSurvey/global/auth/application/strategy/LocalAuthStrategy.java (1)
22-30: Remove or clarify the hardcoding concern—this is intentional for local developmentThis Member stub is designed for local profile testing only (
@Profile({"local", "docker-local"})) and is not meant to represent a database record. The Member object is created fresh in memory and never queried back from the database. Theid=1Lis used as a parameter to filter related entities (surveys, answers) bymemberId, not to fetch the Member itself.No action needed—the implementation is correct for its intended use case (local authentication without OAuth setup).
Likely an incorrect or invalid review comment.
✨ Related Issue
📌 Task Details
authenticate로직을 전략패턴으로 추출@Bean,@Component어노테이션이 중복되어 있던 것 수정💬 Review Requirements (Optional)
Summary by CodeRabbit
릴리스 노트
✏️ Tip: You can customize this high-level summary in your review settings.