[레벨 2] 08. 2단계 - 장바구니/주문 API 변경하기

public class AuthenticationInterceptor implements HandlerInterceptor {
    ...
    @Override
    public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) {
        if (HttpMethod.OPTIONS.matches(request.getMethod())) {
            return true;
        }

        String accessToken = AuthorizationExtractor.extract(request);
        jwtTokenProvider.validateToken(accessToken);

        String payload = jwtTokenProvider.getPayload(accessToken);
        request.setAttribute(ATTRIBUTE_PAYLOAD_KEY, payload);
        return true;
    }
}

`매트`: 이러한 요청을 무조건적으로 true 처리하는 것이 적절한지, `리뷰어님`은 동일한 상황에서 어떠한 방식으로 처리하는지 궁금합니다!

`리뷰어`: cors 관련해서 질문주신게 있네요. 사전 요청에 대해서 무조건 true를 주는게 적절한지를 질문주셨는데요. 어쩔수 없이 Interceptor 로직이 태워지기 때문에 OPTIONS에 대해서는 true를 반환하도록 해야하지 않나 싶네요..

`매트`: 별 다른 방법이 없는 것 같네요 ㅜㅠ 그렇다면 서블릿 이전에 거쳐가는 filter에서 CorsFilter를 등록하는 방식은 어떻게 생각하시나요?? 인터셉터를 거치지 않기 때문에 로직 상 더욱 적합하다고 느껴지는데 이에 대해 철시에 대한 의견이 궁금합니다!!

`리뷰어`: CorsFilter를 등록하더라도 Filter -> Interceptor -> 핸들러 메서드 -> Interceptor -> Filter 순으로 동작하기 때문에 인터셉터를 거치지 않도록 만든다는 방법이 잘 이해가 가지 않네요 ㅠ 필터 구현에 대해 생각하신게 있는걸까요?

`매트`: 아래와 같이 preflight request 처리를 위한 헤더(Access-Control-Allow-Origin, Access-Control-Allow-Methods)를 세팅한 뒤 filter 체이닝을 진행하지 않고 200 상태 코드를 반환하는 형식을 생각하고 있었습니다!

@Component
@Order(Ordered.HIGHEST_PRECEDENCE)
public class CorsFilter implements Filter {
    ...
    @Override
    public void doFilter(ServletRequest req, ServletResponse res, FilterChain chain) throws IOException, ServletException {
        HttpServletRequest request = (HttpServletRequest) req;
        HttpServletResponse response = (HttpServletResponse) res;

        response.setHeader("Access-Control-Allow-Origin", "*");
        response.setHeader("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS");
        response.setHeader("Access-Control-Max-Age", "3600");
        response.setHeader("Access-Control-Allow-Headers", "authorization, content-type, xsrf-token");
        response.addHeader("Access-Control-Expose-Headers", "xsrf-token");

        if("OPTIONS".equalsIgnoreCase(request.getMethod())) {
            response.setStatus(HttpServletResponse.SC_OK);
            return;
        }
        
        chain.doFilter(req, res);
    }
    ...
}

서블릿 앞단에 해당 필터를 등록한 뒤 preflight request 요청이 오게 되면 http 메서드를 확인하여 서블릿 및 인터셉터를 타지 않도록 고민하고 있습니다! 결국 Cors의 preflight 관련 처리를 CorsFilter에서 모두 처리하게 되어 책임을 분리할 수 있다고 생각했습니다!

`리뷰어`: 음 결국 모든 도메인과 url에 대해서 사전 요청 OPTIONS에 대해서는 요청을 허용한다는 의미인가보네요. 보안적인 측면에서는 허용해야할 부분만 열어주는게 제일 베스트라고는 생각하는데요. 그렇다면 지금 현재 방식이 더 낫지 않을까 생각이 들기도 하네요 ㅎㅎ..

@Repository
public class OrdersDetailDao {
    ...
    public List<OrdersDetail> findByOrdersId(Long ordersId) {
        String sql = "SELECT od.id as id, od.quantity as quantity, "
                + "o.id as orderId, "
                + "c.id as customerId, c.username as customerUsername, c.email as customerEmail, "
                + "c.password as customerPassword, c.address as customerAddress, "
                + "c.phone_number as customerPhoneNumber, "
                + "p.id as productId, p.name as productName, p.price as productPrice, "
                + "p.image_url as productImageUrl, p.description as productDescription, p.deleted as productDeleted "
                + "FROM orders_detail od "
                + "JOIN orders o ON od.orders_id = o.id "
                + "JOIN product p ON od.product_id = p.id "
                + "JOIN customer c ON o.customer_id = c.id "
                + "WHERE orders_id = :ordersId";

        SqlParameterSource parameterSource = new MapSqlParameterSource("ordersId", ordersId);
        return jdbcTemplate.query(sql, parameterSource, generateOrderDetailMapper());
    }
    ...
}

