🙌 2단계 - 인가(Authorization) 리뷰 요청 드립니다. (#10)

* feat: add /members/me API

 - more test for new API

* ref: refactoring CheckAuthenticationFilter to AuthorizationFilter

 - 하나의 필터에서 여러 인가 로직을 처리하도록 변경

* feat: add AuthorizationManager for authorize

  - 인증된 사용자 체크를 위한 AuthorityAuthorizationManager
  - 권한 체크를 위한 SecuredAuthorizationManager 추가

* feat: add request matcher for find AuthorizationManager

 - MvcRequestMatcher for matching by HttpMethod And URI
 - AnyRequestMatcher for matching any request

* feat: add AuthorizationManagers for delegate Authorities

 - 인가를 처리하는 매니져들 생성
 - AuthenticatedAuthorizationManager 단순 인증 확인
 - AuthorityAuthorizationManager 메서드에서 어노테이션을 가져와 권한 확인
 - PermitAllAuthorizationManager 모든 요청 허용
 - DenyAllAuthorizationManager 모든 허용 거부

* ref: change test and Add SecureAnnotation to Get /members API

 - 유저 권한 제어를 Interceptor에게 위임하기 위한 Secured 어노테이션 추가

* feat: Filter And Interceptor delegate authorization control to AuthorizationManager

 - 인터셉터와 필터는 더이상 직접 인가를 해주지 않고, AuthorizationManager 를 사용한다.

* feat: add RequestMatcherDelegatingAuthorizationManager and config changed

 - RequestMatcherDelegatingAuthorizationManager 에서 요청에 맞는 인가를 시도한다.

* docs: 요구 사항 정리

* polishing

* ref: Filter 에서 사용하는 인가와 Interceptor 에서 확인하는 인가 분리

 - 필터는 한곳에서 여러 인가를 처리하지만, MethodInterceptor or Aspect 에서는 하나의 인가를 처리 그래서 굳이 합쳐서 처리할 필요가 없는듯함. 만약 필요하다면 RequestMatcher 형식으로 Annotation 을 파라미터로 받도록 처리?
 - RequestMatcherDelegatingAuthorizationManager 는 HttpServletRequest 를 사용하는 매니저만 등록(Filter 사용하는 애들)

* polishing

* ref:

 - AuthorityAuthorizationManager 필터에서 정해진 값과 Authentication.getAuthorities() 비교
 - SecuredMethodInterceptor 인터셉터에서 Annotation 값과 Authentication.getAuthorities() 비교

* polishing

* polishing

* ref: Change AuthorityAuthorizationManager to Generic

 - String과 Collection String을 지원하도록 변경

* ref: use verify not check method

 - AuthorizationDecision 의 널의 위험성과 사용도 안하는 결과를 리턴하면서 결과에 대한 예외 처리도 일괄성을 가지지 못하기에 verify 를 통해 검증까지 위임

* ref: AuthorizationDecision 을 static 객체를 사용하도록 변경

* ref: mvc match logic not Stream change to ForEach

* ref: SecuredAuthorizationManager 에서 AuthorityAuthorizationManager 에게 인가 작업을 위임하도록 변경

 - SecuredAuthorizationManager 에서는 Annotation 값을 통한 동적 인가
 - RequestMatcherDelegatingAuthorizationManager 에서는 인가 작업을 하지 않음

* test: 인증 실패에서는 AccessDeniedException 인가 실패에는 ForbiddenException 나도록 변경

---------

Co-authored-by: MZC01-PARKJUN5 <[email protected]>

Author: parkjun5

README.md

@@ -1 +1,39 @@
 # spring-security-authorization
+
+## 실습
+
+1. [x] GET /members/me 엔드포인트 구현 및 테스트 작성
+2. [x] 권한 검증 로직을 AuthorizationFilter로 리팩터링
+
+## 🚀 1단계 - AuthorizationManager를 활용
+
+요구사항
+
+- [x] AuthorizationManager 를 활용하여 인가 과정 추상화
+- [x] 인가를 처리해줄 AuthorizationManager 생성
+- [x] RequestMatcherDelegatingAuthorizationManager 를 통한 AuthorizationManager 한번에 관리?
+- [x] 인가 과정을 추상화한 AuthorizationManager 를 작성한다. 이 때 필요한 AuthorizationDecision도 함께 작성한다. (실제 AuthorizationManager에는
+  verify도 있는데 이 부분에 대한 구현은 선택)
+- [x] SecuredMethodInterceptor와 Authorization Filter에서 작성된 인가 로직을 AuthorizationManager로 리팩터링 한다.
+
+## 🚀 2단계 - 요청별 권한 검증 정보 분리
+
+요구사항
+
+- [x] 요청별 권한 검증 정보를 별도의 객체로 분리하여 관리
+- [x] RequestMatcherRegistry와 RequestMatcher를 작성하고, RequestMatcher의 구현체를 작성한다.
+- [x] AnyRequestMatcher: 모든 경우 true를 리턴한다.
+- [x] MvcRequestMatcher: method와 pattern(uri)가 같은지 비교하여 리턴한다.
+- [x] RequestMatcherEntry의 T entry는 아래에 해당되는 각 요청별 인가 로직을 담당하는 AuthorizationManager가 된다.
+- [x] /login은 모든 요청을 받을 수 있도록 PermitAllAuthorizationManager로 처리
+- [x] /members/me는 인증된 사용자만에게만 권한을 부여하기 위해 AuthenticatedAuthorizationManager로 처리
+- [x] /members는 "ADMIN" 사용자만에게만 권한을 부여하기 위해 HasAuthorityAuthorizationManager로 처리
+- [x] 그 외 모든 요청은 권한을 제한하기 위해 DenyAllAuthorizationManager로 처리
+
+아래 객체와 시큐리티 코드 확인
+// SpEL
+// Role Authority
+// Role Hierarchy
+// AuthoritiesAuthorizationManager
+// SecureMethodSecurityConfiguration
+// SecuredAuthorizationManager

src/main/java/nextstep/app/SecurityConfig.java

@@ -1,24 +1,30 @@
 package nextstep.app;
 
+import jakarta.servlet.http.HttpServletRequest;
 import nextstep.app.domain.Member;
 import nextstep.app.domain.MemberRepository;
 import nextstep.security.authentication.AuthenticationException;
 import nextstep.security.authentication.BasicAuthenticationFilter;
 import nextstep.security.authentication.UsernamePasswordAuthenticationFilter;
-import nextstep.security.authorization.CheckAuthenticationFilter;
-import nextstep.security.authorization.SecuredAspect;
+import nextstep.security.authorization.AuthorizationFilter;
 import nextstep.security.authorization.SecuredMethodInterceptor;
+import nextstep.security.authorization.manager.*;
 import nextstep.security.config.DefaultSecurityFilterChain;
 import nextstep.security.config.DelegatingFilterProxy;
 import nextstep.security.config.FilterChainProxy;
 import nextstep.security.config.SecurityFilterChain;
 import nextstep.security.context.SecurityContextHolderFilter;
+import nextstep.security.matcher.AnyRequestMatcher;
+import nextstep.security.matcher.MvcRequestMatcher;
+import nextstep.security.matcher.RequestMatcherEntry;
 import nextstep.security.userdetails.UserDetails;
 import nextstep.security.userdetails.UserDetailsService;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.context.annotation.EnableAspectJAutoProxy;
+import org.springframework.http.HttpMethod;
 
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Set;
 
@@ -44,21 +50,32 @@ public FilterChainProxy filterChainProxy(List<SecurityFilterChain> securityFilte
 
     @Bean
     public SecuredMethodInterceptor securedMethodInterceptor() {
-        return new SecuredMethodInterceptor();
+        AuthorityAuthorizationManager<Set<String>> authorityAuthorizationManager = new AuthorityAuthorizationManager<>();
+        return new SecuredMethodInterceptor(new SecuredAuthorizationManager(authorityAuthorizationManager));
     }
 //    @Bean
 //    public SecuredAspect securedAspect() {
 //        return new SecuredAspect();
 //    }
 
+    @Bean
+    public RequestMatcherDelegatingAuthorizationManager requestMatcherDelegatingAuthorizationManager() {
+        List<RequestMatcherEntry<AuthorizationManager<HttpServletRequest>>> mappings = new ArrayList<>();
+        mappings.add(new RequestMatcherEntry<>(new MvcRequestMatcher(HttpMethod.GET, "/members/me"), new AuthenticatedAuthorizationManager()));
+        mappings.add(new RequestMatcherEntry<>(new MvcRequestMatcher(HttpMethod.GET, "/members"), new AuthorityAuthorizationManager<>()));
+        mappings.add(new RequestMatcherEntry<>(new MvcRequestMatcher(HttpMethod.GET, "/search", "/login"), new PermitAllAuthorizationManager()));
+        mappings.add(new RequestMatcherEntry<>(new AnyRequestMatcher(), new DenyAllAuthorizationManager()));
+        return new RequestMatcherDelegatingAuthorizationManager(mappings);
+    }
+
     @Bean
     public SecurityFilterChain securityFilterChain() {
         return new DefaultSecurityFilterChain(
                 List.of(
                         new SecurityContextHolderFilter(),
                         new UsernamePasswordAuthenticationFilter(userDetailsService()),
                         new BasicAuthenticationFilter(userDetailsService()),
-                        new CheckAuthenticationFilter()
+                        new AuthorizationFilter(requestMatcherDelegatingAuthorizationManager())
                 )
         );
     }

src/main/java/nextstep/app/ui/MemberController.java

@@ -2,12 +2,16 @@
 
 import nextstep.app.domain.Member;
 import nextstep.app.domain.MemberRepository;
+import nextstep.security.authentication.AuthenticationException;
+import nextstep.security.authentication.UsernamePasswordAuthenticationToken;
 import nextstep.security.authorization.Secured;
+import nextstep.security.context.SecurityContextHolder;
 import org.springframework.http.ResponseEntity;
 import org.springframework.web.bind.annotation.GetMapping;
 import org.springframework.web.bind.annotation.RestController;
 
 import java.util.List;
+import java.util.NoSuchElementException;
 
 @RestController
 public class MemberController {
@@ -18,16 +22,28 @@ public MemberController(MemberRepository memberRepository) {
         this.memberRepository = memberRepository;
     }
 
+    @Secured("ADMIN")
     @GetMapping("/members")
     public ResponseEntity<List<Member>> list() {
         List<Member> members = memberRepository.findAll();
         return ResponseEntity.ok(members);
     }
 
-    @Secured("ADMIN")
+    @Secured({"ADMIN", "MEMBER"})
     @GetMapping("/search")
     public ResponseEntity<List<Member>> search() {
         List<Member> members = memberRepository.findAll();
         return ResponseEntity.ok(members);
     }
+
+    @GetMapping("/members/me")
+    public ResponseEntity<Member> getMyInfo() {
+        if (SecurityContextHolder.getContext().getAuthentication() instanceof UsernamePasswordAuthenticationToken token
+                && token.getPrincipal() instanceof String email) {
+            Member member = memberRepository.findByEmail(email).orElseThrow(NoSuchElementException::new);
+            return ResponseEntity.ok(member);
+        }
+
+        throw new AuthenticationException();
+    }
 }

src/main/java/nextstep/security/authentication/BasicAuthenticationFilter.java

@@ -3,6 +3,7 @@
 import jakarta.servlet.FilterChain;
 import jakarta.servlet.http.HttpServletRequest;
 import jakarta.servlet.http.HttpServletResponse;
+import nextstep.security.authorization.ForbiddenException;
 import nextstep.security.context.SecurityContext;
 import nextstep.security.context.SecurityContextHolder;
 import nextstep.security.userdetails.UserDetailsService;
@@ -40,6 +41,8 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse
             SecurityContextHolder.setContext(context);
 
             filterChain.doFilter(request, response);
+        } catch (ForbiddenException e) {
+            response.setStatus(HttpServletResponse.SC_FORBIDDEN);
         } catch (Exception e) {
             response.setStatus(HttpServletResponse.SC_UNAUTHORIZED);
         }

src/main/java/nextstep/security/authorization/AccessDeniedException.java

@@ -0,0 +1,8 @@
+package nextstep.security.authorization;
+
+import org.springframework.http.HttpStatus;
+import org.springframework.web.bind.annotation.ResponseStatus;
+
+@ResponseStatus(HttpStatus.FORBIDDEN)
+public class AccessDeniedException extends RuntimeException {
+}

src/main/java/nextstep/security/authorization/AuthorizationDecision.java

@@ -0,0 +1,25 @@
+package nextstep.security.authorization;
+
+public class AuthorizationDecision {
+
+    private static final AuthorizationDecision GRANTED = new AuthorizationDecision(true);
+    private static final AuthorizationDecision DENIED = new AuthorizationDecision(false);
+
+    private final boolean isGranted;
+
+    public static AuthorizationDecision granted() {
+        return GRANTED;
+    }
+
+    public static AuthorizationDecision denied() {
+        return DENIED;
+    }
+
+    protected AuthorizationDecision(final boolean isGranted) {
+        this.isGranted = isGranted;
+    }
+
+    public boolean isDenied() {
+        return !isGranted;
+    }
+}

src/main/java/nextstep/security/authorization/CheckAuthenticationFilter.java renamed to src/main/java/nextstep/security/authorization/AuthorizationFilter.java

@@ -1,38 +1,28 @@
 package nextstep.security.authorization;
 
-import nextstep.security.authentication.Authentication;
-import nextstep.security.context.SecurityContextHolder;
-import org.springframework.web.filter.OncePerRequestFilter;
-
 import jakarta.servlet.FilterChain;
 import jakarta.servlet.ServletException;
 import jakarta.servlet.http.HttpServletRequest;
 import jakarta.servlet.http.HttpServletResponse;
+import nextstep.security.authentication.Authentication;
+import nextstep.security.authorization.manager.RequestMatcherDelegatingAuthorizationManager;
+import nextstep.security.context.SecurityContextHolder;
+import org.springframework.web.filter.OncePerRequestFilter;
+
 import java.io.IOException;
-import java.util.Set;
 
-public class CheckAuthenticationFilter extends OncePerRequestFilter {
-    private static final String DEFAULT_REQUEST_URI = "/members";
+public class AuthorizationFilter extends OncePerRequestFilter {
+
+    private final RequestMatcherDelegatingAuthorizationManager authorizationManager;
+
+    public AuthorizationFilter(RequestMatcherDelegatingAuthorizationManager authorizationManager) {
+        this.authorizationManager = authorizationManager;
+    }
 
     @Override
     protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) throws ServletException, IOException {
-        if (!DEFAULT_REQUEST_URI.equals(request.getRequestURI())) {
-            filterChain.doFilter(request, response);
-            return;
-        }
-
         Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
-        if (authentication == null || !authentication.isAuthenticated()) {
-            response.setStatus(HttpServletResponse.SC_UNAUTHORIZED);
-            return;
-        }
-
-        Set<String> authorities = authentication.getAuthorities();
-        if (!authorities.contains("ADMIN")) {
-            response.setStatus(HttpServletResponse.SC_FORBIDDEN);
-            return;
-        }
-
+        authorizationManager.verifyInFilter(request, authentication);
         filterChain.doFilter(request, response);
     }
 }

src/main/java/nextstep/security/authorization/Secured.java

@@ -8,5 +8,5 @@
 @Target(ElementType.METHOD)
 @Retention(RetentionPolicy.RUNTIME)
 public @interface Secured {
-    String value();
+    String[] value() default {"ADMIN"};
 }

src/main/java/nextstep/security/authorization/SecuredAspect.java

@@ -9,19 +9,24 @@
 import org.aspectj.lang.reflect.MethodSignature;
 
 import java.lang.reflect.Method;
+import java.util.Arrays;
 
 @Aspect
 public class SecuredAspect {
 
     @Before("@annotation(nextstep.security.authorization.Secured)")
     public void checkSecured(JoinPoint joinPoint) throws NoSuchMethodException {
         Method method = getMethodFromJoinPoint(joinPoint);
-        String secured = method.getAnnotation(Secured.class).value();
+        String[] secured = method.getAnnotation(Secured.class).value();
         Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
         if (authentication == null) {
             throw new AuthenticationException();
         }
-        if (!authentication.getAuthorities().contains(secured)) {
+
+        boolean hasNoRole = authentication.getAuthorities()
+                .stream()
+                .noneMatch(auth -> Arrays.asList(secured).contains(auth));
+        if (hasNoRole) {
             throw new ForbiddenException();
         }
     }

src/main/java/nextstep/security/authorization/SecuredMethodInterceptor.java

@@ -1,7 +1,7 @@
 package nextstep.security.authorization;
 
 import nextstep.security.authentication.Authentication;
-import nextstep.security.authentication.AuthenticationException;
+import nextstep.security.authorization.manager.AuthorizationManager;
 import nextstep.security.context.SecurityContextHolder;
 import org.aopalliance.aop.Advice;
 import org.aopalliance.intercept.MethodInterceptor;
@@ -11,29 +11,21 @@
 import org.springframework.aop.framework.AopInfrastructureBean;
 import org.springframework.aop.support.annotation.AnnotationMatchingPointcut;
 
-import java.lang.reflect.Method;
 
 public class SecuredMethodInterceptor implements MethodInterceptor, PointcutAdvisor, AopInfrastructureBean {
 
     private final Pointcut pointcut;
+    private final AuthorizationManager<MethodInvocation> authorizationManager;
 
-    public SecuredMethodInterceptor() {
+    public SecuredMethodInterceptor(AuthorizationManager<MethodInvocation> authorizationManager) {
+        this.authorizationManager = authorizationManager;
         this.pointcut = new AnnotationMatchingPointcut(null, Secured.class);
     }
 
     @Override
     public Object invoke(MethodInvocation invocation) throws Throwable {
-        Method method = invocation.getMethod();
-        if (method.isAnnotationPresent(Secured.class)) {
-            Secured secured = method.getAnnotation(Secured.class);
-            Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
-            if (authentication == null) {
-                throw new AuthenticationException();
-            }
-            if (!authentication.getAuthorities().contains(secured.value())) {
-                throw new ForbiddenException();
-            }
-        }
+        Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
+        authorizationManager.verify(authentication, invocation);
         return invocation.proceed();
     }