[REFACTOR] JWT 처리 방식 변경 및 리팩토링 #223

SSung023 · 2024-07-15T07:43:02Z

PR 타입(하나 이상의 PR 타입을 선택해주세요)

☑ 기능 추가
□ 기능 삭제
☑ 리팩토링
□ 버그 수정
□ 의존성, 환경 변수, 빌드 관련 코드 업데이트

반영 브랜치

refactor/217-refactor-jwt → main

변경 사항

JWT 관련 클래스에 Facade 패턴 적용
코드의 불필요한 부분, 중복 부분 제거
Access token의 처리 방식 변경
- Access token의 저장 위치를 Authorization header로 변경
- Access token의 생성/resolve 방식 변경
- Access token의 정상 작동 확인
JWT 관련 상수 이름 변경
JWT 테스트 코드에 DCI 패턴 적용

테스트 결과

연관된 이슈

#217

리뷰 요구사항(선택)

JwtFacadeTest, TokenServiceTest에 DCI 패턴을 적용해보았는데 제대로 적용한건지 궁금하네요! 도형님의 리뷰를 기다리겠습니다!

- JwtService에 interface 적용 및 구현 클래스 적용 - Cookie의 sameSite 옵션을 strict로 설정

- /api/auth JWT 요청 API의 Request DTO를 의도에 맞도록 수정(TokenRequest)

- JwtService의 의존관계를 TokenRepository에서 TokenService로 변경 - TokenService에 필요한 메서드 추가

- tokenRepository를 활용하는 코드에서 tokenService를 활용하는 코드로 변경

- Access token의 저장 위치를 Authorization header로 변경 - Access token의 생성/resolve 방식 변경 - Access token의 정상 작동 확인 - 불필요한 로직 제거 필요 - JWT 테스트 코드 변경 필요

- JWT 관련 enum 상수 이름 변경

- JWT 중 Access token을 Header에 저장하는 것으로 바꾸면서 Controller 및 JWT 관련 테스트 코드 정상 작동하도록 수정

- JWT 관련 에러 메세지를 세부적으로 나눔 - Refresh token의 탈취 여부를 확인하는 로직 버그 픽스 - Access token 추출 시, 빈 문자열일 때에도 예외를 발생하도록 변경

- TokenService 테스트 코드에 DCI 패턴 적용 - JWT 관련 예외 메세지 추가

- JWT에서 예외 발생 시 처리하는 ExceptionHandlerFilter에서 JWT 관련 Exception 발생 시, Logout 로직이 실행되도록 처리

github-actions · 2024-07-15T07:45:07Z

Test Results

81 files 81 suites 30s ⏱️
273 tests 273 ✅ 0 💤 0 ❌
285 runs 285 ✅ 0 💤 0 ❌

Results for commit 36feda0.

♻️ This comment has been updated with latest results.

- CorsConfigurationSource에 .setExposedHeaders 옵션 설정

- Access token 재발급 여부에 따라 token-reissued에 값 설정

kimdozzi · 2024-07-23T09:30:45Z

src/main/java/com/genius/gitget/global/security/filter/ExceptionHandlerFilter.java

+            //NOTE: 인가 정보가 있는 경우 Logout 처리를 진행...? JWT 관련 예외에서만 진행되는게 맞나? 확인해봐야지
+            if (authentication != null) {
+                UserPrincipal principal = (UserPrincipal) authentication.getPrincipal();
+                String identifier = principal.getUser().getIdentifier();
+                jwtFacade.logout(response, identifier);
+            }
+
+            log.error("ExceptionHandlerFilter에서 작동: " + e.getMessage(), e);
            setErrorResponse(response, e);
        }


이상한 부분이 있는 것 같습니다.
ExceptionHandlerFilter try 내부에서 doFilter를 하기 전에 authentication 의 참/거짓 여부를 파악해야 하고 null이 아닐 떄 doFilter를 수행하는 거 아닌가요? 사실 if (authentication != null) 이라면 정상적인 로그인 흐름인 것 같은데 말이죠...?

authentication == null인 경우에는 사용자가 인증이 되지 않았거나 과거에 인증이 되었지만 세션이 만료되는 경우니까 아마 Spring Security 내부에서 알아서 처리해 줄 것으로 보이고요.. 뭔가 좀 흐름이 이상한 것 같습니다. 정상적으로 로그인/로그아웃 이 진행되던가요?

일단 제 의도는

filterChain을 거치다가 BusinessException(JWT 예외)가 발생했을 때 강제 로그아웃 필요하다고 판단

filterChain에서 발생하는 예외는 @RestControllerAdvice로 처리되지 않아, 해당 예외를 처리할 ExceptionHandlerFilter 사용

catch문에서 BusinessException을 감지해 처리하므로 filterChain에서 발생하는 JWT 예외가 발생했을 때 처리가 가능하다고 판단

Authentication != null의 경우, /api/auth와 같이 JWT를 필요로 하지 않는 API 요청의 경우 필터에서 Authentication을 null로 설정하기 때문에
해당 API들에 대해 예외 처리

