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

[BE/FEAT] 스웨거 비밀번호 설정 #610

Merged
merged 4 commits into from
Jan 4, 2025
Merged

[BE/FEAT] 스웨거 비밀번호 설정 #610

merged 4 commits into from
Jan 4, 2025

Conversation

LJH098
Copy link
Collaborator

@LJH098 LJH098 commented Dec 19, 2024

📌관련 이슈
close #603
🔑 주요 변경사항

  1. prod 환경에서 swagger 사용하지 못하게 막기
  • JwtAuthenticationFilter에서 profile이 prod 이고 request가 Swagger 관련이면 SwaggerCannotProdException 을 throw 한다.
  1. swagger 요청을 web ignore 하지않고 permitAll()
  • swagger 요청이 JwtAuthenticationFilter를 거쳐야 함으로 ignore 하지않고 permitAll 해줌

리뷰어에게

  1. https://devnm.tistory.com/25 를 참고하여 스웨거 비밀번호 설정을 하려고 했으나 JwtAuthenticationPoint와 충돌이 있는 것 같고 swagger-ui/index.html은 200으로 정상적으로 응답을 하나 다른 swagger 요청(css, api-docs 등) 들이 요청되지 않은 현상이 발생함 -> 빈화면만 뜸
    자꾸 이런 현상이 떠서 그냥 prod 환경에서는 swagger를 사용하지 못하는 방식으로 변경

  2. JwtAutehnticationPoint에서 기존에는 request.getAttribute("exception") 이렇게 되있었는데 SwaggerCannotProdException 을 catch 하지못하고 있길래 디버깅 해보니 request의 attribute에 key가 exception으로 되어있는게 아닌 javax.servlet.error.exception 로 되어있어서 이걸로 수정하니 catch 하여 정상적으로 동작함

  • 이전에 exception으로 했을 때 정상 동작을 했나요? 아니면 항상 if (request.getAttribute("exception") == null) 이 부분에 걸려서 체크하지 못한걸까요??

@LJH098 LJH098 requested a review from hwangdaesun December 19, 2024 12:05
@LJH098 LJH098 self-assigned this Dec 19, 2024
Copy link

Overall Project 65.95%

There is no coverage information present for the Files changed

Comment on lines 39 to 46
Object exceptionAttribute = request.getAttribute("javax.servlet.error.exception");
if (exceptionAttribute == null) {
handleAuthenticationException(response);
} else if (exceptionAttribute instanceof Exception exception) {
resolver.resolveException(request, response, null, exception);
} else {
resolver.resolveException(
request, response, null, (Exception) request.getAttribute("exception"));
handleAuthenticationException(response);
}
Copy link
Collaborator

@hwangdaesun hwangdaesun Dec 22, 2024

Choose a reason for hiding this comment

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

기존에는 JwtAuthenticationFilter에서 doFilterInternal메서드 안에 validateAccessToken 메서드 보면, 토큰의 서명이 유효하지 않은 경우, 토큰이 만료된 경우, 지원되지 않는 토큰 경우 이렇게 케이스를 나눠서 각 예외를 던지고, 이 예외를 catch 해서 request.setAttribute("exception", e); 을 한 다음에 JwtAuthenticationPoint 클래스에서 getAttribute가 exception일 경우에 GlobalExceptionHanlder에서 특정 예외를 catch 해서 각 상황에 맞는 예외 응답을 해주고 있습니다.

이렇게 할 경우에 토큰이 만료됬을 경우에도 handleAuthenticationException() 이 실행되서 AOS와 약속한 응답 코드가 나가지 않을 듯 하네요

Copy link
Collaborator

Choose a reason for hiding this comment

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

이거 수정하기 전의 if else 문으로 바꿔서 할 경우 어떤 부분이 안 된 건가요?? 방법 함 찾아볼게요

Copy link
Collaborator Author

@LJH098 LJH098 Dec 26, 2024

Choose a reason for hiding this comment

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

PR에 작성헸듯이

JwtAutehnticationPoint에서 기존에는 request.getAttribute("exception") 이렇게 되있었는데 SwaggerCannotProdException 을 catch 하지못하고 있길래 디버깅 해보니 request의 attribute에 key가 exception으로 되어있는게 아닌 javax.servlet.error.exception 로 되어있어서 이걸로 수정하니 catch 하여 정상적으로 동작함

