[Mod] 토큰 비유효시 반환값 변경 및 토큰 검증 로직 최적화 #167

JunbeomKoreaUniv · 2025-03-05T09:17:14Z

수정한 코드

토큰 비유효시 401 상태코드를 반환하고 status로 구분하도록 코드 수정
(accessTokenEmpty, acceessTokenInvalid, refreshTokenInvalid)
토큰 검증 로직 최적화

변경 사항

자세한 로직은 주석 최대한 상세하게 작성하였으니 확인하시면 코드 파악하기 편하실 것 같습니다

토큰 비유효(만료, 정보 불일치, 리프레시 토큰이 DB에 존재하지 않음 등)시의 반환값 변경.
- 기존에는 엑세스 토큰 비유효와 리프레시 토큰 비유효를 상태코드(401, 403)으로 구분하였는데 403은 토큰 비유효에 적합하지 않은 상태코드임.
- 따라서 상태코드는 403으로 고정하고, 기존 ApiResponseForm의 status로 구분하는 것으로 변경하였음.
  - 엑세스 토큰 비유효시 "accessTokenInvalid", 리프레시 토큰 비유효시 "refreshTokenInvalid", 토큰이 없을 시 "accessTokenEmpty"를 status로 반환함.
토큰 검증 로직 최적화
- 불필요한 검증을 줄여 토큰 검증 시간을 최적화하였음
- 가장 먼저 refreshToken != null인지 확인 -> 토큰 자체가 유효한지 확인 -> DB에서 확인해서 리프레시 토큰 검증을 처리함.
  - refreshToken == null 일 경우 조건절의 refreshToken != null이 false이므로 if문 내부로 진입하지 않고 바로 엑세스토큰 검증 로직으로 넘어감
  - 리프레시토큰의 검증 순서도 가장 시간이 덜 드는 작업부터 검증함.
- 이후 엑세스 토큰 검증 및 토큰이 없을 경우 예외처리
객체지향적 설계
- 기존에 엑세스토큰이든, 리프레시토큰이든 모두 토큰이 유효하지 않으면 InvalidTokenException이라는 예외가 발생해 두 토큰을 구분할 수 없었음.
- 이를 구분하기 위해 InvalidTokenException을 상속한 InvalidAccessTokenException, InvalidRefreshTokenException을 정의하고 비유효시 두 에러를 발생시키게 하였고, catch문에서는 InvalidTokenException을 catch하고 캐치문 안에서 isinstance로 구분하였음.

테스트 결과

[토큰이 없을 경우]

[엑세스 토큰이 있는데 만료됐을 경우]

[리프레시 토큰이 있는데 만료됐을 경우 - 엑세스토큰 없는 버전]

[리프레시 토큰이 있는데 만료됐을 경우 - 엑세스토큰 있는 버전]

- 토큰 비유효시 401 상태코드를 반환하고 status로 구분하도록 코드 수정 (accessTokenEmpty, acceessTokenInvalid, refreshTokenInvalid) - 토큰 검증 로직 최적화

JunbeomKoreaUniv · 2025-03-05T09:18:53Z

아 그리고, NO_CHECK_URLS에 "/**/additional-info"도 추가해두었습니다

생각해보니 위 엔드포인트 온보딩에 통합되면서 삭제했었어서 다시 NO_CHECK_URLS에 제외시켰습니다. 혼란드려서 죄송합니다. 이 코멘트 전체는 무시하셔도 될 것 같아요

github-actions · 2025-03-05T09:20:00Z

실패하는 테스트코드가 있어 PR이 자동으로 닫혔습니다.
Github Action에서 자세한 실패 로그를 확인하고 코드를 수정하세요.

jjjh02 · 2025-03-05T09:40:59Z

ontime-back/src/main/java/devkor/ontime_back/global/jwt/JwtAuthenticationFilter.java

