Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIX/#41] JWT 필터 내 whilelist 처리 오류 해결 #42

Merged
merged 6 commits into from
Jan 17, 2025
Merged

[FIX/#41] JWT 필터 내 whilelist 처리 오류 해결 #42

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ee56a88
[FIX] JwksRequest 요청 검증 시 메이커스 프로덕트 서버가 아닐 경우(즉, 클라이언트)의 요청에서는 SERVER…
sung-silver Jan 17, 2025
11a270c
[FIX] 사용하지 않는 구문 삭제 및 isWhiteRequest 검증 수정
sung-silver Jan 17, 2025
0d73206
[FEAT] INTERNAL SERVER ERROR handle 구현
sung-silver Jan 17, 2025
8367fab
[FEAT] AuthException 뿐만 아니라 서비스 내에서 정의된 모든 Exception을 핸들링하도록 변경
sung-silver Jan 17, 2025
30dc62d
[CHORE] 디버깅을 위해 변경했던 접근 제어자를 다시 private로 변경
sung-silver Jan 17, 2025
10231bc
[MERGE] dev 최신사항 반영
sung-silver Jan 17, 2025
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
17 changes: 17 additions & 0 deletions src/main/java/sopt/makers/authentication/support/code/support/failure/CommonFailure.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package sopt.makers.authentication.support.code.support.failure;

import static lombok.AccessLevel.PRIVATE;

import sopt.makers.authentication.support.code.base.*;

import org.springframework.http.*;

Copy link
Contributor

Choose a reason for hiding this comment

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

P5

애스터리스크 제거해주시면 좋을것 같습니다~

import lombok.*;
Copy link
Contributor

Choose a reason for hiding this comment

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

P5

요기도욥


@Getter
@RequiredArgsConstructor(access = PRIVATE)
public enum CommonFailure implements FailureCode {
INTERNAL_SERVER_ERROR(HttpStatus.INTERNAL_SERVER_ERROR, "서버 내부 오류입니다");
private final HttpStatus status;
private final String message;
}
12 changes: 5 additions & 7 deletions src/main/java/sopt/makers/authentication/support/constant/SystemConstant.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,20 @@ private SystemConstant() {}
private static final String API_PATH_PREFIX = "/api";
private static final String API_VERSION = "/v1";

public static final String API_DEFAULT_PREFIX = API_PATH_PREFIX + API_VERSION;
private static final String API_DEFAULT_PREFIX = API_PATH_PREFIX + API_VERSION;

private static final String PATH_ACTUATOR = "/actuator";
private static final String PATH_AUTH = API_DEFAULT_PREFIX + "/auth";
public static final String PATH_AUTH = API_DEFAULT_PREFIX + "/auth";
private static final String PATH_ERROR = "/error";
private static final String PATH_TEST = "/test";
private static final String PATH_GET_REGISTER_SOCIAL_PLATFORM =
API_PATH_PREFIX + "/social/accounts/social";

public static List<String> WHITE_PATHS =
List.of(PATH_ACTUATOR, PATH_AUTH, PATH_GET_REGISTER_SOCIAL_PLATFORM, PATH_ERROR, PATH_TEST);

public static final String PATTERN_ALL = "/**";
public static final String PATTERN_ERROR_PATH = PATH_ERROR + PATTERN_ALL;
public static final String PATTERN_ACTUATOR = PATH_ACTUATOR + PATTERN_ALL;
public static final String PATTERN_AUTH = PATH_AUTH + PATTERN_ALL;
public static final String PATTERN_TEST = API_DEFAULT_PREFIX + PATH_TEST + PATTERN_ALL;
public static final String PATTERN_ROOT_PATH = "/";

public static final List<String> WHITELIST_WILDCARD =
List.of(PATH_ERROR, PATH_ACTUATOR, PATH_AUTH, PATH_TEST);
}
16 changes: 12 additions & 4 deletions src/main/java/sopt/makers/authentication/support/exception/ApplicationExceptionHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package sopt.makers.authentication.support.exception;

import static sopt.makers.authentication.support.code.support.failure.CommonFailure.INTERNAL_SERVER_ERROR;

import sopt.makers.authentication.support.common.api.BaseResponse;
import sopt.makers.authentication.support.exception.domain.AuthException;
import sopt.makers.authentication.support.exception.base.*;

import org.springframework.http.ResponseEntity;
import org.springframework.http.*;
import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.bind.annotation.RestControllerAdvice;

Expand All @@ -12,9 +14,15 @@
@Slf4j
@RestControllerAdvice
public class ApplicationExceptionHandler {
@ExceptionHandler(RuntimeException.class)
ResponseEntity<BaseResponse<?>> handleInternalException(final RuntimeException e) {
log.error(e.getMessage());
return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR)
.body(BaseResponse.ofFailure(INTERNAL_SERVER_ERROR));
}
Comment on lines +17 to +22
Copy link
Member

Choose a reason for hiding this comment

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

P1

음 우선 filter의 경우 Dispatcher Servlet 도달 이전에 수행되는 계층이라 Dispatcher Servlet 도달 이후 역할을 수행하는 ExceptionHandler(@ControllerAdvice)에 Filter 단에서 발생하는 Custom Exception은 처리를 못합니다.

