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.

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

Conversation

sung-silver
Copy link
Contributor

@sung-silver sung-silver commented Jan 17, 2025

Related Issue 🚀

Work Description ✏️

  • 올바른 경로로 요청을 보내도 500 에러가 반환되는 문제
    • 확인해보니 jwtFilter에서 whitelist면 필더를 넘기는 구문이 없었습니다
    • 또한 리스트에 포함된 경로들이 제대로 설정되지 않는 문제가 있었어요
  • 이에 따라 whitelist일 경우 jwt 검증 없이 다음 필터로 넘기는 구문을 추가하고, constant에 선언된 public list를 수정해주었습니다
  • 또한 다음과 같이 규격화되지 않는 에러가 반환되고 있어 ExceptionHandler에 500 예외에 대한 처리를 추가했습니다
    • 제가 알기로 앱에서는 웹과 다르게 응답 값의 형태가 변경되면 핸들링이 어려운 것으로 알고 있습니다!
      image
  • 더하여 AuthException만 처리되고 있어 BaseException이 발생할 경우 모두 규격화된 에러가 반환되도록 수정했습니다

PR Point 📸

@sung-silver sung-silver added the 🔨 fix 버그를 발견하여 코드를 수정한 경우(spring boot 내의 기능 코드) label Jan 17, 2025
@sung-silver sung-silver self-assigned this Jan 17, 2025
Copy link

height bot commented Jan 17, 2025

Link Height tasks by mentioning a task ID in the pull request title or commit messages, or description and comments with the keyword link (e.g. "Link T-123").

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

@sung-silver sung-silver changed the title Fix/#41 [FIX/#41] JWT 필터 내 whilelist 처리 오류 해결 Jan 17, 2025
@sung-silver
Copy link
Contributor Author

sung-silver commented Jan 17, 2025

@hyunw9 @yummygyudon hotfix라 머지하지만 리뷰는 꼭 남겨주세요!!

@sung-silver sung-silver merged commit e6974ae into dev Jan 17, 2025
1 check passed
@sung-silver sung-silver deleted the fix/#41 branch January 17, 2025 18:05
Copy link
Member

@yummygyudon yummygyudon left a comment

Choose a reason for hiding this comment

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

몇 가지 중요한 내용 리뷰 남겨보았어요..!!
긴급했던 만큼 리뷰를 진행한 후에 반영이 되지 못했지만 아마 예상하기로는 완전히 문제가 해결되지는 않았을 것 같다는 생각입니다...!!

Comment on lines +34 to +38
if (shouldNotFilter(request)) {
filterChain.doFilter(request, response);
return;
}

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으로 요청보냈을 때에는 유빈이가 슬랙에서 요청준 문제는 해결되어서 추가로 요청 들어오면 긴급하게 수정하겠지만 일정상 다음주쯤에 다시 반영 가능할 것 같습니다!

Comment on lines +17 to +22
@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));
}
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.

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

Copy link
Contributor

@hyunw9 hyunw9 left a comment

Choose a reason for hiding this comment

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

중요한 내용은 이미 동규형이 말씀을 해주셨네요 !

ShouldNotFilter는 오버라이드 해서 기존 로직 그대로 첨부하면 될 것 같습니다.

고생하셨습니다 !

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 org.springframework.http.*;

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

요기도욥

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 fix 버그를 발견하여 코드를 수정한 경우(spring boot 내의 기능 코드) size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FIX] JWT 필터 내 whilelist 처리 오류 해결
3 participants