+            // 리프레시 토큰이 있고, 유효할 경우 엑세스토큰 재발급
+            // 이때 조건절의 isRefreshTokenValid에서 토큰이 유효하지 않으면 InvalidRefreshTokenException 발생
+            if (refreshToken != null && jwtTokenProvider.isRefreshTokenValid(refreshToken)) {
+                // 리프레시토큰의 경우 토큰의 유효성 뿐만 아니라 DB에 등록되어 있는지도 확인해야 함


전에 디스코드에서 토큰 유효성을 checkRefreshTokenAndReIssueAccessToken 메서드 내에서 처리하는 쪽으로 말씀하셨던 것 같은데 이 코드에서는 if문에서 처리하게 된 이유가 있을까요?

// refreshToken로 검색 후 accessToken 재발급 후 전송 public void checkRefreshTokenAndReIssueAccessToken(HttpServletResponse response, String refreshToken) throws IOException { if (!jwtTokenProvider.isTokenValid(refreshToken)) { response.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Invalid Refresh token."); } userRepository.findByRefreshToken(refreshToken) // refreshToken으로 유저 찾기 .ifPresent(user -> { String newAccessToken = jwtTokenProvider.createAccessToken(user.getEmail(), user.getId()); // accessToken 생성 log.info("New accessToken issued: " + newAccessToken); // 재발급된 accessToken 출력 jwtTokenProvider.sendAccessToken(response, newAccessToken); // accessToken 전송 // jwtTokenProvider.sendAccessToken(response, jwtTokenProvider.createAccessToken(user.getEmail(), user.getId())); // accessToken 생성 후 전송 }); }

위 기존코드에서 토큰 자체가 유효한지 검증(isTokenValid)를 했는데 비유효할 경우 response.sendError를 날렸음에도 아래 DB를 확인하는 코드가 실행될 여지가 있어 뺐습니다. 그런데 지금 보니까 response.sendError안에 return만 넣어주면 될 것 같긴한데 이를 차치하더라도,
아래 엑세스토큰 검증하는 코드랑 조건절이 유사해서 일관성, 가독성 측면에서도 더 낫다고 생각해 토큰 자체가 유효한지 판단하는 로직을 밖으로 뺐습니다. 리프레시토큰 검증할 때는 if문 내부의 메서드에서 토큰 자체가 유효한지 판단하고, 엑세스토큰 검증할 때는 if문 밖에서 토큰 자체가 유효한지 판단하는게 어색하다고 느꼈습니다.

네~ 코드 가독성 측면에서 이 방법이 더 좋을 것 같네요!

jjjh02 · 2025-03-05T10:14:08Z

ontime-back/src/main/java/devkor/ontime_back/global/jwt/JwtAuthenticationFilter.java

                checkAccessTokenAndAuthentication(request, response, filterChain);
-                return;
+            } else if (accessToken != null){ // 엑세스 토큰이 있는데 유효하지 않은 경우 InvalidAccessTokenException 발생


여기서 accesstoken이 유효하지 않으면 if (accessToken != null && jwtTokenProvider.isAccessTokenValid(accessToken))에서 isAccessTokenValid(accessToken)로 예외 발생하면서 if 조건의 평가가 중단됩니다.
그래서 catch (Exception e)에서 InvalidAccessTokenException을 처리하고 있으므로, else if까지 코드가 도달할 일이 없는걸로 알고 있는데 혹시 어떻게 생각하시나요?

위에서 유효하지않은 refreshtoken이 처리되는 경우도
if (refreshToken != null && jwtTokenProvider.isRefreshTokenValid(refreshToken))에서 예외(InvalidRefreshTokenException)가 발생해서 catch (Exception e)에서 예외가 처리하는 방식으로 코드를 구현하신 것 같아서요.

같은 방식으로 처리하면 어떨까 싶어서 제안드립니다~!

아 말씀하신 부분은 의도했습니다. 메서드명때문에 혼란을 드린 것 같네요
if (accessToken != null && jwtTokenProvider.isAccessTokenValid(accessToken))
로 엑세스토큰 자체가 유효하지 않으면 else if로 도달하지 않게 의도하여 코드 작성하였고
유효하면 첫 번째 else if문에 도달해 checkAccessTokenAndAuthentication메서드가 실행되고 해당 메서드에서는 검증이 일어나지 않고 유저의 권한 정보만 저장하고 스프링 시큐리티 필터체인에서 다음 필터로 넘어가게 됩니다.

checkAccessTokenAndAuthentication의 메서드명을 saveAuthenticationandDoFilter 정도로 수정하겠습니다.

그래서 refreshToken 검증하는 코드랑 흐름이 유사합니다.
if 조건절에서 null인지 먼저 판단하고 토큰 자체의 유효성을 판단한 뒤, 유효하다면 각자 필요한 메서드가 실행됩니다.

유효하면 첫 번째 else if문에 도달해 checkAccessTokenAndAuthentication메서드가 실행되고 해당 메서드에서는 검증이 일어나지 않고 유저의 권한 정보만 저장하고 스프링 시큐리티 필터체인에서 다음 필터로 넘어가게 됩니다.
이 부분이 무슨 의미일까요?
유효하면 else if (accessToken != null) 문에 도달한다는 건가요? 유효한 경우에는 if (accessToken != null && jwtTokenProvider.isAccessTokenValid(accessToken))로 들어가는 것 아닌가요? else if 문으로 넘어오는 경우가 어떤 경우를 의도하고 만드신 것인지 궁금합니다.

아 답변드리면서 깨달았네요. jwtTokenProvider.isAccessTokenValid(accessToken)에서 토큰 자체가 유효하지않으면 Exception을 날리기 때문에 else if랑 역할이 완전히 겹치네요.
코드 검토가 부족했던 것 같습니다.
else if문을 빼면 코드 가독성도 훨씬 좋아지는 것 같네요! else if문은 빼고 푸시하겠습니다~

넵 알겠습니다~!

jjjh02 · 2025-03-05T10:18:53Z

그리고 accessToken == null인 경우를 if문으로 먼저 확인하고, 아닌 경우 그 후에 accessToken 유효성을 검증하는 방법은 어떨까요?

JunbeomKoreaUniv · 2025-03-05T13:22:37Z

그리고 accessToken == null인 경우를 if문으로 먼저 확인하고, 아닌 경우 그 후에 accessToken 유효성을 검증하는 방법은 어떨까요?

저희가 엑세스토큰 + 리프레시토큰이 같이 서버로 들어올 때 리프레시 토큰을 먼저 확인해서 유효하면 재발급하는 것으로 합의했던 것으로 기억해서 리프레시 토큰 먼저 검증하도록 코드 작성하였습니다

이에 더해 엑세스 토큰을 먼저 검증하게 되면 리프레시 토큰을 검증하는 로직이 포함될 수 있는 여지가 있어보입니다.
어떻게 어떻게 잘 짜면 될 것 같기도한데 확실한건 코드가 더 복잡해져 가독성이 저해될 것이고, 지금 짜놓은 코드가 제가 판단했을 때에는 검증 로직도 최적화인 것으로 보이는데 굳이 바꿀 필요가 있을까라는 입장입니다

jjjh02 · 2025-03-05T15:01:43Z

아 앞의 코멘트에 설명이 부족했네요. 리프레시토큰 확인하고 if (accessToken == null)와 if (accessToken != null && jwtTokenProvider.isAccessTokenValid(accessToken))의 순서 변경을 말씀 드렸던 것입니다. null을 먼저 확인하면 뒤에서 null인지는 확인 안하고 valid 여부만 확인해도 되지않을까해서요.

JunbeomKoreaUniv · 2025-03-05T15:15:40Z

근데 그거는 순서 변경 하든 안하든 소요시간이 똑같아보입니다.
원래 코드: accessToken이 null이면 if (accessToken != null에서 if문 탈출하고 if (accessToken == null)문 실행 / accessToken이 null이 아니면 엑세스토큰 검증 진행
순서 변경 한 후 코드: accessToken이 null이면 if (accessToken == null)문 실행 / accessToken이 null이 아니면 if (accessToken == null)에서 if문 탈출하고 if (accessToken != null && jwtTokenProvider.isAccessTokenValid(accessToken))문 실행.

차이가 없어보입니다.
이 의도로 질문하신거 맞을까요?

jjjh02 · 2025-03-05T15:18:46Z

네 저 의도로 질문한 것 맞습니다. if (accessToken != null)로 진행하고 if (accessToken != null && jwtTokenProvider.isAccessTokenValid(accessToken))를 if (jwtTokenProvider.isAccessTokenValid(accessToken))로 진행하는 방법은 어떨까 싶어서 여쭤보았습니다.

JunbeomKoreaUniv · 2025-03-05T15:21:02Z

아 이거는 근데 저는 지금 코드가 더 낫다고 생각하는게 취향의 영역이긴한데 리프레시토큰 검증하는 if문이랑 틀이 똑같아서 처음 보는 사람이 코드 봤을 때 더 이해하기 쉽다고 생각하긴 합니다. 물론 협업하는 사람이 진서님밖에 없긴 하지만요

jjjh02 · 2025-03-05T15:22:50Z

네 알겠습니다. 코드의 가독성도 중요해서 성능적으로 크게 차이는 없을 것 같아서 그대로 진행해도 괜찮을 것 같습니다

JunbeomKoreaUniv · 2025-03-05T15:26:19Z

넵 성능적으로는 전혀 차이 없습니다. 성능차이가 있다고 하더라고 O(1)수준이여서 고려하지 않으셔도 될 것 같습니다.

그러면

(코드 리뷰하면서 요청주신)엑세스 토큰 유효성 검증하는 else if문 삭제와
(따로 요청주신) NO_CHECK_URL에 소셜로그인 URL 추가

해서 푸시 넣겠습니다~

JunbeomKoreaUniv · 2025-03-05T15:29:21Z

2가지 사항 변경해서 반영하였고,
변경 사항 반영 후 엑세스 토큰 만료시 코드 잘 작동하는지 테스트 진행한 내역 첨부합니다.

jjjh02 · 2025-03-05T15:31:17Z

수고 많으셨습니다~!!

JunbeomKoreaUniv and others added 6 commits March 3, 2025 12:31

test:: PR 테스트코드 검증 파이프라인을 테스트하기 위해 테스트코드가 실패하도록 수정

f2c16ac

Merge branch 'main' of https://github.com/DevKor-github/OnTime-back

be621d3

Merge branch 'main' of https://github.com/DevKor-github/OnTime-back

9281c90

Merge branch 'main' of https://github.com/DevKor-github/OnTime-back

2447a83

mod:: 토큰 비유효시 반환값 변경 및 토큰 검증 로직 최적화

151afb3

- 토큰 비유효시 401 상태코드를 반환하고 status로 구분하도록 코드 수정 (accessTokenEmpty, acceessTokenInvalid, refreshTokenInvalid) - 토큰 검증 로직 최적화

Merge branch 'main' into mod/jwt2

2af1c30

github-actions bot closed this Mar 5, 2025

JunbeomKoreaUniv added 2 commits March 5, 2025 18:21

mod:: pr close 파이프라인 작동테스트를 위해 테스트코드 틀리게 조작했던 코드 복구

2e62906

Merge branch 'mod/jwt2' of https://github.com/DevKor-github/OnTime-back

ca7fdb5

JunbeomKoreaUniv reopened this Mar 5, 2025

mod:: 토큰 필요없는 url에 있던 /**/additional-info 제외

a53fe6e

jjjh02 reviewed Mar 5, 2025

View reviewed changes

mod:: 엑세스토큰 검증 코드의 불필요한 분기문 제거 및 소셜로그인 url NO_CHECK_URL에 추가

aefe12a

jjjh02 merged commit 146f16a into main Mar 5, 2025
2 checks passed

jjjh02 deleted the mod/jwt2 branch March 5, 2025 15:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Mod] 토큰 비유효시 반환값 변경 및 토큰 검증 로직 최적화 #167

[Mod] 토큰 비유효시 반환값 변경 및 토큰 검증 로직 최적화 #167

JunbeomKoreaUniv commented Mar 5, 2025 •

edited

Loading

JunbeomKoreaUniv commented Mar 5, 2025 •

edited

Loading

github-actions bot commented Mar 5, 2025

jjjh02 Mar 5, 2025

JunbeomKoreaUniv Mar 5, 2025 •

edited

Loading

jjjh02 Mar 5, 2025

jjjh02 Mar 5, 2025

JunbeomKoreaUniv Mar 5, 2025

jjjh02 Mar 5, 2025

JunbeomKoreaUniv Mar 5, 2025

jjjh02 Mar 5, 2025

jjjh02 commented Mar 5, 2025

JunbeomKoreaUniv commented Mar 5, 2025 •

edited

Loading

jjjh02 commented Mar 5, 2025

JunbeomKoreaUniv commented Mar 5, 2025 •

edited

Loading

jjjh02 commented Mar 5, 2025

JunbeomKoreaUniv commented Mar 5, 2025

jjjh02 commented Mar 5, 2025

JunbeomKoreaUniv commented Mar 5, 2025

JunbeomKoreaUniv commented Mar 5, 2025

jjjh02 commented Mar 5, 2025

[Mod] 토큰 비유효시 반환값 변경 및 토큰 검증 로직 최적화 #167

[Mod] 토큰 비유효시 반환값 변경 및 토큰 검증 로직 최적화 #167

Conversation

JunbeomKoreaUniv commented Mar 5, 2025 • edited Loading

수정한 코드

변경 사항

자세한 로직은 주석 최대한 상세하게 작성하였으니 확인하시면 코드 파악하기 편하실 것 같습니다

테스트 결과

JunbeomKoreaUniv commented Mar 5, 2025 • edited Loading

github-actions bot commented Mar 5, 2025

jjjh02 Mar 5, 2025

Choose a reason for hiding this comment

JunbeomKoreaUniv Mar 5, 2025 • edited Loading

Choose a reason for hiding this comment

jjjh02 Mar 5, 2025

Choose a reason for hiding this comment

jjjh02 Mar 5, 2025

Choose a reason for hiding this comment

JunbeomKoreaUniv Mar 5, 2025

Choose a reason for hiding this comment

jjjh02 Mar 5, 2025

Choose a reason for hiding this comment

JunbeomKoreaUniv Mar 5, 2025

Choose a reason for hiding this comment

jjjh02 Mar 5, 2025

Choose a reason for hiding this comment

jjjh02 commented Mar 5, 2025

JunbeomKoreaUniv commented Mar 5, 2025 • edited Loading

jjjh02 commented Mar 5, 2025

JunbeomKoreaUniv commented Mar 5, 2025 • edited Loading

jjjh02 commented Mar 5, 2025

JunbeomKoreaUniv commented Mar 5, 2025

jjjh02 commented Mar 5, 2025

JunbeomKoreaUniv commented Mar 5, 2025

JunbeomKoreaUniv commented Mar 5, 2025

jjjh02 commented Mar 5, 2025

JunbeomKoreaUniv commented Mar 5, 2025 •

edited

Loading

JunbeomKoreaUniv commented Mar 5, 2025 •

edited

Loading

JunbeomKoreaUniv Mar 5, 2025 •

edited

Loading

JunbeomKoreaUniv commented Mar 5, 2025 •

edited

Loading

JunbeomKoreaUniv commented Mar 5, 2025 •

edited

Loading