그렇기 때문에 직접적인 HttpServletResponse의 Body를 직접 조작해줘야 하는 것으로 알고 있어요!
(Body를 조작하는 메서드를 정의해서 적용하거나 별도 Exception에 따른 조작을 수행하는 Filter를 마지막 순서에 등록하는 방법이 있습니다.)

지금 확인해보니 본 프로젝트에서는 Error Response 조작 메서드의 구현 및 적용이 되어있지 않은 것 같네요..!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

아 필터 단에서의 에러는 아직 반영되지 않은게 맞습니다! 제 의도는 RuntimeException보다 AuthException만 해당 핸들러에 반영되어 있어서 BaseException으로 바꾸는 겸 internalException도 추가한거였어요! 말씀해주신 부분이랑 shouldNotFilter 적용해서 다음주중으로 반영해두겠습니다!

Copy link
Member

Choose a reason for hiding this comment

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

아하아하!!! 에러 처리를 세분화한 내용이군요!! 확인했습니다 :)


@ExceptionHandler(AuthException.class)
ResponseEntity<BaseResponse<?>> authFailureException(final AuthException e) {
@ExceptionHandler(BaseException.class)
ResponseEntity<BaseResponse<?>> handleBusinessException(final BaseException e) {
log.error(e.getError().getMessage());
return ResponseEntity.status(e.getError().getStatus().value())
.body(BaseResponse.ofFailure(e.getError()));
Expand Down
11 changes: 8 additions & 3 deletions ...main/java/sopt/makers/authentication/support/security/filter/JwtAuthenticationFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package sopt.makers.authentication.support.security.filter;

import static sopt.makers.authentication.support.constant.SystemConstant.WHITE_PATHS;
import static sopt.makers.authentication.support.constant.SystemConstant.WHITELIST_WILDCARD;

import sopt.makers.authentication.support.jwt.provider.JwtAuthAccessTokenProvider;
import sopt.makers.authentication.support.security.authentication.CustomAuthentication;
Expand Down Expand Up @@ -31,6 +31,11 @@ protected void doFilterInternal(
final HttpServletRequest request, final HttpServletResponse response, FilterChain filterChain)
throws ServletException, IOException {

if (shouldNotFilter(request)) {
filterChain.doFilter(request, response);
return;
}

Comment on lines +34 to +38
Copy link
Member

Choose a reason for hiding this comment

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

P1
해당 로직은 필요없습니다!
OncePerRequestFilter 소스 코드를 확인하면 이해하기 좋은데요!!
내부적으로 shouldNotFilter 메서드를 통과해야지만 Filter의 로직이 수행되도록 구현되어있어요!!

// org.springframework.web.filter.OncePerRequestFilter 클래스 소스코드 발췌

public abstract class OncePerRequestFilter extends GenericFilterBean {
    ...
    public final void doFilter(ServletRequest request, ServletResponse response, FilterChain filterChain) throws 
ServletException, IOException {
        if (request instanceof HttpServletRequest httpRequest) {
            if (response instanceof HttpServletResponse httpResponse) {
                String alreadyFilteredAttributeName = this.getAlreadyFilteredAttributeName();
                boolean hasAlreadyFilteredAttribute = request.getAttribute(alreadyFilteredAttributeName) != null;
                if (!this.skipDispatch(httpRequest) && !this.shouldNotFilter(httpRequest)) {
                    if (hasAlreadyFilteredAttribute) {
                        if (DispatcherType.ERROR.equals(request.getDispatcherType())) {
                            this.doFilterNestedErrorDispatch(httpRequest, httpResponse, filterChain);
                            return;
                        }

                        filterChain.doFilter(request, response);
                    } else {
                        request.setAttribute(alreadyFilteredAttributeName, Boolean.TRUE);

                        try {
                            // 여기에서 호출되는 것을 알 수 있습니다.
                            this.doFilterInternal(httpRequest, httpResponse, filterChain);
                        } finally {
                            request.removeAttribute(alreadyFilteredAttributeName);
                        }
                    }
                } else {
                    filterChain.doFilter(request, response);
                }

                return;
            }
        }

        throw new ServletException("OncePerRequestFilter only supports HTTP requests");
    }

    ...
    protected boolean shouldNotFilter(HttpServletRequest request) throws ServletException {
        return false;
    }
    protected abstract void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) throws ServletException, IOException;
    ...
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

오호 그렇군요 지금보니 override하는 메서드였네요 어제 문제 확인할때는 해당 내용을 몰랐어서 일단 임의로 수정하긴했는데, 제가 postman으로 요청보냈을 때에는 유빈이가 슬랙에서 요청준 문제는 해결되어서 추가로 요청 들어오면 긴급하게 수정하겠지만 일정상 다음주쯤에 다시 반영 가능할 것 같습니다!

String authorizationToken = getAuthorizationToken(request);
CustomAuthentication authentication = authTokenProvider.parse(authorizationToken);

Expand All @@ -46,8 +51,8 @@ public boolean shouldNotFilter(HttpServletRequest request) {
}

private boolean isWhiteRequest(final HttpServletRequest request) {
String url = request.getRequestURL().toString();
return WHITE_PATHS.stream().anyMatch(url::contains);
String uri = request.getRequestURI();
return WHITELIST_WILDCARD.stream().anyMatch(uri::startsWith);
}

/**
Expand Down
Loading