diff --git a/README.md b/README.md index 61f417c..21ffc7a 100644 --- a/README.md +++ b/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 diff --git a/src/main/java/nextstep/app/SecurityConfig.java b/src/main/java/nextstep/app/SecurityConfig.java index 1683058..928bbd7 100644 --- a/src/main/java/nextstep/app/SecurityConfig.java +++ b/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,13 +50,24 @@ public FilterChainProxy filterChainProxy(List securityFilte @Bean public SecuredMethodInterceptor securedMethodInterceptor() { - return new SecuredMethodInterceptor(); + AuthorityAuthorizationManager> authorityAuthorizationManager = new AuthorityAuthorizationManager<>(); + return new SecuredMethodInterceptor(new SecuredAuthorizationManager(authorityAuthorizationManager)); } // @Bean // public SecuredAspect securedAspect() { // return new SecuredAspect(); // } + @Bean + public RequestMatcherDelegatingAuthorizationManager requestMatcherDelegatingAuthorizationManager() { + List>> 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( @@ -58,7 +75,7 @@ public SecurityFilterChain securityFilterChain() { new SecurityContextHolderFilter(), new UsernamePasswordAuthenticationFilter(userDetailsService()), new BasicAuthenticationFilter(userDetailsService()), - new CheckAuthenticationFilter() + new AuthorizationFilter(requestMatcherDelegatingAuthorizationManager()) ) ); } diff --git a/src/main/java/nextstep/app/ui/MemberController.java b/src/main/java/nextstep/app/ui/MemberController.java index 823cf7e..5bc6afb 100644 --- a/src/main/java/nextstep/app/ui/MemberController.java +++ b/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() { List members = memberRepository.findAll(); return ResponseEntity.ok(members); } - @Secured("ADMIN") + @Secured({"ADMIN", "MEMBER"}) @GetMapping("/search") public ResponseEntity> search() { List members = memberRepository.findAll(); return ResponseEntity.ok(members); } + + @GetMapping("/members/me") + public ResponseEntity 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(); + } } diff --git a/src/main/java/nextstep/security/authentication/BasicAuthenticationFilter.java b/src/main/java/nextstep/security/authentication/BasicAuthenticationFilter.java index 406116f..91b81e1 100644 --- a/src/main/java/nextstep/security/authentication/BasicAuthenticationFilter.java +++ b/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); } diff --git a/src/main/java/nextstep/security/authorization/AccessDeniedException.java b/src/main/java/nextstep/security/authorization/AccessDeniedException.java new file mode 100644 index 0000000..2dc2593 --- /dev/null +++ b/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 { +} diff --git a/src/main/java/nextstep/security/authorization/AuthorizationDecision.java b/src/main/java/nextstep/security/authorization/AuthorizationDecision.java new file mode 100644 index 0000000..e7d2d97 --- /dev/null +++ b/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; + } +} diff --git a/src/main/java/nextstep/security/authorization/CheckAuthenticationFilter.java b/src/main/java/nextstep/security/authorization/AuthorizationFilter.java similarity index 51% rename from src/main/java/nextstep/security/authorization/CheckAuthenticationFilter.java rename to src/main/java/nextstep/security/authorization/AuthorizationFilter.java index 02327ff..a8ae4d5 100644 --- a/src/main/java/nextstep/security/authorization/CheckAuthenticationFilter.java +++ b/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 authorities = authentication.getAuthorities(); - if (!authorities.contains("ADMIN")) { - response.setStatus(HttpServletResponse.SC_FORBIDDEN); - return; - } - + authorizationManager.verifyInFilter(request, authentication); filterChain.doFilter(request, response); } } diff --git a/src/main/java/nextstep/security/authorization/Secured.java b/src/main/java/nextstep/security/authorization/Secured.java index be90056..55a0f0c 100644 --- a/src/main/java/nextstep/security/authorization/Secured.java +++ b/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"}; } diff --git a/src/main/java/nextstep/security/authorization/SecuredAspect.java b/src/main/java/nextstep/security/authorization/SecuredAspect.java index f68f86c..bc21d01 100644 --- a/src/main/java/nextstep/security/authorization/SecuredAspect.java +++ b/src/main/java/nextstep/security/authorization/SecuredAspect.java @@ -9,6 +9,7 @@ import org.aspectj.lang.reflect.MethodSignature; import java.lang.reflect.Method; +import java.util.Arrays; @Aspect public class SecuredAspect { @@ -16,12 +17,16 @@ 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(); } } diff --git a/src/main/java/nextstep/security/authorization/SecuredMethodInterceptor.java b/src/main/java/nextstep/security/authorization/SecuredMethodInterceptor.java index 8ee7409..0d0a2a9 100644 --- a/src/main/java/nextstep/security/authorization/SecuredMethodInterceptor.java +++ b/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 authorizationManager; - public SecuredMethodInterceptor() { + public SecuredMethodInterceptor(AuthorizationManager 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(); } diff --git a/src/main/java/nextstep/security/authorization/manager/AuthenticatedAuthorizationManager.java b/src/main/java/nextstep/security/authorization/manager/AuthenticatedAuthorizationManager.java new file mode 100644 index 0000000..af1d668 --- /dev/null +++ b/src/main/java/nextstep/security/authorization/manager/AuthenticatedAuthorizationManager.java @@ -0,0 +1,17 @@ +package nextstep.security.authorization.manager; + +import jakarta.servlet.http.HttpServletRequest; +import nextstep.security.authentication.Authentication; +import nextstep.security.authorization.AuthorizationDecision; + +public class AuthenticatedAuthorizationManager implements AuthorizationManager { + + @Override + public AuthorizationDecision check(Authentication authentication, HttpServletRequest object) { + if (authentication == null || !authentication.isAuthenticated()) { + return AuthorizationDecision.denied(); + } + + return AuthorizationDecision.granted(); + } +} diff --git a/src/main/java/nextstep/security/authorization/manager/AuthorityAuthorizationManager.java b/src/main/java/nextstep/security/authorization/manager/AuthorityAuthorizationManager.java new file mode 100644 index 0000000..76ffc20 --- /dev/null +++ b/src/main/java/nextstep/security/authorization/manager/AuthorityAuthorizationManager.java @@ -0,0 +1,65 @@ +package nextstep.security.authorization.manager; + +import nextstep.security.authentication.Authentication; +import nextstep.security.authorization.AuthorizationDecision; +import nextstep.security.authorization.ForbiddenException; + +import java.util.Collection; +import java.util.Set; + +public class AuthorityAuthorizationManager implements AuthorizationManager { + + @Override + public AuthorizationDecision check(Authentication authentication, T authorities) { + if (authentication == null || !authentication.isAuthenticated()) { + return AuthorizationDecision.denied(); + } + + boolean hasNotRole = false; + + if (authorities instanceof Collection collection) { + hasNotRole = roleCheckByCollection(authentication, collection); + } else if (authorities instanceof String authority) { + hasNotRole = roleCheckByString(authentication, authority); + } + + if (hasNotRole) { + throw new ForbiddenException(); + } + + return AuthorizationDecision.granted(); + } + + private boolean roleCheckByCollection(Authentication authentication, Collection requiredAuthorities) { + if (requiredAuthorities == null || requiredAuthorities.isEmpty()) { + return false; + } + + Set userAuthorities = authentication.getAuthorities(); + if (userAuthorities == null || userAuthorities.isEmpty()) { + return true; + } + + if (requiredAuthorities.iterator().next() instanceof String) { + + @SuppressWarnings("unchecked") + Collection requires = (Collection) requiredAuthorities; + for (String requiredAuthority : requires) { + if (userAuthorities.contains(requiredAuthority)) { + return false; + } + } + } + + return true; + } + + private boolean roleCheckByString(Authentication authentication, String requiredAuthority) { + Set userAuthorities = authentication.getAuthorities(); + if (userAuthorities == null || userAuthorities.isEmpty()) { + return true; + } + + return !userAuthorities.contains(requiredAuthority); + } +} diff --git a/src/main/java/nextstep/security/authorization/manager/AuthorizationManager.java b/src/main/java/nextstep/security/authorization/manager/AuthorizationManager.java new file mode 100644 index 0000000..d40841b --- /dev/null +++ b/src/main/java/nextstep/security/authorization/manager/AuthorizationManager.java @@ -0,0 +1,18 @@ +package nextstep.security.authorization.manager; + +import nextstep.security.authentication.Authentication; +import nextstep.security.authorization.AccessDeniedException; +import nextstep.security.authorization.AuthorizationDecision; + +@FunctionalInterface +public interface AuthorizationManager { + + default void verify(Authentication authentication, T object) { + AuthorizationDecision decision = check(authentication, object); + if (decision != null && decision.isDenied()) { + throw new AccessDeniedException(); + } + } + + AuthorizationDecision check(Authentication authentication, T object); +} diff --git a/src/main/java/nextstep/security/authorization/manager/DenyAllAuthorizationManager.java b/src/main/java/nextstep/security/authorization/manager/DenyAllAuthorizationManager.java new file mode 100644 index 0000000..28328d3 --- /dev/null +++ b/src/main/java/nextstep/security/authorization/manager/DenyAllAuthorizationManager.java @@ -0,0 +1,14 @@ +package nextstep.security.authorization.manager; + +import jakarta.servlet.http.HttpServletRequest; +import nextstep.security.authentication.Authentication; +import nextstep.security.authorization.AccessDeniedException; +import nextstep.security.authorization.AuthorizationDecision; + +public class DenyAllAuthorizationManager implements AuthorizationManager { + + @Override + public AuthorizationDecision check(Authentication authentication, HttpServletRequest object) { + throw new AccessDeniedException(); + } +} diff --git a/src/main/java/nextstep/security/authorization/manager/PermitAllAuthorizationManager.java b/src/main/java/nextstep/security/authorization/manager/PermitAllAuthorizationManager.java new file mode 100644 index 0000000..083009f --- /dev/null +++ b/src/main/java/nextstep/security/authorization/manager/PermitAllAuthorizationManager.java @@ -0,0 +1,13 @@ +package nextstep.security.authorization.manager; + +import jakarta.servlet.http.HttpServletRequest; +import nextstep.security.authentication.Authentication; +import nextstep.security.authorization.AuthorizationDecision; + +public class PermitAllAuthorizationManager implements AuthorizationManager { + + @Override + public AuthorizationDecision check(Authentication authentication, HttpServletRequest object) { + return AuthorizationDecision.granted(); + } +} diff --git a/src/main/java/nextstep/security/authorization/manager/RequestMatcherDelegatingAuthorizationManager.java b/src/main/java/nextstep/security/authorization/manager/RequestMatcherDelegatingAuthorizationManager.java new file mode 100644 index 0000000..fd78bfe --- /dev/null +++ b/src/main/java/nextstep/security/authorization/manager/RequestMatcherDelegatingAuthorizationManager.java @@ -0,0 +1,40 @@ +package nextstep.security.authorization.manager; + +import jakarta.servlet.http.HttpServletRequest; +import nextstep.security.authentication.Authentication; +import nextstep.security.authorization.AuthorizationDecision; +import nextstep.security.matcher.RequestMatcherEntry; + +import java.util.List; +import java.util.Objects; + +public class RequestMatcherDelegatingAuthorizationManager { + + private final List>> mapping; + + public RequestMatcherDelegatingAuthorizationManager(List>> mapping) { + this.mapping = mapping; + } + + public void verifyInFilter(HttpServletRequest request, Authentication authentication) { + var requestMatcherEntry = mapping.stream() + .filter(it -> Objects.nonNull(it.getMatcher())) + .filter(it -> it.getMatcher().matches(request)) + .filter(it -> Objects.nonNull(it.getEntry())) + .findFirst(); + + requestMatcherEntry.ifPresent(it + -> it.getEntry().verify(authentication, request)); + } + + @Deprecated + public AuthorizationDecision checkInFilter(HttpServletRequest request, Authentication authentication) { + return mapping.stream() + .filter(it -> Objects.nonNull(it.getMatcher())) + .filter(it -> it.getMatcher().matches(request)) + .filter(it -> Objects.nonNull(it.getEntry())) + .findFirst() + .map(it -> it.getEntry().check(authentication, request)) + .orElse(AuthorizationDecision.denied()); + } +} diff --git a/src/main/java/nextstep/security/authorization/manager/SecuredAuthorizationManager.java b/src/main/java/nextstep/security/authorization/manager/SecuredAuthorizationManager.java new file mode 100644 index 0000000..c88d583 --- /dev/null +++ b/src/main/java/nextstep/security/authorization/manager/SecuredAuthorizationManager.java @@ -0,0 +1,34 @@ +package nextstep.security.authorization.manager; + +import nextstep.security.authentication.Authentication; +import nextstep.security.authorization.AuthorizationDecision; +import nextstep.security.authorization.Secured; +import org.aopalliance.intercept.MethodInvocation; + +import java.lang.reflect.Method; +import java.util.Set; + +public class SecuredAuthorizationManager implements AuthorizationManager { + + private final AuthorityAuthorizationManager> authorityAuthorizationManager; + + public SecuredAuthorizationManager(AuthorityAuthorizationManager> authorityAuthorizationManager) { + this.authorityAuthorizationManager = authorityAuthorizationManager; + } + + @Override + public AuthorizationDecision check(Authentication authentication, MethodInvocation methodInvocation) { + if (authentication == null || !authentication.isAuthenticated()) { + return AuthorizationDecision.denied(); + } + + Method method = methodInvocation.getMethod(); + + if (method.isAnnotationPresent(Secured.class)) { + Secured secured = method.getAnnotation(Secured.class); + return authorityAuthorizationManager.check(authentication, Set.of(secured.value())); + } + + return AuthorizationDecision.granted(); + } +} diff --git a/src/main/java/nextstep/security/matcher/AnyRequestMatcher.java b/src/main/java/nextstep/security/matcher/AnyRequestMatcher.java new file mode 100644 index 0000000..3e2e21b --- /dev/null +++ b/src/main/java/nextstep/security/matcher/AnyRequestMatcher.java @@ -0,0 +1,10 @@ +package nextstep.security.matcher; + +import jakarta.servlet.http.HttpServletRequest; + +public class AnyRequestMatcher implements RequestMatcher { + @Override + public boolean matches(HttpServletRequest request) { + return true; + } +} diff --git a/src/main/java/nextstep/security/matcher/MvcRequestMatcher.java b/src/main/java/nextstep/security/matcher/MvcRequestMatcher.java new file mode 100644 index 0000000..2ef2fa5 --- /dev/null +++ b/src/main/java/nextstep/security/matcher/MvcRequestMatcher.java @@ -0,0 +1,31 @@ +package nextstep.security.matcher; + +import jakarta.servlet.http.HttpServletRequest; +import org.springframework.http.HttpMethod; + +public class MvcRequestMatcher implements RequestMatcher { + + private final HttpMethod httpMethod; + private final String[] requestURIs; + + public MvcRequestMatcher(HttpMethod httpMethod, String... requestURIs) { + this.httpMethod = httpMethod; + this.requestURIs = requestURIs; + } + + @Override + public boolean matches(HttpServletRequest request) { + boolean matchMethod = httpMethod.name().equalsIgnoreCase(request.getMethod()); + + boolean matchURI = false; + final String requestURI = request.getRequestURI(); + for (String uri : requestURIs) { + if (uri.equalsIgnoreCase(requestURI)) { + matchURI = true; + break; + } + } + + return matchMethod && matchURI; + } +} diff --git a/src/main/java/nextstep/security/matcher/RequestMatcher.java b/src/main/java/nextstep/security/matcher/RequestMatcher.java new file mode 100644 index 0000000..2c1d4b3 --- /dev/null +++ b/src/main/java/nextstep/security/matcher/RequestMatcher.java @@ -0,0 +1,7 @@ +package nextstep.security.matcher; + +import jakarta.servlet.http.HttpServletRequest; + +public interface RequestMatcher { + boolean matches(HttpServletRequest request); +} diff --git a/src/main/java/nextstep/security/matcher/RequestMatcherEntry.java b/src/main/java/nextstep/security/matcher/RequestMatcherEntry.java new file mode 100644 index 0000000..beeb467 --- /dev/null +++ b/src/main/java/nextstep/security/matcher/RequestMatcherEntry.java @@ -0,0 +1,21 @@ +package nextstep.security.matcher; + + +public class RequestMatcherEntry { + + private final RequestMatcher matcher; + private final T entry; + + public RequestMatcherEntry(RequestMatcher matcher, T entry) { + this.matcher = matcher; + this.entry = entry; + } + + public RequestMatcher getMatcher() { + return matcher; + } + + public T getEntry() { + return entry; + } +} diff --git a/src/test/java/nextstep/app/BasicAuthTest.java b/src/test/java/nextstep/app/BasicAuthTest.java index ac117be..c273fe6 100644 --- a/src/test/java/nextstep/app/BasicAuthTest.java +++ b/src/test/java/nextstep/app/BasicAuthTest.java @@ -5,6 +5,8 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; import org.springframework.boot.test.context.SpringBootTest; @@ -23,7 +25,8 @@ @AutoConfigureMockMvc class BasicAuthTest { private final Member TEST_ADMIN_MEMBER = new Member("a@a.com", "password", "a", "", Set.of("ADMIN")); - private final Member TEST_USER_MEMBER = new Member("b@b.com", "password", "b", "", Set.of()); + private final Member TEST_USER_MEMBER = new Member("b@b.com", "password", "b", "", Set.of("MEMBER")); + private final Member TEST_UN_AUTH_MEMBER = new Member("c@c.com", "password", "c", "", Set.of("MEMBER")); @Autowired private MockMvc mockMvc; @@ -89,4 +92,51 @@ void request_fail_invalid_password() throws Exception { response.andExpect(status().isUnauthorized()); } + + @DisplayName("인증된 사용자는 자신의 정보를 조회할 수 있다.") + @Test + void request_success_members_me() throws Exception { + String token = Base64.getEncoder().encodeToString((TEST_ADMIN_MEMBER.getEmail() + ":" + TEST_ADMIN_MEMBER.getPassword()).getBytes()); + + ResultActions response = mockMvc.perform(get("/members/me") + .header("Authorization", "Basic " + token) + .contentType(MediaType.APPLICATION_FORM_URLENCODED_VALUE) + ); + + response.andExpect(status().isOk()) + .andExpect(MockMvcResultMatchers.jsonPath("$.length()").value(5)) + .andExpect(MockMvcResultMatchers.jsonPath("$.email").value("a@a.com")) + .andExpect(MockMvcResultMatchers.jsonPath("$.password").value("password")) + .andExpect(MockMvcResultMatchers.jsonPath("$.name").value("a")) + .andExpect(MockMvcResultMatchers.jsonPath("$.imageUrl").exists()) + .andExpect(MockMvcResultMatchers.jsonPath("$.roles").value("ADMIN")); + + } + + @DisplayName("인증되지 않은 사용자는 자신의 정보를 조회할 수 없다.") + @Test + void request_fail_members_me() throws Exception { + String token = Base64.getEncoder().encodeToString((TEST_UN_AUTH_MEMBER.getEmail() + ":" + TEST_UN_AUTH_MEMBER.getPassword()).getBytes()); + + ResultActions response = mockMvc.perform(get("/members/me") + .header("Authorization", "Basic " + token) + .contentType(MediaType.APPLICATION_FORM_URLENCODED_VALUE) + ); + + response.andExpect(status().isUnauthorized()); + } + + @DisplayName("허용되지 않은 요청은 접근 불가능해야한다.") + @ParameterizedTest + @ValueSource(strings = {"/member", "/members/my", "/members/me/1"}) + void request_fail_not_permit_url(String url) throws Exception { + String token = Base64.getEncoder().encodeToString((TEST_ADMIN_MEMBER.getEmail() + ":" + TEST_ADMIN_MEMBER.getPassword()).getBytes()); + + ResultActions response = mockMvc.perform(get(url) + .header("Authorization", "Basic " + token) + .contentType(MediaType.APPLICATION_FORM_URLENCODED_VALUE) + ); + + response.andExpect(status().isUnauthorized()); + } } diff --git a/src/test/java/nextstep/app/FormLoginTest.java b/src/test/java/nextstep/app/FormLoginTest.java index 6301c1a..aafe66a 100644 --- a/src/test/java/nextstep/app/FormLoginTest.java +++ b/src/test/java/nextstep/app/FormLoginTest.java @@ -23,7 +23,7 @@ @AutoConfigureMockMvc class FormLoginTest { private final Member TEST_ADMIN_MEMBER = new Member("a@a.com", "password", "a", "", Set.of("ADMIN")); - private final Member TEST_USER_MEMBER = new Member("b@b.com", "password", "b", "", Set.of()); + private final Member TEST_USER_MEMBER = new Member("b@b.com", "password", "b", "", Set.of("MEMBER")); @Autowired private MockMvc mockMvc; diff --git a/src/test/java/nextstep/app/SecuredTest.java b/src/test/java/nextstep/app/SecuredTest.java index 9e672e5..1eecbac 100644 --- a/src/test/java/nextstep/app/SecuredTest.java +++ b/src/test/java/nextstep/app/SecuredTest.java @@ -2,9 +2,12 @@ import nextstep.app.domain.Member; import nextstep.app.domain.MemberRepository; +import nextstep.app.infrastructure.InmemoryMemberRepository; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; import org.springframework.boot.test.context.SpringBootTest; @@ -24,7 +27,6 @@ @AutoConfigureMockMvc class SecuredTest { private final Member TEST_ADMIN_MEMBER = new Member("a@a.com", "password", "a", "", Set.of("ADMIN")); - private final Member TEST_USER_MEMBER = new Member("b@b.com", "password", "b", "", Set.of()); @Autowired private MockMvc mockMvc; @@ -33,15 +35,17 @@ class SecuredTest { private MemberRepository memberRepository; @BeforeEach - void setUp() { + void clear() { memberRepository.save(TEST_ADMIN_MEMBER); - memberRepository.save(TEST_USER_MEMBER); } @DisplayName("ADMIN 권한을 가진 사용자가 요청할 경우 모든 회원 정보를 조회할 수 있다.") - @Test - void request_search_success_with_admin_user() throws Exception { - String token = Base64.getEncoder().encodeToString((TEST_ADMIN_MEMBER.getEmail() + ":" + TEST_ADMIN_MEMBER.getPassword()).getBytes()); + @ParameterizedTest + @ValueSource(strings = {"ADMIN", "MEMBER"}) + void request_search_success_with_admin_user(String role) throws Exception { + Member member = new Member("b@b.com", "password", "b", "", Set.of(role)); + memberRepository.save(member); + String token = Base64.getEncoder().encodeToString((member.getEmail() + ":" + member.getPassword()).getBytes()); ResultActions response = mockMvc.perform(get("/search") .header("Authorization", "Basic " + token) @@ -52,12 +56,30 @@ void request_search_success_with_admin_user() throws Exception { .andExpect(MockMvcResultMatchers.jsonPath("$.length()").value(2)); } - @DisplayName("일반 사용자가 요청할 경우 권한이 없어야 한다.") + @DisplayName("ADMIN 권한을 가진 사용자가 /members를 요청할 경우 모든 회원 정보를 조회할 수 있다.") @Test - void request_search_fail_with_general_user() throws Exception { - String token = Base64.getEncoder().encodeToString((TEST_USER_MEMBER.getEmail() + ":" + TEST_USER_MEMBER.getPassword()).getBytes()); + void request_members_success_with_admin_user() throws Exception { + Member member = new Member("b@b.com", "password", "b", "", Set.of("ADMIN")); + memberRepository.save(member); + String token = Base64.getEncoder().encodeToString((member.getEmail() + ":" + member.getPassword()).getBytes()); - ResultActions response = mockMvc.perform(get("/search") + ResultActions response = mockMvc.perform(get("/members") + .header("Authorization", "Basic " + token) + .contentType(MediaType.APPLICATION_FORM_URLENCODED_VALUE) + ).andDo(print()); + + response.andExpect(status().isOk()) + .andExpect(MockMvcResultMatchers.jsonPath("$.length()").value(2)); + } + + @DisplayName("일반 사용자가 /members를 요청할 경우 권한이 없어야 한다.") + @Test + void request_members_fail_with_general_user() throws Exception { + Member member = new Member("b@b.com", "password", "b", "", Set.of("MEMBER")); + memberRepository.save(member); + String token = Base64.getEncoder().encodeToString((member.getEmail() + ":" + member.getPassword()).getBytes()); + + ResultActions response = mockMvc.perform(get("/members") .header("Authorization", "Basic " + token) .contentType(MediaType.APPLICATION_FORM_URLENCODED_VALUE) ).andDo(print()); diff --git a/src/test/java/nextstep/security/authorization/AuthorizationFilterTest.java b/src/test/java/nextstep/security/authorization/AuthorizationFilterTest.java new file mode 100644 index 0000000..f24c0ae --- /dev/null +++ b/src/test/java/nextstep/security/authorization/AuthorizationFilterTest.java @@ -0,0 +1,66 @@ +package nextstep.security.authorization; + +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import nextstep.security.authorization.manager.*; +import nextstep.security.matcher.AnyRequestMatcher; +import nextstep.security.matcher.MvcRequestMatcher; +import nextstep.security.matcher.RequestMatcherEntry; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.springframework.http.HttpMethod; +import org.springframework.mock.web.MockFilterChain; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; + +import java.util.ArrayList; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +class AuthorizationFilterTest { + + private final AuthorizationFilter authorizationFilter; + + AuthorizationFilterTest() { + List>> 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())); + RequestMatcherDelegatingAuthorizationManager authorizationManager = new RequestMatcherDelegatingAuthorizationManager(mappings); + this.authorizationFilter = new AuthorizationFilter(authorizationManager); + } + + @DisplayName("필터에서는 로그인, 검색 API는 인증을 진행하지 않는다.") + @ParameterizedTest + @ValueSource(strings = {"/login", "/search"}) + void filter_does_not_check_permit_all(String url) throws Exception { + MockHttpServletRequest request = new MockHttpServletRequest(); + MockHttpServletResponse response = new MockHttpServletResponse(); + MockFilterChain chain = new MockFilterChain(); + + request.setMethod("GET"); + request.setRequestURI(url); + authorizationFilter.doFilterInternal(request, response, chain); + + assertThat(chain.getRequest()).isNotNull(); + assertThat(response.getStatus()).isEqualTo(HttpServletResponse.SC_OK); + } + + @DisplayName("필터에서는 인증이 필요한 API는 검사한다. 인가에서 실패하면 Forbidden 발생") + @ParameterizedTest + @ValueSource(strings = {"/members", "/members/me", "/blah/blah"}) + void filter_check_apis(String url) { + MockHttpServletRequest request = new MockHttpServletRequest(); + MockHttpServletResponse response = new MockHttpServletResponse(); + MockFilterChain chain = new MockFilterChain(); + + request.setMethod("GET"); + request.setRequestURI(url); + assertThatThrownBy(() -> authorizationFilter.doFilterInternal(request, response, chain)) + .isInstanceOfAny(AccessDeniedException.class, ForbiddenException.class); + } +}