[인증(Authentication) - 2, 3, 4단계] 김선호 미션 제출합니다. 🚀 #42

haero77 · 2025-02-13T14:37:22Z

브라운 안녕하세요, 지난 1단계 미션 이후 2, 3, 4단계를 모두 구현하여 리뷰 요청드립니다!

이번에도 Spring Security 구현을 보면서 SecurityContextHolderFilter는 뭐하는 녀석일지, SecurityContextRepository에 SecurityContext를 언제 저장해야할지 찾아보며 공부해봤습니다.

구현을 뜯어보며 공부하느라 시간은 오래 걸렸지만,
FilterChainProxy -> SecurityFilterChain -> SecurityContextHolderFilter -> AuthenticationFilter -> AuthetncationManager -> AuthenticationProvider(인증된 Authentication 생성)이라는 인증의 큰 흐름이 드디어 머리 속에 잡혀서 기쁘네요 ㅎㅎ

지난 리뷰에서 남겨주신 더 생각해볼만한 부분들에 대해, 제 나름대로의 근거를 갖고 그 결론을 적용시켜봤습니다. (요건 코멘트로 남겨둘게요.) 요번에도 고민해볼만한 내용 많이 던져주시면 감사하겠습니다..!

첫 번째 미션 함께해주셔서 감사드려요 :-)

…FilterChain를 하나만 유지하는 구조로 변경 - applies feedback next-step#40 (comment)

…ePasswordAuthenticationToken 구현

…er 정의

…icationProvider 구현

…er 정의

…onManager 구현체 ProviderManager 구현

…nFilter로 변경

…r를 상속하도록 변경 - applies feedback next-step#40 (comment)

…textImpl 구현

…rityContext로 관리하도록 변경

…보를 관리하도록 변경

…sitory 구현

… SecurityContextHolder에 적재하는 SecurityContextHolderFilter 구현

…ext를 저장하도록 변경 - SecurityContextPersistenceFilter는 securityContextRepository에 항상 context를 저장하지만,SecurityContextHolderFilter는 각 인증 필터에게 securityContextRepository를 저장할지 말지 자율권을 준다.

haero77 · 2025-02-13T14:58:09Z

src/main/java/nextstep/security/filter/BasicAuthenticationFilter.java

