Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,15 @@ protected Claims validateAndParseToken(final String token) {
}

if (claims.getExpiration().toInstant().isBefore(new Date().toInstant())) {
throw new CustomRuntimeException(ErrorCode.EXPIRED_TOKEN);
throw new ExpiredJwtException(
null,
claims,
"JWT 토큰이 만료되었습니다."
);
}
return claims;
} catch (ExpiredJwtException exception) {
throw exception;
} catch (JwtException | IllegalArgumentException exception) {
Comment on lines +116 to 118

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

catch (ExpiredJwtException exception) 블록은 예외를 잡아서 아무런 추가 작업 없이 다시 던지고 있습니다. 이는 불필요하며 코드를 복잡하게 만듭니다. 이 catch 블록을 제거해도 동작은 동일하게 유지되면서 코드가 더 간결해집니다.

throw new CustomRuntimeException(ErrorCode.INVALID_TOKEN_TYPE);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,26 @@
package life.mosu.mosuserver.global.filter;

import com.fasterxml.jackson.databind.ObjectMapper;
import io.jsonwebtoken.ExpiredJwtException;
import jakarta.servlet.FilterChain;
import jakarta.servlet.ServletException;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import java.io.IOException;
import life.mosu.mosuserver.global.exception.CustomRuntimeException;
import life.mosu.mosuserver.global.exception.ErrorResponse;
import lombok.RequiredArgsConstructor;
import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.stereotype.Component;
import org.springframework.web.filter.OncePerRequestFilter;

@Component
@RequiredArgsConstructor
public class TokenExceptionFilter extends OncePerRequestFilter {

private final ObjectMapper objectMapper;

@Override
protected void doFilterInternal(
final HttpServletRequest request,
Expand All @@ -22,11 +29,43 @@ protected void doFilterInternal(
) throws ServletException, IOException {
try {
filterChain.doFilter(request, response);
} catch (CustomRuntimeException exception) {
response.setStatus(HttpServletResponse.SC_UNAUTHORIZED);
response.setContentType("application/json");
} catch (CustomRuntimeException ex) {

ErrorResponse errorResponse = ErrorResponse.builder()
.status(ex.getStatus().value())
.message(ex.getMessage())
.code(ex.getCode())
.build();

response.setStatus(HttpStatus.UNAUTHORIZED.value());

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

CustomRuntimeException은 자체적으로 HttpStatus를 가지고 있습니다. 하지만 여기서는 HttpStatus.UNAUTHORIZED로 상태 코드를 고정하고 있어, 예외에 담긴 정확한 상태 코드가 무시됩니다. ex.getStatus().value()를 사용하여 동적으로 상태 코드를 설정해야 올바른 오류 응답을 클라이언트에 전달할 수 있습니다.

Suggested change
response.setStatus(HttpStatus.UNAUTHORIZED.value());
response.setStatus(ex.getStatus().value());

response.setContentType(MediaType.APPLICATION_JSON_VALUE);
response.setCharacterEncoding("UTF-8");

objectMapper.writeValue(response.getWriter(), errorResponse);
} catch (ExpiredJwtException ex) {
ErrorResponse errorResponse = ErrorResponse.builder()
.status(HttpStatus.NOT_ACCEPTABLE.value())
.message("토큰이 만료되었습니다.")
.code("TOKEN_EXPIRED")
.build();

response.setStatus(HttpStatus.NOT_ACCEPTABLE.value());
response.setContentType(MediaType.APPLICATION_JSON_VALUE);
response.setCharacterEncoding("UTF-8");
response.sendError(exception.getStatus().value(), exception.getMessage());

objectMapper.writeValue(response.getWriter(), errorResponse);
Comment on lines +46 to +56

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

만료된 토큰에 대해 HttpStatus.NOT_ACCEPTABLE (406)을 반환하고 있습니다. 이는 일반적으로 클라이언트의 Accept 헤더와 서버가 생성할 수 있는 컨텐츠 타입이 맞지 않을 때 사용됩니다. 토큰이 만료된 경우에는 인증 실패를 의미하는 HttpStatus.UNAUTHORIZED (401)를 사용하는 것이 더 일반적이고 표준에 가깝습니다. 이전 구현에서도 UNAUTHORIZED를 사용했던 만큼, 일관성을 위해 변경을 고려해보시는 것이 좋겠습니다.

Suggested change
ErrorResponse errorResponse = ErrorResponse.builder()
.status(HttpStatus.NOT_ACCEPTABLE.value())
.message("토큰이 만료되었습니다.")
.code("TOKEN_EXPIRED")
.build();
response.setStatus(HttpStatus.NOT_ACCEPTABLE.value());
response.setContentType(MediaType.APPLICATION_JSON_VALUE);
response.setCharacterEncoding("UTF-8");
response.sendError(exception.getStatus().value(), exception.getMessage());
objectMapper.writeValue(response.getWriter(), errorResponse);
ErrorResponse errorResponse = ErrorResponse.builder()
.status(HttpStatus.UNAUTHORIZED.value())
.message("토큰이 만료되었습니다.")
.code("TOKEN_EXPIRED")
.build();
response.setStatus(HttpStatus.UNAUTHORIZED.value());
response.setContentType(MediaType.APPLICATION_JSON_VALUE);
response.setCharacterEncoding("UTF-8");
objectMapper.writeValue(response.getWriter(), errorResponse);

} catch (Exception ex) {
ErrorResponse errorResponse = ErrorResponse.builder()
.status(HttpStatus.INTERNAL_SERVER_ERROR.value())
.message("서버 오류가 발생했습니다.")
.code("INTERNAL_SERVER_ERROR")
.build();

response.setStatus(HttpStatus.INTERNAL_SERVER_ERROR.value());
response.setContentType(MediaType.APPLICATION_JSON_VALUE);
response.setCharacterEncoding("UTF-8");

objectMapper.writeValue(response.getWriter(), errorResponse);
}
Comment on lines +32 to 69

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

현재 catch 블록들에는 코드 중복과 로깅 누락이라는 두 가지 개선점이 있습니다.

  1. 코드 중복: 세 개의 catch 블록에서 ErrorResponse를 생성하고 응답을 설정하는 코드가 거의 동일합니다. 이 로직을 별도의 private 메서드로 추출하면 중복을 줄이고 코드 가독성과 유지보수성을 높일 수 있습니다.
  2. 로깅 누락: catch (Exception ex) 블록에서 예외를 로깅하지 않고 있습니다. 예상치 못한 오류를 추적하고 디버깅하기 어렵게 만들 수 있으므로, 클래스에 @Slf4j를 추가하고 log.error("...")를 사용하여 예외를 기록하는 것이 중요합니다.

아래는 리팩토링 예시입니다.

// private helper method
private void writeErrorResponse(HttpServletResponse res, HttpStatus status, String msg, String code) throws IOException {
    res.setStatus(status.value());
    res.setContentType(MediaType.APPLICATION_JSON_VALUE);
    res.setCharacterEncoding("UTF-8");
    ErrorResponse errRes = ErrorResponse.builder()
            .status(status.value()).message(msg).code(code).build();
    objectMapper.writeValue(res.getWriter(), errRes);
}

// catch (Exception ex) block
// log.error("Unhandled exception in filter chain", ex);
// writeErrorResponse(response, HttpStatus.INTERNAL_SERVER_ERROR, "서버 오류가 발생했습니다.", "INTERNAL_SERVER_ERROR");

}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package life.mosu.mosuserver.global.filter;

import io.jsonwebtoken.ExpiredJwtException;
import jakarta.servlet.FilterChain;
import jakarta.servlet.ServletException;
import jakarta.servlet.http.HttpServletRequest;
Expand Down Expand Up @@ -105,9 +106,12 @@ protected void doFilterInternal(

try {
setAuthentication(accessToken);
} catch (ExpiredJwtException e) {
log.error("액세스 토큰 만료: {}", e.getMessage());
throw e;
} catch (CustomRuntimeException e) {
log.error("액세스 토큰 인증 실패: {}", e.getMessage());
throw new CustomRuntimeException(ErrorCode.INVALID_TOKEN);
log.error("유효하지 않은 토큰 인증 실패: {}", e.getMessage());
throw e;
} catch (Exception e) {
log.error("액세스 토큰 인증 실패: {}", e.getMessage());
throw new RuntimeException("액세스 토큰 인증 중 예외 발생", e);
Expand All @@ -120,7 +124,7 @@ private void reissueToken(HttpServletRequest request, HttpServletResponse respon
throws IOException {
final TokenCookies tokenCookies = tokenResolver.resolveTokens(request);
if (!tokenCookies.availableReissue()) {
throw new CustomRuntimeException(ErrorCode.EXPIRED_REFRESH_TOKEN);
throw new CustomRuntimeException(ErrorCode.NOT_FOUND_TOKEN);
}

Token newToken = authTokenManager.reissueToken(tokenCookies);
Expand Down