사이드 프로젝트 리펙토링(1) - jwtFilter

개요

이제 어플리케이션을 어느 정도 만들 줄 알고 필수적인 아키텍처들에 대해 구현 정도는 알 줄 알게 되었습니다. 구현에만 집중하다 보니 해당 스택에 대해 제대로 아는 것도 없고 코딩 상태도 엉망이라는 느낌을 받았습니다. 아키텍처를 깊게 파는 것 보다는 기본 중에 기본인 메인 언어에 대한 공부 및 코딩 스타일을 교정하는 것이 우선이라고 판단하여 리펙토링을 먼저 하기로 했습니다.

JwtFilter

기존 코드

@RequiredArgsConstructor
@Slf4j
//OncePerRequestFilter 한 번만 동작
public class JwtFilter extends OncePerRequestFilter {
    private final JwtUtil jwtUtil;
    private final RefreshTokenRepository refreshTokenRepository;
    private final ExceptionManager exceptionManager;

    @Override
    protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) throws ServletException, IOException {
        try {
        // 1 : 인증정보가 있는가?
            HttpSession session = request.getSession();
            String userEmail ;
            String accessToken;
            final String authorization = request.getHeader("Authorization");
            log.info("authorization: {}", authorization);
            if (authorization == null || !authorization.startsWith("Bearer ")) {
                log.error("authorization is wrong");
                filterChain.doFilter(request, response);
                return;
            } else {
                accessToken = authorization.split(" ")[1];
                userEmail= (String) session.getAttribute(accessToken);
                log.info("요청자: " + userEmail);
            }
		// end 1
   
         // 2 : 엑세스 토큰이 만료됐는가?
            if (jwtUtil.isExpired(accessToken, "ACCESS")) {
                log.error("AccessToken 만료");
                log.info("유저 아이디: " + userEmail);
                // 3 : 리프레쉬 토큰이 만료됐는가?
                if (refreshTokenRepository.findById(userEmail, "refreshToken").isPresent()) {
                    log.info("RefreshToken 확인, AccessToken 재발급");
                    String newAccessToken = jwtUtil.createToken(userEmail, "ACCESS");
                    log.info("재발급 AccessToken : " + newAccessToken);
                    session.setAttribute(newAccessToken, userEmail);
                    response.setHeader("ACCESS", newAccessToken);
                    setAuthentication(userEmail, request, response, filterChain);
                } else {
                    throw new AppException(ErrorCode.TOKEN_EXPIRED, "토큰 만료"); 
                 // end 3 
                }
            } else {
                log.info("AccessToken 정상");
                userEmail = jwtUtil.getUserEmail(accessToken, "ACCESS");
                setAuthentication(userEmail, request, response, filterChain);
            }
         // end 2
            filterChain.doFilter(request, response);
        } catch (AppException e) {
            handleException(response, e);
        }
    }

    private void handleException(HttpServletResponse response, AppException e) throws IOException {
        response.setStatus(e.getErrorCode().getHttpStatus().value());
        response.setContentType("application/json");
        response.setCharacterEncoding("UTF-8");
        response.getWriter().write(new ObjectMapper().writeValueAsString(exceptionManager.createErrorResponse(e)));
    }

    public void setAuthentication(String userEmail, HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) throws ServletException, IOException {
        UsernamePasswordAuthenticationToken authenticationToken =
                new UsernamePasswordAuthenticationToken(userEmail, null, List.of(new SimpleGrantedAuthority("USER")));
        authenticationToken.setDetails(new WebAuthenticationDetailsSource().buildDetails(request));
        SecurityContextHolder.getContext().setAuthentication(authenticationToken);
    }
}

뭔가 메서드화를 하려다 만 모습이네요. 하나하나 고쳐보겠습니다.
우선 1번은 토큰이 없는 사용자에 대한 처리입니다. 토큰이 없다면 해당 JwtFilter를 종료하고 다음 filter로 바로 넘어갑니다. 이를 메서드화 하겠습니다.
일단 메서드화에 앞서 userEmail과 accessToken 변수가 현재 dofilter 내부에 선언되어 있는데 두 변수를 읽어 사용하는 메서드가 많아 전역변수로 처리한 후 진행했습니다.

