[김선호] 미션 4 - 취약점 대응 & 리팩터링 리뷰 요청드립니다 🚀

README.md

@@ -1 +1,136 @@
-# spring-security
+> 미션 4: 취약점 대응 & 리팩토링
+
+<!-- TOC -->
+* [요구사항](#요구사항)
+  * [실습 - 취약점 대응(CsrfFilter)](#실습---취약점-대응csrffilter)
+  * [1단계 - SecurityFilterChain 리팩토링](#1단계---securityfilterchain-리팩토링)
+  * [2단계 - 인증 관련 리팩토링](#2단계---인증-관련-리팩토링)
+  * [3단계 - 인가 관련 리팩토링](#3단계---인가-관련-리팩토링)
+  * [4단계 - Auto Configuration 적용](#4단계---auto-configuration-적용)
+    * [미션을 진행하며 생긴 고민에 대한 흔적](#미션을-진행하며-생긴-고민에-대한-흔적)
+* [플로우차트를 활용한 이해](#플로우차트를-활용한-이해)
+  * [CSRF 공격 대응](#csrf-공격-대응)
+    * [CSRF 토큰 적용 전](#csrf-토큰-적용-전)
+    * [CSRF 토큰 적용 후](#csrf-토큰-적용-후)
+<!-- TOC -->
+
+# 요구사항
+
+## 실습 - 취약점 대응(CsrfFilter)
+
+> CsrfFilter를 이용한 CSRF 공격 대응
+
+- [x] CsrfToken 구현
+- [x] CsrfTokenRepository 구현 - HttpSessionCsrfTokenRepository
+  - [x] CsrfToken 발급/저장/조회
+- [x] CsrfFilter 구현
+  - [x] CsrfTokenRepository를 이용한 CsrfToken 검증
+
+## 1단계 - SecurityFilterChain 리팩토링
+
+> 주요 클래스
+> - `HttpSecurity`
+> - `HttpSecurityConfiguration`
+> - `SecurityConfigurer`
+> - `Customizer`
+> - `@EnableWebSecurity`
+
+- [x] HttpSecurity 구현
+- [x] @EnableWebSecurity, HttpSecurityConfiguration를 이용한 HttpSecurity 빈 등록
+- [x] csrf 필터를 configurer를 이용하여 설정
+
+## 2단계 - 인증 관련 리팩토링
+
+- [x] `.formLogin()` 메서드를 사용하여 폼 로그인 기능을 설정하고, U`sernamePasswordAuthenticationFilter`를 자동으로 추가한다.
+- [x] `.httpBasic()` 메서드를 사용해 HTTP Basic 인증을 설정하고, `BasicAuthenticationFilter`를 자동으로 추가한다.
+- [x] `.securityContext()` 메서드를 사용하여 `SecurityContextHolderFilter` 자동으로 추가
+- [x] oauth2 리팩토링
+  - [x] OAuth2AuthorizationRequestRedirectFilter 등록, OAuth2LoginAuthenticationFilter 등록
+
+## 3단계 - 인가 관련 리팩토링
+
+> 예시 코드
+
+```java
+
+@Bean
+public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception {
+    http
+            .authorizeHttpRequests(authorize -> authorize
+                    .requestMatchers("/public").permitAll()  // /public 경로는 모두 허용
+                    .anyRequest().authenticated())  // 그 외의 경로는 인증 필요
+            .formLogin(Customizer.withDefaults())  // 폼 로그인
+            .httpBasic(Customizer.withDefaults());  // HTTP Basic 인증
+
+    return http.build();
+}
+```
+
+- [x] `authorizeHttpRequests()` 메서드 구현
+  - [x] `AuthorizeHttpRequestsConfigurer`를 이용한 설정
+  - [x] 특정 경로에 대해 인증 없이 접근 가능하도록 설정하고, 나머지 요청에 대해서는 인증이 필요하도록 설정한다.
+  - [x] 특정 경로에 대해서 권한에 따라 접근 가능하게 할지/말지를 설정한다
+
+## 4단계 - Auto Configuration 적용
+
+> Auto Configuration을 통한 기본 SecurityFilterChain 설정
+> 
+> 스프링 부트의 자동 설정을 통해 기본 SecurityFilterChain이 설정되도록 하고, 필요시 사용자가 새로운 SecurityFilterChain을 추가할 수 있도록 한다. 사용자가 새로운 SecurityFilterChain을 정의할 경우, 기본 보안 설정이 비활성화된다.
+> 주요 클래스 
+> - @ConditionalOnDefaultWebSecurity
+> - @DefaultWebSecurityCondition
+> - SpringBootWebSecurityConfiguration
+> - EnableWebSecurity
+> 
+
+![img.png](img.png)
+![img_1.png](img_1.png)
+
+- [x] AutoConfiguration을 이용하여 
+  - [x] 사용자가 SecurityFilterChain을 정의한 경우 기본 SecurityFilterChain이 설정된다. 
+  - [x] 사용자가 SecurityFilterChain을 정의하지 않은 경우 기본 SecurityFilterChain이 설정되지 않는다. 
+
+### 미션을 진행하며 생긴 고민에 대한 흔적 & 피드백 내용
+
+- [x] [CSRF 구현 방식에 대한 고민 (세션, 쿠키 둘 중 어디에 토큰을 저장할지?)](https://github.com/next-step/spring-security-refactoring/pull/9#discussion_r2001099639) 
+- [x] [HttpSecurity에서 필터의 순서를 어떻게 제어할지에 대한 고민](https://github.com/next-step/spring-security-refactoring/pull/9#discussion_r2001125969)
+  - 👉시큐리티에서는 httpSecurity.addFilter에서 OrderedFilter 생성. 또는 httpSecurity.addFilterAtOffsetOf에서  FilterOrderRegistration을 참조하여 필터 순서를 조정.
+- [x] [`AuthorizeHttpRequestsConfigurer::hasRole` 호출 시 roleHierarchy에 대한 의존성을 어떻게 관리할지에 대한 고민](https://github.com/next-step/spring-security-refactoring/pull/9#discussion_r2001147784)
+- [x] [AuthorizationFilter에서 인증이 안 되었을 때 401을 던지는 것이 자연스러운가에 대한 고민](https://github.com/next-step/spring-security-refactoring/pull/9#discussion_r2001166948)
+- [x] [예외 상황에 따라서 어떻게 예외를 핸들링할지에 대한 고민](https://github.com/next-step/spring-security-refactoring/pull/9#discussion_r2005589776)
+- [x] [HttpSecurity를 빈 등록 시 유연성이 떨어지는 문제 고민](https://github.com/next-step/spring-security-refactoring/pull/9#discussion_r2005603658) (구현 완)
+- [x] [SecurityConfigurer의 `init`, `configure` 구분을 어떻게 하고 있는지?](https://github.com/next-step/spring-security-refactoring/pull/9#discussion_r2005616627) (구현 완)  
+
+
+
+# 플로우차트를 활용한 이해
+
+## CSRF 공격 대응
+
+### CSRF 토큰 적용 전
+
+```mermaid
+sequenceDiagram
+    participant User
+    participant HackerSite
+    participant Bank
+    User ->> HackerSite: 악성 링크 클릭
+    HackerSite ->> Bank: POST 이체 요청 (자동 실행)
+    Bank -->> HackerSite: 성공 (세션 쿠키만으로 인증)
+```
+
+### CSRF 토큰 적용 후
+
+```mermaid
+sequenceDiagram
+    participant User
+    participant HackerSite
+    participant Bank
+    User ->> Bank: 로그인
+    Bank -->> User: 세션 쿠키 + CSRF 토큰(HTML 숨김 필드)
+    User ->> HackerSite: 악성 링크 클릭
+    HackerSite ->> Bank: POST 이체 요청 (세션 쿠키만 포함)
+    Bank ->> Bank: 1. 세션에서 CSRF 토큰 조회
+    Bank ->> Bank: 2. 요청의 CSRF 토큰 비교
+    Bank -->> HackerSite: 403 Forbidden (토큰 없음)
+``` 

docs/Questions.md

@@ -0,0 +1,17 @@
+> PR 코멘트로 질문 드리겠습니다..! 🙇‍♂️
+
+- [ ] SSR에서는 HttpSessionCsrfTokenRepository, CSR에서는 CookieCsrfTokenRepository를 사용해야할 것 같은데, 그 이유를 얘기하고 근거가 적절한지 질문.
+
+### 인증 필터간 순서
+
+- 현재 구현한 HttpSecurity의 경우, 어떤 인증 메서드(`csrf()`, `oauth2Login()` 등)를 호출하냐에따라 SecurityFilterChain의 필터 순서가 결정된다.
+- 이 방식은 `authorizeHttpRequests()` 가 다른 인증 메서드보다 먼저 호출될 경우, `AuthorizationFilter`가 다른 인증 필터보다 먼저 수행하기 때문에 인증 전에 인가가 수행되버리는 문제가 있다.
+  - `authorizeHttpRequests()` 가 먼저 호출되어도 필터 체인의 가장 마지막에 순서를 위치시키도록 하는 로직이 필요함. (우선 강제로 순서를 맞춰주는 방식으로 구현)
+
+### authorizationFlter에서는 403을 리턴하는데 인증되지 않은 경우는 401을 리턴해야한다.
+
+- 문제를 해결하기 위해 exceptionTranslationFilter를 지금 구현하는 것은 오버 엔지니어링.
+
+### RoleHierarchy 가 빈으로 등록되었는데, 찾지 못하는 문제?
+
+- httpSecurity.build 보다 hasRole이 먼저 호출되고, 그래서 this.roleHierarchy 가 null이 되는 문제 발생

src/main/java/nextstep/app/SecurityApplication.java

@@ -9,5 +9,4 @@ public class SecurityApplication {
     public static void main(String[] args) {
         SpringApplication.run(SecurityApplication.class, args);
     }
-
 }

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

@@ -1,41 +1,38 @@
 package nextstep.app;
 
+import nextstep.autoconfigure.EnableSpringBootSecurityConfiguration;
 import nextstep.oauth2.OAuth2ClientProperties;
 import nextstep.oauth2.authentication.OAuth2LoginAuthenticationProvider;
 import nextstep.oauth2.registration.ClientRegistration;
 import nextstep.oauth2.registration.ClientRegistrationRepository;
 import nextstep.oauth2.userinfo.OAuth2UserService;
-import nextstep.oauth2.web.OAuth2AuthorizationRequestRedirectFilter;
-import nextstep.oauth2.web.OAuth2AuthorizedClientRepository;
-import nextstep.oauth2.web.OAuth2LoginAuthenticationFilter;
-import nextstep.security.access.AnyRequestMatcher;
-import nextstep.security.access.MvcRequestMatcher;
-import nextstep.security.access.RequestMatcherEntry;
 import nextstep.security.access.hierarchicalroles.RoleHierarchy;
 import nextstep.security.access.hierarchicalroles.RoleHierarchyImpl;
-import nextstep.security.authentication.*;
-import nextstep.security.authorization.*;
-import nextstep.security.config.DefaultSecurityFilterChain;
+import nextstep.security.authentication.AuthenticationManager;
+import nextstep.security.authentication.DaoAuthenticationProvider;
+import nextstep.security.authentication.ProviderManager;
+import nextstep.security.authorization.SecuredMethodInterceptor;
+import nextstep.security.config.Customizer;
 import nextstep.security.config.DelegatingFilterProxy;
 import nextstep.security.config.FilterChainProxy;
 import nextstep.security.config.SecurityFilterChain;
-import nextstep.security.context.SecurityContextHolderFilter;
+import nextstep.security.config.annotation.web.builders.HttpSecurity;
+import nextstep.security.config.annotation.web.configuration.EnableWebSecurity;
 import nextstep.security.userdetails.UserDetailsService;
 import org.springframework.boot.context.properties.EnableConfigurationProperties;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.context.annotation.EnableAspectJAutoProxy;
-import org.springframework.http.HttpMethod;
 
-import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
-
 @Configuration
 @EnableAspectJAutoProxy
 @EnableConfigurationProperties(OAuth2ClientProperties.class)
+@EnableWebSecurity
+@EnableSpringBootSecurityConfiguration
 public class SecurityConfig {
 
     private final UserDetailsService userDetailsService;
@@ -49,8 +46,8 @@ public SecurityConfig(UserDetailsService userDetailsService, OAuth2UserService o
     }
 
     @Bean
-    public DelegatingFilterProxy delegatingFilterProxy() {
-        return new DelegatingFilterProxy(filterChainProxy(List.of(securityFilterChain())));
+    public DelegatingFilterProxy delegatingFilterProxy(HttpSecurity http) {
+        return new DelegatingFilterProxy(filterChainProxy(List.of(securityFilterChain2(http))));
     }
 
     @Bean
@@ -63,13 +60,6 @@ public SecuredMethodInterceptor securedMethodInterceptor() {
         return new SecuredMethodInterceptor();
     }
 
-    @Bean
-    public RoleHierarchy roleHierarchy() {
-        return RoleHierarchyImpl.with()
-                .role("ADMIN").implies("USER")
-                .build();
-    }
-
     @Bean
     public AuthenticationManager authenticationManager() {
         return new ProviderManager(List.of(
@@ -78,26 +68,25 @@ public AuthenticationManager authenticationManager() {
     }
 
     @Bean
-    public SecurityFilterChain securityFilterChain() {
-        return new DefaultSecurityFilterChain(
-                List.of(
-                        new SecurityContextHolderFilter(),
-                        new UsernamePasswordAuthenticationFilter(authenticationManager()),
-                        new BasicAuthenticationFilter(authenticationManager()),
-                        new OAuth2AuthorizationRequestRedirectFilter(clientRegistrationRepository()),
-                        new OAuth2LoginAuthenticationFilter(clientRegistrationRepository(), new OAuth2AuthorizedClientRepository(), authenticationManager()),
-                        new AuthorizationFilter(requestAuthorizationManager())
+    public SecurityFilterChain securityFilterChain2(HttpSecurity http) {
+        return http
+                .authorizeHttpRequests(authorize -> authorize
+                        .requestMatchers("/members").hasRole("ADMIN")
+                        .requestMatchers("/members/me").authenticated()
+                        .anyRequest().permitAll()
                 )
-        );
+                .csrf(c -> c.ignoringRequestMatchers("/login"))
+                .formLogin(Customizer.withDefaults())
+                .httpBasic(Customizer.withDefaults())
+                .oauth2Login(Customizer.withDefaults())
+                .build();
     }
 
     @Bean
-    public RequestMatcherDelegatingAuthorizationManager requestAuthorizationManager() {
-        List<RequestMatcherEntry<AuthorizationManager>> mappings = new ArrayList<>();
-        mappings.add(new RequestMatcherEntry<>(new MvcRequestMatcher(HttpMethod.GET, "/members"), new AuthorityAuthorizationManager(roleHierarchy(), "ADMIN")));
-        mappings.add(new RequestMatcherEntry<>(new MvcRequestMatcher(HttpMethod.GET, "/members/me"), new AuthorityAuthorizationManager(roleHierarchy(), "USER")));
-        mappings.add(new RequestMatcherEntry<>(AnyRequestMatcher.INSTANCE, new PermitAllAuthorizationManager<Void>()));
-        return new RequestMatcherDelegatingAuthorizationManager(mappings);
+    public RoleHierarchy roleHierarchy() {
+        return RoleHierarchyImpl.with()
+                .role("ADMIN").implies("USER")
+                .build();
     }
 
     @Bean

src/main/java/nextstep/app/ui/AccountController.java

@@ -1,6 +1,7 @@
 package nextstep.app.ui;
 
 import jakarta.servlet.http.HttpServletRequest;
+import nextstep.security.web.csrf.CsrfToken;
 import org.springframework.stereotype.Controller;
 import org.springframework.ui.Model;
 import org.springframework.web.bind.annotation.GetMapping;
@@ -17,8 +18,8 @@ public String getAccountPage(HttpServletRequest request, Model model) {
         model.addAttribute("username", "username");
         model.addAttribute("email", "[email protected]");
 
-//        CsrfToken csrfToken = (CsrfToken) request.getAttribute(CsrfToken.class.getName());
-//        model.addAttribute("csrfToken", csrfToken);
+        CsrfToken csrfToken = (CsrfToken) request.getAttribute(CsrfToken.class.getName());
+        model.addAttribute("csrfToken", csrfToken);
 
         return "account";
     }
@@ -30,4 +31,4 @@ public String updateAccount(@RequestParam("username") String username, @RequestP
 
         return "redirect:/account";
     }
-}
+}

src/main/java/nextstep/autoconfigure/EnableSpringBootSecurityConfiguration.java

@@ -0,0 +1,15 @@
+package nextstep.autoconfigure;
+
+import org.springframework.context.annotation.Import;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+@Retention(RetentionPolicy.RUNTIME)
+@Target(ElementType.TYPE)
+@Import(SpringBootWebSecurityConfiguration.class)
+public @interface EnableSpringBootSecurityConfiguration {
+
+}

src/main/java/nextstep/autoconfigure/SpringBootWebSecurityConfiguration.java

@@ -0,0 +1,35 @@
+package nextstep.autoconfigure;
+
+import nextstep.security.config.Customizer;
+import nextstep.security.config.SecurityFilterChain;
+import nextstep.security.config.annotation.web.builders.HttpSecurity;
+import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
+import org.springframework.boot.autoconfigure.security.SecurityProperties;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.core.annotation.Order;
+
+// 클래스에 선언된 빈 생성 메서드 간 빈 의존성이 없으므로 굳이 프록시 객체를 생성하지 않기 위해 proxyBeanMethods = false 옵션 사용
+@Configuration(proxyBeanMethods = false)
+public class SpringBootWebSecurityConfiguration {
+
+    @Configuration(proxyBeanMethods = false)
+    @ConditionalOnMissingBean(SecurityFilterChain.class)
+    static class SecurityFilterChainConfiguration {
+
+        @Bean
+        @Order(SecurityProperties.BASIC_AUTH_ORDER)
+        SecurityFilterChain defaultSecurityFilterChain(HttpSecurity http) {
+            return http
+                    .authorizeHttpRequests((requests) -> requests.anyRequest().authenticated())
+                    .formLogin(Customizer.withDefaults())
+                    .httpBasic(Customizer.withDefaults())
+                    .build();
+        }
+    }
+
+    @Configuration(proxyBeanMethods = false)
+    static class WebSecurityEnablerConfiguration {
+
+    }
+}

src/main/java/nextstep/oauth2/web/OAuth2AuthorizationRequestRedirectFilter.java

@@ -6,6 +6,7 @@
 import jakarta.servlet.http.HttpServletResponse;
 import nextstep.oauth2.endpoint.OAuth2AuthorizationRequest;
 import nextstep.oauth2.registration.ClientRegistrationRepository;
+import org.springframework.util.Assert;
 import org.springframework.web.filter.OncePerRequestFilter;
 
 import java.io.IOException;
@@ -19,6 +20,7 @@ public class OAuth2AuthorizationRequestRedirectFilter extends OncePerRequestFilt
     private final AuthorizationRequestRepository authorizationRequestRepository = new AuthorizationRequestRepository();
 
     public OAuth2AuthorizationRequestRedirectFilter(ClientRegistrationRepository clientRegistrationRepository) {
+        Assert.notNull(clientRegistrationRepository, "clientRegistrationRepository cannot be null");
         authorizationRequestResolver = new OAuth2AuthorizationRequestResolver(clientRegistrationRepository, DEFAULT_AUTHORIZATION_REQUEST_BASE_URI);
     }
 

src/main/java/nextstep/oauth2/web/OAuth2LoginAuthenticationFilter.java

@@ -33,6 +33,11 @@ public class OAuth2LoginAuthenticationFilter extends AbstractAuthenticationProce
 
     public OAuth2LoginAuthenticationFilter(ClientRegistrationRepository clientRegistrationRepository, OAuth2AuthorizedClientRepository authorizedClientRepository, AuthenticationManager authenticationManager) {
         super(DEFAULT_LOGIN_REQUEST_BASE_URI, authenticationManager);
+
+        Assert.notNull(authenticationManager, "authenticationManager cannot be null");
+        Assert.notNull(clientRegistrationRepository, "clientRegistrationRepository cannot be null");
+        Assert.notNull(authorizedClientRepository, "authorizationRequestRepository cannot be null");
+
         this.clientRegistrationRepository = clientRegistrationRepository;
         this.authorizedClientRepository = authorizedClientRepository;
     }

src/main/java/nextstep/security/access/AccessDeniedHandler.java

@@ -0,0 +1,10 @@
+package nextstep.security.access;
+
+import jakarta.servlet.http.HttpServletResponse;
+
+public class AccessDeniedHandler {
+
+    public void handle(HttpServletResponse response) {
+        response.setStatus(HttpServletResponse.SC_FORBIDDEN);
+    }
+}