-
Notifications
You must be signed in to change notification settings - Fork 0
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
[YS-170] refact: 실험 공고 상세 정보 조회 응답에 isAuthor
추가 및 JwtOptionalAuthenticationFilter 추가
#41
Conversation
isAuthor
추가isAuthor
추가
class JwtOptionalAuthenticationFilter( | ||
private val jwtTokenProvider: JwtTokenProvider, | ||
private val handlerExceptionResolver: HandlerExceptionResolver, | ||
) : OncePerRequestFilter() { | ||
override fun doFilterInternal( | ||
request: HttpServletRequest, | ||
response: HttpServletResponse, | ||
filterChain: FilterChain, | ||
) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
공고 상세 정보 조회 API는 비회원도 조회할 수 있어야 합니다.
하지만 회원이 조회하는 경우 isAuthor
값을 위해 JWT에서 memberId를 추출해야 합니다.
현재 JwtAuthenticationFilter
는 비회원 요청에 대해서 JWT를 처리하지 않기 때문에, 비회원이 조회할 경우 JwtOptionalAuthenticationFilter
를 사용하여 JWT가 있을 경우에만 필터를 적용하도록 추가했습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
즉 JwtOptionalAuthenticationFilter
은 요청에 JWT가 있을 경우에만 토큰을 검증하는 필터이군요! JWT 요청이 없는 요청은 검증을 생략하고 통과되구요.
해당 필터를 적용하면, 비회원 요청은 JWT 검증 없이 처리 되고 회원 요청은 JWT를 검증하고 memberId
를 추출해서 사용할 수 있는 것으로 알겠습니다.
@Bean | ||
@Order(2) | ||
fun publicApiSecurityFilterChain( | ||
httpSecurity: HttpSecurity, | ||
jwtTokenProvider: JwtTokenProvider, | ||
handlerExceptionResolver: HandlerExceptionResolver | ||
): SecurityFilterChain = httpSecurity | ||
.securityMatcher("/v1/experiment-posts/{postId}/details") | ||
.csrf { it.disable() } | ||
.cors(Customizer.withDefaults()) | ||
.sessionManagement { | ||
it.sessionCreationPolicy(SessionCreationPolicy.STATELESS) | ||
} | ||
.authorizeHttpRequests { | ||
it.requestMatchers("/v1/experiment-posts/{postId}/details").permitAll() | ||
} | ||
.addFilterBefore( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
인증이 필요하지 않은 모든 API에 필터를 적용하는 것은 비효율적이라 판단하여, authSecurityFilterChain
과 publicApiSecurityFilterChain
으로 분리하였습니다.
공고 상세 정보 조회와 같이 회원과 비회원 모두 접근 가능하지만, 회원인 경우에만 JWT 필터를 적용해야 하는 경우에는 해당 FilterChain에 등록하면 됩니다~
제가 각 필터 체인에 대해 위에 주석을 달아놨는데 혹시 설명이 더 필요하다면 코멘트 달아주세요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
모든 API에 JWT 필터를 적용하면 성능적으로 비효율적이겠네요.
authSecurityFilterChain
을 통해 인증이 필수적인 API를 처리하고,
publicApiSecurityFilterChain
은 회원/비회원 모두 접근이 가능한 API를 처리하되, 회원일 경우 memberId
를 추출해내는 것이 맞다면 👍 버튼 부탁드립니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 필터는 로그인한 회원의 경우, 요청 헤더에 포함된 JWT를 파싱하여 인증 객체를 생성하고 SecurityContext
에 저장하는 역할을 합니다!
더 구체적으로 말하자면, 기존에는 특정 URI에 대해 JWT 필터를 거치지 않고 permitAll 설정을 사용했기 때문에, 요청 헤더에서 JWT 토큰을 추출하는 과정이 생략되었습니다. 이로 인해 AuthenticationUtils
를 통해 현재 로그인한 회원의 memberId
를 가져올 수 없는 문제가 발생했습니다.
이를 해결하기 위해, 요청에 JWT가 포함되어 있다면 이를 검증하고 인증 정보를 설정하도록 처리하고 JWT가 없는 경우에도 예외를 발생시키지 않고 요청 처리를 계속 진행하도록 JwtOptionalAuthenticationFilter
를 추가했습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
결국에는 기존 JWT 필터에 isAuthenticated()
에 걸리는 URI들은 인증 과정이 요청 헤더에 토큰을 추출하지 않는다는 설계방식이었군요.
그런데 현재 요구사항에서는 isAuthor
라는 해당 글의 작성자인지 아닌지 여부를 판단해야하는 응답값 필드가 비회원 유저가 접근 가능한 API 엔드포인트에서 반환해야 하기 때문에, 해당 필터를 만드신 걸로 판단했습니다 👍👍👏
isAuthor
추가isAuthor
추가 및 JwtOptionalAuthenticationFilter 추가
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
지수님 빠르고 깔끔한 작업 감사합니다!
코멘트 남겨서 제가 이해한 바가 맞는지 & 그리고 AuthenticationUtils
의 메서드 관련 부분 확인 부탁드립니다. 😊😊
@Bean | ||
@Order(2) | ||
fun publicApiSecurityFilterChain( | ||
httpSecurity: HttpSecurity, | ||
jwtTokenProvider: JwtTokenProvider, | ||
handlerExceptionResolver: HandlerExceptionResolver | ||
): SecurityFilterChain = httpSecurity | ||
.securityMatcher("/v1/experiment-posts/{postId}/details") | ||
.csrf { it.disable() } | ||
.cors(Customizer.withDefaults()) | ||
.sessionManagement { | ||
it.sessionCreationPolicy(SessionCreationPolicy.STATELESS) | ||
} | ||
.authorizeHttpRequests { | ||
it.requestMatchers("/v1/experiment-posts/{postId}/details").permitAll() | ||
} | ||
.addFilterBefore( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
모든 API에 JWT 필터를 적용하면 성능적으로 비효율적이겠네요.
authSecurityFilterChain
을 통해 인증이 필수적인 API를 처리하고,
publicApiSecurityFilterChain
은 회원/비회원 모두 접근이 가능한 API를 처리하되, 회원일 경우 memberId
를 추출해내는 것이 맞다면 👍 버튼 부탁드립니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 공고를 쓴 작성자인지 확인하는 isAuthor
필드값을 추가함으로써, 더 직관적으로 프론트와 소통할 수 있을 것 같네요. 👍👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getCurrentMemberIdOrNull
에서 authentication?.name
이 null인 경우 if-else문으로 분기해서 코드를 짜셨는데, 코틀린의 Null-Safety 기능을 이용하면 더 간결하게 처리할 수 있을 것 같습니다!
fun getCurrentMemberIdOrNull(): Long? {
return SecurityContextHolder.getContext().authentication?.name?.takeIf { it != "anonymousUser" }?.toLong()
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
생각해보니까 이렇게 코드를 짜면 결국 getCurrentMemberId
메서드와 getCurrentMemberOrNull
메서드는 SecurityContextHolder.getContext()
를 가져온다는 부분에서 로직이 똑같겠군요 🤔
fun getCurrentMemberId(): Long {
return getCurrentMemberIdOrNull()
?: throw //예외던지기
}
다만, Utils 패키지에서 해당 예외를 던지는 설계가 클린 아키텍처에 부합하는지에 대해서는 고민을 좀 해야할 것 같아요.
제 생각에는 Utils 패키지는 단순히 데이터를 반환하거나 null-safe한 전역적 유틸리티 역할만 수행해야 할 것 같아서 해당 패키지에서 예외를 던지는 것은 가급적 지양해야할 것 같네요..
지수님은 어떻게 생각하시나요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
첫 번째 제안 방식이 깔끔해서 채택했습니다. 감사합니다.
다만, 두 번째 제안에서 예외를 던지시는데 해당 함수는 예외를 반환하지 않고 null을 반환하도록 설계되었습니다. 이는 실험 공고 상세 정보 조회 유즈케이스와 같은 시나리오에서 비회원일 경우 isAuthor
처리가 정상적으로 이루어지도록 하기 위함입니다! 따라서 인증되지 않은 사용자의 경우 memberId는 null로 반환되어야 합니다.
그리고 예외 처리 관련해서 의견을 물어보셨는데, 상황에 따라 다를 수는 있지만 저는 utils 패키지는 범용적이고 도메인과 독립적인 작업에만 활용하는 것이 적합하다고 생각합니다!
따라서 특정 도메인 규칙에 의존하는 예외 처리가 필요한 경우, utils의 역할을 명확히 하고 도메인 로직의 책임 분리를 유지하기 위해서라도 utils에서 예외를 던지는 것은 지양하는 것이 좋다고 봐요. 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
좋은 의견 감사합니다
그렇게 된다면 getCurrentMemberId
와 getCurrentMemberIdOrNull
는 공통로직으로 추출하기가 힘들 것 같네요.
SRP 측면에서도 Utils 패키지에 예외처리는 지양하는 걸로 해야겠습니다!
…getCurrentMemberIdOrNull
Quality Gate passedIssues Measures |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
피드백 사항 잘 반영해주셔서 감사합니다 👍
…ticationFilter 추가 (#41) * feat: add JwtOptionalAuthenticationFilter * feat: add getCurrentMemberIdOrNull() * refact: add isAuthor to ExperimentPostDetailResponse * test: refactor GetExperimentPostDetailUseCaseTest test code * refact: use Kotlin Null-Safety to return null for anonymous users in getCurrentMemberIdOrNull
💡 작업 내용
isAuthor
추가✅ 셀프 체크리스트
🙋🏻 확인해주세요
인증이 필요한 API / 인증이 필요하지 않은 API / 인증은 필요없지만 회원인 경우 memberId를 확인해야 하는 API
에 대해 로컬에서 API 테스트를 진행했습니다. 제가 놓친 부분이 있다면 알려주세요~🔗 Jira 티켓
https://yappsocks.atlassian.net/browse/YS-170