리펙토링1

    private static String userEmail;
    private static String accessToken;

    @Override
    protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) throws ServletException, IOException {
        try {

            // 요청 세션 얻기
            HttpSession session = request.getSession();
            if(!extractAuthorizationInfo(request, response, session)){
                log.error("authorization is wrong");
                filterChain.doFilter(request, response);
                return;
            }
     }
            .
            .
            .

       private Boolean extractAuthorizationInfo(HttpServletRequest request, HttpServletResponse response, HttpSession session){
        String authorization = request.getHeader("Authorization");
        log.info("authorization: {}", authorization);
        if (authorization == null || !authorization.startsWith("Bearer ")) {
            return false;
        } else {
            accessToken = authorization.split(" ")[1];
            userEmail= (String) session.getAttribute(accessToken);
            log.info("요청자: " + userEmail);
            return true;
        }
    }

메서드화는 합리적으로 한 것 같습니다. 하지만 사용자 정보를 전역변수로 두는게 옳은지에 대해 의문을 가지던 중 멀티스레딩에 대한 생각을 하게 됐습니다.
// 멀티스레딩 관련 포스팅 준비 중

결과적으로 다음과 같이 처리했습니다.

 @Override
    protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) throws ServletException, IOException {
        try {
            String userEmail;
            String accessToken;
            // 요청 세션 얻기
            HttpSession session = request.getSession();
            String requestUrl = request.getRequestURI();
            log.info("요청 url: " + requestUrl);
            
            AuthenticationInfo authenticationInfo = extractAuthorizationInfo(request, session);
            if(authenticationInfo == null){
                log.error("authorization is wrong");
                filterChain.doFilter(request, response);
                return;
            }else{
                userEmail = authenticationInfo.userEmail;
                accessToken = authenticationInfo.accessToken;
                log.info("요청자: " + userEmail);
            }
           .
           .
           .
   private AuthenticationInfo extractAuthorizationInfo(HttpServletRequest request, HttpSession session){
        String authorization = request.getHeader("Authorization");
        log.info("authorization: {}", authorization);
        if (authorization == null || !authorization.startsWith("Bearer ")) {
            return null;
        } else {
            String accessToken = authorization.split(" ")[1];
            String userEmail= (String) session.getAttribute(accessToken);
            return new AuthenticationInfo(userEmail,accessToken);
        }
   }

인증정보를 extract한 후의 결과 처리를 다시 메서드화 하고 싶었지만 인증정보가 없는 경우 dofilter을 return 해야 현재 filter를 중단하고 다음 filter로 넘어갈 수 있기 때문에 결과처리를 doFilter 내부에 처리했습니다.

리펙토링2

  // 2 : 엑세스 토큰이 만료됐는가
   isExpiredAccessTokenTime(request, response, accessToken, userEmail, session);
  // end 2
            .
            .
            .
    private void isExpiredAccessTokenTime(HttpServletRequest request, HttpServletResponse response, String accessToken, String userEmail, HttpSession session) {
        if (jwtUtil.isExpired(accessToken, Token.TokenType.ACCESS.name())) {
            log.error("{} 만료", Token.TokenName.accessToken.name());
            log.info("유저 아이디 : " + userEmail);
            // 3 : 리프레쉬 토큰이 만료됐는가
            isExpiredRefreshTokenTime(request, response, userEmail, session);
        } else {
            log.info("{} 정상", Token.TokenName.accessToken.name());
            userEmail = jwtUtil.getUserEmail(accessToken, Token.TokenType.ACCESS.name());
            setAuthentication(userEmail, request);
        }
    }

    private void isExpiredRefreshTokenTime(HttpServletRequest request, HttpServletResponse response, String userEmail, HttpSession session) {
        if (refreshTokenRepository.findById(userEmail, Token.TokenType.REFRESH.name()).isPresent()) {
            regenerateAccessToken(request, response, userEmail, session);
        } else {
            throw new AppException(ErrorCode.TOKEN_EXPIRED, "토큰 만료");
        }
    }

    private void regenerateAccessToken(HttpServletRequest request, HttpServletResponse response, String userEmail, HttpSession session) {
        log.info("{} 확인, {} 재발급", Token.TokenName.refreshToken.name(), Token.TokenName.accessToken.name());
        String newAccessToken = jwtUtil.createToken(userEmail, Token.TokenType.ACCESS.name());
        log.info("재발급 {} : " + newAccessToken, Token.TokenName.accessToken.name());

        // 새 토큰으로 업데이트
        session.setAttribute(newAccessToken, userEmail);
        response.setHeader(Token.TokenType.ACCESS.name(), newAccessToken);
        setAuthentication(userEmail, request);
    }
            

긴 if-else 문을 세 개의 메서드로 쪼개니 확실히 가독성이 좋아졌습니다. 그리고 중복되는 매직 넘버 및 리터럴들을 상수화 했습니다.
참고로 상수 관리는 enum으로 했습니다.

import lombok.Getter;
public class Token{
    public enum TokenType{
        ACCESS,REFRESH;
    }
    public enum TokenName{
        accessToken,refreshToken;
    }

    @Getter
    public enum TokenTime {
        INFO(10 * 1000 * 60L, 60 * 1000 * 60L);

        private final Long accessExpiredTime;
        private final Long refreshExpiredTime;

        TokenTime(Long accessExpiredTime, Long refreshExpiredTime) {
            this.accessExpiredTime = accessExpiredTime;
            this.refreshExpiredTime = refreshExpiredTime;
        }
    }
}

리펙토링 결과

    @Override
    protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) throws ServletException, IOException {
        try {
            String userEmail;
            String accessToken;
            // 요청 세션 얻기
            HttpSession session = request.getSession();
            String requestUrl = request.getRequestURI();
            log.info("요청 url : " + requestUrl);
            // 1 : 인증정보가 있는가
            AuthenticationInfo authenticationInfo = extractAuthorizationInfo(request, session);
            if(authenticationInfo == null){
                log.error("{} is wrong", Auth.KeyWord.Authentication.name());
                filterChain.doFilter(request, response);
                return;
            }else{
            // 정보 매핑
                userEmail = authenticationInfo.userEmail;
                accessToken = authenticationInfo.accessToken;
                log.info("요청자 이메일 : " + userEmail);
            // end 정보 매핑
            }
            // end 1

            // 2 : 엑세스 토큰이 만료됐는가
            isExpiredAccessTokenTime(request, response, accessToken, userEmail, session);
            // end 2
            filterChain.doFilter(request, response);
        } catch (AppException e) {
            handleException(response, e);
        }
    }

리팩토링을 진행하며 대부분의 문제를 해결했지만, 여전히 '정보 매핑'과 같이 상태에 따라 결과를 처리하는 부분을 void mappingInfo()와 같은 메서드로 분리해야 하는지에 대해 확신이 서지 않습니다