SwaggerCannotProdException�를 throw 했을 때 if (request.getAttribute("exception") == null) 계속 이부분으로 걸려서 "인증이 필요합니다." 라는 응답을 보내게 됩니다.

즉, 제가 원한건 SwaggerCannotProdException�을 throw 했을 때 "운영환경에서는 Swagger를 볼 수 없습니다." 라는 응답을 보내는건데 계속 "인증이 필요합니다"라는 응답을 보내게됩니다.

Copy link
Collaborator

@hwangdaesun hwangdaesun Dec 27, 2024

Choose a reason for hiding this comment

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

그러면, 기존에 JwtAuthenticationFilter에서 request.setAttribute("exception", e) 한 것처럼 prod 환경일 시에도 이와 동일하게 request.setAttribute("prod환경에서는 swagger 안 됨(예시)", e); 해서 JwtAuthenticationPoint 에서 해당 예외 처리하는 거는 어떨까요?

지금 exception이라는 단어가 상황을 구체적으로 설명하고 있지 않아서 다른 단어로 바꾸는 것도 좋을 듯 하네요.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

단순히 throw SwaagerCannotProdExeption.EXCEPTION 하지말고 직접 request.setAttribute 하라는 걸까요? 대선님이 말한게 정확히 어떤 의도인지 파악하지 못했어요

Copy link
Collaborator

@hwangdaesun hwangdaesun Dec 28, 2024

Choose a reason for hiding this comment

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

기존 코드에서는 토큰 관련 예외가 발생시 JwtAuthenticationFilter에서 request.setAttribute("exception", e)을 하여 JwtAuthenticationPoint의 else 문이 실행되어 커스텀한 응답이 나오는 상황

try {
        if (jwtManager.validateAccessToken(accessToken, request)) {
            ...
        } catch (Exception e) {
            request.setAttribute("exception", e);
        }
}
        
  if (request.getAttribute("exception") == null) {
      handleAuthenticationException(response);
  } else {
      resolver.resolveException(
              request, response, null, (Exception) request.getAttribute("exception"));
  }

토큰과 관련한 예외 처리가 안 되는 문제

현재 PR에서는 prod 환경일 때 swagger에 접속하면 JwtAuthenticationFilter에서 throw SwaagerCannotProdExeption -> JwtAuthenticationPoint에서 해당 예외를 잡아서 처리하고 싶어서 아래와 같이 변경하신 것 같습니다.

Object exceptionAttribute = request.getAttribute("javax.servlet.error.exception");
        if (exceptionAttribute == null) {
            handleAuthenticationException(response);
        } else if (exceptionAttribute instanceof Exception exception) {
            resolver.resolveException(request, response, null, exception);
        } else {
            handleAuthenticationException(response);
        }

토큰 만료 등과 같은 상황에서 else if에 걸려 resolver.resolveException(request, response, null, exception) 가 실행되어야 GlobalExceptionHandler에서 예외에 따라 커스텀하게 응답이 가능한데, 지금 코드에서는 else if에 걸리지 않는 문제 발생합니다.

제안

따라서, SwaggerCannotProdException이 발생할 때도 request.setAttribute("swaggerProdException") 를 사용하여 올바르게 if else문이 동작하도록 수정하는 것은 어떨까요??

if (Boolean.TRUE.equals(springEnvironmentHelper.isProdProfile()) && isSwaggerRequest(request)) {
            request.setAttribute("swaggerProdException");
            throw SwaggerCannotProdException.EXCEPTION;
        }

Comment on lines +47 to +48
if (Boolean.TRUE.equals(springEnvironmentHelper.isProdProfile())
&& isSwaggerRequest(request)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

springEnvironmentHelper.isProdProfile() && isSwaggerRequest(request) 이렇게 하지 않은 이유가 있을까요??

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

sonar lint에 이렇게 Boolean.TRUE를 사용하라고 나와있더라구요
https://next.sonarqube.com/sonarqube/coding_rules?open=java%3AS5411&rule_key=java%3AS5411

Copy link

github-actions bot commented Jan 2, 2025

Overall Project 65.75%

There is no coverage information present for the Files changed

Copy link
Collaborator

@hwangdaesun hwangdaesun left a comment

Choose a reason for hiding this comment

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

굿굿!

@LJH098 LJH098 merged commit fdc5bff into develop Jan 4, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants