[Gibeom] Week8 미션#92
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 886eab92e8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| ){ | ||
| BaseSuccessCode code = MemberSuccessCode.OK; | ||
| return ApiResponse.onSuccess(code, memberService.getInfo(memberId)); | ||
| MemberResDTO.GetInfo result = memberService.getInfo(memberId); |
There was a problem hiding this comment.
AuthMember에서 회원 id를 꺼내도록 수정하세요
이번 변경에서 CustomUserDetailsService가 인증 principal로 AuthMember를 반환하고, SecurityConfig가 대부분의 API를 인증 필요로 바꿨습니다. 이 상태에서 /v1/members/me가 @AuthenticationPrincipal Long을 받으면 타입이 맞지 않아 로그인 후에도 memberId가 null로 들어가고 memberService.getInfo(null) 조회가 실패합니다. AuthMember를 주입받아 getMember().getId()를 꺼내거나 @AuthenticationPrincipal(expression = "member.id")처럼 매핑하세요. 다음으로 Spring Security의 UserDetails principal 바인딩 방식을 학습하면 좋습니다.
Useful? React with 👍 / 👎.
| EMAIL_DUPLICATED(HttpStatus.NOT_FOUND, | ||
| "COMMON404_2", | ||
| "해당 사용자를 찾을 수 없습니다." |
There was a problem hiding this comment.
signUp에서 existsByEmail이 참이면 EMAIL_DUPLICATED를 던지는데, 현재 코드는 404와 "사용자를 찾을 수 없음" 메시지를 반환합니다. 중복 가입은 리소스를 못 찾은 상황이 아니라 검증 실패/충돌 상황이라 클라이언트가 원인을 잘못 이해하고, 예외 코드 학습에서도 NOT_FOUND와 CONFLICT의 책임이 섞입니다. 닉네임 중복처럼 HttpStatus.CONFLICT와 "이미 존재하는 이메일입니다" 메시지로 분리하고, 다음에는 HTTP 상태 코드와 도메인 예외 매핑을 같이 정리해 보세요.
Useful? React with 👍 / 👎.
| ) { | ||
| BaseSuccessCode code = ReviewSuccessCode.OK; | ||
| return ApiResponse.onSuccess(code, reviewService.getMemberReviewsOrderById(memberId,pageSize, cursor, query)); | ||
| ReviewResDTO.Pagination<ReviewResDTO.getReview> result = reviewService.getMemberReviewsOrderByScore(memberId, pageSize, cursor, query); |
There was a problem hiding this comment.
컨트롤러는 여전히 query 파라미터를 받지만 항상 getMemberReviewsOrderByScore만 호출합니다. 그래서 기존처럼 query=id로 커서 조회하면 커서가 있을 때 QUERY_NOT_VALID가 나고, 첫 페이지(cursor=-1)에서는 요청과 다르게 별점순으로 내려갑니다. query 값에 따라 id/score 서비스로 분기하거나 하나의 서비스 내부에서 정렬 전략을 선택하게 바꾸세요. 다음 학습 포인트는 컨트롤러 파라미터가 서비스 책임으로 어떻게 전달되는지와 커서 페이징의 정렬 기준 일관성입니다.
Useful? React with 👍 / 👎.
kjhh2605
left a comment
There was a problem hiding this comment.
[키워드 조사]
Spring Security, 인증과 인가, Stateful/Stateless의 차이를 핵심 키워드로 정리한 점이 좋습니다. 다만 현재 코드가 formLogin 기반 세션 인증을 사용하고 있으므로 Stateful 방식과 연결해 SecurityContext, Session, CSRF 설정이 함께 어떻게 동작하는지 추가로 정리하는 것을 권장합니다. 또한 UserDetailsService, PasswordEncoder, SecurityFilterChain, 401/403 예외 핸들러가 로그인 요청에서 어떤 순서로 호출되는지 흐름도를 작성하면 적용력이 높아집니다.
[코드 리뷰]
회원가입 요청 DTO, 비밀번호 암호화, 중복 이메일/닉네임 검증, UserDetailsService, 인증/인가 예외 핸들러까지 Week8 보안 흐름의 핵심 구성요소를 폭넓게 구현했습니다. 이전 주차에서 진행한 응답 래핑과 페이징 보완도 함께 이어진 점이 좋습니다. 보완할 부분은 인증 주체 타입을 Controller 전반에서 일관되게 맞추는 것, REST API 인증 전략과 formLogin 세션 전략을 분리해 설명하는 것, 권한 모델과 Security 예외 타입을 명확히 하는 것입니다. 인증 기능은 작은 타입 불일치도 런타임 문제로 이어지므로 회원가입 → 로그인 → 인증 필요한 API 호출 흐름을 통합 테스트로 확인하는 것을 권장합니다.
| @AuthenticationPrincipal Long memberId, | ||
| @GetMapping("/v1/members/home") | ||
| public ResponseEntity<ApiResponse<MemberResDTO.HomeResultDto>> getHome( | ||
| @AuthenticationPrincipal AuthMember authMember, |
There was a problem hiding this comment.
AuthMember를 principal로 받는 방향은 좋습니다. 다만 같은 Controller 안에서 다른 API는 아직 @AuthenticationPrincipal Long memberId를 사용하고 있어 인증 주체 타입이 섞입니다. Spring Security가 실제로 주입하는 principal 타입을 하나로 정하고, AuthMember#getMember().getId() 또는 별도 어댑터 방식으로 일관되게 맞추는 것을 권장합니다.
| .requestMatchers(allowUris).permitAll() | ||
| .anyRequest().authenticated() | ||
| ) | ||
| .formLogin(form -> form |
There was a problem hiding this comment.
formLogin을 활성화하면 기본적으로 세션 기반 로그인과 리다이렉트 흐름을 사용합니다. 이번 주차 키워드인 Stateful/Stateless와 직접 연결되는 부분이므로, REST API용 JSON 응답 전략인지 Swagger 테스트용 임시 설정인지 의도를 분리해 두는 것을 권장합니다. JWT 기반 Stateless 인증으로 확장할 계획이라면 session 정책과 CSRF 설정까지 함께 비교하면 이해도가 높아집니다.
|
|
||
| @Override | ||
| public Collection<? extends GrantedAuthority> getAuthorities() { | ||
| return List.of(); |
There was a problem hiding this comment.
현재 권한 목록이 비어 있으면 인증은 가능하더라도 인가 학습으로 확장하기 어렵습니다. 처음에는 ROLE_USER처럼 단순한 기본 권한이라도 Member의 역할 필드나 고정 권한으로 표현해 보고, hasRole/hasAuthority가 어떤 기준으로 동작하는지 확인하는 것을 권장합니다.
| String username | ||
| ) throws UsernameNotFoundException { | ||
| Member member = memberRepository.findByEmail(username) | ||
| .orElseThrow(() -> new MemberException(MemberErrorCode.NOT_FOUND)); |
There was a problem hiding this comment.
UserDetailsService는 Spring Security 인증 흐름 안에서 호출되므로 사용자를 찾지 못했을 때는 UsernameNotFoundException처럼 Security가 기대하는 예외 타입을 사용하는 것이 흐름을 이해하기 좋습니다. 도메인 API에서 사용하는 MemberException과 인증 실패 예외를 분리하면 401 응답 처리와 도메인 예외 처리가 더 명확해집니다.
| Stateless(토큰 기반) : 서버가 상태를 유지하지 않으므로 요청에 포함된 토큰(JWT)로 검증 | ||
|
|
||
| | 구분 | Stateful | Stateless | | ||
| | --- | --- | --- | |
There was a problem hiding this comment.
Stateful/Stateless 비교 표를 작성한 점이 좋습니다. 다만 Markdown 표 구분선의 들여쓰기가 맞지 않아 표가 깨질 수 있으며, 현재 코드의 formLogin은 Stateful 세션 방식에 가깝습니다. 표 아래에 formLogin, SecurityContext, JSESSIONID, JWT 방식의 차이를 현재 구현과 연결해 추가 정리하는 것을 권장합니다.
🔗 연관 이슈
🛠 작업 내용
[x] Spring Security 적용
[x] 회원가입 API구현
[x]ExceptionHandling 구현
🖼 스크린샷 (선택)
👀 리뷰 요구사항 (선택)
🤖 AI 활용
💬 나의 프롬프트
🧠 AI 응답
✅ 내가 최종 선택한 방법 (이유)
💡 나만의 Tip (선택)