Conversation
* [feat] Applicant 도메인 준비 * [ci] tmp/dev 브랜치에서 ci 활성화 (임시) * [chore] 주석 추가 및 수정
* [refactor] 인증 대상 테이블을 applicant로 전환 * [Refactor] JWT claim 이름을 applicantId로 변경 * [Refactor] 일반 인증 흐름의 Member 의존을 Applicant로 변경 * [Refactor] 관리자 인증 흐름의 Member 의존을 Applicant로 변경 * [refactor] Member에서 이관 완료된 인증 엔드포인트, 로직 제거 * [chore] 함수 제거 및 주석 추가
* [refactor]: 지원서 소유자를 applicant로 전환 * [db]: 지원서 외래키를 applicant로 전환 * [test]: 지원서 테스트를 applicant 기준으로 수정 * [test]: applicant 계정 조회 쿼리 테스트 이관 * [test]: applicant repository 테스트 네이밍 정리
Walkthrough지원자 도메인과 지원서/인증 흐름이 회원 기준에서 지원자 기준으로 전환되었습니다. 관련 컨트롤러, 서비스, 저장소, JWT 처리, 마이그레이션, 테스트가 함께 수정되었습니다. Changes지원자 핵심 모델
관리자 계정과 백오피스 처리
인증과 토큰 발급
지원서 처리
지원자 API와 웹 설정
테스트와 마이그레이션
Sequence Diagram(s)sequenceDiagram
participant AuthController
participant AuthService
participant JwtTokenProvider
participant CustomSuccessHandler
AuthController->>AuthService: refreshAccessToken(refreshToken)
AuthService->>JwtTokenProvider: extractApplicantId(refreshToken)
JwtTokenProvider-->>AuthService: applicantId
AuthService-->>AuthController: applicantId
AuthController->>CustomSuccessHandler: onAuthenticationSuccess(response, refreshToken, applicantId)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
Test Results395 tests 395 ✅ 13s ⏱️ Results for commit 9b145c1. ♻️ This comment has been updated with latest results. |
📊테스트 커버리지
|
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/org/ject/support/common/security/AuthenticatedApplicantIdResolver.java (1)
30-36:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAuthenticatedApplicantIdResolver에서 principal 타입 검증 누락으로 ClassCastException(500) 가능
authentication.isAuthenticated()만으로는authentication.getPrincipal()이 항상CustomUserDetails임이 보장되지 않아, 현재(CustomUserDetails) authentication.getPrincipal()캐스팅에서ClassCastException이 발생할 수 있습니다. (실제로AuthenticatedApplicantIdResolverTest#resolveArgumentWithInvalidAuthentication에서 invalid authentication 시ClassCastException이 발생하는 케이스가 존재)
instanceof CustomUserDetails로 타입을 먼저 확인하고 아니면GlobalException(EMPTY_ACCESS_TOKEN)등으로 처리하도록 보완해 주세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/org/ject/support/common/security/AuthenticatedApplicantIdResolver.java` around lines 30 - 36, In AuthenticatedApplicantIdResolver, after retrieving Authentication via SecurityContextHolder.getContext().getAuthentication() and checking authentication.isAuthenticated(), add an instanceof check on authentication.getPrincipal() to ensure it's a CustomUserDetails before casting; if the principal is not a CustomUserDetails, throw the same GlobalException(EMPTY_ACCESS_TOKEN) (or similar invalid-token error) instead of casting directly in the code path that currently does (CustomUserDetails) authentication.getPrincipal() and returns userDetails.getApplicantId().
🧹 Nitpick comments (1)
src/test/java/org/ject/support/domain/applicant/controller/ApplicantControllerTest.java (1)
119-120: ⚡ Quick winSecurityContext 정리는
@AfterEach로 일원화하는 편이 안전합니다.본문 말미 수동 정리는 assertion 실패 시 실행되지 않아 다음 테스트를 오염시킬 수 있습니다. 공통 teardown으로 이동하세요.
예시 수정
+ import org.junit.jupiter.api.AfterEach; ... + `@AfterEach` + void tearDown() { + SecurityContextHolder.clearContext(); + }- // 테스트 후 인증 정보 초기화 - SecurityContextHolder.clearContext();Also applies to: 144-145, 167-168, 192-193, 217-218
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/org/ject/support/domain/applicant/controller/ApplicantControllerTest.java` around lines 119 - 120, Multiple tests in ApplicantControllerTest call SecurityContextHolder.clearContext() manually at the end of test methods, which can be skipped on assertion failure; instead add a single `@AfterEach` teardown method in the ApplicantControllerTest class that calls SecurityContextHolder.clearContext(), then remove the manual SecurityContextHolder.clearContext() calls from individual tests (the occurrences currently present at the ends of test methods). Ensure the new method is annotated with org.junit.jupiter.api.AfterEach and is placed in the ApplicantControllerTest class so all tests are cleaned up reliably.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/org/ject/support/common/security/jwt/JwtTokenProvider.java`:
- Around line 169-173: The reissueAccessToken method currently trusts the
caller-supplied applicantId, opening a forgery path; change reissueAccessToken
to extract the user id from the validated refresh token (via
getAuthenticationByToken / Authentication.getPrincipal() or the token subject)
and compare it to the provided applicantId, and if they differ throw a security
exception (e.g., BadCredentialsException or custom JwtException); alternatively,
ignore the parameter and pass the token-derived id into createAccessToken so
only the token's internal identity is trusted (update calls to validateToken,
getAuthenticationByToken and createAccessToken accordingly).
In `@src/main/java/org/ject/support/domain/applicant/entity/Applicant.java`:
- Around line 118-155: The entity exposes and mutates the internal
interestedDomains list directly causing NPEs and state leaks; update
Applicant.toEditor(), Applicant.edit(ApplicantEditor) and
Applicant.deleteProfile() to defensively copy and null-safe the list: in
toEditor() pass a copy (e.g., new List from this.interestedDomains or empty list
if null) instead of the raw reference; in edit(...) assign a defensive copy of
editor.interestedDomains (or an empty list when null) rather than the raw list
reference; in deleteProfile() avoid calling interestedDomains.clear() on a
potentially null list—either set interestedDomains to an empty list instance or
null-safe clear after checking for null. Ensure all three methods use the same
defensive-copy pattern to prevent external mutation and NPEs.
In
`@src/main/java/org/ject/support/domain/applicant/repository/ApplicantQueryRepositoryImpl.java`:
- Around line 29-37: Modify findEmailsByIdsAndNotSubmitted to take a recruitId
parameter and return distinct emails for applicants in that recruit who have no
SUBMITTED application rows: replace the current join-based query with a
queryFactory.selectDistinct(applicant.email).from(applicant).where(applicant.id.in(applicantIds),
applicant.isDeleted.eq(false), applicant.recruit.id.eq(recruitId),
JPAExpressions.selectOne().from(apply).where(apply.applicant.id.eq(applicant.id),
apply.status.eq(SUBMITTED)).notExists()) so the method uses selectDistinct and a
NOT EXISTS(subquery for SUBMITTED) check instead of the current join to avoid
false positives and duplicate emails.
In
`@src/main/java/org/ject/support/domain/applicant/service/ApplicantService.java`:
- Around line 39-57: The current registerTempApplicant uses
applicantRepository.findByEmail(email) and throws
ApplicantException(ALREADY_EXIST_APPLICANT) which incorrectly blocks
reapplications across semesters and leaves a race condition; change the
existence check to use the recruit-scoped key (e.g.,
applicantRepository.findByEmailAndRecruitId(email, recruitId) or
findByEmailAndSemesterId(email, semesterId)) inside registerTempApplicant,
update createTempApplicantWithPin to accept recruitId/semesterId, and rely on a
DB unique constraint on (email, recruit_id) plus transactional handling (or
catch DataIntegrityViolationException) instead of a simple check-then-insert to
avoid concurrent duplicate inserts.
In `@src/main/java/org/ject/support/domain/auth/service/AuthService.java`:
- Around line 101-104: findApplicant currently calls
applicantRepository.findByEmail(email) which will throw at runtime if multiple
applicants share the same email; change the lookup to handle duplicates
deterministically: use a repository method that returns multiple results (e.g.,
findAllByEmail or findTopByEmailOrderByCreatedAtDesc) and update findApplicant
to (1) if none found throw ApplicantException(NOT_FOUND_APPLICANT), (2) if
exactly one return it, and (3) if multiple either return a deterministic single
record (e.g., most recently created) or throw a new DuplicateApplicantException
(or a specific ApplicantException code) so the authentication path does not
crash with a 500; update references to applicantRepository.findByEmail, the
findApplicant method, and exception usage (ApplicantException /
NOT_FOUND_APPLICANT) accordingly.
In
`@src/test/java/org/ject/support/common/security/jwt/JwtAuthenticationFilterTest.java`:
- Line 111: 현재 검증은 findByIdAndRoleIn(1L, Role.backofficeRoles()) 호출만 막고 있어 다른 인자
호출을 놓칠 수 있으니, repository 접근 자체가 없어야 하는 비백오피스 시나리오에 맞게 더 강한 검증으로 교체하세요: 해당 라인의
verify(applicantRepository, never()).findByIdAndRoleIn(...) 대신
applicantRepository에 대해 verifyNoInteractions(applicantRepository)을 사용해 어떤 메서드
호출도 허용하지 않도록 변경하고, 필요하면 findByIdAndRoleIn 메서드명이 테스트에서 참조되는 부분을 함께 정리하세요.
In
`@src/test/java/org/ject/support/domain/applicant/controller/ApplicantControllerTest.java`:
- Around line 103-105: The stub uses
lenient().doNothing().when(applicantService).registerInitialProfile(any(),
eq(applicantId)) which allows the controller to pass a wrong/null applicantId
unnoticed; update each stub (and similar stubs at the other occurrences) to
either stub with the exact applicantId (replace any() with eq(applicantId)) or
keep the lenient stub but add a corresponding verify call after the controller
invocation: verify(applicantService).registerInitialProfile(any(),
eq(applicantId)); doing this for registerInitialProfile and the other service
methods mentioned will ensure the applicantId argument is asserted with
eq(applicantId).
---
Outside diff comments:
In
`@src/main/java/org/ject/support/common/security/AuthenticatedApplicantIdResolver.java`:
- Around line 30-36: In AuthenticatedApplicantIdResolver, after retrieving
Authentication via SecurityContextHolder.getContext().getAuthentication() and
checking authentication.isAuthenticated(), add an instanceof check on
authentication.getPrincipal() to ensure it's a CustomUserDetails before casting;
if the principal is not a CustomUserDetails, throw the same
GlobalException(EMPTY_ACCESS_TOKEN) (or similar invalid-token error) instead of
casting directly in the code path that currently does (CustomUserDetails)
authentication.getPrincipal() and returns userDetails.getApplicantId().
---
Nitpick comments:
In
`@src/test/java/org/ject/support/domain/applicant/controller/ApplicantControllerTest.java`:
- Around line 119-120: Multiple tests in ApplicantControllerTest call
SecurityContextHolder.clearContext() manually at the end of test methods, which
can be skipped on assertion failure; instead add a single `@AfterEach` teardown
method in the ApplicantControllerTest class that calls
SecurityContextHolder.clearContext(), then remove the manual
SecurityContextHolder.clearContext() calls from individual tests (the
occurrences currently present at the ends of test methods). Ensure the new
method is annotated with org.junit.jupiter.api.AfterEach and is placed in the
ApplicantControllerTest class so all tests are cleaned up reliably.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: ebb526df-5bd8-45c8-8054-dbc59bd9e2db
📒 Files selected for processing (76)
.github/workflows/continuous-intergration.ymlsrc/main/java/org/ject/support/admin/account/dto/AdminAccountCreateRequest.javasrc/main/java/org/ject/support/admin/account/dto/AdminAccountResponse.javasrc/main/java/org/ject/support/admin/account/service/AdminAccountService.javasrc/main/java/org/ject/support/admin/apply/dto/AdminApplyDetailResponse.javasrc/main/java/org/ject/support/admin/apply/dto/AdminApplyResponse.javasrc/main/java/org/ject/support/admin/apply/repository/AdminApplyQueryRepositoryImpl.javasrc/main/java/org/ject/support/admin/apply/repository/AdminApplyRepository.javasrc/main/java/org/ject/support/admin/apply/service/AdminApplyService.javasrc/main/java/org/ject/support/admin/apply/service/ApplyPassService.javasrc/main/java/org/ject/support/admin/auth/service/AdminAuthService.javasrc/main/java/org/ject/support/admin/component/AdminMemberComponent.javasrc/main/java/org/ject/support/common/security/AuthenticatedApplicantIdResolver.javasrc/main/java/org/ject/support/common/security/CustomSuccessHandler.javasrc/main/java/org/ject/support/common/security/CustomUserDetailService.javasrc/main/java/org/ject/support/common/security/CustomUserDetails.javasrc/main/java/org/ject/support/common/security/config/WebConfig.javasrc/main/java/org/ject/support/common/security/jwt/JwtAuthenticationFilter.javasrc/main/java/org/ject/support/common/security/jwt/JwtTokenProvider.javasrc/main/java/org/ject/support/domain/applicant/controller/ApplicantApiSpec.javasrc/main/java/org/ject/support/domain/applicant/controller/ApplicantController.javasrc/main/java/org/ject/support/domain/applicant/dto/ApplicantAccountProjection.javasrc/main/java/org/ject/support/domain/applicant/dto/ApplicantDto.javasrc/main/java/org/ject/support/domain/applicant/dto/ApplicantProfileResponse.javasrc/main/java/org/ject/support/domain/applicant/entity/Applicant.javasrc/main/java/org/ject/support/domain/applicant/entity/ApplicantEditor.javasrc/main/java/org/ject/support/domain/applicant/exception/ApplicantErrorCode.javasrc/main/java/org/ject/support/domain/applicant/exception/ApplicantException.javasrc/main/java/org/ject/support/domain/applicant/repository/ApplicantQueryRepository.javasrc/main/java/org/ject/support/domain/applicant/repository/ApplicantQueryRepositoryImpl.javasrc/main/java/org/ject/support/domain/applicant/repository/ApplicantRepository.javasrc/main/java/org/ject/support/domain/applicant/service/ApplicantService.javasrc/main/java/org/ject/support/domain/apply/controller/ApplyApiSpec.javasrc/main/java/org/ject/support/domain/apply/controller/ApplyController.javasrc/main/java/org/ject/support/domain/apply/domain/Apply.javasrc/main/java/org/ject/support/domain/apply/repository/ApplicationFormRepository.javasrc/main/java/org/ject/support/domain/apply/repository/ApplyRepository.javasrc/main/java/org/ject/support/domain/apply/service/ApplyService.javasrc/main/java/org/ject/support/domain/apply/service/ApplyUsecase.javasrc/main/java/org/ject/support/domain/apply/service/RemindApplyService.javasrc/main/java/org/ject/support/domain/auth/controller/AuthController.javasrc/main/java/org/ject/support/domain/auth/controller/DevAuthController.javasrc/main/java/org/ject/support/domain/auth/service/AuthService.javasrc/main/java/org/ject/support/domain/member/controller/MemberApiSpec.javasrc/main/java/org/ject/support/domain/member/dto/MemberProfileResponse.javasrc/main/java/org/ject/support/domain/member/repository/MemberQueryRepository.javasrc/main/java/org/ject/support/domain/member/repository/MemberQueryRepositoryImpl.javasrc/main/java/org/ject/support/domain/member/service/MemberService.javasrc/main/resources/db/migration/V32__convert_apply_member_to_applicant.sqlsrc/test/java/org/ject/support/admin/account/controller/AdminAccountControllerTest.javasrc/test/java/org/ject/support/admin/account/service/AdminAccountServiceTest.javasrc/test/java/org/ject/support/admin/apply/repository/AdminApplyQueryRepositoryTest.javasrc/test/java/org/ject/support/admin/apply/service/AdminApplyServiceTest.javasrc/test/java/org/ject/support/admin/apply/service/ApplyPassServiceTest.javasrc/test/java/org/ject/support/admin/auth/service/AdminAuthServiceTest.javasrc/test/java/org/ject/support/admin/component/AdminMemberComponentTest.javasrc/test/java/org/ject/support/common/security/AuthenticatedApplicantIdResolverTest.javasrc/test/java/org/ject/support/common/security/CustomUserDetailServiceTest.javasrc/test/java/org/ject/support/common/security/CustomUserDetailsTest.javasrc/test/java/org/ject/support/common/security/jwt/JwtAuthenticationFilterTest.javasrc/test/java/org/ject/support/common/security/jwt/JwtTokenProviderTest.javasrc/test/java/org/ject/support/domain/applicant/ApplicantTest.javasrc/test/java/org/ject/support/domain/applicant/controller/ApplicantControllerTest.javasrc/test/java/org/ject/support/domain/applicant/repository/ApplicantRepositoryTest.javasrc/test/java/org/ject/support/domain/applicant/service/ApplicantServiceTest.javasrc/test/java/org/ject/support/domain/apply/controller/ApplyControllerTest.javasrc/test/java/org/ject/support/domain/apply/domain/ApplyTest.javasrc/test/java/org/ject/support/domain/apply/repository/ApplyRepositoryTest.javasrc/test/java/org/ject/support/domain/apply/service/ApplyServiceTest.javasrc/test/java/org/ject/support/domain/auth/AuthControllerTest.javasrc/test/java/org/ject/support/domain/auth/AuthServiceTest.javasrc/test/java/org/ject/support/domain/member/repository/MemberAccountQueryRepositoryTest.javasrc/test/java/org/ject/support/domain/member/repository/MemberQueryRepositoryTest.javasrc/test/java/org/ject/support/domain/recruit/repository/ApplicationFormRepositoryTest.javasrc/test/java/org/ject/support/testconfig/AuthenticatedUser.javasrc/test/java/org/ject/support/testconfig/WithAuthenticatedUserSecurityContextFactory.java
💤 Files with no reviewable changes (7)
- src/main/java/org/ject/support/domain/member/repository/MemberQueryRepository.java
- src/test/java/org/ject/support/domain/member/repository/MemberAccountQueryRepositoryTest.java
- src/main/java/org/ject/support/domain/member/controller/MemberApiSpec.java
- src/main/java/org/ject/support/domain/member/dto/MemberProfileResponse.java
- src/main/java/org/ject/support/domain/member/service/MemberService.java
- src/test/java/org/ject/support/domain/member/repository/MemberQueryRepositoryTest.java
- src/main/java/org/ject/support/domain/member/repository/MemberQueryRepositoryImpl.java
#️⃣연관된 이슈
close #561
📝 작업 내용
하위 이슈작업 마무리로 dev에 병합합니다
Summary by CodeRabbit
Release Notes