AbstractAuthenticationProcessingFilter 구현 메서드에 관한 질문

[dongkyunkimdev]

public class JwtAuthenticationFilter extends AbstractAuthenticationProcessingFilter {
...
	@Override
	protected boolean requiresAuthentication(HttpServletRequest request, HttpServletResponse response) {
		return matcher.matches(request);
	}
...
	@Override
	protected void successfulAuthentication(HttpServletRequest request, HttpServletResponse response, FilterChain chain,
		Authentication authResult) throws IOException, ServletException {
		SecurityContextHolder.getContext().setAuthentication(authResult);
		super.successfulAuthentication(request, response, chain, authResult);
	}

	@Override
	protected void unsuccessfulAuthentication(HttpServletRequest request, HttpServletResponse response,
		AuthenticationException failed) throws IOException, ServletException {
		SecurityContextHolder.clearContext();
		super.unsuccessfulAuthentication(request, response, failed);
	}

}

JwtAuthenticationFilter에서 Override한 위 메서드들에 대해서 궁금한게 있습니다!

1. requiresAuthentication를 override 한 이유

AbstractAuthenticationProcessingFilter의 requiresAuthentication method는 아래와 같습니다.

protected boolean requiresAuthentication(HttpServletRequest request, HttpServletResponse response) {
		if (this.requiresAuthenticationRequestMatcher.matches(request)) {
			return true;
		}
		if (this.logger.isTraceEnabled()) {
			this.logger
					.trace(LogMessage.format("Did not match request to %s", this.requiresAuthenticationRequestMatcher));
		}
		return false;
	}

Override해서 구현한 return matcher.matches(request);의 기능이 AbstractAuthenticationProcessingFilter에서 이미 구현되어 있는 것 같은데, 새롭게 구현해서 사용하는 이유가 궁금합니다!

2. successfulAuthentication, unsuccessfulAuthentication를 override한 이유

AbstractAuthenticationProcessingFilter의 successfulAuthentication method는 아래와 같습니다.

	protected void successfulAuthentication(HttpServletRequest request, HttpServletResponse response, FilterChain chain,
			Authentication authResult) throws IOException, ServletException {
		SecurityContextHolder.getContext().setAuthentication(authResult);
		if (this.logger.isDebugEnabled()) {
			this.logger.debug(LogMessage.format("Set SecurityContextHolder to %s", authResult));
		}
		this.rememberMeServices.loginSuccess(request, response, authResult);
		if (this.eventPublisher != null) {
			this.eventPublisher.publishEvent(new InteractiveAuthenticationSuccessEvent(authResult, this.getClass()));
		}
		this.successHandler.onAuthenticationSuccess(request, response, authResult);
	}

현재 선필님이 작성하신 코드에서는

	@Override
	protected void successfulAuthentication(HttpServletRequest request, HttpServletResponse response, FilterChain chain,
		Authentication authResult) throws IOException, ServletException {
		SecurityContextHolder.getContext().setAuthentication(authResult);
		super.successfulAuthentication(request, response, chain, authResult);
	}

SecurityContextHolder.getContext().setAuthentication(authResult); 로 컨텍스트에 인증 객체를 저장하고 super.successfulAuthentication();를 호출하는데, 인증 객체를 저장하는 코드를 반복해서 추가 구현한 이유가 궁금합니다!!
unsuccessfulAuthentication도 같은 맥락입니다!

[slolee]

질문과 약간은 관련이 없지만 successfulAuthentication 을 오버라이딩 하는 이유중에 하나는,

기본적으로 오버라이딩을 하지 않을 경우 AbstractAuthenticationProcessingFilter 에 구현된 super.successfulAuthentication() 을 호출하게 되는데 해당 메소드에서는 내부적으로 이전 인증 실패에 대한 요청인 savedRequest 를 가져와서 처리하게 되는걸로 알고있습니다.

선필님이 작성하신 코드를 직접 실행해보진 않았지만 제가 개인적으로 프로젝트를 진행했을 때의 이슈로 JWT 인증시

이런 이유로 실패하는 케이스가 있었는데요.

저 같은 경우는 Spring Security 에서 제공해주는 로그인 폼을 사용하지 않고 별도의 /auth/login API 를 통해서 로그인처리를 했고, Spring Security 를 이용해서는 단순히 JWT 토큰 인증에 대한 처리만 했었습니다.

JWT 토큰 인증에 성공해서 super.successfulAuthentication() 이 호출되었을 때 이전에 인증에 실패한 요청이 당연히 없기 때문에 savedRequest 는 null 이고 이에 따라 기본값으로 설정된 경로인 "/" 로 리다이렉트를 시키게 되었고 "/" 로 요청을 보내는 와중에 또 JWT 토큰 검증을 위한 Spring Seucrity Filter 를 만나게 되고.... 무한 반복 이었던 이슈였습니다.

그래서 저는 기본적으로 구현된 super.successfulAuthentication() 말고 아래 핸들러를 직접 구현해서 사용했었습니다..!

override fun successfulAuthentication(
        request: HttpServletRequest,
        response: HttpServletResponse,
        chain: FilterChain,
        authResult: Authentication
    ) {
        val context = SecurityContextHolder.createEmptyContext()
        context.authentication = authResult
        SecurityContextHolder.setContext(context)
        chain.doFilter(request, response)
    }

아니면 "/" 경로에 대해서 해당 필터를 예외처리해도 해결이 가능했습니다.

선필님 코드랑은 다른 케이스지만 이것때문에 예전에 고생한 기억이 떠올라 공유드려봅니다 ㅎㅎㅎ