`리뷰어`: 복잡한 쿼리문에 대해서도 질문주셨는데요. 저도 가능하면 한방쿼리를 통해서 조회하는 쿼리를 많이 사용하기는 한거 같아요. 다만, 반드시 `EXPLAIN`으로 해당 쿼리의 `실행계획`이 어떻게 되는지 확인이 필요하구요. 적절한 인덱스를 타지 않아 부하가 심하다고 생각되면 각각 따로 조회해서 조합해줬던거 같네요. 

`매트`: 아직 DB에 대한 개념이 부족하여 주신 리뷰를 완벽하게 이해 하진 못했네요 🥲 `EXPLAIN`과 `인덱스` 키워드를 통해 관련 학습 진행해보겠습니다!!

`리뷰어`: 기존에는 `AuthenticationPrincipalArgumentResolver`에서 `토큰 검증 및 전달`까지 해줬던걸로 보이는데요. `AuthenticationInterceptor`와 `AuthenticationPrincipalArgumentResolver`로 나눠진것 같네요. 위와 같이 다시 구성하신 이유가 있을까요?

추가로 인터셉터에서 토큰 검증을 해서 attribute로 넘기기때문에 여기서는 getAttribute로 세팅하는 부분만 남은걸로 보이는데요. 만약 getAttribute("payload")에 값이 없는 경우는 어떻게 되나요?

`매트`: ArgumentResolver의 사용 목적에 대해 고민해보았습나다. 그러던 중 ArgumentResolver을 만들기 위해 사용하는 HandlerMethodArgumentResolver의 설명을 확인하였습니다.

> Strategy interface for resolving method parameters into argument values in the context of a given.

Spring에서는 ArgumentResolver를 하나의 전략 인터페이스로 인식하고 있으며 목적은 parameter가 원하는 형태의 객체로 바인딩하는 것이라 생각합니다. 이러한 근거를 기반으로 인증 및 인가에 대한 로직들을 모두 interceptor로 이동하였습니다!

또한 관련 attribute를 누락한 이유는 해당 `AuthenticationPrincipleArgumentResolver`는 항상 interceptor를 거칠 때만 사용해서 관련 검증을 누락시켰습니다! 철시가 언급하신 것 처럼 만약 interceptor를 거치지 않고 argument를 사용하게 되면 관련 atrribute가 존재하지 않게 되므로 관련 처리 진행하였습니다!

`리뷰어`: 현재는 Interceptor로 검증하는 곳과 AuthenticationPrincipleArgumentResolver가 사용되는 곳이 동일하기 때문에 문제가 없지만 만약 추후에 기능이 추가된다고 가정했을때 두 사용하는 곳이 달라질 수 있어서 질문드린거이긴해요. Interceptor의 사용처와 AuthenticationPrincipleArgumentResolver의 사용처를 같게 만드는 것은 사람(개발자)이 하게되는 것이니 휴먼에러의 여지도 있어서요.

값을 바인딩한다는 것에 초점을 맞추신걸로 보이는데 AuthenticationPrincipleArgumentResolver에서도 Authorization 헤더를 가지고 바인딩이 가능한만큼 AuthenticationPrincipleArgumentResolver이 검증부분을 가져가도 괜찮지 않을까 의견드려요.

`매트`: 리뷰어님의 의견에도 동의합니다! 언급해주신 것에 더 나아가 Token에 대한 검증은 필요하지만 LoginCustomer가 필요하지 않은 핸들러가 존재할 때를 고민해보았습니다. 예를들면 admin과 관련된 Role이 추가되어 id라는 식별자를 기반으로 URL(ex. /api/customers/1)에 접근하게 되면 이때 LoginCustomer는 사용되지 않기 때문에 AuthenticationPrincipleArgumentResolver의 검증 로직은 사용할 수 없으며 결국 추가적인 Interceptor를 통해 처리해야 하므로 중복된 코드를 야기하게 된다고 생각하여 현재 작성한 코드가 좀 더 확장에 유연할 수 있다고 판단합니다!