diff --git a/src/main/java/nextstep/app/config/AuthConfig.java b/src/main/java/nextstep/app/config/AuthConfig.java index e59870d..c130393 100644 --- a/src/main/java/nextstep/app/config/AuthConfig.java +++ b/src/main/java/nextstep/app/config/AuthConfig.java @@ -2,8 +2,9 @@ import nextstep.security.access.matcher.MvcRequestMatcher; import nextstep.security.authentication.*; -import nextstep.security.authorization.LoginAuthorizationFilter; +import nextstep.security.authorization.SessionAuthorizationFilter; import nextstep.security.authorization.RoleAuthorizationFilter; +import nextstep.security.config.AuthorizeRequestMatcherRegistry; import nextstep.security.config.DefaultSecurityFilterChain; import nextstep.security.config.FilterChainProxy; import nextstep.security.config.SecurityFilterChain; @@ -29,6 +30,15 @@ public AuthConfig(UserDetailsService userDetailsService) { this.userDetailsService = userDetailsService; } + @Bean + public AuthorizeRequestMatcherRegistry requestMatcherRegistryMapping() { + AuthorizeRequestMatcherRegistry requestMatcherRegistry = new AuthorizeRequestMatcherRegistry(); + requestMatcherRegistry + .matcher(new MvcRequestMatcher(HttpMethod.GET, "/members")).hasAuthority("ADMIN") + .matcher(new MvcRequestMatcher(HttpMethod.GET, "/members/me")).authenticated(); + return requestMatcherRegistry; + } + @Bean public DelegatingFilterProxy securityFilterChainProxy() { return new DelegatingFilterProxy("filterChainProxy"); @@ -50,8 +60,8 @@ public SecurityFilterChain loginSecurityFilterChain() { public SecurityFilterChain membersSecurityFilterChain() { List filters = new ArrayList<>(); filters.add(new BasicAuthenticationFilter(authenticationManager())); - filters.add(new LoginAuthorizationFilter(securityContextRepository())); - filters.add(new RoleAuthorizationFilter()); + filters.add(new SessionAuthorizationFilter(securityContextRepository(), requestMatcherRegistryMapping())); + filters.add(new RoleAuthorizationFilter(requestMatcherRegistryMapping())); return new DefaultSecurityFilterChain(new MvcRequestMatcher(HttpMethod.GET, "/members"), filters); } @@ -64,5 +74,4 @@ public SecurityContextRepository securityContextRepository() { public AuthenticationManager authenticationManager() { return new AuthenticationManager(new UsernamePasswordAuthenticationProvider(userDetailsService)); } - } diff --git a/src/main/java/nextstep/app/ui/MemberController.java b/src/main/java/nextstep/app/ui/MemberController.java index fce08d7..226b300 100644 --- a/src/main/java/nextstep/app/ui/MemberController.java +++ b/src/main/java/nextstep/app/ui/MemberController.java @@ -25,4 +25,11 @@ public ResponseEntity> list() { return ResponseEntity.ok(members); } + @GetMapping("/members/me") + public ResponseEntity me() { + Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); + String email = authentication.getPrincipal().toString(); + Member member = memberRepository.findByEmail(email).orElseThrow(); + return ResponseEntity.ok(member); + } } diff --git a/src/main/java/nextstep/security/access/matcher/MvcRequestMatcher.java b/src/main/java/nextstep/security/access/matcher/MvcRequestMatcher.java index abfb3d4..0b02326 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().contains(pattern) || request.getRequestURI().startsWith(pattern); } } diff --git a/src/main/java/nextstep/security/authorization/RoleAuthorizationFilter.java b/src/main/java/nextstep/security/authorization/RoleAuthorizationFilter.java index 42e97e0..1276531 100644 --- a/src/main/java/nextstep/security/authorization/RoleAuthorizationFilter.java +++ b/src/main/java/nextstep/security/authorization/RoleAuthorizationFilter.java @@ -1,10 +1,9 @@ package nextstep.security.authorization; -import nextstep.security.access.matcher.MvcRequestMatcher; import nextstep.security.authentication.Authentication; +import nextstep.security.config.AuthorizeRequestMatcherRegistry; import nextstep.security.context.SecurityContextHolder; import nextstep.security.exception.AuthenticationException; -import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; import org.springframework.web.filter.GenericFilterBean; @@ -17,22 +16,22 @@ import java.io.IOException; public class RoleAuthorizationFilter extends GenericFilterBean { - private static final MvcRequestMatcher DEFAULT_REQUEST_MATCHER = new MvcRequestMatcher(HttpMethod.GET, - "/members"); - private static final String ADMIN_ROLE = "ADMIN"; + private final AuthorizeRequestMatcherRegistry requestMatcherRegistry; + + public RoleAuthorizationFilter(AuthorizeRequestMatcherRegistry requestMatcherRegistry) { + this.requestMatcherRegistry = requestMatcherRegistry; + } + @Override public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException { try { - if (!DEFAULT_REQUEST_MATCHER.matches((HttpServletRequest) request)) { - chain.doFilter(request, response); - return; - } - + String attribute = requestMatcherRegistry.getAttribute((HttpServletRequest) request); Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); + boolean isAuthorized = requestMatcherRegistry.isAuthorized(attribute, authentication); - if (!authentication.getAuthorities().contains(ADMIN_ROLE)) { + if (!isAuthorized) { ((HttpServletResponse) response).sendError(HttpStatus.FORBIDDEN.value(), HttpStatus.FORBIDDEN.getReasonPhrase()); return; } diff --git a/src/main/java/nextstep/security/authorization/LoginAuthorizationFilter.java b/src/main/java/nextstep/security/authorization/SessionAuthorizationFilter.java similarity index 63% rename from src/main/java/nextstep/security/authorization/LoginAuthorizationFilter.java rename to src/main/java/nextstep/security/authorization/SessionAuthorizationFilter.java index a5ad5e0..53723e5 100644 --- a/src/main/java/nextstep/security/authorization/LoginAuthorizationFilter.java +++ b/src/main/java/nextstep/security/authorization/SessionAuthorizationFilter.java @@ -1,11 +1,10 @@ package nextstep.security.authorization; -import nextstep.security.access.matcher.MvcRequestMatcher; +import nextstep.security.config.AuthorizeRequestMatcherRegistry; import nextstep.security.context.SecurityContext; import nextstep.security.context.SecurityContextHolder; import nextstep.security.context.SecurityContextRepository; import nextstep.security.exception.AuthenticationException; -import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; import org.springframework.web.filter.GenericFilterBean; @@ -17,33 +16,33 @@ import javax.servlet.http.HttpServletResponse; import java.io.IOException; -public class LoginAuthorizationFilter extends GenericFilterBean { - private static final MvcRequestMatcher DEFAULT_REQUEST_MATCHER = new MvcRequestMatcher(HttpMethod.GET, - "/members"); +public class SessionAuthorizationFilter extends GenericFilterBean { private final SecurityContextRepository securityContextRepository; - public LoginAuthorizationFilter(SecurityContextRepository securityContextRepository) { + private final AuthorizeRequestMatcherRegistry requestMatcherRegistry; + + public SessionAuthorizationFilter(SecurityContextRepository securityContextRepository, AuthorizeRequestMatcherRegistry requestMatcherRegistry) { this.securityContextRepository = securityContextRepository; + this.requestMatcherRegistry = requestMatcherRegistry; } @Override public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException { try { - if (!DEFAULT_REQUEST_MATCHER.matches((HttpServletRequest) request)) { - chain.doFilter(request, response); - return; - } - + String attribute = requestMatcherRegistry.getAttribute((HttpServletRequest) request); SecurityContext loadedContext = securityContextRepository.loadContext((HttpServletRequest) request); if (loadedContext == null) { ((HttpServletResponse) response).sendError(HttpStatus.FORBIDDEN.value(), HttpStatus.FORBIDDEN.getReasonPhrase()); return; } + boolean isAuthorized = requestMatcherRegistry.isAuthorized(attribute, loadedContext.getAuthentication()); - SecurityContext context = SecurityContextHolder.createEmptyContext(); - context.setAuthentication(loadedContext.getAuthentication()); - SecurityContextHolder.setContext(context); + if (isAuthorized) { + SecurityContext context = SecurityContextHolder.createEmptyContext(); + context.setAuthentication(loadedContext.getAuthentication()); + SecurityContextHolder.setContext(context); + } } catch (AuthenticationException e) { ((HttpServletResponse) response).sendError(HttpStatus.UNAUTHORIZED.value(), HttpStatus.UNAUTHORIZED.getReasonPhrase()); return; 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..39c235d --- /dev/null +++ b/src/main/java/nextstep/security/config/AuthorizeRequestMatcherRegistry.java @@ -0,0 +1,82 @@ +package nextstep.security.config; + +import nextstep.security.access.matcher.RequestMatcher; +import nextstep.security.authentication.Authentication; + +import javax.servlet.http.HttpServletRequest; +import java.util.HashMap; +import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +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"; + public static final String AUTHORITY = "hasAuthority"; + + 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(AUTHORITY + "(" + authority + ")"); + } + + public AuthorizeRequestMatcherRegistry authenticated() { + return access(AUTHENTICATED); + } + + private AuthorizeRequestMatcherRegistry access(String attribute) { + return addMapping(requestMatcher, attribute); + } + } + + public Boolean isAuthorized(String attribute, Authentication authentication) { + if (attribute.contains(AuthorizedUrl.AUTHORITY)) { + Pattern pattern = Pattern.compile("\\((.*?)\\)"); + Matcher matcher = pattern.matcher(attribute); + if(matcher.find()) { + String role = matcher.group(1).trim(); + return authentication.getAuthorities().contains(role); + } + } else if (attribute.contains(AuthorizedUrl.AUTHENTICATED)) { + return authentication.isAuthenticated(); + } + + return false; + } + +} diff --git a/src/test/java/nextstep/app/MemberAcceptanceTest.java b/src/test/java/nextstep/app/MemberAcceptanceTest.java index dc861af..617cef0 100644 --- a/src/test/java/nextstep/app/MemberAcceptanceTest.java +++ b/src/test/java/nextstep/app/MemberAcceptanceTest.java @@ -24,36 +24,86 @@ void get_members_after_form_login() { params.put("username", TEST_MEMBER.getEmail()); params.put("password", TEST_MEMBER.getPassword()); - ExtractableResponse loginResponse = RestAssured.given().log().all() - .formParams(params) - .contentType(MediaType.APPLICATION_FORM_URLENCODED_VALUE) - .when() - .post("/login") - .then().log().all() - .extract(); + ExtractableResponse loginResponse = requestLogin(params); + + ExtractableResponse memberResponse = requestMembers(loginResponse); + assertThat(memberResponse.statusCode()).isEqualTo(HttpStatus.OK.value()); + List members = memberResponse.jsonPath().getList(".", Member.class); + assertThat(members.size()).isEqualTo(2); + } + + @Test + void get_members_before_form_login() { ExtractableResponse memberResponse = RestAssured.given().log().all() - .cookies(loginResponse.cookies()) .contentType(MediaType.APPLICATION_JSON_VALUE) .when() .get("/members") .then().log().all() .extract(); + assertThat(memberResponse.statusCode()).isEqualTo(HttpStatus.FORBIDDEN.value()); + } + + @Test + void get_members_me_after_form_login() { + Map params = new HashMap<>(); + params.put("username", TEST_MEMBER.getEmail()); + params.put("password", TEST_MEMBER.getPassword()); + + ExtractableResponse loginResponse = requestLogin(params); + + ExtractableResponse memberResponse = requestMembersMe(loginResponse); + assertThat(memberResponse.statusCode()).isEqualTo(HttpStatus.OK.value()); - List members = memberResponse.jsonPath().getList(".", Member.class); - assertThat(members.size()).isEqualTo(2); + assertThat(memberResponse.jsonPath().getString("email")).isEqualTo(TEST_MEMBER.getEmail()); } @Test - void get_members_before_form_login() { + void get_members_me_before_form_login() { + ExtractableResponse memberResponse = requestMembersMe(null); + + assertThat(memberResponse.statusCode()).isEqualTo(HttpStatus.FORBIDDEN.value()); + } + + private static ExtractableResponse requestLogin(Map params) { + ExtractableResponse loginResponse = RestAssured.given().log().all() + .formParams(params) + .contentType(MediaType.APPLICATION_FORM_URLENCODED_VALUE) + .when() + .post("/login") + .then().log().all() + .extract(); + return loginResponse; + } + + private static ExtractableResponse requestMembers(ExtractableResponse loginResponse) { ExtractableResponse memberResponse = RestAssured.given().log().all() + .cookies(loginResponse.cookies()) .contentType(MediaType.APPLICATION_JSON_VALUE) .when() .get("/members") .then().log().all() .extract(); + return memberResponse; + } - assertThat(memberResponse.statusCode()).isEqualTo(HttpStatus.FORBIDDEN.value()); + private static ExtractableResponse requestMembersMe(ExtractableResponse loginResponse) { + if (loginResponse == null) { + return RestAssured.given().log().all() + .contentType(MediaType.APPLICATION_JSON_VALUE) + .when() + .get("/members/me") + .then().log().all() + .extract(); + } else { + return RestAssured.given().log().all() + .contentType(MediaType.APPLICATION_JSON_VALUE) + .cookies(loginResponse.cookies()) + .when() + .get("/members/me") + .then().log().all() + .extract(); + } } } diff --git a/src/test/java/nextstep/app/MemberTest.java b/src/test/java/nextstep/app/MemberTest.java index 1961fb4..ac650d0 100644 --- a/src/test/java/nextstep/app/MemberTest.java +++ b/src/test/java/nextstep/app/MemberTest.java @@ -1,6 +1,8 @@ package nextstep.app; import nextstep.security.authentication.Authentication; +import nextstep.security.authentication.UsernamePasswordAuthentication; +import nextstep.security.context.SecurityContext; import nextstep.security.context.SecurityContextHolder; import nextstep.app.domain.Member; import nextstep.app.infrastructure.InmemoryMemberRepository; @@ -9,11 +11,14 @@ import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.http.MediaType; +import org.springframework.mock.web.MockHttpSession; import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.ResultActions; import org.springframework.test.web.servlet.result.MockMvcResultMatchers; import java.util.Base64; +import java.util.Collections; +import java.util.Set; import static org.assertj.core.api.Assertions.assertThat; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; @@ -29,8 +34,10 @@ public class MemberTest { private MockMvc mockMvc; @Test - void request_success_with_admin_user() throws Exception { - ResultActions response = requestWithBasicAuth(TEST_ADMIN_MEMBER.getEmail(), TEST_ADMIN_MEMBER.getPassword()); + void members_request_success_with_admin_user() throws Exception { + ResultActions response = requestMembersWithBasicAuthAndSession( + TEST_ADMIN_MEMBER.getEmail(), TEST_ADMIN_MEMBER.getPassword(), TEST_ADMIN_MEMBER.getRoles() + ); response.andExpect(status().isOk()) .andExpect(MockMvcResultMatchers.jsonPath("$.length()").value(2)); @@ -41,8 +48,10 @@ void request_success_with_admin_user() throws Exception { } @Test - void request_fail_with_general_user() throws Exception { - ResultActions response = requestWithBasicAuth(TEST_USER_MEMBER.getEmail(), TEST_USER_MEMBER.getPassword()); + void members_request_fail_with_general_user() throws Exception { + ResultActions response = requestMembersWithBasicAuthAndSession( + TEST_USER_MEMBER.getEmail(), TEST_USER_MEMBER.getPassword(), TEST_USER_MEMBER.getRoles() + ); response.andExpect(status().isForbidden()); @@ -52,25 +61,66 @@ void request_fail_with_general_user() throws Exception { } @Test - void request_fail_with_no_user() throws Exception { - ResultActions response = requestWithBasicAuth("none", "none"); + void members_request_fail_with_no_user() throws Exception { + ResultActions response = requestMembersWithBasicAuthAndSession("none", "none", Collections.emptySet()); + + response.andExpect(status().isUnauthorized()); + } + + @Test + void members_request_fail_invalid_password() throws Exception { + ResultActions response = requestMembersWithBasicAuthAndSession(TEST_ADMIN_MEMBER.getEmail(), "invalid", Collections.emptySet()); response.andExpect(status().isUnauthorized()); } @Test - void request_fail_invalid_password() throws Exception { - ResultActions response = requestWithBasicAuth(TEST_ADMIN_MEMBER.getEmail(), "invalid"); + void members_me_request_success() throws Exception { + ResultActions response = requestMembersMeWithBasicAuthAndSession( + TEST_USER_MEMBER.getEmail(), TEST_ADMIN_MEMBER.getPassword(), TEST_ADMIN_MEMBER.getRoles() + ); + + response.andExpect(status().isOk()) + .andExpect(MockMvcResultMatchers.jsonPath("$.email").value(TEST_USER_MEMBER.getEmail())); + + Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); + assertThat(authentication.isAuthenticated()).isTrue(); + } + + @Test + void members_me_request_fail() throws Exception { + ResultActions response = requestMembersMeWithBasicAuthAndSession( + TEST_USER_MEMBER.getEmail(), "invalid", Collections.emptySet() + ); response.andExpect(status().isUnauthorized()); } - private ResultActions requestWithBasicAuth(String username, String password) throws Exception { + private ResultActions requestMembersWithBasicAuthAndSession(String username, String password, Set roles) throws Exception { String token = Base64.getEncoder().encodeToString((username + ":" + password).getBytes()); return mockMvc.perform(get("/members") .header("Authorization", "Basic " + token) + .session(getLoginSession(username, password, roles)) .contentType(MediaType.APPLICATION_FORM_URLENCODED_VALUE) ); } + + private ResultActions requestMembersMeWithBasicAuthAndSession(String username, String password, Set roles) throws Exception { + String token = Base64.getEncoder().encodeToString((username + ":" + password).getBytes()); + + return mockMvc.perform(get("/members/me") + .header("Authorization", "Basic " + token) + .session(getLoginSession(username, password, roles)) + .contentType(MediaType.APPLICATION_FORM_URLENCODED_VALUE) + ); + } + + private MockHttpSession getLoginSession(String username, String password, Set roles) { + MockHttpSession session = new MockHttpSession(); + SecurityContext context = SecurityContextHolder.createEmptyContext(); + context.setAuthentication(UsernamePasswordAuthentication.ofAuthenticated(username, password, roles)); + session.setAttribute("SPRING_SECURITY_CONTEXT", context); + return session; + } }