From b35062195e5877845a465ea4a092857f77d7409f Mon Sep 17 00:00:00 2001 From: JoJo Date: Thu, 8 Dec 2022 13:12:21 +0900 Subject: [PATCH 1/9] feat: add AuthorizeRequestMatcherRegistry --- .../access/matcher/MvcRequestMatcher.java | 2 +- .../AuthorizeRequestMatcherRegistry.java | 62 +++++++++++++++++++ 2 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 src/main/java/nextstep/security/config/AuthorizeRequestMatcherRegistry.java diff --git a/src/main/java/nextstep/security/access/matcher/MvcRequestMatcher.java b/src/main/java/nextstep/security/access/matcher/MvcRequestMatcher.java index abfb3d4..eca7322 100644 --- a/src/main/java/nextstep/security/access/matcher/MvcRequestMatcher.java +++ b/src/main/java/nextstep/security/access/matcher/MvcRequestMatcher.java @@ -28,6 +28,6 @@ public boolean matches(HttpServletRequest request) { return false; } - return request.getRequestURI().contains(pattern); + return request.getRequestURI().equals(pattern); } } diff --git a/src/main/java/nextstep/security/config/AuthorizeRequestMatcherRegistry.java b/src/main/java/nextstep/security/config/AuthorizeRequestMatcherRegistry.java new file mode 100644 index 0000000..ff5f505 --- /dev/null +++ b/src/main/java/nextstep/security/config/AuthorizeRequestMatcherRegistry.java @@ -0,0 +1,62 @@ +package nextstep.security.config; + +import nextstep.security.access.matcher.RequestMatcher; + +import javax.servlet.http.HttpServletRequest; +import java.util.HashMap; +import java.util.Map; + +public class AuthorizeRequestMatcherRegistry { + private final Map mappings = new HashMap<>(); + + public AuthorizedUrl matcher(RequestMatcher requestMatcher) { + return new AuthorizedUrl(requestMatcher); + } + + AuthorizeRequestMatcherRegistry addMapping(RequestMatcher requestMatcher, String attributes) { + mappings.put(requestMatcher, attributes); + return this; + } + + public String getAttribute(HttpServletRequest request) { + for (Map.Entry entry : mappings.entrySet()) { + if (entry.getKey().matches(request)) { + return entry.getValue(); + } + } + + return null; + } + + public class AuthorizedUrl { + public static final String PERMIT_ALL = "permitAll"; + public static final String DENY_ALL = "denyAll"; + public static final String AUTHENTICATED = "authenticated"; + private final RequestMatcher requestMatcher; + + public AuthorizedUrl(RequestMatcher requestMatcher) { + this.requestMatcher = requestMatcher; + } + + public AuthorizeRequestMatcherRegistry permitAll() { + return access(PERMIT_ALL); + } + + public AuthorizeRequestMatcherRegistry denyAll() { + return access(DENY_ALL); + } + + public AuthorizeRequestMatcherRegistry hasAuthority(String authority) { + return access("hasAuthority(" + authority + ")"); + } + + public AuthorizeRequestMatcherRegistry authenticated() { + return access(AUTHENTICATED); + } + + private AuthorizeRequestMatcherRegistry access(String attribute) { + return addMapping(requestMatcher, attribute); + } + } + +} From e136e3ec31543777893b866ef35f5cef25587269 Mon Sep 17 00:00:00 2001 From: hyunssooo Date: Sun, 11 Dec 2022 16:20:19 +0900 Subject: [PATCH 2/9] =?UTF-8?q?fix:=20=EC=9D=B8=EA=B0=80=20=EA=B3=BC?= =?UTF-8?q?=EC=A0=95=EC=97=90=EC=84=9C=20=EC=9D=B8=EC=A6=9D=20=EC=A0=95?= =?UTF-8?q?=EB=B3=B4=20=EB=B6=88=EB=9F=AC=EC=98=A4=EB=8A=94=20=EB=B0=A9?= =?UTF-8?q?=EB=B2=95=20=EC=88=98=EC=A0=95?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../authorization/AuthorizationFilter.java | 35 +++++++++++++++++-- .../HttpSessionSecurityContextRepository.java | 2 +- .../context/SecurityContextHolder.java | 2 +- 3 files changed, 34 insertions(+), 5 deletions(-) diff --git a/src/main/java/nextstep/security/authorization/AuthorizationFilter.java b/src/main/java/nextstep/security/authorization/AuthorizationFilter.java index 98065a6..9924e51 100644 --- a/src/main/java/nextstep/security/authorization/AuthorizationFilter.java +++ b/src/main/java/nextstep/security/authorization/AuthorizationFilter.java @@ -1,14 +1,18 @@ package nextstep.security.authorization; import java.io.IOException; +import java.util.Optional; import javax.servlet.FilterChain; import javax.servlet.ServletException; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import nextstep.security.authentication.Authentication; import nextstep.security.context.SecurityContext; +import nextstep.security.context.SecurityContextHolder; import nextstep.security.context.SecurityContextRepository; +import nextstep.security.exception.AuthenticationException; import nextstep.security.exception.AuthorizationException; import org.springframework.http.HttpStatus; import org.springframework.web.filter.GenericFilterBean; @@ -29,16 +33,41 @@ public void doFilter( ServletResponse response, FilterChain chain ) throws IOException, ServletException { + preAuthorization((HttpServletRequest) request); try { - final SecurityContext context = securityContextRepository.loadContext((HttpServletRequest) request); - if (context.getAuthentication().getAuthorities().stream().noneMatch(roleManager::hasRole)) { + final Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); + if (authentication == null) { + throw new AuthenticationException(); + } + + if (authentication.getAuthorities().stream().noneMatch(roleManager::hasRole)) { throw new AuthorizationException(); } + } catch (AuthenticationException e) { + ((HttpServletResponse) response).sendError( + HttpStatus.UNAUTHORIZED.value(), + HttpStatus.UNAUTHORIZED.getReasonPhrase() + ); + return; } catch (AuthorizationException e) { - ((HttpServletResponse) response).sendError(HttpStatus.FORBIDDEN.value(), HttpStatus.FORBIDDEN.getReasonPhrase()); + ((HttpServletResponse) response).sendError( + HttpStatus.FORBIDDEN.value(), + HttpStatus.FORBIDDEN.getReasonPhrase() + ); return; } chain.doFilter(request, response); } + + private void preAuthorization(HttpServletRequest request) { + final SecurityContext context = (SecurityContext) request.getSession().getAttribute( + SecurityContextHolder.SPRING_SECURITY_CONTEXT_KEY + ); + + final Authentication authentication = Optional.ofNullable(context) + .map(SecurityContext::getAuthentication) + .orElseGet(() -> securityContextRepository.loadContext(request).getAuthentication()); + SecurityContextHolder.getContext().setAuthentication(authentication); + } } diff --git a/src/main/java/nextstep/security/context/HttpSessionSecurityContextRepository.java b/src/main/java/nextstep/security/context/HttpSessionSecurityContextRepository.java index 45f3c56..5b8ab7e 100644 --- a/src/main/java/nextstep/security/context/HttpSessionSecurityContextRepository.java +++ b/src/main/java/nextstep/security/context/HttpSessionSecurityContextRepository.java @@ -5,7 +5,7 @@ import javax.servlet.http.HttpSession; public class HttpSessionSecurityContextRepository implements SecurityContextRepository { - public static final String SPRING_SECURITY_CONTEXT_KEY = "SPRING_SECURITY_CONTEXT"; + private static final String SPRING_SECURITY_CONTEXT_KEY = "SPRING_SECURITY_CONTEXT"; @Override public SecurityContext loadContext(HttpServletRequest request) { diff --git a/src/main/java/nextstep/security/context/SecurityContextHolder.java b/src/main/java/nextstep/security/context/SecurityContextHolder.java index f295d33..4f83053 100644 --- a/src/main/java/nextstep/security/context/SecurityContextHolder.java +++ b/src/main/java/nextstep/security/context/SecurityContextHolder.java @@ -1,7 +1,7 @@ package nextstep.security.context; public class SecurityContextHolder { - public static final String SPRING_SECURITY_CONTEXT_KEY = "SECURITY_CONTEXT"; + public static final String SPRING_SECURITY_CONTEXT_KEY = "SPRING_SECURITY_CONTEXT"; private static final ThreadLocal contextHolder; static { From facf63bb6efa204704b29bffce03985f7ad9ab89 Mon Sep 17 00:00:00 2001 From: hyunssooo Date: Mon, 12 Dec 2022 10:03:05 +0900 Subject: [PATCH 3/9] =?UTF-8?q?feat:=20SecurityFilterChain=20=ED=95=98?= =?UTF-8?q?=EB=82=98=EB=A7=8C=20=EC=82=AC=EC=9A=A9=ED=95=98=EB=8F=84?= =?UTF-8?q?=EB=A1=9D=20=EB=B3=80=EA=B2=BD?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../java/nextstep/app/config/AuthConfig.java | 45 ++++++------------- .../authorization/AuthorizationFilter.java | 33 +++++++++++--- .../AuthorizationSecurityFilterChain.java | 27 +++++++++++ .../exception/AccessDeniedException.java | 5 +++ 4 files changed, 72 insertions(+), 38 deletions(-) create mode 100644 src/main/java/nextstep/security/config/AuthorizationSecurityFilterChain.java create mode 100644 src/main/java/nextstep/security/exception/AccessDeniedException.java diff --git a/src/main/java/nextstep/app/config/AuthConfig.java b/src/main/java/nextstep/app/config/AuthConfig.java index 7e4e0f1..2cfcc69 100644 --- a/src/main/java/nextstep/app/config/AuthConfig.java +++ b/src/main/java/nextstep/app/config/AuthConfig.java @@ -1,13 +1,14 @@ package nextstep.app.config; +import java.util.List; import nextstep.security.access.matcher.MvcRequestMatcher; import nextstep.security.authentication.AuthenticationManager; import nextstep.security.authentication.BasicAuthenticationFilter; import nextstep.security.authentication.UsernamePasswordAuthenticationFilter; import nextstep.security.authentication.UsernamePasswordAuthenticationProvider; import nextstep.security.authorization.AuthorizationFilter; -import nextstep.security.authorization.RoleManager; -import nextstep.security.config.DefaultSecurityFilterChain; +import nextstep.security.config.AuthorizationSecurityFilterChain; +import nextstep.security.config.AuthorizeRequestMatcherRegistry; import nextstep.security.config.FilterChainProxy; import nextstep.security.config.SecurityFilterChain; import nextstep.security.context.HttpSessionSecurityContextRepository; @@ -17,6 +18,7 @@ import org.springframework.context.annotation.Configuration; import org.springframework.http.HttpMethod; import org.springframework.web.filter.DelegatingFilterProxy; +import org.springframework.web.method.support.HandlerMethodArgumentResolver; import org.springframework.web.servlet.config.annotation.WebMvcConfigurer; @Configuration @@ -34,48 +36,29 @@ public DelegatingFilterProxy securityFilterChainProxy() { } @Bean - public FilterChainProxy filterChainProxy( - SecurityFilterChain loginSecurityFilterChain, - SecurityFilterChain membersSecurityFilterChain - ) { - return new FilterChainProxy( - loginSecurityFilterChain, - membersSecurityFilterChain - ); + public FilterChainProxy filterChainProxy(SecurityFilterChain securityFilterChain) { + return new FilterChainProxy(securityFilterChain); } - @Bean - public SecurityFilterChain loginSecurityFilterChain( + public SecurityFilterChain securityFilterChain( AuthenticationManager authenticationManager, SecurityContextRepository securityContextRepository ) { - return new DefaultSecurityFilterChain( - new MvcRequestMatcher( - HttpMethod.POST, - "/login" - ), + return new AuthorizationSecurityFilterChain( new UsernamePasswordAuthenticationFilter( authenticationManager, securityContextRepository - ) - ); - } - - @Bean - public SecurityFilterChain membersSecurityFilterChain( - AuthenticationManager authenticationManager, - SecurityContextRepository securityContextRepository - ) { - return new DefaultSecurityFilterChain( - new MvcRequestMatcher( - HttpMethod.GET, - "/members" ), new BasicAuthenticationFilter( authenticationManager, securityContextRepository ), - new AuthorizationFilter(securityContextRepository, new RoleManager("ADMIN")) + new AuthorizationFilter( + securityContextRepository, + new AuthorizeRequestMatcherRegistry() + .matcher(new MvcRequestMatcher(HttpMethod.GET, "/members")).hasAuthority("ADMIN") + .matcher(new MvcRequestMatcher(HttpMethod.GET, "/members/me")).authenticated() + ) ); } diff --git a/src/main/java/nextstep/security/authorization/AuthorizationFilter.java b/src/main/java/nextstep/security/authorization/AuthorizationFilter.java index 9924e51..96f16a3 100644 --- a/src/main/java/nextstep/security/authorization/AuthorizationFilter.java +++ b/src/main/java/nextstep/security/authorization/AuthorizationFilter.java @@ -9,9 +9,12 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import nextstep.security.authentication.Authentication; +import nextstep.security.config.AuthorizeRequestMatcherRegistry; +import nextstep.security.config.AuthorizeRequestMatcherRegistry.AuthorizedUrl; import nextstep.security.context.SecurityContext; import nextstep.security.context.SecurityContextHolder; import nextstep.security.context.SecurityContextRepository; +import nextstep.security.exception.AccessDeniedException; import nextstep.security.exception.AuthenticationException; import nextstep.security.exception.AuthorizationException; import org.springframework.http.HttpStatus; @@ -20,11 +23,14 @@ public class AuthorizationFilter extends GenericFilterBean { private final SecurityContextRepository securityContextRepository; - private final RoleManager roleManager; + private final AuthorizeRequestMatcherRegistry authorizeRequestMatcherRegistry; - public AuthorizationFilter(SecurityContextRepository securityContextRepository, RoleManager roleManager) { + public AuthorizationFilter( + SecurityContextRepository securityContextRepository, + AuthorizeRequestMatcherRegistry authorizeRequestMatcherRegistry + ) { this.securityContextRepository = securityContextRepository; - this.roleManager = roleManager; + this.authorizeRequestMatcherRegistry = authorizeRequestMatcherRegistry; } @Override @@ -36,11 +42,18 @@ public void doFilter( preAuthorization((HttpServletRequest) request); try { final Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); - if (authentication == null) { - throw new AuthenticationException(); - } - if (authentication.getAuthorities().stream().noneMatch(roleManager::hasRole)) { + final String attribute = authorizeRequestMatcherRegistry.getAttribute((HttpServletRequest) request); + if (attribute == null || AuthorizedUrl.PERMIT_ALL.equals(attribute)) { + chain.doFilter(request, response); + return; + } else if (AuthorizedUrl.DENY_ALL.equals(attribute)) { + throw new AccessDeniedException(); + } else if (AuthorizedUrl.AUTHENTICATED.equals(attribute)) { + if (authentication == null) { + throw new AuthenticationException(); + } + } else if (authentication.getAuthorities().stream().noneMatch(it -> attribute.contains(it))) { throw new AuthorizationException(); } } catch (AuthenticationException e) { @@ -55,6 +68,12 @@ public void doFilter( HttpStatus.FORBIDDEN.getReasonPhrase() ); return; + } catch (AccessDeniedException e) { + ((HttpServletResponse) response).sendError( + HttpStatus.BAD_REQUEST.value(), + HttpStatus.BAD_REQUEST.getReasonPhrase() + ); + return; } chain.doFilter(request, response); diff --git a/src/main/java/nextstep/security/config/AuthorizationSecurityFilterChain.java b/src/main/java/nextstep/security/config/AuthorizationSecurityFilterChain.java new file mode 100644 index 0000000..7800413 --- /dev/null +++ b/src/main/java/nextstep/security/config/AuthorizationSecurityFilterChain.java @@ -0,0 +1,27 @@ +package nextstep.security.config; + +import java.util.List; +import javax.servlet.Filter; +import javax.servlet.http.HttpServletRequest; + +public class AuthorizationSecurityFilterChain implements SecurityFilterChain { + private final List filters; + + public AuthorizationSecurityFilterChain(List filters) { + this.filters = filters; + } + + public AuthorizationSecurityFilterChain(Filter... filters) { + this(List.of(filters)); + } + + @Override + public boolean matches(HttpServletRequest request) { + return true; + } + + @Override + public List getFilters() { + return filters; + } +} diff --git a/src/main/java/nextstep/security/exception/AccessDeniedException.java b/src/main/java/nextstep/security/exception/AccessDeniedException.java new file mode 100644 index 0000000..3eb62c1 --- /dev/null +++ b/src/main/java/nextstep/security/exception/AccessDeniedException.java @@ -0,0 +1,5 @@ +package nextstep.security.exception; + +public class AccessDeniedException extends RuntimeException { + +} From 76a93c2129ce96c8a922531a3e6819a54e5536c2 Mon Sep 17 00:00:00 2001 From: hyunssooo Date: Mon, 12 Dec 2022 10:05:48 +0900 Subject: [PATCH 4/9] =?UTF-8?q?feat:=20PreAuthorizationFilter=20=EC=B6=94?= =?UTF-8?q?=EA=B0=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../java/nextstep/app/config/AuthConfig.java | 3 +- .../authorization/AuthorizationFilter.java | 22 +------- .../authorization/PreAuthorizationFilter.java | 54 +++++++++++++++++++ 3 files changed, 57 insertions(+), 22 deletions(-) create mode 100644 src/main/java/nextstep/security/authorization/PreAuthorizationFilter.java diff --git a/src/main/java/nextstep/app/config/AuthConfig.java b/src/main/java/nextstep/app/config/AuthConfig.java index 2cfcc69..77a48a4 100644 --- a/src/main/java/nextstep/app/config/AuthConfig.java +++ b/src/main/java/nextstep/app/config/AuthConfig.java @@ -7,6 +7,7 @@ import nextstep.security.authentication.UsernamePasswordAuthenticationFilter; import nextstep.security.authentication.UsernamePasswordAuthenticationProvider; import nextstep.security.authorization.AuthorizationFilter; +import nextstep.security.authorization.PreAuthorizationFilter; import nextstep.security.config.AuthorizationSecurityFilterChain; import nextstep.security.config.AuthorizeRequestMatcherRegistry; import nextstep.security.config.FilterChainProxy; @@ -53,8 +54,8 @@ public SecurityFilterChain securityFilterChain( authenticationManager, securityContextRepository ), + new PreAuthorizationFilter(securityContextRepository), new AuthorizationFilter( - securityContextRepository, new AuthorizeRequestMatcherRegistry() .matcher(new MvcRequestMatcher(HttpMethod.GET, "/members")).hasAuthority("ADMIN") .matcher(new MvcRequestMatcher(HttpMethod.GET, "/members/me")).authenticated() diff --git a/src/main/java/nextstep/security/authorization/AuthorizationFilter.java b/src/main/java/nextstep/security/authorization/AuthorizationFilter.java index 96f16a3..c8b3331 100644 --- a/src/main/java/nextstep/security/authorization/AuthorizationFilter.java +++ b/src/main/java/nextstep/security/authorization/AuthorizationFilter.java @@ -1,7 +1,6 @@ package nextstep.security.authorization; import java.io.IOException; -import java.util.Optional; import javax.servlet.FilterChain; import javax.servlet.ServletException; import javax.servlet.ServletRequest; @@ -11,9 +10,7 @@ import nextstep.security.authentication.Authentication; import nextstep.security.config.AuthorizeRequestMatcherRegistry; import nextstep.security.config.AuthorizeRequestMatcherRegistry.AuthorizedUrl; -import nextstep.security.context.SecurityContext; import nextstep.security.context.SecurityContextHolder; -import nextstep.security.context.SecurityContextRepository; import nextstep.security.exception.AccessDeniedException; import nextstep.security.exception.AuthenticationException; import nextstep.security.exception.AuthorizationException; @@ -22,14 +19,9 @@ public class AuthorizationFilter extends GenericFilterBean { - private final SecurityContextRepository securityContextRepository; private final AuthorizeRequestMatcherRegistry authorizeRequestMatcherRegistry; - public AuthorizationFilter( - SecurityContextRepository securityContextRepository, - AuthorizeRequestMatcherRegistry authorizeRequestMatcherRegistry - ) { - this.securityContextRepository = securityContextRepository; + public AuthorizationFilter(AuthorizeRequestMatcherRegistry authorizeRequestMatcherRegistry) { this.authorizeRequestMatcherRegistry = authorizeRequestMatcherRegistry; } @@ -39,7 +31,6 @@ public void doFilter( ServletResponse response, FilterChain chain ) throws IOException, ServletException { - preAuthorization((HttpServletRequest) request); try { final Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); @@ -78,15 +69,4 @@ public void doFilter( chain.doFilter(request, response); } - - private void preAuthorization(HttpServletRequest request) { - final SecurityContext context = (SecurityContext) request.getSession().getAttribute( - SecurityContextHolder.SPRING_SECURITY_CONTEXT_KEY - ); - - final Authentication authentication = Optional.ofNullable(context) - .map(SecurityContext::getAuthentication) - .orElseGet(() -> securityContextRepository.loadContext(request).getAuthentication()); - SecurityContextHolder.getContext().setAuthentication(authentication); - } } diff --git a/src/main/java/nextstep/security/authorization/PreAuthorizationFilter.java b/src/main/java/nextstep/security/authorization/PreAuthorizationFilter.java new file mode 100644 index 0000000..6a8dde0 --- /dev/null +++ b/src/main/java/nextstep/security/authorization/PreAuthorizationFilter.java @@ -0,0 +1,54 @@ +package nextstep.security.authorization; + +import java.io.IOException; +import java.util.Optional; +import javax.servlet.FilterChain; +import javax.servlet.ServletException; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServletRequest; +import nextstep.security.authentication.Authentication; +import nextstep.security.context.SecurityContext; +import nextstep.security.context.SecurityContextHolder; +import nextstep.security.context.SecurityContextRepository; +import org.springframework.web.filter.GenericFilterBean; + +public class PreAuthorizationFilter extends GenericFilterBean { + + private final SecurityContextRepository securityContextRepository; + + public PreAuthorizationFilter(SecurityContextRepository securityContextRepository) { + this.securityContextRepository = securityContextRepository; + } + + @Override + public void doFilter( + ServletRequest request, + ServletResponse response, + FilterChain chain + ) throws IOException, ServletException { + try { + if (SecurityContextHolder.getContext().getAuthentication() != null) { + chain.doFilter(request, response); + return; + } + + final HttpServletRequest httpServletRequest = (HttpServletRequest) request; + SecurityContext context = Optional.ofNullable( + (SecurityContext) httpServletRequest.getSession() + .getAttribute(SecurityContextHolder.SPRING_SECURITY_CONTEXT_KEY) + ) + .orElseGet(() -> securityContextRepository.loadContext(httpServletRequest)); + + final Authentication authentication = Optional.ofNullable(context) + .map(it -> it.getAuthentication()) + .orElse(null); + + SecurityContextHolder.getContext().setAuthentication(authentication); + + } catch (Exception ignored) { + + } + chain.doFilter(request, response); + } +} From 3d06e2d8c75a85bdf4d79efdc7ea177c311820c9 Mon Sep 17 00:00:00 2001 From: hyunssooo Date: Mon, 12 Dec 2022 10:06:21 +0900 Subject: [PATCH 5/9] =?UTF-8?q?feat:=20members/me=20api=20=EC=B6=94?= =?UTF-8?q?=EA=B0=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../java/nextstep/app/config/AuthConfig.java | 4 ++ .../app/config/LoginUserArgumentResolver.java | 31 ++++++++++++++++ .../nextstep/app/ui/MemberController.java | 17 ++++++--- .../java/nextstep/app/ui/dto/LoginUser.java | 20 ++++++++++ .../nextstep/app/MemberAcceptanceTest.java | 37 +++++++++++++++++++ 5 files changed, 104 insertions(+), 5 deletions(-) create mode 100644 src/main/java/nextstep/app/config/LoginUserArgumentResolver.java create mode 100644 src/main/java/nextstep/app/ui/dto/LoginUser.java diff --git a/src/main/java/nextstep/app/config/AuthConfig.java b/src/main/java/nextstep/app/config/AuthConfig.java index 77a48a4..881f87a 100644 --- a/src/main/java/nextstep/app/config/AuthConfig.java +++ b/src/main/java/nextstep/app/config/AuthConfig.java @@ -73,4 +73,8 @@ public AuthenticationManager authenticationManager() { return new AuthenticationManager(new UsernamePasswordAuthenticationProvider(userDetailsService)); } + @Override + public void addArgumentResolvers(List resolvers) { + resolvers.add(new LoginUserArgumentResolver()); + } } diff --git a/src/main/java/nextstep/app/config/LoginUserArgumentResolver.java b/src/main/java/nextstep/app/config/LoginUserArgumentResolver.java new file mode 100644 index 0000000..9f11daf --- /dev/null +++ b/src/main/java/nextstep/app/config/LoginUserArgumentResolver.java @@ -0,0 +1,31 @@ +package nextstep.app.config; + +import javax.servlet.http.HttpServletRequest; +import nextstep.app.ui.dto.LoginUser; +import nextstep.security.context.SecurityContext; +import org.springframework.core.MethodParameter; +import org.springframework.web.bind.support.WebDataBinderFactory; +import org.springframework.web.context.request.NativeWebRequest; +import org.springframework.web.method.support.HandlerMethodArgumentResolver; +import org.springframework.web.method.support.ModelAndViewContainer; + +public class LoginUserArgumentResolver implements HandlerMethodArgumentResolver { + + @Override + public boolean supportsParameter(MethodParameter parameter) { + return parameter.getParameterType().equals(LoginUser.class); + } + + @Override + public LoginUser resolveArgument( + MethodParameter parameter, + ModelAndViewContainer mavContainer, + NativeWebRequest webRequest, + WebDataBinderFactory binderFactory + ) { + final HttpServletRequest request = (HttpServletRequest) webRequest.getNativeRequest(); + final SecurityContext context = (SecurityContext) request.getSession().getAttribute("SPRING_SECURITY_CONTEXT"); + final String email = context.getAuthentication().getPrincipal().toString(); + return new LoginUser(email); + } +} diff --git a/src/main/java/nextstep/app/ui/MemberController.java b/src/main/java/nextstep/app/ui/MemberController.java index fce08d7..a527474 100644 --- a/src/main/java/nextstep/app/ui/MemberController.java +++ b/src/main/java/nextstep/app/ui/MemberController.java @@ -1,16 +1,17 @@ package nextstep.app.ui; -import nextstep.security.authentication.Authentication; -import nextstep.security.context.SecurityContextHolder; +import java.util.List; import nextstep.app.domain.Member; import nextstep.app.domain.MemberRepository; +import nextstep.app.ui.dto.LoginUser; +import nextstep.security.exception.AuthenticationException; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RestController; -import java.util.List; - @RestController +@RequestMapping("/members") public class MemberController { private final MemberRepository memberRepository; @@ -19,10 +20,16 @@ public MemberController(MemberRepository memberRepository) { this.memberRepository = memberRepository; } - @GetMapping("/members") + @GetMapping public ResponseEntity> list() { List members = memberRepository.findAll(); return ResponseEntity.ok(members); } + @GetMapping("/me") + public ResponseEntity me(LoginUser loginUser) { + final Member member = memberRepository.findByEmail(loginUser.getEmail()) + .orElseThrow(() -> new AuthenticationException()); + return ResponseEntity.ok(member); + } } diff --git a/src/main/java/nextstep/app/ui/dto/LoginUser.java b/src/main/java/nextstep/app/ui/dto/LoginUser.java new file mode 100644 index 0000000..acbecba --- /dev/null +++ b/src/main/java/nextstep/app/ui/dto/LoginUser.java @@ -0,0 +1,20 @@ +package nextstep.app.ui.dto; + +public class LoginUser { + private final String email; + + public LoginUser(String email) { + this.email = email; + } + + public String getEmail() { + return email; + } + + @Override + public String toString() { + return "LoginUser{" + + "email='" + email + '\'' + + '}'; + } +} diff --git a/src/test/java/nextstep/app/MemberAcceptanceTest.java b/src/test/java/nextstep/app/MemberAcceptanceTest.java index 264733e..44755e9 100644 --- a/src/test/java/nextstep/app/MemberAcceptanceTest.java +++ b/src/test/java/nextstep/app/MemberAcceptanceTest.java @@ -57,6 +57,43 @@ void get_members_after_form_login_user() { assertThat(memberResponse.statusCode()).isEqualTo(HttpStatus.FORBIDDEN.value()); } + @DisplayName("인증된 사용자는 200 응답을 받고 members/me 조회가 가능하다.") + @Test + void get_me_after_form_login_user() { + final String loginEmail = USER.getEmail(); + final ExtractableResponse loginResponse = 로그인(loginEmail, USER.getPassword()); + + ExtractableResponse memberResponse = RestAssured.given().log().all() + .cookies(loginResponse.cookies()) + .contentType(MediaType.APPLICATION_JSON_VALUE) + .when() + .get("/members/me") + .then().log().all() + .extract(); + + assertAll( + () -> assertThat(memberResponse.statusCode()).isEqualTo(HttpStatus.OK.value()), + () -> { + Member me = memberResponse.as(Member.class); + assertThat(me.getEmail()).isEqualTo(loginEmail); + } + ); + } + + @DisplayName("인증되지 않은 사용자는 401 응답을 받고 members/me 조회가 불가능하다.") + @Test + void get_me_after_form_not_login_user() { + + ExtractableResponse memberResponse = RestAssured.given().log().all() + .contentType(MediaType.APPLICATION_JSON_VALUE) + .when() + .get("/members/me") + .then().log().all() + .extract(); + + assertThat(memberResponse.statusCode()).isEqualTo(HttpStatus.UNAUTHORIZED.value()); + } + private ExtractableResponse 로그인(String username, String password) { return RestAssured.given().log().all() .formParams(Map.of( From 499702f60677a3e2cce3ba3fac173b52ec60c6f2 Mon Sep 17 00:00:00 2001 From: hyunssooo Date: Mon, 12 Dec 2022 18:56:01 +0900 Subject: [PATCH 6/9] =?UTF-8?q?fix:=20rename=20AuthorizationSecurityFilter?= =?UTF-8?q?Chain=20->=20DefaultSecurityFilterChain,=20=EA=B5=AC=ED=98=84?= =?UTF-8?q?=20=EB=82=B4=EC=9A=A9=20=EB=B3=80=EA=B2=BD?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../java/nextstep/app/config/AuthConfig.java | 4 +- .../AuthorizationSecurityFilterChain.java | 27 -------- .../config/DefaultSecurityFilterChain.java | 16 ++--- .../security/config/FilterChainProxyTest.java | 66 +++++++++++-------- 4 files changed, 45 insertions(+), 68 deletions(-) delete mode 100644 src/main/java/nextstep/security/config/AuthorizationSecurityFilterChain.java diff --git a/src/main/java/nextstep/app/config/AuthConfig.java b/src/main/java/nextstep/app/config/AuthConfig.java index 881f87a..cd8cb3e 100644 --- a/src/main/java/nextstep/app/config/AuthConfig.java +++ b/src/main/java/nextstep/app/config/AuthConfig.java @@ -8,7 +8,7 @@ import nextstep.security.authentication.UsernamePasswordAuthenticationProvider; import nextstep.security.authorization.AuthorizationFilter; import nextstep.security.authorization.PreAuthorizationFilter; -import nextstep.security.config.AuthorizationSecurityFilterChain; +import nextstep.security.config.DefaultSecurityFilterChain; import nextstep.security.config.AuthorizeRequestMatcherRegistry; import nextstep.security.config.FilterChainProxy; import nextstep.security.config.SecurityFilterChain; @@ -45,7 +45,7 @@ public SecurityFilterChain securityFilterChain( AuthenticationManager authenticationManager, SecurityContextRepository securityContextRepository ) { - return new AuthorizationSecurityFilterChain( + return new DefaultSecurityFilterChain( new UsernamePasswordAuthenticationFilter( authenticationManager, securityContextRepository diff --git a/src/main/java/nextstep/security/config/AuthorizationSecurityFilterChain.java b/src/main/java/nextstep/security/config/AuthorizationSecurityFilterChain.java deleted file mode 100644 index 7800413..0000000 --- a/src/main/java/nextstep/security/config/AuthorizationSecurityFilterChain.java +++ /dev/null @@ -1,27 +0,0 @@ -package nextstep.security.config; - -import java.util.List; -import javax.servlet.Filter; -import javax.servlet.http.HttpServletRequest; - -public class AuthorizationSecurityFilterChain implements SecurityFilterChain { - private final List filters; - - public AuthorizationSecurityFilterChain(List filters) { - this.filters = filters; - } - - public AuthorizationSecurityFilterChain(Filter... filters) { - this(List.of(filters)); - } - - @Override - public boolean matches(HttpServletRequest request) { - return true; - } - - @Override - public List getFilters() { - return filters; - } -} diff --git a/src/main/java/nextstep/security/config/DefaultSecurityFilterChain.java b/src/main/java/nextstep/security/config/DefaultSecurityFilterChain.java index f27eabf..fc8b2ec 100644 --- a/src/main/java/nextstep/security/config/DefaultSecurityFilterChain.java +++ b/src/main/java/nextstep/security/config/DefaultSecurityFilterChain.java @@ -1,29 +1,23 @@ package nextstep.security.config; - -import nextstep.security.access.matcher.RequestMatcher; - +import java.util.List; import javax.servlet.Filter; import javax.servlet.http.HttpServletRequest; -import java.util.List; public class DefaultSecurityFilterChain implements SecurityFilterChain { - - private final RequestMatcher requestMatcher; private final List filters; - public DefaultSecurityFilterChain(RequestMatcher requestMatcher, List filters) { - this.requestMatcher = requestMatcher; + public DefaultSecurityFilterChain(List filters) { this.filters = filters; } - public DefaultSecurityFilterChain(RequestMatcher requestMatcher, Filter... filters) { - this(requestMatcher, List.of(filters)); + public DefaultSecurityFilterChain(Filter... filters) { + this(List.of(filters)); } @Override public boolean matches(HttpServletRequest request) { - return requestMatcher.matches(request); + return true; } @Override diff --git a/src/test/java/nextstep/security/config/FilterChainProxyTest.java b/src/test/java/nextstep/security/config/FilterChainProxyTest.java index 475d08d..47354f1 100644 --- a/src/test/java/nextstep/security/config/FilterChainProxyTest.java +++ b/src/test/java/nextstep/security/config/FilterChainProxyTest.java @@ -1,20 +1,23 @@ package nextstep.security.config; -import nextstep.security.access.matcher.MvcRequestMatcher; +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertAll; + +import java.io.IOException; +import javax.servlet.Filter; +import javax.servlet.FilterChain; +import javax.servlet.ServletException; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServletRequest; import nextstep.security.fixture.MockFilterChain; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.http.HttpMethod; import org.springframework.mock.web.MockHttpServletRequest; -import javax.servlet.*; -import javax.servlet.http.HttpServletRequest; -import java.io.IOException; -import java.util.List; - -import static org.assertj.core.api.Assertions.assertThat; +class FilterChainProxyTest { -public class FilterChainProxyTest { private FilterChainProxy filterChainProxy; private TestFilter loginTestFilter; private TestFilter membersTestFilter; @@ -22,36 +25,36 @@ public class FilterChainProxyTest { @BeforeEach void setUp() { loginTestFilter = new TestFilter(); - SecurityFilterChain loginFilterChain = new DefaultSecurityFilterChain( - new MvcRequestMatcher(HttpMethod.POST, "/login"), - List.of(loginTestFilter) - ); - membersTestFilter = new TestFilter(); - SecurityFilterChain membersFilterChain = new DefaultSecurityFilterChain( - new MvcRequestMatcher(HttpMethod.GET, "/members"), - List.of(membersTestFilter) - ); - filterChainProxy = new FilterChainProxy(List.of(loginFilterChain, membersFilterChain)); + filterChainProxy = new FilterChainProxy( + new DefaultSecurityFilterChain( + loginTestFilter, + membersTestFilter + ) + ); } @Test void login() throws ServletException, IOException { HttpServletRequest request = new MockHttpServletRequest(HttpMethod.POST.name(), "/login"); - filterChainProxy.doFilter(request, null, null); + filterChainProxy.doFilter(request, null, new MockFilterChain()); - assertThat(loginTestFilter.count).isEqualTo(1); - assertThat(membersTestFilter.count).isEqualTo(0); + assertAll( + () -> assertThat(loginTestFilter.count).isEqualTo(1), + () -> assertThat(membersTestFilter.count).isEqualTo(1) + ); } @Test void members() throws ServletException, IOException { HttpServletRequest request = new MockHttpServletRequest(HttpMethod.GET.name(), "/members"); - filterChainProxy.doFilter(request, null, null); + filterChainProxy.doFilter(request, null, new MockFilterChain()); - assertThat(loginTestFilter.count).isEqualTo(0); - assertThat(membersTestFilter.count).isEqualTo(1); + assertAll( + () -> assertThat(loginTestFilter.count).isEqualTo(1), + () -> assertThat(membersTestFilter.count).isEqualTo(1) + ); } @Test @@ -59,17 +62,24 @@ void none() throws ServletException, IOException { HttpServletRequest request = new MockHttpServletRequest(HttpMethod.GET.name(), "/test"); filterChainProxy.doFilter(request, null, new MockFilterChain()); - - assertThat(loginTestFilter.count).isEqualTo(0); - assertThat(membersTestFilter.count).isEqualTo(0); + assertAll( + () -> assertThat(loginTestFilter.count).isEqualTo(1), + () -> assertThat(membersTestFilter.count).isEqualTo(1) + ); } private static class TestFilter implements Filter { + private int count = 0; @Override - public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException { + public void doFilter( + ServletRequest request, + ServletResponse response, + FilterChain chain + ) throws ServletException, IOException { count++; + chain.doFilter(request, response); } } } From 44ebd1602d900d4d71eb58ba12b94014e765b2dc Mon Sep 17 00:00:00 2001 From: hyunssooo Date: Tue, 13 Dec 2022 00:10:49 +0900 Subject: [PATCH 7/9] =?UTF-8?q?feat:=20RoleManager=20=EC=A0=81=EC=9A=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../authorization/AuthorizationFilter.java | 25 ++++++++------ .../security/authorization/RoleManager.java | 20 ----------- .../manager/AuthenticationRoleManager.java | 11 ++++++ .../manager/AuthorizationRoleManager.java | 23 +++++++++++++ .../manager/DenyAllRoleManager.java | 11 ++++++ .../manager/PermitAllRoleManager.java | 11 ++++++ .../authorization/manager/RoleManager.java | 8 +++++ .../AuthorizeRequestMatcherRegistry.java | 34 +++++++++++-------- 8 files changed, 97 insertions(+), 46 deletions(-) delete mode 100644 src/main/java/nextstep/security/authorization/RoleManager.java create mode 100644 src/main/java/nextstep/security/authorization/manager/AuthenticationRoleManager.java create mode 100644 src/main/java/nextstep/security/authorization/manager/AuthorizationRoleManager.java create mode 100644 src/main/java/nextstep/security/authorization/manager/DenyAllRoleManager.java create mode 100644 src/main/java/nextstep/security/authorization/manager/PermitAllRoleManager.java create mode 100644 src/main/java/nextstep/security/authorization/manager/RoleManager.java diff --git a/src/main/java/nextstep/security/authorization/AuthorizationFilter.java b/src/main/java/nextstep/security/authorization/AuthorizationFilter.java index c8b3331..046a67c 100644 --- a/src/main/java/nextstep/security/authorization/AuthorizationFilter.java +++ b/src/main/java/nextstep/security/authorization/AuthorizationFilter.java @@ -1,6 +1,7 @@ package nextstep.security.authorization; import java.io.IOException; +import java.util.Optional; import javax.servlet.FilterChain; import javax.servlet.ServletException; import javax.servlet.ServletRequest; @@ -8,8 +9,8 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import nextstep.security.authentication.Authentication; +import nextstep.security.authorization.manager.RoleManager; import nextstep.security.config.AuthorizeRequestMatcherRegistry; -import nextstep.security.config.AuthorizeRequestMatcherRegistry.AuthorizedUrl; import nextstep.security.context.SecurityContextHolder; import nextstep.security.exception.AccessDeniedException; import nextstep.security.exception.AuthenticationException; @@ -32,19 +33,21 @@ public void doFilter( FilterChain chain ) throws IOException, ServletException { try { - final Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); + final Authentication authentication = Optional.ofNullable( + SecurityContextHolder + .getContext() + .getAuthentication() + ).orElseThrow(AuthenticationException::new); - final String attribute = authorizeRequestMatcherRegistry.getAttribute((HttpServletRequest) request); - if (attribute == null || AuthorizedUrl.PERMIT_ALL.equals(attribute)) { + final RoleManager roleManager = authorizeRequestMatcherRegistry.getRoleManager((HttpServletRequest) request); + + if (roleManager == null) { chain.doFilter(request, response); return; - } else if (AuthorizedUrl.DENY_ALL.equals(attribute)) { - throw new AccessDeniedException(); - } else if (AuthorizedUrl.AUTHENTICATED.equals(attribute)) { - if (authentication == null) { - throw new AuthenticationException(); - } - } else if (authentication.getAuthorities().stream().noneMatch(it -> attribute.contains(it))) { + } + + + if (!roleManager.check(authentication)) { throw new AuthorizationException(); } } catch (AuthenticationException e) { diff --git a/src/main/java/nextstep/security/authorization/RoleManager.java b/src/main/java/nextstep/security/authorization/RoleManager.java deleted file mode 100644 index 884fb4b..0000000 --- a/src/main/java/nextstep/security/authorization/RoleManager.java +++ /dev/null @@ -1,20 +0,0 @@ -package nextstep.security.authorization; - -import java.util.Set; - -public class RoleManager { - - private final Set roles; - - public RoleManager(Set roles) { - this.roles = roles; - } - - public RoleManager(String... roles) { - this(Set.of(roles)); - } - - public boolean hasRole(String role) { - return roles.contains(role); - } -} diff --git a/src/main/java/nextstep/security/authorization/manager/AuthenticationRoleManager.java b/src/main/java/nextstep/security/authorization/manager/AuthenticationRoleManager.java new file mode 100644 index 0000000..35b803b --- /dev/null +++ b/src/main/java/nextstep/security/authorization/manager/AuthenticationRoleManager.java @@ -0,0 +1,11 @@ +package nextstep.security.authorization.manager; + +import nextstep.security.authentication.Authentication; + +public class AuthenticationRoleManager implements RoleManager { + + @Override + public boolean check(Authentication authentication) { + return authentication.isAuthenticated(); + } +} diff --git a/src/main/java/nextstep/security/authorization/manager/AuthorizationRoleManager.java b/src/main/java/nextstep/security/authorization/manager/AuthorizationRoleManager.java new file mode 100644 index 0000000..17138ea --- /dev/null +++ b/src/main/java/nextstep/security/authorization/manager/AuthorizationRoleManager.java @@ -0,0 +1,23 @@ +package nextstep.security.authorization.manager; + +import java.util.Set; +import nextstep.security.authentication.Authentication; + +public class AuthorizationRoleManager implements RoleManager { + + private final Set authorities; + + public AuthorizationRoleManager(Set authorities) { + this.authorities = authorities; + } + + public AuthorizationRoleManager(String... authorities) { + this(Set.of(authorities)); + } + + @Override + public boolean check(Authentication authentication) { + return authentication.getAuthorities().stream() + .anyMatch(authorities::contains); + } +} diff --git a/src/main/java/nextstep/security/authorization/manager/DenyAllRoleManager.java b/src/main/java/nextstep/security/authorization/manager/DenyAllRoleManager.java new file mode 100644 index 0000000..9e6cc84 --- /dev/null +++ b/src/main/java/nextstep/security/authorization/manager/DenyAllRoleManager.java @@ -0,0 +1,11 @@ +package nextstep.security.authorization.manager; + +import nextstep.security.authentication.Authentication; + +public class DenyAllRoleManager implements RoleManager { + + @Override + public boolean check(Authentication authentication) { + return false; + } +} diff --git a/src/main/java/nextstep/security/authorization/manager/PermitAllRoleManager.java b/src/main/java/nextstep/security/authorization/manager/PermitAllRoleManager.java new file mode 100644 index 0000000..d2d85cc --- /dev/null +++ b/src/main/java/nextstep/security/authorization/manager/PermitAllRoleManager.java @@ -0,0 +1,11 @@ +package nextstep.security.authorization.manager; + +import nextstep.security.authentication.Authentication; + +public class PermitAllRoleManager implements RoleManager { + + @Override + public boolean check(Authentication authentication) { + return true; + } +} diff --git a/src/main/java/nextstep/security/authorization/manager/RoleManager.java b/src/main/java/nextstep/security/authorization/manager/RoleManager.java new file mode 100644 index 0000000..302281a --- /dev/null +++ b/src/main/java/nextstep/security/authorization/manager/RoleManager.java @@ -0,0 +1,8 @@ +package nextstep.security.authorization.manager; + +import nextstep.security.authentication.Authentication; + +public interface RoleManager { + + boolean check(Authentication authentication); +} diff --git a/src/main/java/nextstep/security/config/AuthorizeRequestMatcherRegistry.java b/src/main/java/nextstep/security/config/AuthorizeRequestMatcherRegistry.java index ff5f505..6f31234 100644 --- a/src/main/java/nextstep/security/config/AuthorizeRequestMatcherRegistry.java +++ b/src/main/java/nextstep/security/config/AuthorizeRequestMatcherRegistry.java @@ -1,25 +1,29 @@ package nextstep.security.config; -import nextstep.security.access.matcher.RequestMatcher; - -import javax.servlet.http.HttpServletRequest; import java.util.HashMap; import java.util.Map; +import javax.servlet.http.HttpServletRequest; +import nextstep.security.access.matcher.RequestMatcher; +import nextstep.security.authorization.manager.AuthenticationRoleManager; +import nextstep.security.authorization.manager.AuthorizationRoleManager; +import nextstep.security.authorization.manager.DenyAllRoleManager; +import nextstep.security.authorization.manager.PermitAllRoleManager; +import nextstep.security.authorization.manager.RoleManager; public class AuthorizeRequestMatcherRegistry { - private final Map mappings = new HashMap<>(); + private final Map mappings = new HashMap<>(); public AuthorizedUrl matcher(RequestMatcher requestMatcher) { return new AuthorizedUrl(requestMatcher); } - AuthorizeRequestMatcherRegistry addMapping(RequestMatcher requestMatcher, String attributes) { - mappings.put(requestMatcher, attributes); + AuthorizeRequestMatcherRegistry addMapping(RequestMatcher requestMatcher, RoleManager roleManager) { + mappings.put(requestMatcher, roleManager); return this; } - public String getAttribute(HttpServletRequest request) { - for (Map.Entry entry : mappings.entrySet()) { + public RoleManager getRoleManager(HttpServletRequest request) { + for (var entry : mappings.entrySet()) { if (entry.getKey().matches(request)) { return entry.getValue(); } @@ -39,23 +43,23 @@ public AuthorizedUrl(RequestMatcher requestMatcher) { } public AuthorizeRequestMatcherRegistry permitAll() { - return access(PERMIT_ALL); + return access(new PermitAllRoleManager()); } public AuthorizeRequestMatcherRegistry denyAll() { - return access(DENY_ALL); + return access(new DenyAllRoleManager()); } - public AuthorizeRequestMatcherRegistry hasAuthority(String authority) { - return access("hasAuthority(" + authority + ")"); + public AuthorizeRequestMatcherRegistry hasAuthority(String... authorities) { + return access(new AuthorizationRoleManager(authorities)); } public AuthorizeRequestMatcherRegistry authenticated() { - return access(AUTHENTICATED); + return access(new AuthenticationRoleManager()); } - private AuthorizeRequestMatcherRegistry access(String attribute) { - return addMapping(requestMatcher, attribute); + private AuthorizeRequestMatcherRegistry access(RoleManager roleManager) { + return addMapping(requestMatcher, roleManager); } } From fc595d1e90ae8c11abcfae1fa06646836613bb98 Mon Sep 17 00:00:00 2001 From: hyunssooo Date: Tue, 13 Dec 2022 00:16:42 +0900 Subject: [PATCH 8/9] =?UTF-8?q?refactor:=20=EC=82=AC=EC=9A=A9=ED=95=98?= =?UTF-8?q?=EC=A7=80=20=EC=95=8A=EB=8A=94=20=EC=BD=94=EB=93=9C=20=EC=A0=9C?= =?UTF-8?q?=EA=B1=B0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../security/authorization/AuthorizationFilter.java | 8 -------- .../security/exception/AccessDeniedException.java | 5 ----- 2 files changed, 13 deletions(-) delete mode 100644 src/main/java/nextstep/security/exception/AccessDeniedException.java diff --git a/src/main/java/nextstep/security/authorization/AuthorizationFilter.java b/src/main/java/nextstep/security/authorization/AuthorizationFilter.java index 046a67c..70b9f94 100644 --- a/src/main/java/nextstep/security/authorization/AuthorizationFilter.java +++ b/src/main/java/nextstep/security/authorization/AuthorizationFilter.java @@ -12,7 +12,6 @@ import nextstep.security.authorization.manager.RoleManager; import nextstep.security.config.AuthorizeRequestMatcherRegistry; import nextstep.security.context.SecurityContextHolder; -import nextstep.security.exception.AccessDeniedException; import nextstep.security.exception.AuthenticationException; import nextstep.security.exception.AuthorizationException; import org.springframework.http.HttpStatus; @@ -42,7 +41,6 @@ public void doFilter( final RoleManager roleManager = authorizeRequestMatcherRegistry.getRoleManager((HttpServletRequest) request); if (roleManager == null) { - chain.doFilter(request, response); return; } @@ -62,12 +60,6 @@ public void doFilter( HttpStatus.FORBIDDEN.getReasonPhrase() ); return; - } catch (AccessDeniedException e) { - ((HttpServletResponse) response).sendError( - HttpStatus.BAD_REQUEST.value(), - HttpStatus.BAD_REQUEST.getReasonPhrase() - ); - return; } chain.doFilter(request, response); diff --git a/src/main/java/nextstep/security/exception/AccessDeniedException.java b/src/main/java/nextstep/security/exception/AccessDeniedException.java deleted file mode 100644 index 3eb62c1..0000000 --- a/src/main/java/nextstep/security/exception/AccessDeniedException.java +++ /dev/null @@ -1,5 +0,0 @@ -package nextstep.security.exception; - -public class AccessDeniedException extends RuntimeException { - -} From 3a4a9a535b1ea4121510204c47430111961d128a Mon Sep 17 00:00:00 2001 From: hyunssooo Date: Sun, 25 Dec 2022 02:11:17 +0900 Subject: [PATCH 9/9] =?UTF-8?q?feat:=20=ED=94=BC=EB=93=9C=EB=B0=B1=20?= =?UTF-8?q?=EB=B0=98=EC=98=81?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../java/nextstep/app/config/AuthConfig.java | 8 +++-- .../app/config/LoginUserArgumentResolver.java | 9 +++-- .../authorization/AuthorizationFilter.java | 9 +++-- .../authorization/PreAuthorizationFilter.java | 26 +++------------ ...=> AuthenticatedAuthorizationManager.java} | 2 +- ...ava => AuthorityAuthorizationManager.java} | 6 ++-- ...Manager.java => AuthorizationManager.java} | 2 +- ....java => DenyAllAuthorizationManager.java} | 2 +- ...ava => PermitAllAuthorizationManager.java} | 2 +- .../AuthorizeRequestMatcherRegistry.java | 33 +++++++++---------- .../config/DefaultSecurityFilterChain.java | 12 ++++--- .../security/config/FilterChainProxyTest.java | 2 ++ 12 files changed, 51 insertions(+), 62 deletions(-) rename src/main/java/nextstep/security/authorization/manager/{AuthenticationRoleManager.java => AuthenticatedAuthorizationManager.java} (74%) rename src/main/java/nextstep/security/authorization/manager/{AuthorizationRoleManager.java => AuthorityAuthorizationManager.java} (67%) rename src/main/java/nextstep/security/authorization/manager/{RoleManager.java => AuthorizationManager.java} (80%) rename src/main/java/nextstep/security/authorization/manager/{DenyAllRoleManager.java => DenyAllAuthorizationManager.java} (73%) rename src/main/java/nextstep/security/authorization/manager/{PermitAllRoleManager.java => PermitAllAuthorizationManager.java} (73%) diff --git a/src/main/java/nextstep/app/config/AuthConfig.java b/src/main/java/nextstep/app/config/AuthConfig.java index cd8cb3e..a9014c3 100644 --- a/src/main/java/nextstep/app/config/AuthConfig.java +++ b/src/main/java/nextstep/app/config/AuthConfig.java @@ -1,6 +1,8 @@ package nextstep.app.config; import java.util.List; +import javax.servlet.Filter; +import nextstep.security.access.matcher.AnyRequestMatcher; import nextstep.security.access.matcher.MvcRequestMatcher; import nextstep.security.authentication.AuthenticationManager; import nextstep.security.authentication.BasicAuthenticationFilter; @@ -8,8 +10,8 @@ import nextstep.security.authentication.UsernamePasswordAuthenticationProvider; import nextstep.security.authorization.AuthorizationFilter; import nextstep.security.authorization.PreAuthorizationFilter; -import nextstep.security.config.DefaultSecurityFilterChain; import nextstep.security.config.AuthorizeRequestMatcherRegistry; +import nextstep.security.config.DefaultSecurityFilterChain; import nextstep.security.config.FilterChainProxy; import nextstep.security.config.SecurityFilterChain; import nextstep.security.context.HttpSessionSecurityContextRepository; @@ -40,12 +42,13 @@ public DelegatingFilterProxy securityFilterChainProxy() { public FilterChainProxy filterChainProxy(SecurityFilterChain securityFilterChain) { return new FilterChainProxy(securityFilterChain); } + @Bean public SecurityFilterChain securityFilterChain( AuthenticationManager authenticationManager, SecurityContextRepository securityContextRepository ) { - return new DefaultSecurityFilterChain( + final List filters = List.of( new UsernamePasswordAuthenticationFilter( authenticationManager, securityContextRepository @@ -61,6 +64,7 @@ public SecurityFilterChain securityFilterChain( .matcher(new MvcRequestMatcher(HttpMethod.GET, "/members/me")).authenticated() ) ); + return new DefaultSecurityFilterChain(AnyRequestMatcher.INSTANCE, filters); } @Bean diff --git a/src/main/java/nextstep/app/config/LoginUserArgumentResolver.java b/src/main/java/nextstep/app/config/LoginUserArgumentResolver.java index 9f11daf..0ea9918 100644 --- a/src/main/java/nextstep/app/config/LoginUserArgumentResolver.java +++ b/src/main/java/nextstep/app/config/LoginUserArgumentResolver.java @@ -1,8 +1,8 @@ package nextstep.app.config; -import javax.servlet.http.HttpServletRequest; import nextstep.app.ui.dto.LoginUser; -import nextstep.security.context.SecurityContext; +import nextstep.security.authentication.Authentication; +import nextstep.security.context.SecurityContextHolder; import org.springframework.core.MethodParameter; import org.springframework.web.bind.support.WebDataBinderFactory; import org.springframework.web.context.request.NativeWebRequest; @@ -23,9 +23,8 @@ public LoginUser resolveArgument( NativeWebRequest webRequest, WebDataBinderFactory binderFactory ) { - final HttpServletRequest request = (HttpServletRequest) webRequest.getNativeRequest(); - final SecurityContext context = (SecurityContext) request.getSession().getAttribute("SPRING_SECURITY_CONTEXT"); - final String email = context.getAuthentication().getPrincipal().toString(); + final Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); + final String email = authentication.getPrincipal().toString(); return new LoginUser(email); } } diff --git a/src/main/java/nextstep/security/authorization/AuthorizationFilter.java b/src/main/java/nextstep/security/authorization/AuthorizationFilter.java index 70b9f94..33d09c9 100644 --- a/src/main/java/nextstep/security/authorization/AuthorizationFilter.java +++ b/src/main/java/nextstep/security/authorization/AuthorizationFilter.java @@ -9,7 +9,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import nextstep.security.authentication.Authentication; -import nextstep.security.authorization.manager.RoleManager; +import nextstep.security.authorization.manager.AuthorizationManager; import nextstep.security.config.AuthorizeRequestMatcherRegistry; import nextstep.security.context.SecurityContextHolder; import nextstep.security.exception.AuthenticationException; @@ -38,14 +38,13 @@ public void doFilter( .getAuthentication() ).orElseThrow(AuthenticationException::new); - final RoleManager roleManager = authorizeRequestMatcherRegistry.getRoleManager((HttpServletRequest) request); + final AuthorizationManager authorizationManager = authorizeRequestMatcherRegistry.getAuthorizationManager((HttpServletRequest) request); - if (roleManager == null) { + if (authorizationManager == null) { return; } - - if (!roleManager.check(authentication)) { + if (!authorizationManager.check(authentication)) { throw new AuthorizationException(); } } catch (AuthenticationException e) { diff --git a/src/main/java/nextstep/security/authorization/PreAuthorizationFilter.java b/src/main/java/nextstep/security/authorization/PreAuthorizationFilter.java index 6a8dde0..42af3d0 100644 --- a/src/main/java/nextstep/security/authorization/PreAuthorizationFilter.java +++ b/src/main/java/nextstep/security/authorization/PreAuthorizationFilter.java @@ -7,7 +7,6 @@ import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; -import nextstep.security.authentication.Authentication; import nextstep.security.context.SecurityContext; import nextstep.security.context.SecurityContextHolder; import nextstep.security.context.SecurityContextRepository; @@ -27,28 +26,13 @@ public void doFilter( ServletResponse response, FilterChain chain ) throws IOException, ServletException { - try { - if (SecurityContextHolder.getContext().getAuthentication() != null) { - chain.doFilter(request, response); - return; - } + final HttpServletRequest httpServletRequest = (HttpServletRequest) request; - final HttpServletRequest httpServletRequest = (HttpServletRequest) request; - SecurityContext context = Optional.ofNullable( - (SecurityContext) httpServletRequest.getSession() - .getAttribute(SecurityContextHolder.SPRING_SECURITY_CONTEXT_KEY) - ) - .orElseGet(() -> securityContextRepository.loadContext(httpServletRequest)); + final Optional securityContext = Optional.ofNullable( + securityContextRepository.loadContext(httpServletRequest) + ); + securityContext.ifPresent(it -> SecurityContextHolder.setContext(it)); - final Authentication authentication = Optional.ofNullable(context) - .map(it -> it.getAuthentication()) - .orElse(null); - - SecurityContextHolder.getContext().setAuthentication(authentication); - - } catch (Exception ignored) { - - } chain.doFilter(request, response); } } diff --git a/src/main/java/nextstep/security/authorization/manager/AuthenticationRoleManager.java b/src/main/java/nextstep/security/authorization/manager/AuthenticatedAuthorizationManager.java similarity index 74% rename from src/main/java/nextstep/security/authorization/manager/AuthenticationRoleManager.java rename to src/main/java/nextstep/security/authorization/manager/AuthenticatedAuthorizationManager.java index 35b803b..09b768f 100644 --- a/src/main/java/nextstep/security/authorization/manager/AuthenticationRoleManager.java +++ b/src/main/java/nextstep/security/authorization/manager/AuthenticatedAuthorizationManager.java @@ -2,7 +2,7 @@ import nextstep.security.authentication.Authentication; -public class AuthenticationRoleManager implements RoleManager { +public class AuthenticatedAuthorizationManager implements AuthorizationManager { @Override public boolean check(Authentication authentication) { diff --git a/src/main/java/nextstep/security/authorization/manager/AuthorizationRoleManager.java b/src/main/java/nextstep/security/authorization/manager/AuthorityAuthorizationManager.java similarity index 67% rename from src/main/java/nextstep/security/authorization/manager/AuthorizationRoleManager.java rename to src/main/java/nextstep/security/authorization/manager/AuthorityAuthorizationManager.java index 17138ea..453c22a 100644 --- a/src/main/java/nextstep/security/authorization/manager/AuthorizationRoleManager.java +++ b/src/main/java/nextstep/security/authorization/manager/AuthorityAuthorizationManager.java @@ -3,15 +3,15 @@ import java.util.Set; import nextstep.security.authentication.Authentication; -public class AuthorizationRoleManager implements RoleManager { +public class AuthorityAuthorizationManager implements AuthorizationManager { private final Set authorities; - public AuthorizationRoleManager(Set authorities) { + public AuthorityAuthorizationManager(Set authorities) { this.authorities = authorities; } - public AuthorizationRoleManager(String... authorities) { + public AuthorityAuthorizationManager(String... authorities) { this(Set.of(authorities)); } diff --git a/src/main/java/nextstep/security/authorization/manager/RoleManager.java b/src/main/java/nextstep/security/authorization/manager/AuthorizationManager.java similarity index 80% rename from src/main/java/nextstep/security/authorization/manager/RoleManager.java rename to src/main/java/nextstep/security/authorization/manager/AuthorizationManager.java index 302281a..2c03ffd 100644 --- a/src/main/java/nextstep/security/authorization/manager/RoleManager.java +++ b/src/main/java/nextstep/security/authorization/manager/AuthorizationManager.java @@ -2,7 +2,7 @@ import nextstep.security.authentication.Authentication; -public interface RoleManager { +public interface AuthorizationManager { boolean check(Authentication authentication); } diff --git a/src/main/java/nextstep/security/authorization/manager/DenyAllRoleManager.java b/src/main/java/nextstep/security/authorization/manager/DenyAllAuthorizationManager.java similarity index 73% rename from src/main/java/nextstep/security/authorization/manager/DenyAllRoleManager.java rename to src/main/java/nextstep/security/authorization/manager/DenyAllAuthorizationManager.java index 9e6cc84..71bd73c 100644 --- a/src/main/java/nextstep/security/authorization/manager/DenyAllRoleManager.java +++ b/src/main/java/nextstep/security/authorization/manager/DenyAllAuthorizationManager.java @@ -2,7 +2,7 @@ import nextstep.security.authentication.Authentication; -public class DenyAllRoleManager implements RoleManager { +public class DenyAllAuthorizationManager implements AuthorizationManager { @Override public boolean check(Authentication authentication) { diff --git a/src/main/java/nextstep/security/authorization/manager/PermitAllRoleManager.java b/src/main/java/nextstep/security/authorization/manager/PermitAllAuthorizationManager.java similarity index 73% rename from src/main/java/nextstep/security/authorization/manager/PermitAllRoleManager.java rename to src/main/java/nextstep/security/authorization/manager/PermitAllAuthorizationManager.java index d2d85cc..3f7b118 100644 --- a/src/main/java/nextstep/security/authorization/manager/PermitAllRoleManager.java +++ b/src/main/java/nextstep/security/authorization/manager/PermitAllAuthorizationManager.java @@ -2,7 +2,7 @@ import nextstep.security.authentication.Authentication; -public class PermitAllRoleManager implements RoleManager { +public class PermitAllAuthorizationManager implements AuthorizationManager { @Override public boolean check(Authentication authentication) { diff --git a/src/main/java/nextstep/security/config/AuthorizeRequestMatcherRegistry.java b/src/main/java/nextstep/security/config/AuthorizeRequestMatcherRegistry.java index 6f31234..1ceeb52 100644 --- a/src/main/java/nextstep/security/config/AuthorizeRequestMatcherRegistry.java +++ b/src/main/java/nextstep/security/config/AuthorizeRequestMatcherRegistry.java @@ -4,25 +4,25 @@ import java.util.Map; import javax.servlet.http.HttpServletRequest; import nextstep.security.access.matcher.RequestMatcher; -import nextstep.security.authorization.manager.AuthenticationRoleManager; -import nextstep.security.authorization.manager.AuthorizationRoleManager; -import nextstep.security.authorization.manager.DenyAllRoleManager; -import nextstep.security.authorization.manager.PermitAllRoleManager; -import nextstep.security.authorization.manager.RoleManager; +import nextstep.security.authorization.manager.AuthenticatedAuthorizationManager; +import nextstep.security.authorization.manager.AuthorizationManager; +import nextstep.security.authorization.manager.AuthorityAuthorizationManager; +import nextstep.security.authorization.manager.DenyAllAuthorizationManager; +import nextstep.security.authorization.manager.PermitAllAuthorizationManager; public class AuthorizeRequestMatcherRegistry { - private final Map mappings = new HashMap<>(); + private final Map mappings = new HashMap<>(); public AuthorizedUrl matcher(RequestMatcher requestMatcher) { return new AuthorizedUrl(requestMatcher); } - AuthorizeRequestMatcherRegistry addMapping(RequestMatcher requestMatcher, RoleManager roleManager) { - mappings.put(requestMatcher, roleManager); + AuthorizeRequestMatcherRegistry addMapping(RequestMatcher requestMatcher, AuthorizationManager authorizationManager) { + mappings.put(requestMatcher, authorizationManager); return this; } - public RoleManager getRoleManager(HttpServletRequest request) { + public AuthorizationManager getAuthorizationManager(HttpServletRequest request) { for (var entry : mappings.entrySet()) { if (entry.getKey().matches(request)) { return entry.getValue(); @@ -33,9 +33,6 @@ public RoleManager getRoleManager(HttpServletRequest request) { } public class AuthorizedUrl { - public static final String PERMIT_ALL = "permitAll"; - public static final String DENY_ALL = "denyAll"; - public static final String AUTHENTICATED = "authenticated"; private final RequestMatcher requestMatcher; public AuthorizedUrl(RequestMatcher requestMatcher) { @@ -43,23 +40,23 @@ public AuthorizedUrl(RequestMatcher requestMatcher) { } public AuthorizeRequestMatcherRegistry permitAll() { - return access(new PermitAllRoleManager()); + return access(new PermitAllAuthorizationManager()); } public AuthorizeRequestMatcherRegistry denyAll() { - return access(new DenyAllRoleManager()); + return access(new DenyAllAuthorizationManager()); } public AuthorizeRequestMatcherRegistry hasAuthority(String... authorities) { - return access(new AuthorizationRoleManager(authorities)); + return access(new AuthorityAuthorizationManager(authorities)); } public AuthorizeRequestMatcherRegistry authenticated() { - return access(new AuthenticationRoleManager()); + return access(new AuthenticatedAuthorizationManager()); } - private AuthorizeRequestMatcherRegistry access(RoleManager roleManager) { - return addMapping(requestMatcher, roleManager); + private AuthorizeRequestMatcherRegistry access(AuthorizationManager authorizationManager) { + return addMapping(requestMatcher, authorizationManager); } } diff --git a/src/main/java/nextstep/security/config/DefaultSecurityFilterChain.java b/src/main/java/nextstep/security/config/DefaultSecurityFilterChain.java index fc8b2ec..23abebd 100644 --- a/src/main/java/nextstep/security/config/DefaultSecurityFilterChain.java +++ b/src/main/java/nextstep/security/config/DefaultSecurityFilterChain.java @@ -3,21 +3,25 @@ import java.util.List; import javax.servlet.Filter; import javax.servlet.http.HttpServletRequest; +import nextstep.security.access.matcher.RequestMatcher; public class DefaultSecurityFilterChain implements SecurityFilterChain { + + private final RequestMatcher requestMatcher; private final List filters; - public DefaultSecurityFilterChain(List filters) { + public DefaultSecurityFilterChain(RequestMatcher requestMatcher, List filters) { + this.requestMatcher = requestMatcher; this.filters = filters; } - public DefaultSecurityFilterChain(Filter... filters) { - this(List.of(filters)); + public DefaultSecurityFilterChain(RequestMatcher requestMatcher, Filter... filters) { + this(requestMatcher, List.of(filters)); } @Override public boolean matches(HttpServletRequest request) { - return true; + return requestMatcher.matches(request); } @Override diff --git a/src/test/java/nextstep/security/config/FilterChainProxyTest.java b/src/test/java/nextstep/security/config/FilterChainProxyTest.java index 47354f1..863e6f1 100644 --- a/src/test/java/nextstep/security/config/FilterChainProxyTest.java +++ b/src/test/java/nextstep/security/config/FilterChainProxyTest.java @@ -10,6 +10,7 @@ import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; +import nextstep.security.access.matcher.AnyRequestMatcher; import nextstep.security.fixture.MockFilterChain; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -29,6 +30,7 @@ void setUp() { filterChainProxy = new FilterChainProxy( new DefaultSecurityFilterChain( + AnyRequestMatcher.INSTANCE, loginTestFilter, membersTestFilter )