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

DRAW-392 401 에러 발생 처리 하기 #6

Open
wants to merge 4 commits into
base: feature/DRAW-347
Choose a base branch
from

Conversation

comforest
Copy link
Contributor

Related Jira ✔

Description ✔

PR Rule ✔

P1: 꼭 반영해주세요 (Request changes)
P2: 적극적으로 고려해주세요 (Request changes)
P3: 웬만하면 반영해 주세요 (Comment)
P4: 반영해도 좋고 넘어가도 좋습니다 (Approve)
P5: 그냥 사소한 의견입니다 (Approve)

@comforest comforest self-assigned this Oct 12, 2024
@SunwoongH SunwoongH changed the title DRAW-392 fix: 401 에러 발생 처리 하기 DRAW-392 401 에러 발생 처리 하기 Oct 14, 2024
@@ -8,7 +8,7 @@ dependencies {
implementation(project(":core"))

implementation("org.springframework.boot:spring-boot-starter-web:${Versions.SPRING_BOOT}")
implementation("org.springframework.boot:spring-boot-starter-security:${Versions.SPRING_BOOT}")
api("org.springframework.boot:spring-boot-starter-security:${Versions.SPRING_BOOT}")
Copy link
Member

Choose a reason for hiding this comment

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

P5
@PreAuthorize의 isAuthenticated() 메서드를 사용하는 과정에서 시큐리티 참조가 필요해서 api로 변경하신 걸까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

넹 맞습니다. 더불어서 ExceptionHandler에서 Security 에러 처리도 해야해서 변경 했습니다.

@@ -17,6 +18,7 @@ class RoomController(

@Operation(summary = "현재 참여 중인 방 정보")
@GetMapping("/api/v1/playing-room")
@PreAuthorize("isAuthenticated()")
Copy link
Member

Choose a reason for hiding this comment

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

P3
app:support:auth 패키지 하위 클래스 중
@PreAuthorize("isAuthenticated()")
annotation class NeedLogin 이 있습니다. 해당 어노테이션 클래스를 사용하면 시큐리티 의존 관계를 끊을 수 있을 것 같아서 의견 드립니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

보편적으로 사용하는 클래스들은 가급적 커스텀 하지 않고 사용하는 걸 선호하는데
(Spring 개발자가 볼 때 편하기때문에)

의존성을 줄이기 위한 방편으로 좋은 것 같습니다.

Exception Handler와 함께 의존선 줄이는 방향으로 처리했습니다.

@@ -32,7 +32,7 @@ internal class TokenAuthenticationFilter(

private fun getAccessToken(request: HttpServletRequest): String? {
val accessToken = request.getHeader(HEADER_AUTHORIZATION)
if (accessToken.isNullOrBlank().not() && accessToken.startsWith(HEADER_BEARER)) {
if (accessToken.isNullOrBlank().not() && accessToken.lowercase().startsWith(HEADER_BEARER)) {
Copy link
Member

Choose a reason for hiding this comment

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

P5
lowercase()를 사용하신 이유가 궁금합니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

bearer 가 대소문자 구분이 애매해서 처리해뒀습니다
https://datatracker.ietf.org/doc/html/rfc6750#section-6.1.1
image

Comment on lines 30 to 33
@ExceptionHandler(AccessDeniedException::class)
protected fun handleException(ex: AccessDeniedException): ExceptionResponseEntity {
return handleException(UnAuthorizedException)
}
Copy link
Member

Choose a reason for hiding this comment

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

P5
api 패키지 내에서 401은 언제 발생하는지 알 수 있을까요? 필터 단에서 핸들링한 후 AuthenticationException으로 통일해서 내려주도록 설정되어 있는 것으로 알고 있습니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@PreAuthorize
이게 필터 이후 Exception 이 발생해서 처리가 필요합니다

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants