Conversation
There was a problem hiding this comment.
레디스를 사용하기 위한 세팅부터, 알맞는 코드까지 작성하시느라 고생하셨습니다.
컨트롤러 리뷰에서 언급한 토큰 인증 필터 체인 사용과 이에 따른 서비스 로직의 책임 분리, 또한 Redis 조작과 관련된 서비스 로직의 책임 분리를 조금 더 신경쓰면 골져스한 코드가 완성될 것 같습니다!
마지막으로 AuthService, MemberService, JwtAuthenticationFilterTest, JwtTokenResolverTest 클래스의 몇몇 테스트가 실패했는데, 깃허브 CI가 로컬 환경과 다소 달라서 발생하는 문제인 것으로 보입니다. 해결 부탁 드립니다!
| @@ -23,4 +23,11 @@ public CsResponse<AuthLoginResponse> login( | |||
| AuthLoginResponse token = authService.loginMember(request); | |||
| return CsResponse.of(AuthSuccessCode.LOGIN_SUCCESS, token); | |||
| } | |||
| CsResponse<AuthLoginResponse> login( | ||
| @RequestBody AuthLoginRequest request | ||
| ); | ||
|
|
| UNAUTHORIZED_FAILED_VALIDATION(8002, "인증에 실패했습니다."), | ||
| UNAUTHORIZED_PASSWORD(8011, "비밀번호가 일치하지 않습니다."), | ||
|
|
||
| INTERNAL_SERVER_ERROR(5000, "서버에서 예기치 못한 오류가 발생했습니다.") |
There was a problem hiding this comment.
5000 에러는 GlobalExceptionHandler에 구현되어 있기 때문에 이넘 작성할 필요 없어용
There was a problem hiding this comment.
AuthController 리뷰에서 언급한 필터 체인 사용과 별개로,
각 메서드가 토큰 발급 혹은 인증과 레디스 토큰 관리를 모두 담당하고 있어 단일 책임 원칙 (SRP)에 부합하지 않은 설계라고 보여집니다.
Redis 조작 로직을 별도의 메서드로 분리하거나, 더욱 책임을 분산시키기 위해서 별도의 클래스로 작성하여 레디스 관련 로직을 모두 위임하는 방법도 좋을 것 같습니다.
| .authorizeHttpRequests(authz -> authz | ||
| .requestMatchers(PathRequest.toH2Console()).permitAll() // H2 콘솔 경로 허용 | ||
| .requestMatchers("/member/register", "/auth/login", "/swagger-ui/**", "/v3/api-docs/**").permitAll() | ||
| .requestMatchers("/member/register", "/auth/login", "auth/logout", "/swagger-ui/**", "/v3/api-docs/**").permitAll() |
There was a problem hiding this comment.
auth/logout은 잘못된 URI이므로 /auth/logout 으로 수정 바랍니다
There was a problem hiding this comment.
Authorization 헤더가 누락된 요청을 받는 경우 컨트롤러까지 요청이 전달되지 않고 Jwt 인증 필터 체인에서 예외를 발생하기 때문에 컨트롤러에서 발생시키는 헤더를 핸들링하는 GlobalExceptionHandler에는 메서드가 필요하지 않을 것으로 보입니다
| return generateToken(email, refreshTokenExpireTime, hmacRefreshKey); | ||
| } | ||
|
|
||
| public String generateTestToken(String email, Long expireTime) { |
There was a problem hiding this comment.
generateTestToken 제거해 주세요!
테스트 때문에 실제 서비스 로직에 필요 없는 코드가 추가되면 안됩니다
There was a problem hiding this comment.
헤더로 액세스 토큰을 받는 것과 더불어 @IdentifiedUser 어노테이션을 사용해 Member 객체 또한 받는 것이 좋을 것 같습니다.
현재 로직으로는 인증 필터 체인을 거치지 않고 컨트롤러 단으로 요청이 바로 전달되기 때문에, 서비스 로직에서 토큰 추출, 인증, 관리까지 모두 담당하게 되어 책임이 지나치게 무거워집니다.
구현되어 있는 인증 필터 체인을 사용해서 토큰 추출 및 인증에 대한 책임을 필터 체인에 인계하고, 서비스 로직에서는 토큰 블랙리스트 관리만 담당하게 하는 것이 좋은 코드가 될 것 같습니다~!
There was a problem hiding this comment.
JwtUtil의 책임이 당초 설계할 때 예상했던 것 보다 점점 무거워지는 것 같아요
추후에 JwtUtil 클래스의 책임을 분산하는 방안에 대해서 고민해보면 좋을 것 같습니다!
2 participants
#️⃣ 연관된 이슈
close #21
사용자 로그아웃 기능을 처리하는 API를 구현합니다.
클라이언트는 Authorization 헤더에 Bearer 토큰 형식의 Access Token을 담아 요청을 보냅니다. 서버는 해당 토큰을 검증하고, 유효한 경우 토큰을 무효화 처리하여 사용자의 세션을 안전하게 종료시킵니다.
🔎 작업 내용
✅ Checklist
💬 리뷰 요구사항(선택)