[신성수] 인가(Authorization) 리뷰 요청 드립니다. #28
Conversation
| final AuthorizationManager<HttpServletRequest> authorizationManager = new RequestAuthorizationManager(List.of( | ||
| createMvcMatcher(GET, "/members", new AuthorityAuthorizationManager<>("ADMIN")), | ||
| createMvcMatcher(GET, "/members/me", new AuthenticatedAuthorizationManager<>()), | ||
| createMvcMatcher(GET, "/search", new PermitAllAuthorizationManager<>()) | ||
| ), createDefaultMatcher(new DenyAllAuthorizationManager<>())); |
| @@ -0,0 +1,5 @@ | |||
| package nextstep.security.authorization.manager; | |||
|
|
|||
| public interface AuthorizationResult { | |||
There was a problem hiding this comment.
미션내의 요구사항은 AuthorizationDecision이었으나 최근 버전에서의 추상화를 잘 구현해주셨네요
| @@ -0,0 +1,20 @@ | |||
| package nextstep.security.authorization.manager; | |||
|
|
|||
| public enum AuthorizationDecision implements AuthorizationResult { | |||
There was a problem hiding this comment.
true와 false로 구성되어있어 enum으로 생각해볼 수도 있으나 실제 구현체에서는 AuthorizationDecision을 상속받아 구현하는 새로운 구현체들도 있어 enum으로 사용되지는 않습니다.
일반적으로 enum을 구성하는 것은 몇가지 고정값이 있기 때문에 enum으로 표현하지는 않고 정말 static한 정보들로 표기될때에만 사용되어요. 예를들면 HttpStatus는 400, 401과 같은 코드가 있기 때문에 각각을 표기하는 것처럼요.
그래서 사실은 AuthorizationDecision은 granted가 있다 없다로 생성되는 객체일뿐 enum에 적합하진 않습니다.
| - [x] SecuredAuthorizationManager 구현 | ||
| - [x] RequestAuthorizationManager 구현 | ||
| - [x] SecuredAuthorizationManager 구현 |
There was a problem hiding this comment.
| - [x] SecuredAuthorizationManager 구현 | |
| - [x] RequestAuthorizationManager 구현 | |
| - [x] SecuredAuthorizationManager 구현 | |
| - [x] SecuredAuthorizationManager 구현 | |
| - [x] RequestAuthorizationManager 구현 |
중복되어있네요 :)
| return authentication != null | ||
| && authentication.isAuthenticated() | ||
| && authentication.getAuthorities() | ||
| .contains(method.getAnnotation(Secured.class).value()); |
There was a problem hiding this comment.
SecuredAuthorizationManager의 구현 내용과 AuthorityAuthorizationManager의 구현 내용을 생각해보면 서로 비슷한 동작이 반복되는 것을 볼 수 있는데요.
AuthorizationManager들의 구현체들은 각각 따로 구성되는 것이 아닌 서로를 사용할 수 있도록 연결할 수 있어요. 그래서 AuthorityAuthorizationManager에서 authority가 있는지를 체크하는 로직을 현재와 같이 전부 몰아넣고, SecuredAuthorizationManager는 @Secured에 있는 값을 꺼내 AuthorityAuthorizationManager에게 인가여부를 묻는 형태로 구성이 될 수 있습니다.
늦게 제출해서 죄송합니다. 😢
주말만 시간이 허용되어서 그래도 열심히 PR올리도록 하겠습니다.
요구사항 정리
🚀 1단계 - AuthorizationManager를 활용
🚀 2단계 - 요청별 권한 검증 정보 분리