From 313c0113a94c096ed6d1b8f73afa003724ab2a26 Mon Sep 17 00:00:00 2001 From: boorownie Date: Fri, 24 Jan 2025 17:18:23 +0900 Subject: [PATCH 01/13] practice - /members/me --- .../nextstep/app/ui/MemberController.java | 17 +++++++++++++ src/test/java/nextstep/app/BasicAuthTest.java | 24 +++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/src/main/java/nextstep/app/ui/MemberController.java b/src/main/java/nextstep/app/ui/MemberController.java index 823cf7e..79c905b 100644 --- a/src/main/java/nextstep/app/ui/MemberController.java +++ b/src/main/java/nextstep/app/ui/MemberController.java @@ -2,7 +2,10 @@ import nextstep.app.domain.Member; import nextstep.app.domain.MemberRepository; +import nextstep.security.authentication.Authentication; +import nextstep.security.authentication.AuthenticationException; import nextstep.security.authorization.Secured; +import nextstep.security.context.SecurityContextHolder; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.RestController; @@ -30,4 +33,18 @@ public ResponseEntity> search() { List members = memberRepository.findAll(); return ResponseEntity.ok(members); } + + @GetMapping("/members/me") + public ResponseEntity me() { + Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); + if (authentication == null) { + throw new AuthenticationException(); + } + + String email = authentication.getPrincipal().toString(); + Member member = memberRepository.findByEmail(email) + .orElseThrow(RuntimeException::new); + + return ResponseEntity.ok(member); + } } diff --git a/src/test/java/nextstep/app/BasicAuthTest.java b/src/test/java/nextstep/app/BasicAuthTest.java index ac117be..2dea9bb 100644 --- a/src/test/java/nextstep/app/BasicAuthTest.java +++ b/src/test/java/nextstep/app/BasicAuthTest.java @@ -89,4 +89,28 @@ 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("$.email").value(TEST_ADMIN_MEMBER.getEmail())); + } + + @DisplayName("인증되지 않은 사용자는 자신의 정보를 조회할 수 없다.") + @Test + void request_fail_members_me() throws Exception { + ResultActions response = mockMvc.perform(get("/members/me") + .contentType(MediaType.APPLICATION_FORM_URLENCODED_VALUE) + ); + + response.andExpect(status().isUnauthorized()); + } } From 7e036b1c1a423ddf6f6ffa28f52f0c73068189e5 Mon Sep 17 00:00:00 2001 From: boorownie Date: Fri, 24 Jan 2025 17:19:30 +0900 Subject: [PATCH 02/13] practice - authorization filter --- .../java/nextstep/app/SecurityConfig.java | 5 +- .../BasicAuthenticationFilter.java | 7 ++- .../authorization/AuthorizationFilter.java | 52 +++++++++++++++++++ .../CheckAuthenticationFilter.java | 38 -------------- 4 files changed, 60 insertions(+), 42 deletions(-) create mode 100644 src/main/java/nextstep/security/authorization/AuthorizationFilter.java delete mode 100644 src/main/java/nextstep/security/authorization/CheckAuthenticationFilter.java diff --git a/src/main/java/nextstep/app/SecurityConfig.java b/src/main/java/nextstep/app/SecurityConfig.java index 1683058..e2bfc3f 100644 --- a/src/main/java/nextstep/app/SecurityConfig.java +++ b/src/main/java/nextstep/app/SecurityConfig.java @@ -5,8 +5,7 @@ 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.config.DefaultSecurityFilterChain; import nextstep.security.config.DelegatingFilterProxy; @@ -58,7 +57,7 @@ public SecurityFilterChain securityFilterChain() { new SecurityContextHolderFilter(), new UsernamePasswordAuthenticationFilter(userDetailsService()), new BasicAuthenticationFilter(userDetailsService()), - new CheckAuthenticationFilter() + new AuthorizationFilter() ) ); } diff --git a/src/main/java/nextstep/security/authentication/BasicAuthenticationFilter.java b/src/main/java/nextstep/security/authentication/BasicAuthenticationFilter.java index 406116f..05cc1ad 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,8 +41,12 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse SecurityContextHolder.setContext(context); filterChain.doFilter(request, response); - } catch (Exception e) { + } catch (AuthenticationException e) { response.setStatus(HttpServletResponse.SC_UNAUTHORIZED); + } catch (ForbiddenException e) { + response.setStatus(HttpServletResponse.SC_FORBIDDEN); + } catch (Exception e) { + response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); } } diff --git a/src/main/java/nextstep/security/authorization/AuthorizationFilter.java b/src/main/java/nextstep/security/authorization/AuthorizationFilter.java new file mode 100644 index 0000000..b1e100b --- /dev/null +++ b/src/main/java/nextstep/security/authorization/AuthorizationFilter.java @@ -0,0 +1,52 @@ +package nextstep.security.authorization; + +import nextstep.security.authentication.Authentication; +import nextstep.security.authentication.AuthenticationException; +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 java.io.IOException; + +public class AuthorizationFilter extends OncePerRequestFilter { + + @Override + protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) throws ServletException, IOException { + Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); + boolean isGranted = checkAuthorization(authentication, request); + + if (!isGranted) { + throw new ForbiddenException(); + } + + filterChain.doFilter(request, response); + } + + private boolean checkAuthorization(Authentication authentication, HttpServletRequest httpRequest) { + if (httpRequest.getRequestURI().equals("/members")) { + if (authentication == null) { + throw new AuthenticationException(); + } + + return authentication.getAuthorities().stream() + .anyMatch(authority -> authority.equals("ADMIN")); + } + + if (httpRequest.getRequestURI().equals("/members/me")) { + if (authentication == null) { + throw new AuthenticationException(); + } + + return authentication.isAuthenticated(); + } + + if (httpRequest.getRequestURI().equals("/search")) { + return true; + } + + return false; + } +} diff --git a/src/main/java/nextstep/security/authorization/CheckAuthenticationFilter.java b/src/main/java/nextstep/security/authorization/CheckAuthenticationFilter.java deleted file mode 100644 index 02327ff..0000000 --- a/src/main/java/nextstep/security/authorization/CheckAuthenticationFilter.java +++ /dev/null @@ -1,38 +0,0 @@ -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 java.io.IOException; -import java.util.Set; - -public class CheckAuthenticationFilter extends OncePerRequestFilter { - private static final String DEFAULT_REQUEST_URI = "/members"; - - @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; - } - - filterChain.doFilter(request, response); - } -} From c95315045de1f9336b3736f1af3afc370f3c062e Mon Sep 17 00:00:00 2001 From: parkSeryu Date: Mon, 10 Feb 2025 00:50:06 +0900 Subject: [PATCH 03/13] =?UTF-8?q?feat:=20AuthorizationManager,=20Authoriza?= =?UTF-8?q?tionDecision=20=EA=B5=AC=ED=98=84?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../authorization/AuthorizationDecision.java | 13 +++++++++++++ .../authorization/AuthorizationManager.java | 8 ++++++++ 2 files changed, 21 insertions(+) create mode 100644 src/main/java/nextstep/security/authorization/AuthorizationDecision.java create mode 100644 src/main/java/nextstep/security/authorization/AuthorizationManager.java 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..e789944 --- /dev/null +++ b/src/main/java/nextstep/security/authorization/AuthorizationDecision.java @@ -0,0 +1,13 @@ +package nextstep.security.authorization; + +public class AuthorizationDecision { + private final boolean isGranted; + + public AuthorizationDecision(boolean isGranted) { + this.isGranted = isGranted; + } + + public boolean isGranted() { + return isGranted; + } +} diff --git a/src/main/java/nextstep/security/authorization/AuthorizationManager.java b/src/main/java/nextstep/security/authorization/AuthorizationManager.java new file mode 100644 index 0000000..42b8610 --- /dev/null +++ b/src/main/java/nextstep/security/authorization/AuthorizationManager.java @@ -0,0 +1,8 @@ +package nextstep.security.authorization; + +import nextstep.security.authentication.Authentication; + +@FunctionalInterface +public interface AuthorizationManager { + AuthorizationDecision check(Authentication authentication, T object); +} From 439e1a5e785321bf3f581a71397f44ee18be32c9 Mon Sep 17 00:00:00 2001 From: parkSeryu Date: Mon, 10 Feb 2025 01:08:36 +0900 Subject: [PATCH 04/13] =?UTF-8?q?refactor:=20=EA=B8=B0=EC=A1=B4=EC=9D=98?= =?UTF-8?q?=20AuthorizationFilter=20=EC=9D=B8=EA=B0=80=20=EB=A1=9C?= =?UTF-8?q?=EC=A7=81=EC=9D=84=20RequestAuthorizationManager=EB=A1=9C=20?= =?UTF-8?q?=EB=A6=AC=ED=8C=A9=ED=86=A0=EB=A7=81?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../java/nextstep/app/SecurityConfig.java | 10 ++++- .../authorization/AuthorizationFilter.java | 39 +++++-------------- .../web/RequestAuthorizationManager.java | 35 +++++++++++++++++ src/test/java/nextstep/app/BasicAuthTest.java | 2 +- 4 files changed, 55 insertions(+), 31 deletions(-) create mode 100644 src/main/java/nextstep/security/authorization/web/RequestAuthorizationManager.java diff --git a/src/main/java/nextstep/app/SecurityConfig.java b/src/main/java/nextstep/app/SecurityConfig.java index e2bfc3f..14e01d5 100644 --- a/src/main/java/nextstep/app/SecurityConfig.java +++ b/src/main/java/nextstep/app/SecurityConfig.java @@ -1,12 +1,15 @@ 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.AuthorizationFilter; +import nextstep.security.authorization.AuthorizationManager; import nextstep.security.authorization.SecuredMethodInterceptor; +import nextstep.security.authorization.web.RequestAuthorizationManager; import nextstep.security.config.DefaultSecurityFilterChain; import nextstep.security.config.DelegatingFilterProxy; import nextstep.security.config.FilterChainProxy; @@ -50,6 +53,11 @@ public SecuredMethodInterceptor securedMethodInterceptor() { // return new SecuredAspect(); // } + @Bean + public AuthorizationManager requestAuthorizationManager() { + return new RequestAuthorizationManager(); + } + @Bean public SecurityFilterChain securityFilterChain() { return new DefaultSecurityFilterChain( @@ -57,7 +65,7 @@ public SecurityFilterChain securityFilterChain() { new SecurityContextHolderFilter(), new UsernamePasswordAuthenticationFilter(userDetailsService()), new BasicAuthenticationFilter(userDetailsService()), - new AuthorizationFilter() + new AuthorizationFilter(requestAuthorizationManager()) ) ); } diff --git a/src/main/java/nextstep/security/authorization/AuthorizationFilter.java b/src/main/java/nextstep/security/authorization/AuthorizationFilter.java index b1e100b..0fc45de 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 nextstep.security.authentication.Authentication; -import nextstep.security.authentication.AuthenticationException; import nextstep.security.context.SecurityContextHolder; import org.springframework.web.filter.OncePerRequestFilter; @@ -13,40 +12,22 @@ public class AuthorizationFilter extends OncePerRequestFilter { + private final AuthorizationManager authorizationManager; + + public AuthorizationFilter(AuthorizationManager authorizationManager) { + this.authorizationManager = authorizationManager; + } + @Override - protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) throws ServletException, IOException { + protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) + throws ServletException, IOException { Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); - boolean isGranted = checkAuthorization(authentication, request); + AuthorizationDecision authorizationDecision = authorizationManager.check(authentication, request); - if (!isGranted) { + if (!authorizationDecision.isGranted()) { throw new ForbiddenException(); } filterChain.doFilter(request, response); } - - private boolean checkAuthorization(Authentication authentication, HttpServletRequest httpRequest) { - if (httpRequest.getRequestURI().equals("/members")) { - if (authentication == null) { - throw new AuthenticationException(); - } - - return authentication.getAuthorities().stream() - .anyMatch(authority -> authority.equals("ADMIN")); - } - - if (httpRequest.getRequestURI().equals("/members/me")) { - if (authentication == null) { - throw new AuthenticationException(); - } - - return authentication.isAuthenticated(); - } - - if (httpRequest.getRequestURI().equals("/search")) { - return true; - } - - return false; - } } diff --git a/src/main/java/nextstep/security/authorization/web/RequestAuthorizationManager.java b/src/main/java/nextstep/security/authorization/web/RequestAuthorizationManager.java new file mode 100644 index 0000000..25b4a43 --- /dev/null +++ b/src/main/java/nextstep/security/authorization/web/RequestAuthorizationManager.java @@ -0,0 +1,35 @@ +package nextstep.security.authorization.web; + +import jakarta.servlet.http.HttpServletRequest; +import nextstep.security.authentication.Authentication; +import nextstep.security.authorization.AuthorizationDecision; +import nextstep.security.authorization.AuthorizationManager; + +public class RequestAuthorizationManager implements AuthorizationManager { + @Override + public AuthorizationDecision check(Authentication authentication, HttpServletRequest httpRequest) { + if (httpRequest.getRequestURI().equals("/members")) { + if (authentication == null) { + return new AuthorizationDecision(false); + } + + return new AuthorizationDecision( + authentication.getAuthorities().stream() + .anyMatch(authority -> authority.equals("ADMIN"))); + } + + if (httpRequest.getRequestURI().equals("/members/me")) { + if (authentication == null) { + return new AuthorizationDecision(false); + } + + return new AuthorizationDecision(authentication.isAuthenticated()); + } + + if (httpRequest.getRequestURI().equals("/search")) { + return new AuthorizationDecision(true); + } + + return new AuthorizationDecision(false); + } +} diff --git a/src/test/java/nextstep/app/BasicAuthTest.java b/src/test/java/nextstep/app/BasicAuthTest.java index 2dea9bb..c201f69 100644 --- a/src/test/java/nextstep/app/BasicAuthTest.java +++ b/src/test/java/nextstep/app/BasicAuthTest.java @@ -111,6 +111,6 @@ void request_fail_members_me() throws Exception { .contentType(MediaType.APPLICATION_FORM_URLENCODED_VALUE) ); - response.andExpect(status().isUnauthorized()); + response.andExpect(status().isForbidden()); } } From a63a21ece56e2fa4c3aee6fa44afa581be31db86 Mon Sep 17 00:00:00 2001 From: parkSeryu Date: Mon, 10 Feb 2025 20:30:09 +0900 Subject: [PATCH 05/13] =?UTF-8?q?test:=20=ED=85=8C=EC=8A=A4=ED=8A=B8=20?= =?UTF-8?q?=EC=BD=94=EB=93=9C=20=EA=B5=AC=EC=A1=B0=ED=99=94?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - testFixture 추가 - 기존 테스트 코드 구조 변경 - 사용하지 않는 테스트 클래스 제거 --- src/test/java/nextstep/app/BasicAuthTest.java | 6 ++---- src/test/java/nextstep/app/FormLoginTest.java | 7 ++----- src/test/java/nextstep/app/SecuredTest.java | 12 ++++-------- .../SecurityAuthorizationApplicationTests.java | 13 ------------- .../security/fixture/MemberTestFixture.java | 18 ++++++++++++++++++ 5 files changed, 26 insertions(+), 30 deletions(-) delete mode 100644 src/test/java/nextstep/app/SecurityAuthorizationApplicationTests.java create mode 100644 src/test/java/nextstep/security/fixture/MemberTestFixture.java diff --git a/src/test/java/nextstep/app/BasicAuthTest.java b/src/test/java/nextstep/app/BasicAuthTest.java index c201f69..f281e01 100644 --- a/src/test/java/nextstep/app/BasicAuthTest.java +++ b/src/test/java/nextstep/app/BasicAuthTest.java @@ -1,6 +1,5 @@ package nextstep.app; -import nextstep.app.domain.Member; import nextstep.app.domain.MemberRepository; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; @@ -14,16 +13,15 @@ import org.springframework.test.web.servlet.result.MockMvcResultMatchers; import java.util.Base64; -import java.util.Set; +import static nextstep.security.fixture.MemberTestFixture.TEST_ADMIN_MEMBER; +import static nextstep.security.fixture.MemberTestFixture.TEST_USER_MEMBER; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @SpringBootTest @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()); @Autowired private MockMvc mockMvc; diff --git a/src/test/java/nextstep/app/FormLoginTest.java b/src/test/java/nextstep/app/FormLoginTest.java index 6301c1a..dfed774 100644 --- a/src/test/java/nextstep/app/FormLoginTest.java +++ b/src/test/java/nextstep/app/FormLoginTest.java @@ -1,6 +1,5 @@ package nextstep.app; -import nextstep.app.domain.Member; import nextstep.app.domain.MemberRepository; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; @@ -13,8 +12,8 @@ import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.ResultActions; -import java.util.Set; - +import static nextstep.security.fixture.MemberTestFixture.TEST_ADMIN_MEMBER; +import static nextstep.security.fixture.MemberTestFixture.TEST_USER_MEMBER; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @@ -22,8 +21,6 @@ @SpringBootTest @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()); @Autowired private MockMvc mockMvc; diff --git a/src/test/java/nextstep/app/SecuredTest.java b/src/test/java/nextstep/app/SecuredTest.java index 9e672e5..e9f19c6 100644 --- a/src/test/java/nextstep/app/SecuredTest.java +++ b/src/test/java/nextstep/app/SecuredTest.java @@ -1,7 +1,7 @@ package nextstep.app; -import nextstep.app.domain.Member; import nextstep.app.domain.MemberRepository; +import nextstep.security.fixture.MemberTestFixture; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; @@ -13,9 +13,7 @@ import org.springframework.test.web.servlet.ResultActions; import org.springframework.test.web.servlet.result.MockMvcResultMatchers; -import java.util.Base64; -import java.util.Set; - +import static nextstep.security.fixture.MemberTestFixture.*; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.print; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @@ -23,8 +21,6 @@ @SpringBootTest @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; @@ -41,7 +37,7 @@ void setUp() { @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()); + String token = createAdminToken(); ResultActions response = mockMvc.perform(get("/search") .header("Authorization", "Basic " + token) @@ -55,7 +51,7 @@ void request_search_success_with_admin_user() throws Exception { @DisplayName("일반 사용자가 요청할 경우 권한이 없어야 한다.") @Test void request_search_fail_with_general_user() throws Exception { - String token = Base64.getEncoder().encodeToString((TEST_USER_MEMBER.getEmail() + ":" + TEST_USER_MEMBER.getPassword()).getBytes()); + String token = createMemberToken(); ResultActions response = mockMvc.perform(get("/search") .header("Authorization", "Basic " + token) diff --git a/src/test/java/nextstep/app/SecurityAuthorizationApplicationTests.java b/src/test/java/nextstep/app/SecurityAuthorizationApplicationTests.java deleted file mode 100644 index 759cc3f..0000000 --- a/src/test/java/nextstep/app/SecurityAuthorizationApplicationTests.java +++ /dev/null @@ -1,13 +0,0 @@ -package nextstep.app; - -import org.junit.jupiter.api.Test; -import org.springframework.boot.test.context.SpringBootTest; - -@SpringBootTest -class SecurityAuthorizationApplicationTests { - - @Test - void contextLoads() { - } - -} diff --git a/src/test/java/nextstep/security/fixture/MemberTestFixture.java b/src/test/java/nextstep/security/fixture/MemberTestFixture.java new file mode 100644 index 0000000..5b0c3bd --- /dev/null +++ b/src/test/java/nextstep/security/fixture/MemberTestFixture.java @@ -0,0 +1,18 @@ +package nextstep.security.fixture; + +import java.util.Base64; +import java.util.Set; +import nextstep.app.domain.Member; + +public class MemberTestFixture { + public static final Member TEST_ADMIN_MEMBER = new Member("a@a.com", "password", "a", "", Set.of("ADMIN")); + public static final Member TEST_USER_MEMBER = new Member("b@b.com", "password", "b", "", Set.of("USER")); + + public static String createAdminToken(){ + return Base64.getEncoder().encodeToString((TEST_ADMIN_MEMBER.getEmail() + ":" + TEST_ADMIN_MEMBER.getPassword()).getBytes()); + } + + public static String createMemberToken(){ + return Base64.getEncoder().encodeToString((TEST_USER_MEMBER.getEmail() + ":" + TEST_USER_MEMBER.getPassword()).getBytes()); + } +} From 8cdf3dc3f265e9874857725eb5c88c3c384c6f63 Mon Sep 17 00:00:00 2001 From: parkSeryu Date: Mon, 10 Feb 2025 23:22:25 +0900 Subject: [PATCH 06/13] =?UTF-8?q?refactor:=20=EA=B8=B0=EC=A1=B4=EC=9D=98?= =?UTF-8?q?=20SecuredMethodInterceptor=20=EC=9D=B8=EA=B0=80=20=EB=A1=9C?= =?UTF-8?q?=EC=A7=81=EC=9D=84=20SecuredAuthorizationManager=EB=A1=9C=20?= =?UTF-8?q?=EB=A6=AC=ED=8C=A9=ED=86=A0=EB=A7=81?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../java/nextstep/app/SecurityConfig.java | 10 ++++- .../SecuredMethodInterceptor.java | 20 +++++----- .../method/SecuredAuthorizationManager.java | 38 +++++++++++++++++++ 3 files changed, 56 insertions(+), 12 deletions(-) create mode 100644 src/main/java/nextstep/security/authorization/method/SecuredAuthorizationManager.java diff --git a/src/main/java/nextstep/app/SecurityConfig.java b/src/main/java/nextstep/app/SecurityConfig.java index 14e01d5..3767a12 100644 --- a/src/main/java/nextstep/app/SecurityConfig.java +++ b/src/main/java/nextstep/app/SecurityConfig.java @@ -9,6 +9,7 @@ import nextstep.security.authorization.AuthorizationFilter; import nextstep.security.authorization.AuthorizationManager; import nextstep.security.authorization.SecuredMethodInterceptor; +import nextstep.security.authorization.method.SecuredAuthorizationManager; import nextstep.security.authorization.web.RequestAuthorizationManager; import nextstep.security.config.DefaultSecurityFilterChain; import nextstep.security.config.DelegatingFilterProxy; @@ -17,6 +18,7 @@ import nextstep.security.context.SecurityContextHolderFilter; import nextstep.security.userdetails.UserDetails; import nextstep.security.userdetails.UserDetailsService; +import org.aopalliance.intercept.MethodInvocation; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.EnableAspectJAutoProxy; @@ -46,13 +48,19 @@ public FilterChainProxy filterChainProxy(List securityFilte @Bean public SecuredMethodInterceptor securedMethodInterceptor() { - return new SecuredMethodInterceptor(); + return new SecuredMethodInterceptor(securedAuthorizationManager()); } + // @Bean // public SecuredAspect securedAspect() { // return new SecuredAspect(); // } + @Bean + public AuthorizationManager securedAuthorizationManager(){ + return new SecuredAuthorizationManager(); + } + @Bean public AuthorizationManager requestAuthorizationManager() { return new RequestAuthorizationManager(); diff --git a/src/main/java/nextstep/security/authorization/SecuredMethodInterceptor.java b/src/main/java/nextstep/security/authorization/SecuredMethodInterceptor.java index 8ee7409..c9bdc53 100644 --- a/src/main/java/nextstep/security/authorization/SecuredMethodInterceptor.java +++ b/src/main/java/nextstep/security/authorization/SecuredMethodInterceptor.java @@ -15,25 +15,23 @@ public class SecuredMethodInterceptor implements MethodInterceptor, PointcutAdvisor, AopInfrastructureBean { + private final AuthorizationManager authorizationManager; private final Pointcut pointcut; - 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(); + AuthorizationDecision authorizationDecision = authorizationManager.check(authentication, invocation); + + if (!authorizationDecision.isGranted()) { + throw new ForbiddenException(); } + return invocation.proceed(); } diff --git a/src/main/java/nextstep/security/authorization/method/SecuredAuthorizationManager.java b/src/main/java/nextstep/security/authorization/method/SecuredAuthorizationManager.java new file mode 100644 index 0000000..7eb245e --- /dev/null +++ b/src/main/java/nextstep/security/authorization/method/SecuredAuthorizationManager.java @@ -0,0 +1,38 @@ +package nextstep.security.authorization.method; + +import java.lang.reflect.Method; +import java.util.Collections; +import java.util.Set; +import nextstep.security.authentication.Authentication; +import nextstep.security.authentication.AuthenticationException; +import nextstep.security.authorization.AuthorizationDecision; +import nextstep.security.authorization.AuthorizationManager; +import nextstep.security.authorization.Secured; +import org.aopalliance.intercept.MethodInvocation; + +public class SecuredAuthorizationManager implements AuthorizationManager { + @Override + public AuthorizationDecision check(Authentication authentication, MethodInvocation invocation) { + if (authentication == null) { + throw new AuthenticationException(); + } + + Set authorities = getAuthorities(invocation); + + boolean hasAuthority = authentication.getAuthorities().stream() + .anyMatch(authorities::contains); + + return new AuthorizationDecision(hasAuthority); + } + + private Set getAuthorities(MethodInvocation invocation) { + Method method = invocation.getMethod(); + + if (!method.isAnnotationPresent(Secured.class)) { + return Collections.emptySet(); + } + + Secured secured = method.getAnnotation(Secured.class); + return Set.of(secured.value()); + } +} From 21e639b61bc3071271c24380e75bfcb430921d9c Mon Sep 17 00:00:00 2001 From: parkSeryu Date: Tue, 11 Feb 2025 00:03:18 +0900 Subject: [PATCH 07/13] =?UTF-8?q?feat:=20RequestMatcherRegistry=EC=99=80?= =?UTF-8?q?=20RequestMatcher=EB=A5=BC=20=EC=9E=91=EC=84=B1=ED=95=98?= =?UTF-8?q?=EA=B3=A0,=20RequestMatcher=EC=9D=98=20=EA=B5=AC=ED=98=84?= =?UTF-8?q?=EC=B2=B4=EB=A5=BC=20=EC=9E=91=EC=84=B1?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ...MatcherDelegatingAuthorizationManager.java | 22 +++++++++ .../security/util/AnyRequestMatcher.java | 14 ++++++ .../security/util/MvcRequestMatcher.java | 20 ++++++++ .../security/util/RequestMatcher.java | 7 +++ .../security/util/RequestMatcherEntry.java | 19 ++++++++ .../security/util/MvcRequestMatcherTest.java | 47 +++++++++++++++++++ 6 files changed, 129 insertions(+) create mode 100644 src/main/java/nextstep/security/authorization/web/RequestMatcherDelegatingAuthorizationManager.java create mode 100644 src/main/java/nextstep/security/util/AnyRequestMatcher.java create mode 100644 src/main/java/nextstep/security/util/MvcRequestMatcher.java create mode 100644 src/main/java/nextstep/security/util/RequestMatcher.java create mode 100644 src/main/java/nextstep/security/util/RequestMatcherEntry.java create mode 100644 src/test/java/nextstep/security/util/MvcRequestMatcherTest.java diff --git a/src/main/java/nextstep/security/authorization/web/RequestMatcherDelegatingAuthorizationManager.java b/src/main/java/nextstep/security/authorization/web/RequestMatcherDelegatingAuthorizationManager.java new file mode 100644 index 0000000..3815a78 --- /dev/null +++ b/src/main/java/nextstep/security/authorization/web/RequestMatcherDelegatingAuthorizationManager.java @@ -0,0 +1,22 @@ +package nextstep.security.authorization.web; + +import jakarta.servlet.http.HttpServletRequest; +import java.util.List; +import nextstep.security.authentication.Authentication; +import nextstep.security.authorization.AuthorizationDecision; +import nextstep.security.authorization.AuthorizationManager; +import nextstep.security.util.RequestMatcherEntry; + +public class RequestMatcherDelegatingAuthorizationManager implements AuthorizationManager { + + private final List> mappings; + + public RequestMatcherDelegatingAuthorizationManager(List> mappings) { + this.mappings = mappings; + } + + @Override + public AuthorizationDecision check(Authentication authentication, HttpServletRequest object) { + return null; + } +} diff --git a/src/main/java/nextstep/security/util/AnyRequestMatcher.java b/src/main/java/nextstep/security/util/AnyRequestMatcher.java new file mode 100644 index 0000000..67e72d9 --- /dev/null +++ b/src/main/java/nextstep/security/util/AnyRequestMatcher.java @@ -0,0 +1,14 @@ +package nextstep.security.util; + +import jakarta.servlet.http.HttpServletRequest; + +public class AnyRequestMatcher implements RequestMatcher { + + public static final RequestMatcher INSTANCE = new AnyRequestMatcher(); + + @Override + public boolean matches(HttpServletRequest request) { + return true; + } + +} diff --git a/src/main/java/nextstep/security/util/MvcRequestMatcher.java b/src/main/java/nextstep/security/util/MvcRequestMatcher.java new file mode 100644 index 0000000..1b1089f --- /dev/null +++ b/src/main/java/nextstep/security/util/MvcRequestMatcher.java @@ -0,0 +1,20 @@ +package nextstep.security.util; + +import jakarta.servlet.http.HttpServletRequest; +import org.springframework.http.HttpMethod; + +public class MvcRequestMatcher implements RequestMatcher { + + private final HttpMethod method; + private final String pattern; + + public MvcRequestMatcher(HttpMethod method, String pattern) { + this.method = method; + this.pattern = pattern; + } + + @Override + public boolean matches(HttpServletRequest request) { + return this.method.equals(HttpMethod.valueOf(request.getMethod())) && request.getRequestURI().equals(pattern); + } +} diff --git a/src/main/java/nextstep/security/util/RequestMatcher.java b/src/main/java/nextstep/security/util/RequestMatcher.java new file mode 100644 index 0000000..6e77696 --- /dev/null +++ b/src/main/java/nextstep/security/util/RequestMatcher.java @@ -0,0 +1,7 @@ +package nextstep.security.util; + +import jakarta.servlet.http.HttpServletRequest; + +public interface RequestMatcher { + boolean matches(HttpServletRequest request); +} diff --git a/src/main/java/nextstep/security/util/RequestMatcherEntry.java b/src/main/java/nextstep/security/util/RequestMatcherEntry.java new file mode 100644 index 0000000..9153740 --- /dev/null +++ b/src/main/java/nextstep/security/util/RequestMatcherEntry.java @@ -0,0 +1,19 @@ +package nextstep.security.util; + +public class RequestMatcherEntry { + private final RequestMatcher requestMatcher; + private final T entry; + + public RequestMatcherEntry(RequestMatcher requestMatcher, T entry) { + this.requestMatcher = requestMatcher; + this.entry = entry; + } + + public RequestMatcher getRequestMatcher() { + return requestMatcher; + } + + public T getEntry() { + return entry; + } +} diff --git a/src/test/java/nextstep/security/util/MvcRequestMatcherTest.java b/src/test/java/nextstep/security/util/MvcRequestMatcherTest.java new file mode 100644 index 0000000..0632f0d --- /dev/null +++ b/src/test/java/nextstep/security/util/MvcRequestMatcherTest.java @@ -0,0 +1,47 @@ +package nextstep.security.util; + +import static org.assertj.core.api.Assertions.*; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.springframework.http.HttpMethod; +import org.springframework.mock.web.MockHttpServletRequest; + +@DisplayName("MvcRequestMatcher 테스트") +class MvcRequestMatcherTest { + + @DisplayName("matches() - http 메서드와 uri가 같으면 true 반환") + @Test + void MatchesWhenMethodAndPatternMatch() { + // given + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setMethod("GET"); + request.setRequestURI("/test"); + + MvcRequestMatcher matcher = new MvcRequestMatcher(HttpMethod.GET, "/test"); + + // when + boolean matches = matcher.matches(request); + + // then + assertThat(matches).isTrue(); + } + + @DisplayName("matches() - http 메서드와 uri중 하나라도 다르면 false 반환") + @Test + void MatchesWhenMethodAndPatternIsNotMatch() { + // given + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setMethod("POST"); + request.setRequestURI("/test"); + + MvcRequestMatcher matcher = new MvcRequestMatcher(HttpMethod.GET, "/test"); + + // when + boolean matches = matcher.matches(request); + + // then + assertThat(matches).isFalse(); + } + +} \ No newline at end of file From 706c626d206bce1b9d2ce80b933f6b5970176398 Mon Sep 17 00:00:00 2001 From: parkSeryu Date: Tue, 11 Feb 2025 21:15:17 +0900 Subject: [PATCH 08/13] =?UTF-8?q?feat:=20=EA=B0=81=20=EC=9A=94=EC=B2=AD?= =?UTF-8?q?=EB=B3=84=20=EC=9D=B8=EA=B0=80=20=EB=A1=9C=EC=A7=81=EC=9D=84=20?= =?UTF-8?q?=EB=8B=B4=EB=8B=B9=ED=95=98=EB=8A=94=20AuthorizationManager?= =?UTF-8?q?=EB=93=A4=20=EC=9E=91=EC=84=B1?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../java/nextstep/app/SecurityConfig.java | 23 ++++++++++-- .../AuthenticatedAuthorizationManager.java | 16 +++++++++ .../web/AuthorityAuthorizationManager.java | 30 ++++++++++++++++ .../web/DenyAllAuthorizationManager.java | 13 +++++++ .../web/PermitAllAuthorizationManager.java | 13 +++++++ .../web/RequestAuthorizationManager.java | 35 ------------------- ...MatcherDelegatingAuthorizationManager.java | 12 +++++-- 7 files changed, 102 insertions(+), 40 deletions(-) create mode 100644 src/main/java/nextstep/security/authorization/web/AuthenticatedAuthorizationManager.java create mode 100644 src/main/java/nextstep/security/authorization/web/AuthorityAuthorizationManager.java create mode 100644 src/main/java/nextstep/security/authorization/web/DenyAllAuthorizationManager.java create mode 100644 src/main/java/nextstep/security/authorization/web/PermitAllAuthorizationManager.java delete mode 100644 src/main/java/nextstep/security/authorization/web/RequestAuthorizationManager.java diff --git a/src/main/java/nextstep/app/SecurityConfig.java b/src/main/java/nextstep/app/SecurityConfig.java index 3767a12..7d9afbe 100644 --- a/src/main/java/nextstep/app/SecurityConfig.java +++ b/src/main/java/nextstep/app/SecurityConfig.java @@ -1,6 +1,7 @@ package nextstep.app; import jakarta.servlet.http.HttpServletRequest; +import java.util.ArrayList; import nextstep.app.domain.Member; import nextstep.app.domain.MemberRepository; import nextstep.security.authentication.AuthenticationException; @@ -10,7 +11,10 @@ import nextstep.security.authorization.AuthorizationManager; import nextstep.security.authorization.SecuredMethodInterceptor; import nextstep.security.authorization.method.SecuredAuthorizationManager; -import nextstep.security.authorization.web.RequestAuthorizationManager; +import nextstep.security.authorization.web.AuthenticatedAuthorizationManager; +import nextstep.security.authorization.web.AuthorityAuthorizationManager; +import nextstep.security.authorization.web.DenyAllAuthorizationManager; +import nextstep.security.authorization.web.RequestMatcherDelegatingAuthorizationManager; import nextstep.security.config.DefaultSecurityFilterChain; import nextstep.security.config.DelegatingFilterProxy; import nextstep.security.config.FilterChainProxy; @@ -18,6 +22,10 @@ import nextstep.security.context.SecurityContextHolderFilter; import nextstep.security.userdetails.UserDetails; import nextstep.security.userdetails.UserDetailsService; +import nextstep.security.util.AnyRequestMatcher; +import nextstep.security.util.MvcRequestMatcher; +import nextstep.security.util.RequestMatcherEntry; +import nextstep.security.authorization.web.PermitAllAuthorizationManager; import org.aopalliance.intercept.MethodInvocation; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -25,6 +33,7 @@ import java.util.List; import java.util.Set; +import org.springframework.http.HttpMethod; @EnableAspectJAutoProxy @Configuration @@ -57,13 +66,21 @@ public SecuredMethodInterceptor securedMethodInterceptor() { // } @Bean - public AuthorizationManager securedAuthorizationManager(){ + public AuthorizationManager securedAuthorizationManager() { return new SecuredAuthorizationManager(); } @Bean public AuthorizationManager requestAuthorizationManager() { - return new RequestAuthorizationManager(); + List> mappings = new ArrayList<>(); + mappings.add(new RequestMatcherEntry<>(new MvcRequestMatcher(HttpMethod.GET, "/members"), + new AuthorityAuthorizationManager("ADMIN"))); + mappings.add(new RequestMatcherEntry<>(new MvcRequestMatcher(HttpMethod.GET, "/members/me"), + new AuthenticatedAuthorizationManager())); + mappings.add(new RequestMatcherEntry<>(new MvcRequestMatcher(HttpMethod.GET, "/search"), + new PermitAllAuthorizationManager())); + mappings.add(new RequestMatcherEntry<>(AnyRequestMatcher.INSTANCE, new DenyAllAuthorizationManager())); + return new RequestMatcherDelegatingAuthorizationManager(mappings); } @Bean diff --git a/src/main/java/nextstep/security/authorization/web/AuthenticatedAuthorizationManager.java b/src/main/java/nextstep/security/authorization/web/AuthenticatedAuthorizationManager.java new file mode 100644 index 0000000..6ed87e6 --- /dev/null +++ b/src/main/java/nextstep/security/authorization/web/AuthenticatedAuthorizationManager.java @@ -0,0 +1,16 @@ +package nextstep.security.authorization.web; + +import jakarta.servlet.http.HttpServletRequest; +import nextstep.security.authentication.Authentication; +import nextstep.security.authorization.AuthorizationDecision; +import nextstep.security.authorization.AuthorizationManager; + +public class AuthenticatedAuthorizationManager implements AuthorizationManager { + @Override + public AuthorizationDecision check(Authentication authentication, HttpServletRequest object) { + if (authentication != null && authentication.isAuthenticated()) { + return new AuthorizationDecision(true); + } + return new AuthorizationDecision(false); + } +} diff --git a/src/main/java/nextstep/security/authorization/web/AuthorityAuthorizationManager.java b/src/main/java/nextstep/security/authorization/web/AuthorityAuthorizationManager.java new file mode 100644 index 0000000..21e6407 --- /dev/null +++ b/src/main/java/nextstep/security/authorization/web/AuthorityAuthorizationManager.java @@ -0,0 +1,30 @@ +package nextstep.security.authorization.web; + +import jakarta.servlet.http.HttpServletRequest; +import nextstep.security.authentication.Authentication; +import nextstep.security.authentication.AuthenticationException; +import nextstep.security.authorization.AuthorizationDecision; +import nextstep.security.authorization.AuthorizationManager; +import nextstep.security.authorization.ForbiddenException; + +public class AuthorityAuthorizationManager implements AuthorizationManager { + + private final String authority; + + public AuthorityAuthorizationManager(String authority) { + this.authority = authority; + } + + @Override + public AuthorizationDecision check(Authentication authentication, HttpServletRequest object) { + if (authentication == null) { + throw new AuthenticationException(); + } + + boolean hasAuthority = authentication.getAuthorities().stream() + .anyMatch(authority::equals); + + return new AuthorizationDecision(hasAuthority); + + } +} diff --git a/src/main/java/nextstep/security/authorization/web/DenyAllAuthorizationManager.java b/src/main/java/nextstep/security/authorization/web/DenyAllAuthorizationManager.java new file mode 100644 index 0000000..1e963b8 --- /dev/null +++ b/src/main/java/nextstep/security/authorization/web/DenyAllAuthorizationManager.java @@ -0,0 +1,13 @@ +package nextstep.security.authorization.web; + +import jakarta.servlet.http.HttpServletRequest; +import nextstep.security.authentication.Authentication; +import nextstep.security.authorization.AuthorizationDecision; +import nextstep.security.authorization.AuthorizationManager; + +public class DenyAllAuthorizationManager implements AuthorizationManager { + @Override + public AuthorizationDecision check(Authentication authentication, HttpServletRequest object) { + return new AuthorizationDecision(false); + } +} diff --git a/src/main/java/nextstep/security/authorization/web/PermitAllAuthorizationManager.java b/src/main/java/nextstep/security/authorization/web/PermitAllAuthorizationManager.java new file mode 100644 index 0000000..131a805 --- /dev/null +++ b/src/main/java/nextstep/security/authorization/web/PermitAllAuthorizationManager.java @@ -0,0 +1,13 @@ +package nextstep.security.authorization.web; + +import jakarta.servlet.http.HttpServletRequest; +import nextstep.security.authentication.Authentication; +import nextstep.security.authorization.AuthorizationDecision; +import nextstep.security.authorization.AuthorizationManager; + +public class PermitAllAuthorizationManager implements AuthorizationManager { + @Override + public AuthorizationDecision check(Authentication authentication, HttpServletRequest object) { + return new AuthorizationDecision(true); + } +} diff --git a/src/main/java/nextstep/security/authorization/web/RequestAuthorizationManager.java b/src/main/java/nextstep/security/authorization/web/RequestAuthorizationManager.java deleted file mode 100644 index 25b4a43..0000000 --- a/src/main/java/nextstep/security/authorization/web/RequestAuthorizationManager.java +++ /dev/null @@ -1,35 +0,0 @@ -package nextstep.security.authorization.web; - -import jakarta.servlet.http.HttpServletRequest; -import nextstep.security.authentication.Authentication; -import nextstep.security.authorization.AuthorizationDecision; -import nextstep.security.authorization.AuthorizationManager; - -public class RequestAuthorizationManager implements AuthorizationManager { - @Override - public AuthorizationDecision check(Authentication authentication, HttpServletRequest httpRequest) { - if (httpRequest.getRequestURI().equals("/members")) { - if (authentication == null) { - return new AuthorizationDecision(false); - } - - return new AuthorizationDecision( - authentication.getAuthorities().stream() - .anyMatch(authority -> authority.equals("ADMIN"))); - } - - if (httpRequest.getRequestURI().equals("/members/me")) { - if (authentication == null) { - return new AuthorizationDecision(false); - } - - return new AuthorizationDecision(authentication.isAuthenticated()); - } - - if (httpRequest.getRequestURI().equals("/search")) { - return new AuthorizationDecision(true); - } - - return new AuthorizationDecision(false); - } -} diff --git a/src/main/java/nextstep/security/authorization/web/RequestMatcherDelegatingAuthorizationManager.java b/src/main/java/nextstep/security/authorization/web/RequestMatcherDelegatingAuthorizationManager.java index 3815a78..2e60d5d 100644 --- a/src/main/java/nextstep/security/authorization/web/RequestMatcherDelegatingAuthorizationManager.java +++ b/src/main/java/nextstep/security/authorization/web/RequestMatcherDelegatingAuthorizationManager.java @@ -5,10 +5,13 @@ import nextstep.security.authentication.Authentication; import nextstep.security.authorization.AuthorizationDecision; import nextstep.security.authorization.AuthorizationManager; +import nextstep.security.util.RequestMatcher; import nextstep.security.util.RequestMatcherEntry; public class RequestMatcherDelegatingAuthorizationManager implements AuthorizationManager { + private static final AuthorizationDecision DENY = new AuthorizationDecision(false); + private final List> mappings; public RequestMatcherDelegatingAuthorizationManager(List> mappings) { @@ -16,7 +19,12 @@ public RequestMatcherDelegatingAuthorizationManager(List mapping : mappings) { + if (mapping.getRequestMatcher().matches(request)) { + return mapping.getEntry().check(authentication, request); + } + } + return DENY; } } From 0f8d213a2a10b8991fe40d867f287545f02c7c83 Mon Sep 17 00:00:00 2001 From: parkSeryu Date: Wed, 12 Feb 2025 22:51:09 +0900 Subject: [PATCH 09/13] =?UTF-8?q?remove:=20=ED=94=BC=EB=93=9C=EB=B0=B1=20?= =?UTF-8?q?=EB=B0=98=EC=98=81=20-=20app=20=ED=8C=A8=ED=82=A4=EC=A7=80?= =?UTF-8?q?=EC=9D=98=20=EB=B6=88=ED=95=84=EC=9A=94=ED=95=9C=20=EC=BD=94?= =?UTF-8?q?=EB=93=9C=20=EC=A0=9C=EA=B1=B0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/main/java/nextstep/app/ui/MemberController.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/main/java/nextstep/app/ui/MemberController.java b/src/main/java/nextstep/app/ui/MemberController.java index 79c905b..11d680b 100644 --- a/src/main/java/nextstep/app/ui/MemberController.java +++ b/src/main/java/nextstep/app/ui/MemberController.java @@ -37,9 +37,6 @@ public ResponseEntity> search() { @GetMapping("/members/me") public ResponseEntity me() { Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); - if (authentication == null) { - throw new AuthenticationException(); - } String email = authentication.getPrincipal().toString(); Member member = memberRepository.findByEmail(email) From 0119ccaa46fb89838f1c06281336f2f593e877a6 Mon Sep 17 00:00:00 2001 From: parkSeryu Date: Wed, 12 Feb 2025 22:58:36 +0900 Subject: [PATCH 10/13] =?UTF-8?q?refactor:=20=ED=94=BC=EB=93=9C=EB=B0=B1?= =?UTF-8?q?=20=EB=B0=98=EC=98=81=20-=20match=20=EB=A9=94=EC=86=8C=EB=93=9C?= =?UTF-8?q?=EC=9D=98=20null-safe=ED=95=9C=20=EC=BD=94=EB=93=9C=EB=A1=9C=20?= =?UTF-8?q?=EB=B3=80=EA=B2=BD?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/main/java/nextstep/security/util/MvcRequestMatcher.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/nextstep/security/util/MvcRequestMatcher.java b/src/main/java/nextstep/security/util/MvcRequestMatcher.java index 1b1089f..fc6ba85 100644 --- a/src/main/java/nextstep/security/util/MvcRequestMatcher.java +++ b/src/main/java/nextstep/security/util/MvcRequestMatcher.java @@ -15,6 +15,6 @@ public MvcRequestMatcher(HttpMethod method, String pattern) { @Override public boolean matches(HttpServletRequest request) { - return this.method.equals(HttpMethod.valueOf(request.getMethod())) && request.getRequestURI().equals(pattern); + return this.method.equals(HttpMethod.valueOf(request.getMethod())) && pattern.equals(request.getRequestURI()); } } From f87ce1e7e3e05c6495aa05e91c6a3b8d4992b585 Mon Sep 17 00:00:00 2001 From: parkSeryu Date: Thu, 13 Feb 2025 00:07:42 +0900 Subject: [PATCH 11/13] =?UTF-8?q?refactor:=20=ED=94=BC=EB=93=9C=EB=B0=B1?= =?UTF-8?q?=20=EB=B0=98=EC=98=81=20-=20=EC=9D=B8=EC=8A=A4=ED=84=B4?= =?UTF-8?q?=EC=8A=A4=ED=99=94=20->=20=EC=83=81=EC=88=98=20=EC=82=AC?= =?UTF-8?q?=EC=9A=A9=EC=9C=BC=EB=A1=9C=20=EB=B3=80=EA=B2=BD?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../security/authorization/AuthorizationDecision.java | 10 +++++++--- .../method/SecuredAuthorizationManager.java | 2 +- .../web/AuthenticatedAuthorizationManager.java | 4 ++-- .../web/AuthorityAuthorizationManager.java | 2 +- .../authorization/web/DenyAllAuthorizationManager.java | 2 +- .../web/PermitAllAuthorizationManager.java | 2 +- .../RequestMatcherDelegatingAuthorizationManager.java | 4 +--- 7 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/main/java/nextstep/security/authorization/AuthorizationDecision.java b/src/main/java/nextstep/security/authorization/AuthorizationDecision.java index e789944..825f5e7 100644 --- a/src/main/java/nextstep/security/authorization/AuthorizationDecision.java +++ b/src/main/java/nextstep/security/authorization/AuthorizationDecision.java @@ -3,11 +3,15 @@ public class AuthorizationDecision { private final boolean isGranted; - public AuthorizationDecision(boolean isGranted) { - this.isGranted = isGranted; + public AuthorizationDecision(boolean granted) { + this.granted = granted; } public boolean isGranted() { - return isGranted; + return granted; + } + + public static AuthorizationDecision of(boolean granted) { + return granted ? ALLOW : DENY; } } diff --git a/src/main/java/nextstep/security/authorization/method/SecuredAuthorizationManager.java b/src/main/java/nextstep/security/authorization/method/SecuredAuthorizationManager.java index 7eb245e..b7454d7 100644 --- a/src/main/java/nextstep/security/authorization/method/SecuredAuthorizationManager.java +++ b/src/main/java/nextstep/security/authorization/method/SecuredAuthorizationManager.java @@ -22,7 +22,7 @@ public AuthorizationDecision check(Authentication authentication, MethodInvocati boolean hasAuthority = authentication.getAuthorities().stream() .anyMatch(authorities::contains); - return new AuthorizationDecision(hasAuthority); + return AuthorizationDecision.of(hasAuthority); } private Set getAuthorities(MethodInvocation invocation) { diff --git a/src/main/java/nextstep/security/authorization/web/AuthenticatedAuthorizationManager.java b/src/main/java/nextstep/security/authorization/web/AuthenticatedAuthorizationManager.java index 6ed87e6..7787aac 100644 --- a/src/main/java/nextstep/security/authorization/web/AuthenticatedAuthorizationManager.java +++ b/src/main/java/nextstep/security/authorization/web/AuthenticatedAuthorizationManager.java @@ -9,8 +9,8 @@ public class AuthenticatedAuthorizationManager implements AuthorizationManager { @Override public AuthorizationDecision check(Authentication authentication, HttpServletRequest object) { - return new AuthorizationDecision(false); + return AuthorizationDecision.DENY; } } diff --git a/src/main/java/nextstep/security/authorization/web/PermitAllAuthorizationManager.java b/src/main/java/nextstep/security/authorization/web/PermitAllAuthorizationManager.java index 131a805..fa96f58 100644 --- a/src/main/java/nextstep/security/authorization/web/PermitAllAuthorizationManager.java +++ b/src/main/java/nextstep/security/authorization/web/PermitAllAuthorizationManager.java @@ -8,6 +8,6 @@ public class PermitAllAuthorizationManager implements AuthorizationManager { @Override public AuthorizationDecision check(Authentication authentication, HttpServletRequest object) { - return new AuthorizationDecision(true); + return AuthorizationDecision.ALLOW; } } diff --git a/src/main/java/nextstep/security/authorization/web/RequestMatcherDelegatingAuthorizationManager.java b/src/main/java/nextstep/security/authorization/web/RequestMatcherDelegatingAuthorizationManager.java index 2e60d5d..cb3f6f9 100644 --- a/src/main/java/nextstep/security/authorization/web/RequestMatcherDelegatingAuthorizationManager.java +++ b/src/main/java/nextstep/security/authorization/web/RequestMatcherDelegatingAuthorizationManager.java @@ -10,8 +10,6 @@ public class RequestMatcherDelegatingAuthorizationManager implements AuthorizationManager { - private static final AuthorizationDecision DENY = new AuthorizationDecision(false); - private final List> mappings; public RequestMatcherDelegatingAuthorizationManager(List> mappings) { @@ -25,6 +23,6 @@ public AuthorizationDecision check(Authentication authentication, HttpServletReq return mapping.getEntry().check(authentication, request); } } - return DENY; + return AuthorizationDecision.DENY; } } From 83a4efd95662b7e9d2590f72ee5a56173a16477d Mon Sep 17 00:00:00 2001 From: parkSeryu Date: Thu, 13 Feb 2025 02:30:06 +0900 Subject: [PATCH 12/13] =?UTF-8?q?refactor:=20=ED=94=BC=EB=93=9C=EB=B0=B1?= =?UTF-8?q?=20=EB=B0=98=EC=98=81=20-=20=EC=97=AD=ED=95=A0=EA=B3=BC=20?= =?UTF-8?q?=EC=B1=85=EC=9E=84=20=EB=B6=84=EB=A6=AC?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../java/nextstep/app/SecurityConfig.java | 9 ++----- .../method/SecuredAuthorizationManager.java | 27 +++++++++++-------- .../web/AuthorityAuthorizationManager.java | 26 +++++++++++------- 3 files changed, 35 insertions(+), 27 deletions(-) diff --git a/src/main/java/nextstep/app/SecurityConfig.java b/src/main/java/nextstep/app/SecurityConfig.java index 7d9afbe..fb68c0f 100644 --- a/src/main/java/nextstep/app/SecurityConfig.java +++ b/src/main/java/nextstep/app/SecurityConfig.java @@ -60,21 +60,16 @@ public SecuredMethodInterceptor securedMethodInterceptor() { return new SecuredMethodInterceptor(securedAuthorizationManager()); } -// @Bean -// public SecuredAspect securedAspect() { -// return new SecuredAspect(); -// } - @Bean public AuthorizationManager securedAuthorizationManager() { - return new SecuredAuthorizationManager(); + return new SecuredAuthorizationManager(); } @Bean public AuthorizationManager requestAuthorizationManager() { List> mappings = new ArrayList<>(); mappings.add(new RequestMatcherEntry<>(new MvcRequestMatcher(HttpMethod.GET, "/members"), - new AuthorityAuthorizationManager("ADMIN"))); + new AuthorityAuthorizationManager(Set.of("ADMIN")))); mappings.add(new RequestMatcherEntry<>(new MvcRequestMatcher(HttpMethod.GET, "/members/me"), new AuthenticatedAuthorizationManager())); mappings.add(new RequestMatcherEntry<>(new MvcRequestMatcher(HttpMethod.GET, "/search"), diff --git a/src/main/java/nextstep/security/authorization/method/SecuredAuthorizationManager.java b/src/main/java/nextstep/security/authorization/method/SecuredAuthorizationManager.java index b7454d7..ddc03ee 100644 --- a/src/main/java/nextstep/security/authorization/method/SecuredAuthorizationManager.java +++ b/src/main/java/nextstep/security/authorization/method/SecuredAuthorizationManager.java @@ -1,31 +1,36 @@ package nextstep.security.authorization.method; import java.lang.reflect.Method; +import java.util.Collection; import java.util.Collections; import java.util.Set; import nextstep.security.authentication.Authentication; -import nextstep.security.authentication.AuthenticationException; import nextstep.security.authorization.AuthorizationDecision; import nextstep.security.authorization.AuthorizationManager; import nextstep.security.authorization.Secured; +import nextstep.security.authorization.web.AuthorityAuthorizationManager; import org.aopalliance.intercept.MethodInvocation; public class SecuredAuthorizationManager implements AuthorizationManager { - @Override - public AuthorizationDecision check(Authentication authentication, MethodInvocation invocation) { - if (authentication == null) { - throw new AuthenticationException(); - } - Set authorities = getAuthorities(invocation); + private AuthorityAuthorizationManager> authorityAuthorizationManager; + + public void setAuthorityAuthorizationManager(Collection authorities) { + authorityAuthorizationManager = new AuthorityAuthorizationManager<>(authorities); + } - boolean hasAuthority = authentication.getAuthorities().stream() - .anyMatch(authorities::contains); + @Override + public AuthorizationDecision check(Authentication authentication, MethodInvocation invocation) { + Collection authorities = getAuthorities(invocation); - return AuthorizationDecision.of(hasAuthority); + if (authorities.isEmpty()) { + return null; + } + setAuthorityAuthorizationManager(authorities); + return authorities.isEmpty() ? null : authorityAuthorizationManager.check(authentication, authorities); } - private Set getAuthorities(MethodInvocation invocation) { + private Collection getAuthorities(MethodInvocation invocation) { Method method = invocation.getMethod(); if (!method.isAnnotationPresent(Secured.class)) { diff --git a/src/main/java/nextstep/security/authorization/web/AuthorityAuthorizationManager.java b/src/main/java/nextstep/security/authorization/web/AuthorityAuthorizationManager.java index 7eba78c..e99689f 100644 --- a/src/main/java/nextstep/security/authorization/web/AuthorityAuthorizationManager.java +++ b/src/main/java/nextstep/security/authorization/web/AuthorityAuthorizationManager.java @@ -1,30 +1,38 @@ package nextstep.security.authorization.web; -import jakarta.servlet.http.HttpServletRequest; +import java.util.Collection; import nextstep.security.authentication.Authentication; import nextstep.security.authentication.AuthenticationException; import nextstep.security.authorization.AuthorizationDecision; import nextstep.security.authorization.AuthorizationManager; -import nextstep.security.authorization.ForbiddenException; -public class AuthorityAuthorizationManager implements AuthorizationManager { +public class AuthorityAuthorizationManager implements AuthorizationManager { - private final String authority; + private final Collection authorities; - public AuthorityAuthorizationManager(String authority) { - this.authority = authority; + public AuthorityAuthorizationManager(Collection authorities) { + this.authorities = authorities; } + @Override - public AuthorizationDecision check(Authentication authentication, HttpServletRequest object) { + public AuthorizationDecision check(Authentication authentication, T object) { if (authentication == null) { throw new AuthenticationException(); } - boolean hasAuthority = authentication.getAuthorities().stream() - .anyMatch(authority::equals); + boolean hasAuthority = isAuthorized(authentication, authorities); return AuthorizationDecision.of(hasAuthority); + } + + private boolean isAuthorized(Authentication authentication, Collection authorities) { + for (String authority : authentication.getAuthorities()) { + if (authorities.contains(authority)) { + return true; + } + } + return false; } } From eca73524ce639ea7c889f282146877547a3be6b3 Mon Sep 17 00:00:00 2001 From: parkSeryu Date: Thu, 13 Feb 2025 02:35:55 +0900 Subject: [PATCH 13/13] =?UTF-8?q?refactor:=20=ED=94=BC=EB=93=9C=EB=B0=B1?= =?UTF-8?q?=20=EB=B0=98=EC=98=81=20check=20->=20authorize=EB=A1=9C=20?= =?UTF-8?q?=EB=B3=80=EA=B2=BD?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../security/authorization/AuthorizationDecision.java | 9 +++++++-- .../security/authorization/AuthorizationFilter.java | 5 +++-- .../security/authorization/AuthorizationManager.java | 6 ++++++ .../security/authorization/SecuredMethodInterceptor.java | 8 +++----- .../security/authorization/web/AuthorizationResult.java | 5 +++++ 5 files changed, 24 insertions(+), 9 deletions(-) create mode 100644 src/main/java/nextstep/security/authorization/web/AuthorizationResult.java diff --git a/src/main/java/nextstep/security/authorization/AuthorizationDecision.java b/src/main/java/nextstep/security/authorization/AuthorizationDecision.java index 825f5e7..859efcf 100644 --- a/src/main/java/nextstep/security/authorization/AuthorizationDecision.java +++ b/src/main/java/nextstep/security/authorization/AuthorizationDecision.java @@ -1,7 +1,12 @@ package nextstep.security.authorization; -public class AuthorizationDecision { - private final boolean isGranted; +import nextstep.security.authorization.web.AuthorizationResult; + +public class AuthorizationDecision implements AuthorizationResult { + public static final AuthorizationDecision ALLOW = new AuthorizationDecision(true); + public static final AuthorizationDecision DENY = new AuthorizationDecision(false); + + private final boolean granted; public AuthorizationDecision(boolean granted) { this.granted = granted; diff --git a/src/main/java/nextstep/security/authorization/AuthorizationFilter.java b/src/main/java/nextstep/security/authorization/AuthorizationFilter.java index 0fc45de..36c3b40 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 nextstep.security.authentication.Authentication; +import nextstep.security.authorization.web.AuthorizationResult; import nextstep.security.context.SecurityContextHolder; import org.springframework.web.filter.OncePerRequestFilter; @@ -22,9 +23,9 @@ public AuthorizationFilter(AuthorizationManager authorizatio protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) throws ServletException, IOException { Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); - AuthorizationDecision authorizationDecision = authorizationManager.check(authentication, request); + AuthorizationResult authorizeResult = authorizationManager.authorize(authentication, request); - if (!authorizationDecision.isGranted()) { + if (!authorizeResult.isGranted()) { throw new ForbiddenException(); } diff --git a/src/main/java/nextstep/security/authorization/AuthorizationManager.java b/src/main/java/nextstep/security/authorization/AuthorizationManager.java index 42b8610..4be5334 100644 --- a/src/main/java/nextstep/security/authorization/AuthorizationManager.java +++ b/src/main/java/nextstep/security/authorization/AuthorizationManager.java @@ -1,8 +1,14 @@ package nextstep.security.authorization; import nextstep.security.authentication.Authentication; +import nextstep.security.authorization.web.AuthorizationResult; @FunctionalInterface public interface AuthorizationManager { + @Deprecated AuthorizationDecision check(Authentication authentication, T object); + + default AuthorizationResult authorize(Authentication authentication, T object) { + return check(authentication, object); + } } diff --git a/src/main/java/nextstep/security/authorization/SecuredMethodInterceptor.java b/src/main/java/nextstep/security/authorization/SecuredMethodInterceptor.java index c9bdc53..3490828 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.web.AuthorizationResult; import nextstep.security.context.SecurityContextHolder; import org.aopalliance.aop.Advice; import org.aopalliance.intercept.MethodInterceptor; @@ -11,8 +11,6 @@ import org.springframework.aop.framework.AopInfrastructureBean; import org.springframework.aop.support.annotation.AnnotationMatchingPointcut; -import java.lang.reflect.Method; - public class SecuredMethodInterceptor implements MethodInterceptor, PointcutAdvisor, AopInfrastructureBean { private final AuthorizationManager authorizationManager; @@ -26,9 +24,9 @@ public SecuredMethodInterceptor(AuthorizationManager authoriza @Override public Object invoke(MethodInvocation invocation) throws Throwable { Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); - AuthorizationDecision authorizationDecision = authorizationManager.check(authentication, invocation); + AuthorizationResult authorizationResult = authorizationManager.authorize(authentication, invocation); - if (!authorizationDecision.isGranted()) { + if (!authorizationResult.isGranted()) { throw new ForbiddenException(); } diff --git a/src/main/java/nextstep/security/authorization/web/AuthorizationResult.java b/src/main/java/nextstep/security/authorization/web/AuthorizationResult.java new file mode 100644 index 0000000..d18b14e --- /dev/null +++ b/src/main/java/nextstep/security/authorization/web/AuthorizationResult.java @@ -0,0 +1,5 @@ +package nextstep.security.authorization.web; + +public interface AuthorizationResult { + boolean isGranted(); +}