-
Notifications
You must be signed in to change notification settings - Fork 0
[FIX] 인증/인가 개선 및 리팩토링 #321
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
Conversation
userId를 Long -> String으로 casting하여 타입이 일관적이지 않았음
from UserAuthentication to CustomAuthenticationToken
HashMap인 validatorMap을 통해 ResourceAuthorizationValidator의 구현체를 저장하고 사용
…자동 매핑 기능 추가 @Autowired를 통해 List<ResourceAuthorizationValidator>를 주입, for loop를 통해 validatorMap에 validator의 resourceType을 key로 저장
- 존재하지 않는 resourceType인 경우 null에 대해 exception throw - Validator에서 permission 체크하도록
Optional과 orElseThrow를 통해 null 처리 간결하게 변경
권한 X = 본인의 리소스가 아닌 경우
from hasPermission to authorizeResourceAccess
as-is - userId와 feed로 바로 삭제 - deleteByUserIdAndFeed()로 삭제 to-be - 좋아요 한 적 있는지 확인하고 삭제 - findByUserIdAndFeed()로 조회 후, delete로 삭제
src/main/java/org/websoso/WSSServer/auth/validator/FeedAccessValidator.java
Show resolved
Hide resolved
src/main/java/org/websoso/WSSServer/auth/validator/FeedAccessValidator.java
Outdated
Show resolved
Hide resolved
src/main/java/org/websoso/WSSServer/auth/validator/FeedAuthorizationValidator.java
Outdated
Show resolved
Hide resolved
sansan20535
left a comment
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.
리뷰 남기면서 많이 배웠습니다! 감사합니다 : )
src/main/java/org/websoso/WSSServer/auth/validator/NovelAuthorizationValidator.java
Show resolved
Hide resolved
src/main/java/org/websoso/WSSServer/auth/ResourceAuthorizationHandler.java
Outdated
Show resolved
Hide resolved
src/main/java/org/websoso/WSSServer/auth/validator/NotificationAuthorizationValidator.java
Show resolved
Hide resolved
src/main/java/org/websoso/WSSServer/auth/validator/UserNovelAuthorizationValidator.java
Show resolved
Hide resolved
src/main/java/org/websoso/WSSServer/controller/NotificationController.java
Show resolved
Hide resolved
rinarina0429
left a comment
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.
우기온앤온 이렇게나 큰 pr을 . . .
수고하셨습니다 이전 Controller보다 훨씬 깔끔하네요! 더 나은 방식이 있을지는 같이 더 찾아봐요 ~
커멘트 한번씩만 확인 부탁드립니다아
src/main/java/org/websoso/WSSServer/auth/validator/NotificationAuthorizationValidator.java
Show resolved
Hide resolved
src/main/java/org/websoso/WSSServer/auth/validator/UserNovelAuthorizationValidator.java
Show resolved
Hide resolved
src/main/java/org/websoso/WSSServer/controller/NotificationController.java
Show resolved
Hide resolved
GiJungPark
left a comment
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.
빠른 피드백 반영 확인했습니다. :)
Related Issue
Key Changes
인증 사용자 조회 및 검증
AS-IS)
Controller 단에서 Principal.getName()을 통해 userId 문자열을 얻고 Long 타입으로 캐스팅,
추출한 userId로 userService.getUserOrException 호출하는 로직이 Controller에서 중복 발생
TO-BE)
Spring Security에서 제공하는 @AuthenticationPrincipal 사용,
CustomArgumentResolver를 통해 인증 사용자 정보 주입,
Resolver에서 사용자 검증이라는 관심사를 전담 및 일관된 처리
리소스 존재 여부 검증 및 리소스 접근 제어
AS-IS)
요청에 해당하는 리소스 존재 여부 검증을 위해 findById().orElseThrow()를 Service 단에서 반복 호출,
숨김 혹은 차단 리소스 접근 가능 여부 검증을 위해 반복되는 로직 발생
TO-BE)
AuthorizationValidator나 AccessValidator에서 수행하여 관심사 분리,
Validator.canAcces로 일관된 처리
리소스 조작 가능 여부 검증
AS-IS)
Service 단에서 리소스 조작 가능 여부를 직접 확인
TO-BE)
@PreAuthorize + AuthorizationService.validate로 위임
AuthorizationService.validate -> ResourceAuthorizationHandler -> XXXAuthorizationValidator
전체 인증 인가 플로우
To Reviewers
볼륨이 좀 크네요..! 죄송합니다 ㅎㅎ,,
References