feat: 프로필 수정 본인 확인 가드 추가 및 통합 테스트 수정#219
Conversation
# Conflicts: # src/test/java/com/codeit/team5/mopl/user/controller/UserControllerIntegrationTest.java # src/test/java/com/codeit/team5/mopl/user/controller/UserControllerTest.java # src/test/java/com/codeit/team5/mopl/user/service/UserServiceTest.java
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthrough사용자 프로필 수정 API에 본인 확인 인가 가드가 추가되었습니다. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/com/codeit/team5/mopl/config/SecurityConfig.java`:
- Around line 60-61: SecurityConfig still leaves a global permit-all fallback,
so any /api/** route not explicitly matched remains anonymously accessible.
Update the HttpSecurity rules in SecurityConfig to remove the final
anyRequest().permitAll() and replace it with a stricter default such as
anyRequest().denyAll() or anyRequest().authenticated(), while keeping the
existing requestMatchers for /api/users/* intact. Make sure the fallback matches
the linked issue goal of eliminating global public access and only allows routes
that are explicitly intended to be open.
In `@src/main/java/com/codeit/team5/mopl/user/controller/UserController.java`:
- Around line 67-68: 이미지 파싱이 userService.update 호출 전에 먼저 실행되어 비소유자 요청에도
MultipartFiles.toImageResource(image) 검증과 getBytes()가 선행되는 문제를 수정하세요.
UserController의 update 흐름에서 userDetails.getId(), userId, request,
MultipartFiles.toImageResource(image) 호출 순서를 조정해 소유자 확인이 먼저 일어나도록 하거나, 이미지 변환을
지연 평가하도록 update API를 변경해 validateOwner가 먼저 실행되게 하세요. UserController.update,
userService.update, validateOwner, MultipartFiles.toImageResource를 기준으로 위치를 찾아
반영하면 됩니다.
In
`@src/test/java/com/codeit/team5/mopl/follow/controller/FollowControllerIntegrationTest.java`:
- Around line 37-40: FollowControllerIntegrationTest currently relies on
SpringBootTest with SecurityConfig, JwtAuthenticationFilter, and JwtProperties,
but the test profile lacks the required jwt.* configuration. Add the missing JWT
test settings for this test by moving the common inline values into
application-test.yml so JwtProperties can bind successfully and the integration
context starts consistently.
In
`@src/test/java/com/codeit/team5/mopl/user/controller/UserControllerIntegrationTest.java`:
- Around line 233-236: The invalid UUID integration test is asserting the wrong
contract by expecting a 500 response in UserControllerIntegrationTest. Update
the expectation in the test that exercises get("/api/users/{userId}",
"invalid-uuid") to validate 400 Bad Request instead, and if the current runtime
still returns 500, fix the exception-to-response mapping in the
controller/advice path used by UserController so path-variable UUID parsing
failures are translated to a client error rather than a server error.
In `@src/test/java/com/codeit/team5/mopl/user/controller/UserControllerTest.java`:
- Line 345: The updateUser test currently uses any() for both the authenticated
principal id and the target userId, so it can’t detect swapped or duplicated IDs
in UserControllerTest. Update the test around updateUser to use two different
UUIDs for the AuthenticationPrincipal and path parameter, then verify the
userService.update call with explicit argument matching so the first parameter
is the principal id and the target id is the path userId. Apply the same
tightening to the related successful-path assertions in the nearby updateUser
test cases that currently mirror this pattern.
🪄 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: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6214fb78-0334-48a6-995a-f8faddf62d3d
📒 Files selected for processing (12)
gradlewsrc/main/java/com/codeit/team5/mopl/config/SecurityConfig.javasrc/main/java/com/codeit/team5/mopl/follow/entity/Follow.javasrc/main/java/com/codeit/team5/mopl/user/controller/UserController.javasrc/main/java/com/codeit/team5/mopl/user/controller/api/UserApi.javasrc/main/java/com/codeit/team5/mopl/user/exception/UserForbiddenException.javasrc/main/java/com/codeit/team5/mopl/user/service/UserService.javasrc/main/resources/application.ymlsrc/test/java/com/codeit/team5/mopl/follow/controller/FollowControllerIntegrationTest.javasrc/test/java/com/codeit/team5/mopl/user/controller/UserControllerIntegrationTest.javasrc/test/java/com/codeit/team5/mopl/user/controller/UserControllerTest.javasrc/test/java/com/codeit/team5/mopl/user/service/UserServiceTest.java
shong9124
left a comment
There was a problem hiding this comment.
인증쪽 반영된 거 확인했습니다! 팔로우쪽 테스트도 활성화해주신 것도 확인했습니다~
코드 이상 없는 것 같아서 승인합니다!
고생하셨어요!!
관련 이슈
작업 내용
변경 사항
프로필 수정 본인 확인 (#149)
UserService.update에validateOwner(현재 사용자 == 대상 userId) → 불일치 시 403UserForbiddenException(403) 추가,@AuthenticationPrincipal로 인증 주체 식별SecurityConfig:PATCH /api/users/*인증 필요단건 조회 IDOR (#128 이슈 2)
GET /api/users/*인증 필요 → 비로그인 401getById존재 확인(404)은 기존 로직 유지테스트 강화 (#129)
getUser_notFound404 +exceptionType/message검증 (slice·통합)getUser비인증 401 케이스 추가,invalid-uuid500 확정 검사FollowControllerIntegrationTestauthentication()주입 방식으로 복구Swagger CSRF (#217)
springdoc.swagger-ui.csrf.enabled=true→ Swagger UI가 XSRF-TOKEN 쿠키를 X-XSRF-TOKEN 헤더로 자동 주입테스트 내용
체크리스트
리뷰 포인트
스크린샷 / 참고 자료
🔗 관련 이슈
#149#128#129#217📝 작업 내용
이 PR은 사용자 리소스에 대한 보안 정책(인가/인증) 강제와 API 에러 계약을 테스트에서 더 정확히 고정하기 위해 진행됐습니다.
기존에는 프로필 수정 시 “요청자가 본인인지” 검증이 수정 로직 진입 전에 확실히 강제되지 않았고, 사용자 단건 조회도 인증 없이 접근될 수 있어 보안 경계가 느슨했습니다. 또한 통합 테스트가 응답 상태/바디를 느슨하게 검증해 실제 API 계약과 어긋날 여지가 있었습니다.
주요 변경사항
프로필 수정(PATCH /api/users/{userId})에 본인 확인 인가 가드 추가
UserController에서@AuthenticationPrincipal로 인증 주체의 ID를 주입하고,UserService.update로 전달 인자를 확장했습니다.UserService.update내부에서validateOwner(currentUserId, userId)를 호출해 **요청자와 대상userId불일치 시UserForbiddenException(403)**을 던지도록 변경했습니다.requesterId,userId를 포함하도록 고정했습니다.사용자 단건 조회(GET /api/users/{userId}) 인증 요구 강화
SecurityConfig에서GET /api/users/*및PATCH /api/users/*를 엔드포인트별로authenticated()처리하도록 조정했습니다(단,/api/users컬렉션은permitAll유지).401 Unauthorized**로 처리되는 케이스를 추가/정비했습니다.email,role,locked를 포함한 공개 형태를 유지했습니다.통합 테스트/예외 응답 계약을 명세 수준으로 정밀화
getUser_notFound는 단순 상태 코드 범위 대신 정확한404+exceptionType+message응답 바디를 검증하도록 강화했습니다.getUser에 비인증 401 케이스와 **invalid-uuid케이스의 현재 기대값 검증(500)**을 추가했습니다.FollowControllerIntegrationTest는 인증 주입 방식을 복원(authentication()post-processor +csrf()반영)해 실제 시큐리티 필터 동작에 맞게 실행되도록 했습니다.springdoc.swagger-ui.csrf.enabled=true를 설정했습니다.📌 리뷰 포인트 / 트레이드오프
SecurityConfig는 전역 규칙(anyRequest().permitAll())이 남아있는 상태에서/api/users/*등 주요 엔드포인트를 명시적으로 인증 처리하는 방식으로 맞춰져 있습니다. 다른 리소스까지 의도대로 완전히 닫히는지 여부는 범위 밖이며(측정하지 않음), 이번 PR은 사용자 관련 정책과 테스트 신뢰성에 집중했습니다.📊 변경 효과 요약 (가능한 경우에만)
UserControllerIntegrationTest:+54/-19UserControllerTest:+24/-11FollowControllerIntegrationTest:+200/-208application.yml:+5/-0(springdoc.swagger-ui.csrf.enabled=true)application-test.yml:+6/-0(테스트용 JWT 설정 추가)UserService: 업데이트 시그니처 변경 및 본인검증 로직 추가UserForbiddenException: 신규 추가 (403)</final_summary>