였습니다..!!

그런데 다시 곱씹다 보니 의심되는 점은, filterChain이 아닌 프로덕션 코드에서 BusinessException이 발생할 때마다 해당 로직이 작동되어 로그아웃 처리가 될 여지가 있을 것 같습니다.
(4번의 경우 컨트롤러 테스트를 돌리다가 발견 했는데, filterChain 예외에만 반응 할 거라는 예상에 맞지 않음)

다시 한 번 검토해보겠습니다!
하마터면 버그를 만들뻔했네요..! 의견 감사합니다!

이상한 부분이 있는 것 같습니다. ExceptionHandlerFilter try 내부에서 doFilter를 하기 전에 authentication 의 참/거짓 여부를 파악해야 하고 null이 아닐 떄 doFilter를 수행하는 거 아닌가요? 사실 if (authentication != null) 이라면 정상적인 로그인 흐름인 것 같은데 말이죠...?

authentication == null인 경우에는 사용자가 인증이 되지 않았거나 과거에 인증이 되었지만 세션이 만료되는 경우니까 아마 Spring Security 내부에서 알아서 처리해 줄 것으로 보이고요.. 뭔가 좀 흐름이 이상한 것 같습니다. 정상적으로 로그인/로그아웃 이 진행되던가요?

오늘 한 번 검토를 해본 결과, if(authentication != null) 이하 로직은 JWT exception 발생 시에도 동작하지 않는 필요 없는 로직이었습니다 ㅎㅎ;;

위에 제가 단 댓글에서 3번까지는 맞는 말(JWT 관련 Exception 발생 시에만 catch문의 로직 실행)이었지만, 애초에 JwtAuthenticationFilter에서 Exception이 발생하여 ExceptionHandlerFilter의 catch()문까지 온 경우 사용자가 인증이 되지 않아 authentication이 null로 설정된 상태입니다.
때문에 if(authentication != null) 조건문을 통과할 일이 없었네요😅

수정해서 push 했습니다! 좋은 의견 감사합니다!

kimdozzi · 2024-07-23T09:54:57Z

src/main/java/com/genius/gitget/global/security/service/JwtFacadeImpl.java

