swp2022/swp-server

JWT 인증 시 오류가 GlobalExceptionHandler로 전달되지 않는 문제

0chil opened this issue · 9 comments

0chil commented

JWT 인증 시 비정상 토큰에 대한 오류가 GlobalExceptionHandler로 전달되지 않는 문제가 발생하고 있습니다.

기존 상황

Pull Request #2
SecurityConfig.class 에 Handler를 적용해 스프링 시큐리티 Exception을 미리 정의한 핸들러로 보내 처리하는 방식입니다.
당연히 오류가 GlobalExceptionHandler로 전달되지 않는 상황이었습니다.

image

현 상태 진단

  1. JwtProvider(JWT 발급하는 역할) 에서 사전에 정의된 Exception(com.swp.common.exception.ForbiddenException)을
    상속하는 Exception (com.swp.auth.exception.InvalidTokenException)을 Throw 합니다.

image

  1. 상기 Exception들은 catch하는 곳이 없어 Spring Framework의 내부 구현체까지 전달되다가 org.springframework.security.core.AuthenticationException으로 변환되어 JwtAuthenticationEntryPoint로 전달되는 상황입니다.

image

  1. 이 Exception은 org.springframework.web.servlet.HandlerExceptionResolver 를 통해서 GlobalExceptionHandler로 전달되나, ApiException의 자식이 아니기 때문에 500 Internal ErrorJSON형태로 Response됩니다.

image

고려해본 개선 방안

  1. 현 상태 진단의 (3)에서 이야기한 Exception의 전달 -> 알맞은 Exception이 아니므로 500 Internal Error
    -> AuthenticationException을 핸들링 하도록 추가한다 (아직 시도 안 함)

  2. JwtAuthentication 전에 JwtExceptionFilter를 추가하여 Exception을 받는다
    -> 이를 HandlerExceptionResolver를 통해 GlobalExceptionHandler로 전달 하려고 한다 (아직 시도 안 함)

JwtExceptionFilter는 추가하고 SecurityConfig에 등록해두었습니다

image

0chil commented

image

개선방안 2 을 시도하여 일단 원하는 대로 구현은 되었습니다.

image

코드 리뷰 요청 드리겠습니다

음 사소한 동작을 위해 너무 변경 사항이 많은 것 같아요.
Spring에서 만들어 놓은 AuthenticateException이 발생한다면, 단순히 GlobalExceptionHandler에서 AuthenticateException을 403으로 처리해도록 코드 몇 줄만 추가하면 되는거 아닌가요??

0chil commented

그 방향으로 해결해보려고 했으나 AuthorizationException은 원래 발생한 ApiException과는 아예 다른 객체라서 Exception의 detailedMessage가 전달이 되지 않는 문제가 있었습니다 🥲

GlobalExceptionHandler의 아래 부분을 보면

@RestControllerAdvice
public class GlobalExceptionHandler extends ResponseEntityExceptionHandler {

    @ExceptionHandler(Exception.class)
    protected ResponseEntity<Object> handleApiException(Exception exception, WebRequest request) {
        if (!Objects.nonNull(exception)) return null;
        if(exception instanceof ApiException) {
            return handleExceptionInternal(exception, null, new HttpHeaders(), ((ApiException) exception).getStatus(), request);
        } else return handleExceptionInternal(exception, null, new HttpHeaders(), INTERNAL_SERVER_ERROR, request);
    }

    //..
}
  • handleApiException() 메소드에는 @ExceptionHandler(Exception.class)가 적용되어 있어요.
    이 어노테이션은 즉 Exception 및 하위 클래스는 모두 이 메소드에서 처리한다. 라는 것을 의미해요.

  • 따라서 지금은 이 메소드 내에 if(exception instanceof ApiException)이 있어서 ApiException만 처리하게 되어 있는데,
    만약 아래 처럼 분기를 더 추가하면 어떨까요?

@RestControllerAdvice
public class GlobalExceptionHandler extends ResponseEntityExceptionHandler {

    @ExceptionHandler(Exception.class)
    protected ResponseEntity<Object> handleApiException(Exception exception, WebRequest request) {
        if (!Objects.nonNull(exception)) return null;
        if(exception instanceof ApiException) {
            return handleExceptionInternal(exception, null, new HttpHeaders(), ((ApiException) exception).getStatus(), request);
        } else if(exception instanceof AuthenticateException) {
			return handleExceptionInternal(exception, null, new HttpHeaders(), HttpStatus.UNAUTHORIZED, request);
		} else return handleExceptionInternal(exception, null, new HttpHeaders(), INTERNAL_SERVER_ERROR, request);
    }

    //..
}
0chil commented

GlobalExceptionHandler의 아래 부분을 보면

@RestControllerAdvice
public class GlobalExceptionHandler extends ResponseEntityExceptionHandler {

    @ExceptionHandler(Exception.class)
    protected ResponseEntity<Object> handleApiException(Exception exception, WebRequest request) {
        if (!Objects.nonNull(exception)) return null;
        if(exception instanceof ApiException) {
            return handleExceptionInternal(exception, null, new HttpHeaders(), ((ApiException) exception).getStatus(), request);
        } else return handleExceptionInternal(exception, null, new HttpHeaders(), INTERNAL_SERVER_ERROR, request);
    }

    //..
}
  • handleApiException() 메소드에는 @ExceptionHandler(Exception.class)가 적용되어 있어요.
    이 어노테이션은 즉 Exception 및 하위 클래스는 모두 이 메소드에서 처리한다. 라는 것을 의미해요.
  • 따라서 지금은 이 메소드 내에 if(exception instanceof ApiException)이 있어서 ApiException만 처리하게 되어 있는데,
    만약 아래 처럼 분기를 더 추가하면 어떨까요?
@RestControllerAdvice
public class GlobalExceptionHandler extends ResponseEntityExceptionHandler {

    @ExceptionHandler(Exception.class)
    protected ResponseEntity<Object> handleApiException(Exception exception, WebRequest request) {
        if (!Objects.nonNull(exception)) return null;
        if(exception instanceof ApiException) {
            return handleExceptionInternal(exception, null, new HttpHeaders(), ((ApiException) exception).getStatus(), request);
        } else if(exception instanceof AuthenticateException) {
			return handleExceptionInternal(exception, null, new HttpHeaders(), HttpStatus.UNAUTHORIZED, request);
		} else return handleExceptionInternal(exception, null, new HttpHeaders(), INTERNAL_SERVER_ERROR, request);
    }

    //..
}

네 이 내용이 위에 얘기한 내용과 같은 내용인데요,
아래 처럼 ApiException의 메시지 대신, AuthenticateException에 사전 정의된 메시지만 나오는 문제가 있습니다.
스크린샷 2022-05-11 오후 1 26 34

여기까지 보셨다면.. 사실 spring security의 인증 과정에서 발생하는 org.springframework.security.core.AuthenticationException은 메시지가 저희가 컨트롤 할 수 없는걸로 알고 있긴 해요 ㅎㅎ 그래서 저도 개인 프로젝트에서 성철님이 겪은 이슈를 동일하게 겪어서, Spring Security에서 발생하는 예외들은 AuthenticationException을 상속한 예외 클래스를 만들어서 throw 하도록 했어요.

아래는 제 개인 플젝 코드 중 일부에요. Kotlin이긴 하지만, 이해하는 데 어려운건 없을 거에요!!

// AuthenticationException 상속하는 예외 클래스 정의
import org.springframework.security.core.AuthenticationException

class AuthenticateException(message: String) : AuthenticationException(message)

// JWT 인증 과정(spring security 과정 내에서 진행됨)에서 위의 예외를 throw
private fun extractAllClaims(token: String): Claims {
        try {
            return Jwts.parser().setSigningKey(jwtProperties.secret).parseClaimsJws(token).body
        } catch (expiredJwtException: ExpiredJwtException) {
            throw AuthenticateException("Jwt 토큰이 만료되었습니다.")
        } catch (unsupportedJwtException: UnsupportedJwtException) {
            throw AuthenticateException("지원되지 않는 Jwt 토큰입니다.")
        } catch (malformedJwtException: MalformedJwtException) {
            throw AuthenticateException("잘못된 형식의 Jwt 토큰입니다.")
        } catch (signatureException: SignatureException) {
            throw AuthenticateException("Jwt Signature이 잘못된 값입니다.")
        } catch (illegalArgumentException: IllegalArgumentException) {
            throw AuthenticateException("Jwt 헤더 값이 잘못되었습니다.")
        }
    }
}
0chil commented

오류를 처리하기 위한 Filter를 추가하고, Exception을 중간에 낚아 채 직접 보내는 방법은 �별로 좋은 구조는 아니라고 생각해요.
그래서 최후의 최후에 사용하고 싶은 방법입니다.
이게 조금 더 자연스러운 방법으로 보이니 제안해주신 방법을 고려해보겠습니다.

네네 정해진 답은 없어요!! 이렇게 논의해 가면서 모두가 납득하는 더 좋은 방법으로 가는게 중요하다고 생각해요!

0chil commented

해결!