[고정완] - 인가(Authorization) 리뷰 요청 드립니다. (#9)

* refactor: Security 에서의 공통 Exception 처리를 FilterChainProxy 에서 담당하도록 변경

* test: 자신의 정보를 조회하는데 인증이 필요한 부분에 대한 테스트 작성

* feat: CheckAuthenticationFilter 로직을 대체하는 RequestAuthorizationManager 구현

* feat: RequestMatcher 와 RequestMatcherEntry 구현

* feat: 4가지 AuthorizationManager 구현; PermitAll, DenyAll, Authenticated, HasAuthority

* feat: AuthorizationManager 에서 RequestMatcherEntry 를 활용하도록 Config 에 Bean 을 등록함

* refactor: SecuredMethodInterceptor 의 인가 로직을 SecuredAuthorizationManager 으로 이동

* fix: Secured 어노테이션이 없으면, MethodInterceptor 에서 인증 및 인가 과정을 거치지 않도록 수정함

* refactor: Rename HasAuthorityAuthorizationManager to AuthorityAuthorizationManager

* fix: security 패키지에서 stream 문을 사용하지 않도록 변경

* fix: 삼항연산자 대신 if 문으로 early return 을 해주도록 모두 변경

* fix: Secured 어노테이션이 없는 메서드일 경우 인가 값이 false 가 되도록 수정

* refactor: deprecated 된 AuthorizationManager.check 대신, AuthorizationResult 인터페이스를 반환하는 AuthorizationManager.authorize 로 대체

* test: Fixture 를 enum 으로 생성

* test: 어드민 유저와 일반 유저의 인가 테스트 케이스를 분리

Author: ghojeong

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

@@ -1,13 +1,19 @@
 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.AuthenticatedAuthorizationManager;
+import nextstep.security.authorization.manager.AuthorityAuthorizationManager;
+import nextstep.security.authorization.manager.AuthorizationManager;
+import nextstep.security.authorization.manager.DenyAllAuthorizationManager;
+import nextstep.security.authorization.manager.PermitAllAuthorizationManager;
+import nextstep.security.authorization.manager.RequestAuthorizationManager;
 import nextstep.security.config.DefaultSecurityFilterChain;
 import nextstep.security.config.DelegatingFilterProxy;
 import nextstep.security.config.FilterChainProxy;
@@ -22,6 +28,10 @@
 import java.util.List;
 import java.util.Set;
 
+import static nextstep.security.authorization.matcher.RequestMatcherEntry.createDefaultMatcher;
+import static nextstep.security.authorization.matcher.RequestMatcherEntry.createMvcMatcher;
+import static org.springframework.http.HttpMethod.GET;
+
 @EnableAspectJAutoProxy
 @Configuration
 public class SecurityConfig {
@@ -46,19 +56,20 @@ public FilterChainProxy filterChainProxy(List<SecurityFilterChain> securityFilte
     public SecuredMethodInterceptor securedMethodInterceptor() {
         return new SecuredMethodInterceptor();
     }
-//    @Bean
-//    public SecuredAspect securedAspect() {
-//        return new SecuredAspect();
-//    }
 
     @Bean
     public SecurityFilterChain securityFilterChain() {
+        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<>()));
         return new DefaultSecurityFilterChain(
                 List.of(
                         new SecurityContextHolderFilter(),
                         new UsernamePasswordAuthenticationFilter(userDetailsService()),
                         new BasicAuthenticationFilter(userDetailsService()),
-                        new CheckAuthenticationFilter()
+                        new AuthorizationFilter(authorizationManager)
                 )
         );
     }

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.Authentication;
+import nextstep.security.authentication.AuthenticationException;
 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.Optional;
 
 @RestController
 public class MemberController {
@@ -30,4 +34,22 @@ public ResponseEntity<List<Member>> search() {
         List<Member> members = memberRepository.findAll();
         return ResponseEntity.ok(members);
     }
+
+    @Secured("USER")
+    @GetMapping("/members/me")
+    public ResponseEntity<Member> me() {
+        final Authentication authentication = SecurityContextHolder
+                .getContext().getAuthentication();
+        return ResponseEntity.ok(
+                Optional.ofNullable(authentication)
+                        .flatMap(this::findMe)
+                        .orElseThrow(AuthenticationException::new)
+        );
+    }
+
+    private Optional<Member> findMe(Authentication authentication) {
+        return memberRepository.findByEmail(
+                authentication.getPrincipal().toString()
+        );
+    }
 }

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

@@ -1,6 +1,7 @@
 package nextstep.security.authentication;
 
 import jakarta.servlet.FilterChain;
+import jakarta.servlet.ServletException;
 import jakarta.servlet.http.HttpServletRequest;
 import jakarta.servlet.http.HttpServletResponse;
 import nextstep.security.context.SecurityContext;
@@ -10,6 +11,7 @@
 import org.springframework.util.StringUtils;
 import org.springframework.web.filter.OncePerRequestFilter;
 
+import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.util.Base64;
 import java.util.List;
@@ -26,23 +28,23 @@ public BasicAuthenticationFilter(UserDetailsService userDetailsService) {
     }
 
     @Override
-    protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) {
-        try {
-            Authentication authentication = convert(request);
-            if (authentication == null) {
-                filterChain.doFilter(request, response);
-                return;
-            }
-
-            Authentication authResult = this.authenticationManager.authenticate(authentication);
-            SecurityContext context = SecurityContextHolder.createEmptyContext();
-            context.setAuthentication(authResult);
-            SecurityContextHolder.setContext(context);
-
+    protected void doFilterInternal(
+            HttpServletRequest request,
+            HttpServletResponse response,
+            FilterChain filterChain
+    ) throws ServletException, IOException {
+        Authentication authentication = convert(request);
+        if (authentication == null) {
             filterChain.doFilter(request, response);
-        } catch (Exception e) {
-            response.setStatus(HttpServletResponse.SC_UNAUTHORIZED);
+            return;
         }
+
+        Authentication authResult = this.authenticationManager.authenticate(authentication);
+        SecurityContext context = SecurityContextHolder.createEmptyContext();
+        context.setAuthentication(authResult);
+        SecurityContextHolder.setContext(context);
+
+        filterChain.doFilter(request, response);
     }
 
     private Authentication convert(HttpServletRequest request) {

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

@@ -0,0 +1,37 @@
+package nextstep.security.authorization;
+
+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.authentication.AuthenticationException;
+import nextstep.security.authorization.manager.AuthorizationManager;
+import nextstep.security.context.SecurityContextHolder;
+import org.springframework.web.filter.OncePerRequestFilter;
+
+import java.io.IOException;
+
+public class AuthorizationFilter extends OncePerRequestFilter {
+    private final AuthorizationManager<HttpServletRequest> authorizationManager;
+
+    public AuthorizationFilter(AuthorizationManager<HttpServletRequest> authorizationManager) {
+        this.authorizationManager = authorizationManager;
+    }
+
+    @Override
+    protected void doFilterInternal(
+            HttpServletRequest request,
+            HttpServletResponse response,
+            FilterChain filterChain
+    ) throws ServletException, IOException {
+        final Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
+        if (authentication == null || !authentication.isAuthenticated()) {
+            throw new AuthenticationException();
+        }
+        if (!authorizationManager.authorize(authentication, request).isGranted()) {
+            throw new ForbiddenException();
+        }
+        filterChain.doFilter(request, response);
+    }
+}

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

@@ -2,6 +2,7 @@
 
 import nextstep.security.authentication.Authentication;
 import nextstep.security.authentication.AuthenticationException;
+import nextstep.security.authorization.manager.SecuredAuthorizationManager;
 import nextstep.security.context.SecurityContextHolder;
 import org.aopalliance.aop.Advice;
 import org.aopalliance.intercept.MethodInterceptor;
@@ -11,8 +12,6 @@
 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;
@@ -23,16 +22,17 @@ public SecuredMethodInterceptor() {
 
     @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();
-            }
+        if (!invocation.getMethod().isAnnotationPresent(Secured.class)) {
+            return invocation.proceed();
+        }
+        final Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
+        if (authentication == null || !authentication.isAuthenticated()) {
+            throw new AuthenticationException();
+        }
+        if (!SecuredAuthorizationManager.getInstance().authorize(
+                authentication, invocation
+        ).isGranted()) {
+            throw new ForbiddenException();
         }
         return invocation.proceed();
     }

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

@@ -0,0 +1,12 @@
+package nextstep.security.authorization.manager;
+
+import nextstep.security.authentication.Authentication;
+
+public class AuthenticatedAuthorizationManager<T> implements AuthorizationManager<T> {
+    @Override
+    public AuthorizationResult authorize(Authentication authentication, T target) {
+        return AuthorizationDecision.of(
+                authentication != null && authentication.isAuthenticated()
+        );
+    }
+}

src/main/java/nextstep/security/authorization/manager/AuthenticatedAuthorizationManager.java

@@ -0,0 +1,33 @@
+package nextstep.security.authorization.manager;
+
+import nextstep.security.authentication.Authentication;
+
+import java.util.Set;
+
+public class AuthorityAuthorizationManager<T> implements AuthorizationManager<T> {
+    private final Set<String> authorities;
+
+    public AuthorityAuthorizationManager(String... authorities) {
+        this.authorities = Set.of(authorities);
+    }
+
+    @Override
+    public AuthorizationResult authorize(Authentication authentication, T target) {
+        return AuthorizationDecision.of(isGranted(authentication));
+    }
+
+    private boolean isGranted(Authentication authentication) {
+        return authentication != null
+                && authentication.isAuthenticated()
+                && anyMatch(authentication);
+    }
+
+    private boolean anyMatch(Authentication authentication) {
+        for (var authority : authentication.getAuthorities()) {
+            if (authorities.contains(authority)) {
+                return true;
+            }
+        }
+        return false;
+    }
+}

src/main/java/nextstep/security/authorization/manager/AuthorityAuthorizationManager.java

@@ -0,0 +1,22 @@
+package nextstep.security.authorization.manager;
+
+public enum AuthorizationDecision implements AuthorizationResult {
+    GRANTED(true), NOT_GRANTED(false);
+
+    private final boolean granted;
+
+    AuthorizationDecision(boolean granted) {
+        this.granted = granted;
+    }
+
+    public static AuthorizationDecision of(boolean granted) {
+        if (granted) {
+            return GRANTED;
+        }
+        return NOT_GRANTED;
+    }
+
+    public boolean isGranted() {
+        return this.granted;
+    }
+}

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

@@ -0,0 +1,8 @@
+package nextstep.security.authorization.manager;
+
+import nextstep.security.authentication.Authentication;
+
+@FunctionalInterface
+public interface AuthorizationManager<T> {
+    AuthorizationResult authorize(Authentication authentication, T target);
+}