@@ -87,46 +85,48 @@ private ResponseCookie setTokenToCookie(String tokenPrefix, String token, long m
                .path("/")
                .maxAge(maxAgeSeconds)
                .httpOnly(true)
-                .sameSite("None")
+                .sameSite("Strict")


이건 어떤 부분인가요 ?

CSRF 공격을 막기 위한 방법입니다!
sameSite 옵션을 strict로 설정하면 같은 도메인으로 요청했을 때에만 쿠키를 전송하게 됩니다.
Refresh token의 경우 쿠키로 자동전송해 CSRF 공격에 취약해 해당 옵션을 strict로 설정했습니다!

kimdozzi · 2024-07-23T09:55:54Z

src/main/java/com/genius/gitget/global/security/service/JwtFacadeImpl.java

-        UserDetails principal = customUserDetailsService.loadUserByUsername(getUserPk(token, ACCESS_SECRET_KEY));
-        return new UsernamePasswordAuthenticationToken(principal, "", principal.getAuthorities());
+    @Override
+    public String resolveAccessToken(HttpServletRequest request) {


resolveAccessToken의 역할이 무엇인지 궁금합니다!

Access token을 헤더에 담는 방식으로 변경하면서 헤더에서 access token을 추출하는 기능을 담당하는 메서드입니다!
기존에는 Access, Refresh 모두 쿠키를 이용했기 때문에 하나의 메서드를 같이 사용할 수 있었지만,
방식 변경으로 인해 Access token 추출용 메서드를 하나 새로 만들었습니다 :)

- ExceptionHandlerFilter에서 불필요한 로직 제거

- Facade 구현체 이름을 FacadeImpl에서 FacadeService로 통일

kimdozzi

고생하셨습니다 ~ ^^

SSung023 added 14 commits July 10, 2024 17:34

feat: JwtService에 인터페이스 적용

1d20d97

- JwtService에 interface 적용 및 구현 클래스 적용 - Cookie의 sameSite 옵션을 strict로 설정

refactor: JWT 요청 API의 Request DTO 변경

dccd526

- /api/auth JWT 요청 API의 Request DTO를 의도에 맞도록 수정(TokenRequest)

refactor: 필요없는 구문 삭제

48bc68d

refactor: JwtService의 의존 관계 변경

b9997a7

- JwtService의 의존관계를 TokenRepository에서 TokenService로 변경 - TokenService에 필요한 메서드 추가

refactor: tokenService를 활용하는 코드로 리팩토링

64a4be1

- tokenRepository를 활용하는 코드에서 tokenService를 활용하는 코드로 변경

refactor: Facade 패턴 적용

31bed44

feat: Access token 처리 방식 변경

e008d4d

- Access token의 저장 위치를 Authorization header로 변경 - Access token의 생성/resolve 방식 변경 - Access token의 정상 작동 확인 - 불필요한 로직 제거 필요 - JWT 테스트 코드 변경 필요

refactor: enum 이름 변경

9e43778

- JWT 관련 enum 상수 이름 변경

test: JWT 처리 방식 변경에 의한 테스트 코드 변경

f9bd69f

- JWT 중 Access token을 Header에 저장하는 것으로 바꾸면서 Controller 및 JWT 관련 테스트 코드 정상 작동하도록 수정

fix: JWT 에러 메세지 변경 및 버그 픽스

df83264

- JWT 관련 에러 메세지를 세부적으로 나눔 - Refresh token의 탈취 여부를 확인하는 로직 버그 픽스 - Access token 추출 시, 빈 문자열일 때에도 예외를 발생하도록 변경

test: JwtFacade 테스트 코드에 DCI 패턴 적용

4b0ffae

test: TokenService 테스트 코드에 DCI 패턴 적용

b057291

- TokenService 테스트 코드에 DCI 패턴 적용 - JWT 관련 예외 메세지 추가

feat: JWT 처리 도중 Exception 발생 시, logout하도록 처리

a63536b

- JWT에서 예외 발생 시 처리하는 ExceptionHandlerFilter에서 JWT 관련 Exception 발생 시, Logout 로직이 실행되도록 처리

Merge branch 'main' into refactor/217-refactor-jwt

630ba8c

SSung023 added ⚙️ refactor 리팩토링 🧑🏻‍💻 BE 백엔드 관련 코드 labels Jul 15, 2024

SSung023 self-assigned this Jul 15, 2024

SSung023 linked an issue Jul 15, 2024 that may be closed by this pull request

[REFACTOR] JWT 코드 리팩토링 #217

Closed

8 tasks

SSung023 changed the title ~~[REFACTOR] JWT 처리 방식 변경~~ [REFACTOR] JWT 처리 방식 변경 및 리팩토링 Jul 15, 2024

SSung023 added 5 commits July 15, 2024 16:48

fix: logout 처리 위치 변경

1625373

fix: custom header가 전달되지 않는 버그 픽스

d3c115b

fix: 브라우저에서 Authorization 헤더 접근이 안되는 버그 픽스

3cc8913

- CorsConfigurationSource에 .setExposedHeaders 옵션 설정

feat: Access token 재발급 여부 관련 헤더 추가

e8e2879

- Access token 재발급 여부에 따라 token-reissued에 값 설정

fix: 브라우저에서 token-reissued 헤더 접근이 안되는 버그 픽스

8a987b5

kimdozzi reviewed Jul 23, 2024

View reviewed changes

SSung023 added 2 commits July 24, 2024 21:57

refactor: 불필요한 로직 제거

c079067

- ExceptionHandlerFilter에서 불필요한 로직 제거

refactor: Facade 구현체 이름 통일

36feda0

- Facade 구현체 이름을 FacadeImpl에서 FacadeService로 통일

kimdozzi approved these changes Jul 25, 2024

View reviewed changes

SSung023 merged commit c36bd15 into main Jul 25, 2024
2 checks passed

SSung023 deleted the refactor/217-refactor-jwt branch July 25, 2024 06:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[REFACTOR] JWT 처리 방식 변경 및 리팩토링 #223

[REFACTOR] JWT 처리 방식 변경 및 리팩토링 #223

SSung023 commented Jul 15, 2024 •

edited

Loading

github-actions bot commented Jul 15, 2024 •

edited

Loading

kimdozzi Jul 23, 2024

SSung023 Jul 23, 2024

SSung023 Jul 24, 2024

kimdozzi Jul 23, 2024

SSung023 Jul 23, 2024

kimdozzi Jul 23, 2024

SSung023 Jul 23, 2024

kimdozzi left a comment

[REFACTOR] JWT 처리 방식 변경 및 리팩토링 #223

[REFACTOR] JWT 처리 방식 변경 및 리팩토링 #223

Conversation

SSung023 commented Jul 15, 2024 • edited Loading

PR 타입(하나 이상의 PR 타입을 선택해주세요)

반영 브랜치

변경 사항

테스트 결과

연관된 이슈

리뷰 요구사항(선택)

github-actions bot commented Jul 15, 2024 • edited Loading

Test Results

kimdozzi Jul 23, 2024

Choose a reason for hiding this comment

SSung023 Jul 23, 2024

Choose a reason for hiding this comment

SSung023 Jul 24, 2024

Choose a reason for hiding this comment

kimdozzi Jul 23, 2024

Choose a reason for hiding this comment

SSung023 Jul 23, 2024

Choose a reason for hiding this comment

kimdozzi Jul 23, 2024

Choose a reason for hiding this comment

SSung023 Jul 23, 2024

Choose a reason for hiding this comment

kimdozzi left a comment

Choose a reason for hiding this comment

SSung023 commented Jul 15, 2024 •

edited

Loading

github-actions bot commented Jul 15, 2024 •

edited

Loading