[Sangwan] week09 미션#104
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e5bf384d41
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| filterChain.doFilter(request, response); | ||
| } catch (Exception e) { | ||
| securityResponseWriter.writeFailure(response, GeneralErrorCode.UNAUTHORIZED); |
There was a problem hiding this comment.
try-catch가 filterChain.doFilter까지 감싸져 있어 JWT 검증과 무관한 하위 체인 예외도 모두 UNAUTHORIZED로 변환됩니다. 이렇게 되면 실제 실패 원인과 HTTP 의미가 사라져 예외 처리 흐름을 학습하기 어렵고, 문제 재현/디버깅도 힘들어집니다. 토큰 추출·검증 구간만 별도로 예외 처리하고 체인 내부 예외는 상위 예외 처리로 전달되게 분리하세요. 다음으로는 Spring Security의 필터 체인 예외 전파(OncePerRequestFilter, ExceptionTranslationFilter)를 함께 학습해보면 좋습니다.
Useful? React with 👍 / 👎.
| Member member = memberRepository.findBySocialTypeAndSocialId(providerId, socialUid) | ||
| .orElseGet(() -> { | ||
| Member newMember = MemberConverter.toMember(dto); | ||
| memberRepository.save(newMember); |
There was a problem hiding this comment.
신규 소셜 회원 생성 조건이 socialType + socialId만이라서, 같은 이메일로 이미 가입된 계정이 있으면 save 시 DB 유니크 제약에서 터져 500으로 끝납니다. Spring/JPA에서는 이런 도메인 충돌을 DB 예외로 터뜨리기보다 서비스 레이어에서 의도를 가진 비즈니스 예외로 변환해 주는 습관이 중요합니다. findByEmail 기반으로 계정 연결/차단 정책을 먼저 결정하고, 중복 상황은 MemberException으로 일관되게 반환하세요. 다음으로는 유니크 제약과 도메인 예외 매핑 전략을 학습해보세요.
Useful? React with 👍 / 👎.
kjhh2605
left a comment
There was a problem hiding this comment.
[키워드 조사]
이번 PR diff에는 Week9 keyword_summary 신규 파일이 확인되지 않았습니다. 기존 keyword_summary 원본은 위키 raw 경로에 보관했습니다. JWT와 OAuth2 구현을 함께 진행한 만큼 Authorization Code 흐름, Kakao 사용자 정보의 필수·선택 동의 항목, 일반 회원과 소셜 회원의 계정 연결 정책을 추가로 정리하는 것을 권장합니다.
[코드 리뷰]
회원가입, 일반 로그인, JWT 필터, OAuth2 성공 핸들러, 인증 사용자 기준의 me API까지 한 흐름으로 연결한 점이 좋습니다. SecurityResponseWriter로 보안 예외 응답을 통일한 방향도 적절합니다. 다만 OAuth 사용자 정보가 항상 존재한다고 가정하는 부분과 JWT 필터 위치는 실제 인증 실패·콜백 실패 상황에서 동작을 흔들 수 있으므로 보완이 필요합니다.
|
|
||
| SocialType providerId; | ||
| String socialUid; | ||
| Map<String, Object> attributes = oAuthMember.getAttribute("kakao_account"); |
There was a problem hiding this comment.
kakao_account가 없거나 동의 항목에서 제외되면 바로 아래 attributes.get(...)에서 예외가 발생할 수 있습니다. 외부 OAuth 응답은 항상 완전하다고 보기 어렵기 때문에 null 검증 후 도메인 예외로 변환하는 흐름을 먼저 두는 것을 권장합니다.
| OAuthDTO dto; | ||
| switch (providerId) { | ||
| case KAKAO -> { | ||
| String email = attributes.get("email").toString(); |
There was a problem hiding this comment.
Kakao 이메일은 사용자 동의 상태에 따라 내려오지 않을 수 있습니다. toString()을 바로 호출하기보다 이메일이 필수인 정책인지, 임시 회원을 허용할지, 추가 동의를 요구할지 정책을 정하고 예외 처리를 분리하는 것을 권장합니다.
| //세션 | ||
| .sessionManagement(AbstractHttpConfigurer::disable) | ||
| //JWT 필터 | ||
| .addFilterAfter(jwtAuthFilter(), LogoutFilter.class) |
There was a problem hiding this comment.
JWT 인증 필터는 일반적으로 UsernamePasswordAuthenticationFilter 앞에 배치해 컨트롤러 진입 전에 인증 객체를 구성합니다. LogoutFilter 뒤에 둔 이유가 명확하지 않다면 표준 위치와 현재 위치의 실행 순서를 비교해 보는 것을 권장합니다.
|
|
||
| ### 토큰 기반 인증 (Stateless) | ||
|
|
||
| 토큰 기반 인증은 로그인 성공 시 서버가 JWT 토큰을 발급하고, 클라이언트가 이를 저장(로컬 스토리지, 세션 스토리지 등)하는 방식이다. 이후 요청 시 Authorization: Bearer <token> 헤더에 토큰을 담아 전송하면, 서버는 DB 조회 없이 토큰의 서명만 검증하여 사용자를 식별한다. |
There was a problem hiding this comment.
토큰 기반 인증에서 서버가 DB를 전혀 조회하지 않는다고 단정하면 Refresh Token 저장소, 토큰 블랙리스트, 사용자 상태 확인 같은 예외 전략이 빠져 보일 수 있습니다. Access Token 검증만 놓고 보면 서명 검증으로 처리할 수 있지만, 재발급·로그아웃·강제 만료 정책에서는 서버 저장소 조회가 필요할 수 있음을 함께 구분하는 것을 권장합니다.
|
|
||
| ### 액세스 토큰 (Access Token) | ||
|
|
||
| 액세스 토큰은 실제 API 요청 시 인증에 사용하는 메인 JWT 토큰이다. 모든 Private API 호출 시 `Authorization: Bearer <access_token>` 헤더에 담아 전송한다. |
There was a problem hiding this comment.
Authorization 헤더 예시의 백틱이 원문에서 닫히지 않아 Markdown 가독성이 낮아질 수 있습니다. 인증 헤더 형식은 실습에서 자주 복사되는 부분이므로 코드 표기와 설명을 정확히 맞추는 것을 권장합니다.
| | 구분 | 액세스 토큰 (Access Token) | 리프레시 토큰 (Refresh Token) | | ||
| | --- | --- | --- | | ||
| | 역할 | API 인증에 직접 사용 | 액세스 토큰 재발급용 | | ||
| | 유효기간 | 짧음 (30분 ~ 4시간) | 김 (7일 ~ 30일) | |
There was a problem hiding this comment.
리프레시 토큰 유효기간 설명에 오탈자가 있습니다. 개념 자체는 적절하게 정리되어 있으므로 김을 긴 기간처럼 의미가 분명한 표현으로 수정하면 비교표의 전달력이 높아집니다.
🔗 연관 이슈
closes #102
🛠 작업 내용
🖼 스크린샷 (선택)
👀 리뷰 요구사항 (선택)
🤖 AI 활용
💬 나의 프롬프트
카카오 로그인 왜 안됨
코드 최적화할 부분 있어?
🧠 AI 응답
env에서 "" 빼야됨
아직 남아있는 memberId 파라미터 제거
Security 응답 작성 방식 통일. 지금 SecurityConfig는 직접 JSON 문자열을 만들고, JwtAuthFilter, OAuthSuccessHandler는 new ObjectMapper()를 매번 만들고 있어. 동작은 되는데 한 곳으로 모으면 코드가 덜 흩어져. 예를 들면 ObjectMapper를 주입받거나, 작은 SecurityResponseWriter 같은 헬퍼로 401/403/OAuth 성공 응답을 통일하면 깔끔해.
✅ 내가 최종 선택한 방법 (이유)
빼주고 그래들에서도 자동으로 빼주게끔 처리
memberId 입력 제거하고 @AuthenticationPrincipal AuthMember 기준으로 현재 로그인 회원 id를 사용하게 변경해
그렇게 해줘
💡 나만의 Tip (선택)