-
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
✨ API 인증 필터 구현 #30
✨ API 인증 필터 구현 #30
Conversation
Walkthrough이 변경 사항은 "3days API"의 OpenAPI 문서화를 설정하기 위한 새로운 구성 클래스를 도입하는 Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (6)
support/common/src/main/kotlin/com/threedays/support/common/security/SecurityContext.kt (1)
1-5
: LGTM! 클래스 구현이 잘 되었습니다.클래스 구현이 간단하고 명확합니다. 제네릭을 사용하여 다양한 인증 구현을 유연하게 처리할 수 있도록 한 점이 좋습니다.
클래스에 KDoc 주석을 추가하여 문서화를 개선하는 것이 좋겠습니다. 예를 들어:
/** * 인증 정보를 캡슐화하는 보안 컨텍스트 클래스입니다. * * @param T 인증 타입. [Authentication]의 하위 타입이어야 합니다. * @property authentication 현재 인증 정보 */ class SecurityContext<T: Authentication>( val authentication: T, )domain/src/main/kotlin/com/threedays/domain/auth/vo/UserAuthentication.kt (2)
1-8
: 클래스 구조와 목적이 명확합니다.
UserAuthentication
클래스는 사용자 인증 정보를 나타내는 간단하고 명확한 구조를 가지고 있습니다.Authentication
인터페이스를 구현하여 시스템의 인증 메커니즘과 잘 통합될 것으로 보입니다.이 클래스가 어떻게 사용되는지, 그리고
Authentication
인터페이스의 다른 구현체가 있는지 고려해보시기 바랍니다. 필요하다면 이 클래스의 사용 예시나 문서화를 추가하는 것이 도움이 될 수 있습니다.
6-8
: data class 사용이 적절합니다.
UserAuthentication
을 data class로 정의한 것은 좋은 선택입니다. 이는equals()
,hashCode()
,toString()
메서드를 자동으로 생성하여 객체 비교와 디버깅을 용이하게 합니다.
Authentication
인터페이스에 정의된 메서드가 있다면, 이를 구현하는 것을 고려해보세요. 현재 코드에서는 인터페이스의 요구사항을 모두 충족하고 있는지 확인하기 어렵습니다.support/common/src/main/kotlin/com/threedays/support/common/security/SecurityContextHolder.kt (1)
7-10
: getContext() 메서드 구현이 적절합니다.제네릭 메서드를 사용하여 타입 안전한 컨텍스트 검색을 가능하게 했습니다. 널 가능 타입을 반환하는 것도 적절합니다. 형식 소거로 인해 불가피한 비검사 캐스트와 경고 억제도 이 경우에는 허용됩니다.
메서드에 KDoc 주석을 추가하여 반환값이 null일 수 있는 상황을 설명하면 좋을 것 같습니다.
bootstrap/api/src/main/kotlin/com/threedays/bootstrap/api/support/config/OpenApiConfig.kt (1)
10-19
: 클래스 선언과 어노테이션이 적절합니다.
OpenApiConfig
클래스의 선언과 어노테이션이 올바르게 구성되어 있습니다. Spring 구성 클래스로 적절히 설정되어 있으며, OpenAPI 정의와 보안 스키마가 잘 정의되어 있습니다.JWT 베어러 토큰을 사용하는 보안 구성은 일반적이고 안전한 접근 방식입니다.
API에 대한 추가 정보를 제공하는 것을 고려해 보시기 바랍니다. 예를 들어,
@OpenAPIDefinition
의info
속성에 버전, 설명, 연락처 정보 등을 추가할 수 있습니다.bootstrap/api/src/main/kotlin/com/threedays/bootstrap/api/support/security/AuthenticationFilter.kt (1)
43-51
: 토큰 추출 로직을 간결하게 리팩토링할 수 있습니다.
extractToken
메서드에서 불필요한 조건문을 줄이고 코드를 간결하게 작성할 수 있습니다.다음과 같이 수정할 수 있습니다:
private fun extractToken(request: HttpServletRequest): String? { return request.getHeader(AUTH_TOKEN_HEADER) ?.takeIf { it.startsWith(BEARER_PREFIX) } ?.substring(BEARER_PREFIX.length) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (8)
- bootstrap/api/src/main/kotlin/com/threedays/bootstrap/api/support/config/OpenApiConfig.kt (1 hunks)
- bootstrap/api/src/main/kotlin/com/threedays/bootstrap/api/support/exception/ControllerAdvice.kt (1 hunks)
- bootstrap/api/src/main/kotlin/com/threedays/bootstrap/api/support/security/AuthenticationFilter.kt (1 hunks)
- bootstrap/api/src/main/kotlin/com/threedays/bootstrap/api/user/UserController.kt (2 hunks)
- domain/src/main/kotlin/com/threedays/domain/auth/vo/UserAuthentication.kt (1 hunks)
- support/common/src/main/kotlin/com/threedays/support/common/security/Authentication.kt (1 hunks)
- support/common/src/main/kotlin/com/threedays/support/common/security/SecurityContext.kt (1 hunks)
- support/common/src/main/kotlin/com/threedays/support/common/security/SecurityContextHolder.kt (1 hunks)
✅ Files skipped from review due to trivial changes (2)
- bootstrap/api/src/main/kotlin/com/threedays/bootstrap/api/support/exception/ControllerAdvice.kt
- support/common/src/main/kotlin/com/threedays/support/common/security/Authentication.kt
🧰 Additional context used
🔇 Additional comments (10)
support/common/src/main/kotlin/com/threedays/support/common/security/SecurityContextHolder.kt (6)
1-2
: 패키지 선언이 적절합니다.패키지 이름이 프로젝트 구조를 잘 반영하고 있으며, 표준 명명 규칙을 따르고 있습니다.
3-3
: SecurityContextHolder 객체 선언이 적절합니다.싱글톤 패턴을 위해
object
를 사용한 것은 보안 컨텍스트 홀더에 적합한 선택입니다. 이름도 그 목적을 명확히 나타내고 있습니다.
5-5
: ThreadLocal 속성이 적절하게 사용되었습니다.
ThreadLocal
을 사용하여 보안 컨텍스트의 스레드 안전성을 보장하고 있습니다. 제네릭 타입에out
을 사용하여 공변성을 허용한 것도 적절합니다. 이는 다양한Authentication
하위 타입을 지원할 수 있게 해줍니다.
12-14
: setContext() 메서드가 올바르게 구현되었습니다.
SecurityContext<out Authentication>
를 매개변수로 사용하여Authentication
의 하위 타입을 허용하는 것이 적절합니다. ThreadLocal의 set 메서드를 사용한 구현도 간단하고 정확합니다.
16-18
: clearContext() 메서드가 적절하게 구현되었습니다.ThreadLocal의 remove 메서드를 호출하여 컨텍스트를 정확하게 제거하고 있습니다. 메서드 이름도 그 목적을 명확히 나타내고 있습니다.
1-20
: 전반적으로 우수한 구현입니다.
SecurityContextHolder
의 구현이 잘 설계되어 있고, 스레드 로컬 보안 컨텍스트 관리에 대한 모범 사례를 따르고 있습니다. 스레드 안전성, 타입 안전성, 그리고 유연성을 모두 고려한 점이 돋보입니다. 약간의 문서화 개선만으로도 더욱 완벽해질 것 같습니다.bootstrap/api/src/main/kotlin/com/threedays/bootstrap/api/support/config/OpenApiConfig.kt (2)
1-7
: 패키지 선언과 임포트가 적절합니다.패키지 선언이 올바르게 되어 있으며, 필요한 모든 임포트가 존재합니다. OpenAPI 구성에 필요한 적절한 클래스들이 임포트되어 있습니다.
20-20
: 클래스 본문이 비어 있는 것이 적절합니다.
OpenApiConfig
클래스의 본문이 비어 있는 것은 이 사용 사례에 적합합니다. 이 클래스는 어노테이션을 통해 구성 정보를 제공하는 역할을 하므로 추가적인 메서드나 필드가 필요하지 않습니다.bootstrap/api/src/main/kotlin/com/threedays/bootstrap/api/user/UserController.kt (2)
5-5
: 새로운 import 문이 적절히 추가되었습니다.새로 추가된
getMyUserInfo()
메서드에 필요한 클래스들이 정확하게 import 되었습니다. 이는 코드의 가독성과 유지보수성을 향상시킵니다.Also applies to: 11-11, 15-16
24-24
: 생성자 매개변수 뒤에 쉼표 추가는 좋은 스타일 개선입니다.생성자 매개변수 목록 끝에 쉼표를 추가한 것은 좋은 변경사항입니다. 이는 향후 매개변수를 추가할 때 더 깔끔한 diff를 만들어 주며, 코드의 일관성을 유지하는 데 도움이 됩니다.
bootstrap/api/src/main/kotlin/com/threedays/bootstrap/api/user/UserController.kt
Outdated
Show resolved
Hide resolved
...rap/api/src/main/kotlin/com/threedays/bootstrap/api/support/security/AuthenticationFilter.kt
Show resolved
Hide resolved
...rap/api/src/main/kotlin/com/threedays/bootstrap/api/support/security/AuthenticationFilter.kt
Outdated
Show resolved
Hide resolved
...rap/api/src/main/kotlin/com/threedays/bootstrap/api/support/security/AuthenticationFilter.kt
Outdated
Show resolved
Hide resolved
bootstrap/api/src/main/kotlin/com/threedays/bootstrap/api/support/config/OpenApiConfig.kt
Show resolved
Hide resolved
bootstrap/api/src/main/kotlin/com/threedays/bootstrap/api/user/UserController.kt
Outdated
Show resolved
Hide resolved
|
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- bootstrap/api/src/main/kotlin/com/threedays/bootstrap/api/support/exception/GlobalExceptionHandlingFilter.kt (1 hunks)
- bootstrap/api/src/main/kotlin/com/threedays/bootstrap/api/support/security/AuthenticationFilter.kt (1 hunks)
- bootstrap/api/src/main/kotlin/com/threedays/bootstrap/api/user/UserController.kt (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- bootstrap/api/src/main/kotlin/com/threedays/bootstrap/api/support/security/AuthenticationFilter.kt
- bootstrap/api/src/main/kotlin/com/threedays/bootstrap/api/user/UserController.kt
논의사항
UserController 보면
이렇게 securityContext를 get하는 부분이 있는데요, 아무래도 다음 인증이 필요한 API를 작성할때 똑같은 코드가 또나올 겁니다.
WEAVE할때는 Interceptor로 파라미터 수정해서 집어넣어줬었는데, 지금은 openapi generator로 인해 생성되는 코드이다 보니까 메서드 파라미터 수정이 불가능합니다.
그래서 제안드리는건 별도로
withUserAuthentication
라는 고차함수를 정의하구요아래처럼 처리하는거 어떻게 생각하시나요?
Summary by CodeRabbit
Summary by CodeRabbit
새로운 기능
OpenApiConfig
클래스 추가.AuthenticationFilter
클래스 추가.getMyUserInfo
메서드가UserController
에 추가되었으나 구현은 미완료.UserAuthentication
,Authentication
,SecurityContext
,SecurityContextHolder
클래스 및 인터페이스 추가.GlobalExceptionHandlingFilter
클래스 추가.버그 수정
ControllerAdvice
클래스의 패키지 구조 변경으로 예외 처리의 목적이 명확해짐.