[dongkyunkimdev]

아하 super.successfulAuthentication(); 에서 내부적으로 savedRequest를 사용하는군요!! 정보 공유 감사합니다 찬준님! 🙇‍♂️

[seonpilKim]

1. requiresAuthentication를 override 한 이유

먼저 의도를 설명드리자면, 다른 이유가 있다기 보다는 단순히 커스텀 matcher를 사용해서 인증 필터를 건너뛸 paths를 비교하는 메소드가 requiresAuthentication()이라서 재정의하려는 생각으로 구현 했던 것 같습니다.

그리고 동균님께서 지적해 주신 김에 코드를 다시 살펴봤는데, 리팩토링을 고려할 만한 부분을 찾은 것 같아서 두 분의 의견을 한 번 여쭤보고 싶습니다..!

우선, 생성자 JwtAuthenticationFilter() 에서 다른 부모 생성자를 호출하게 되면 requiresAuthentication() 메소드를 굳이 재정의 할 필요가 없어질 것 같습니다.

// AbstractAuthenticationProcessingFilter.java
protected AbstractAuthenticationProcessingFilter(RequestMatcher requiresAuthenticationRequestMatcher) {
     Assert.notNull(requiresAuthenticationRequestMatcher, "requiresAuthenticationRequestMatcher cannot be null");
    this.requiresAuthenticationRequestMatcher = requiresAuthenticationRequestMatcher;
}

위의 부모 생성자를 호출하도록 변경해보면, JwtAuthenticationFilter.java을 다음과 같이 리팩토링 할 수 있을 것 같습니다.

public class JwtAuthenticationFilter extends AbstractAuthenticationProcessingFilter {

	private final JwtUtils jwtUtils;
-	private final RequestMatcher matcher;

	public JwtAuthenticationFilter(RequestMatcher matcher, JwtUtils jwtUtils) {
-		super(ALL_PATTERN);
+		super(matcher);
		this.jwtUtils = jwtUtils;
-		this.matcher = matcher;
	}

-	@Override
-	protected boolean requiresAuthentication(HttpServletRequest request, HttpServletResponse response) {
-		return matcher.matches(request);
-	}

다만, 아쉬운 점은 SkipPathRequestMatcher.java의 matches() 메소드가 매치되는 경우 false를 반환하고 있어서, 가독성이 떨어지지 않을까 고민이 생기게 되더라구요.

위의 방식처럼 수정을 하는 것과, 혹은 기존 코드에서 아래와 같이 수정하는 방식 중 어떤 게 더 나을까요?🤔

public class SkipPathRequestMatcher implements RequestMatcher {
        ...
	@Override
	public boolean matches(HttpServletRequest request) {
-		return !matcher.matches(request);
+		return matcher.matches(request);
	}
}

public class JwtAuthenticationFilter extends AbstractAuthenticationProcessingFilter {
	...
	@Override
	protected boolean requiresAuthentication(HttpServletRequest request, HttpServletResponse response) {
-		return matcher.matches(request);
+ 		return !matcher.matches(request);
	}

2. successfulAuthentication, unsuccessfulAuthentication를 override한 이유

AbstractAuthenticationProcessingFilter.java에 구현된 successfulAuthentication()와 unsuccessfulAuthentication() 메소드를 보면 this.rememberMeServices.xxx() 메소드를 호출하는 부분이 보이더라구요.
저희가 인증 로직에서 rememberMe 방식은 고려하고 있지 않기 때문에, 해당 부분을 호출하는 코드를 제거하는 방식으로 최적화가 가능하지 않을까? 라는 가벼운 생각에 재정의 했었습니다.

---(수정)---
이제 보니 super.successHandler.onAuthenticationSuccess()를 호출해야 하는데 super.successfulAuthentication()를 호출하고 있었군요...😬 이 부분도 아래와 같이 수정해야 겠네요. (unsuccessfulAuthentication()도 마찬가지)

public class JwtAuthenticationFilter extends AbstractAuthenticationProcessingFilter {
        ...
	@Override
	protected void successfulAuthentication(HttpServletRequest request, HttpServletResponse response, FilterChain chain,
		Authentication authResult) throws IOException, ServletException {
		SecurityContextHolder.getContext().setAuthentication(authResult);
-		super.successfulAuthentication(request, response, chain, authResult);		
+		super.successHandler.onAuthenticationSuccess(request, response, chain, authResult);
	}
}

[slolee]

저는 전자의 경우를 선호하는데, 어차피 Matcher 클래스의 이름이 SkipPathRequestMatcher 이기 때문에 AbstractAuthenticationProcessingFilter 의 생성자로 전달했을 경우 크게 가독성이 떨어지는 않는다고 생각해요.

"이 필터는 이러한 요청에 대해서 스킵 되는구나" 정도로 생각할 수 있을 것 같아요.

[slolee]

오히려 구조가 심플해져서 나아지는 부분도 있다고 생각이 들구요!

[dongkyunkimdev]

1. 찬준님 의견처럼 부모 생성자를 호출하는 쪽에 동의합니다!! 개인적으로 전자가 흐름을 파악하기 더 좋게 느껴집니다
2. 아 잘못 작성하신 거였군요~~ pr에서 rememberMe 방식을 사용하지 않는다고 했는데, super.successfulAuthentication(request, response, chain, authResult);를 호출해서 의아했었습니다
   구체적인 답변 감사드립니다!! 🙇

[seonpilKim]

의견 감사합니다!
그리고 예리하게 제 실수(?)를 찾아주신 동균님 정말 감사합니다😊😊