-public class BasicAuthenticationFilter implements Filter {
+public class BasicAuthenticationFilter extends OncePerRequestFilter {


Filter 대신 OncePerRequestFilter 이나 GenericFilterBean을 고려해보면 어떨까요?
어떤 차이가 있을까요?

남겨주신 위 리뷰를 참고해서, 인증 필터가 OncePerRequestFilter를 상속하도록 변경해봤습니다.
생각해보니 인증 필터가 HTTP 요청당 2번 이상 수행될 필요가 없을 것 같더라고요...!

(실제 Security 구현을 보니 GenericFilterBean을 상속하면서도 요청당 한 번 수행을 보장하고 있는 것 같던데, 왜 OncePerRequestFilter를 안 쓰는지 이건 좀 더 찾아봐야겠습니다.)

OncePerRequestFilter의 동작 방법을 확인해보시면서 어떻게 요청당 한번 동작을 보장하는지 방법을 찾아보시면 좋을 것 같아요ㅋㅋ

haero77 · 2025-02-13T15:05:33Z

src/main/java/nextstep/app/security/AppSecurityConfiguration.java

    @Bean
    public SecurityFilterChain formLoginSecurityFilterChain() {
        return new DefaultSecurityFilterChain(
-                (httpServletRequest) -> httpServletRequest.getRequestURI().equals("/login"),
-                List.of(
-                        new FormLoginFilter(userDetailsService)
-                )
-        );
-    }
-
-    @Bean
-    public SecurityFilterChain basicAuthenticationSecurityFilterChain() {
-        return new DefaultSecurityFilterChain(
-                (httpServletRequest) -> httpServletRequest.getRequestURI().equals("/members"),
                List.of(
+                        new FormLoginFilter(userDetailsService),
                        new BasicAuthenticationFilter(userDetailsService)
                )
        );


SecurityFilterChain을 각각 만들어 주셨네요!
만약 하나의 SecurityFilterChain으로 만들기 위해서는 어떻게 하면 좋을까요?

엔드포인트 1개 당 SecurityFilterChain 1개를 만드는 기존 구조에서, 엔드포인트 개수와 관계없이 SecurityFilterChain을 하나만 사용하도록 변경했습니다. 이제 엔드포인트가 추가될 때마다 SecurityFilterChain를 만들지 않아도 되니 유지보수 비용은 줄어드는데, 그럼 SecurityFilterChain은 어느 경우에 따로 만드는 것이 효율적인지에 대한 고민이 들더라고요.

찾아보고 고민해본 결과 인증 방식 자체가 달라지는 경우(JWT, Oauth 등)에 적용하면 효과적일 것 같은데, 막상 구현하다보면 그냥 인증 필터 내부에서 RequestMatcher로 잘 매칭시키면 되니까 또 SecurityFilterChain을 한 개로 유지해볼 수도 있겠다는 생각이 들더라고요. SecurityFilterChain의 분리에 대한 기준점을 어떻게 잡아나가면 좋을지 조언해주시면 감사하겠습니다 :)

말씀하신 대로 다른 인증 방식의 경우 다른 필터를 타야할 수 있는데 그럴 때 SecurityFilterChain을 분리하는게 좋을 것 같아요.
물론 matcher로 잘 분리할 수 있지만 같은 필터지만 다른 provider를 적용해야 하는 경우도 있을 수 있으니 그럴 땐 나누는게 좋겠네요.

boorownie

인증 남은 미션 잘 수행해주셨습니다!
특별히 코드 개선의 코멘트를 남길 부분이 없어서
고민해보시면 좋을 부분에 대해 코멘트 남겼습니다!
확인해보시고 답변 남겨주세요~

boorownie · 2025-02-14T06:56:02Z

src/main/java/nextstep/app/security/AppSecurityConfiguration.java

@@ -17,20 +20,13 @@ public AppSecurityConfiguration(UserDetailsService userDetailsService) {

    @Bean
    public SecurityFilterChain formLoginSecurityFilterChain() {


피드백 반영 👍

boorownie · 2025-02-14T06:56:58Z

src/main/java/nextstep/security/AuthenticationException.java

@@ -0,0 +1,12 @@
+package nextstep.security;
+
+public class AuthenticationException extends Exception{


Exception을 상속한 이유가 있나요?
checked exception과 unchecked exception에 대해 고민해보셨나요?

boorownie · 2025-02-14T06:57:12Z

src/main/java/nextstep/security/authentication/AuthenticationManager.java

+    /**
+     * Attempts to authenticate the passed {@link Authentication} object, returning a
+     * fully populated <code>Authentication</code> object if successful.
+     * @param authentication the authentication request object
+     * @return a fully authenticated object including credentials
+     * @throws AuthenticationException if authentication fails
+     */


주석도 가져오신건가요?ㅋㅋㅋ

boorownie · 2025-02-14T06:57:31Z

src/main/java/nextstep/security/authentication/DaoAuthenticationProvider.java

+            throw new AuthenticationException("username or password invalid");
+        }
+
+        // matches password. authenticated.


이 주석은 어떤 의미인가요?

boorownie · 2025-02-14T07:01:48Z

src/test/java/nextstep/app/FormLoginTest.java

+         HttpSession session = loginResponse.andReturn().getRequest().getSession();
+         assertThat(session).isNotNull();
+         assertThat(session.getAttribute("SPRING_SECURITY_CONTEXT")).isNotNull();


이상한 스페이스 발견!

boorownie · 2025-02-14T07:03:10Z

src/main/java/nextstep/security/filter/UsernamePasswordAuthenticationFilter.java

+import java.util.Map;
+
+// 인증 정보를 추출하고, AuthenticationManager 에게 비밀번호 맞고 틀리는 것을 검증하는 책임을 위임한다.
+public class UsernamePasswordAuthenticationFilter extends OncePerRequestFilter {


이 필터도 OncePerRequestFilter가 적절한지 앞서 남긴 코멘트에서 처럼 OncePerRequestFilter의 동작 방식을 같이 확인해보면 좋겠네요!

boorownie · 2025-02-14T07:05:09Z

src/main/java/nextstep/security/filter/UsernamePasswordAuthenticationFilter.java

+    public UsernamePasswordAuthenticationFilter(
+            UserDetailsService userDetailsService1,
+            SecurityContextRepository contextRepository
+    ) {


SecurityContextRepository를 생성자로 주입받을 때 어떤 장점이 있을까요?

boorownie · 2025-02-14T07:05:20Z

src/main/java/nextstep/app/security/AppSecurityConfiguration.java

+                        new UsernamePasswordAuthenticationFilter(userDetailsService, contextRepository),
+                        new BasicAuthenticationFilter(userDetailsService, contextRepository)


userDetailsService를 인자로 넘기는 것과 authenticationManager를 넘기는 방법도 있을 것 같은데
둘의 차이는 어떻게 생각하시나요?

haero77 added 21 commits February 13, 2025 23:34

refactor(feedback): API URI 마다 SecurityFilterChain을 생성하지 않도록 Security…

200fb25

…FilterChain를 하나만 유지하는 구조로 변경 - applies feedback next-step#40 (comment)

feat(step2): 인증 정보(사용자명, 인증 상태 등)를 담는 Authentication 정의 & 구현체 Usernam…

dfbd707

…ePasswordAuthenticationToken 구현

feat: 인증 예외를 의미하는 AuthenticationException 정의

470703c

feat(step2): 구현체 타입에 따라 인증된 Authentication을 리턴하는 AuthenticationProvid…

2975d27

…er 정의

feat(step2): UsernamePasswordAuthenticationToken의 인증을 지원하는 DaoAuthent…

523128f

…icationProvider 구현

feat(step2): 인증 요청 받은 authentication에 대한 인증을 시도하는 AuthenticationManag…

13bf0b7

…er 정의

feat(step2): 주입 받은 AuthenticationProvider를 순회하며 인증을 시도하는 Authenticati…

6c7ae54

…onManager 구현체 ProviderManager 구현

refactor(step2): 폼 로그인 인증 처리를 AuthenticationManager에 위임하도록 변경

f708698

refactor(step2): 폼 로그인 인증을 처리하는 필터의 이름을 UsernamePasswordAuthenticatio…

951231b

…nFilter로 변경

refactor(step2): Basic 인증 처리를 AuthenticationManager에 위임하도록 변경

57bc51c

refactor(feedback): 인증 필터가 요청당 한 번만 수행되는 것을 보장하도록 OncePerRequestFilte…

4244c33

…r를 상속하도록 변경 - applies feedback next-step#40 (comment)

feat(step3): Authentication을 보관하는 SecurityContext 정의, 구현체 SecurityCon…

3122529

…textImpl 구현

feat(step3): 각 스레드별로 독립된 SecurityContext를 관리하는 SecurityContextHolder 구현

02080d2

refactor(step3): BasicAuthenticationFilter에서 인증된 Authentication을 Secu…

b8b7894

…rityContext로 관리하도록 변경

refactor: 폼 로그인 인증 - username과 password를 추출하여 인증하는 로직을 메서드로 분리

ff9812e

refactor(step3): HTTP Session을 사용하지 않고 SecurityContextHolder를 통해 인증 정…

4de355e

…보를 관리하도록 변경

feat(step4): 사용자의 인증 요청 이후 사용자의 인증을 유지하기 위한 SecurityContextRepository 정의

302079c

feat(step4): 인증을 HTTP Session에 저장하기 위한 HttpSessionSecurityContextRepo…

424470a

…sitory 구현

feat(step4): SecurityContextRepository를 사용하여 SecurityContext를 얻고, 이것을…

4ae90bc

… SecurityContextHolder에 적재하는 SecurityContextHolderFilter 구현

test(step4): HTTP Session에 인증 정보 저장 후 인증 유지 테스트 추가

ec51df0

haero77 commented Feb 13, 2025

View reviewed changes

haero77 mentioned this pull request Feb 13, 2025

[인증(Authentication) - 1단계] 김선호 미션 제출합니다. 🚀 #40

Merged

2 tasks

boorownie requested changes Feb 14, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[인증(Authentication) - 2, 3, 4단계] 김선호 미션 제출합니다. 🚀 #42

[인증(Authentication) - 2, 3, 4단계] 김선호 미션 제출합니다. 🚀 #42

haero77 commented Feb 13, 2025 •

edited

Loading

haero77 Feb 13, 2025 •

edited

Loading

boorownie Feb 14, 2025

haero77 Feb 13, 2025 •

edited

Loading

boorownie Feb 14, 2025

boorownie left a comment

boorownie Feb 14, 2025

boorownie Feb 14, 2025

boorownie Feb 14, 2025

boorownie Feb 14, 2025

boorownie Feb 14, 2025

boorownie Feb 14, 2025

boorownie Feb 14, 2025

boorownie Feb 14, 2025

		public class BasicAuthenticationFilter implements Filter {
		public class BasicAuthenticationFilter extends OncePerRequestFilter {

		@@ -17,20 +20,13 @@ public AppSecurityConfiguration(UserDetailsService userDetailsService) {

		@Bean
		public SecurityFilterChain formLoginSecurityFilterChain() {

		@@ -0,0 +1,12 @@
		package nextstep.security;

		public class AuthenticationException extends Exception{

		new UsernamePasswordAuthenticationFilter(userDetailsService, contextRepository),
		new BasicAuthenticationFilter(userDetailsService, contextRepository)

[인증(Authentication) - 2, 3, 4단계] 김선호 미션 제출합니다. 🚀 #42

Are you sure you want to change the base?

[인증(Authentication) - 2, 3, 4단계] 김선호 미션 제출합니다. 🚀 #42

Conversation

haero77 commented Feb 13, 2025 • edited Loading

haero77 Feb 13, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

haero77 Feb 13, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

boorownie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

haero77 commented Feb 13, 2025 •

edited

Loading

haero77 Feb 13, 2025 •

edited

Loading

haero77 Feb 13, 2025 •

edited

Loading