[인증(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에 대해 고민해보셨나요?

여기서는 메서드를 호출하는 쪽에서 명시적으로 예외를 핸들링하게 하기 위해서 사용했습니다.
AuthenticationProvider -> AuthenticationManager -> AuthenticationFilter 이렇게 예외가 올라오도록 설계한 후, AuthenticationFilter 에서 예외를 처리하는 방식을 택했습니다.

AuthenticationFilter에게는 AuthenticationException까지 핸들링하는 책임이 있다고 생각해서, AuthenticationFilter 하위 레이어인 AuthenticationManager, AuthenticationProvider 에서 예외를 체크 예외로 던지는게 낫다고 생각했어요. 언체크 예외를 던지면 AuthenticationException이 발생했는지 눈치 채기가 어렵다 보니까, 일부러 이런 방식을 택했습니다.

시큐리티 코드를 보니 실제로는 ProviderManager에서 예외를 핸들링 해주고 있는데, 미션에서는 별도의 예외 처리 레이어를 두기가 애매하더라고요. 그래서 HttpServletRequest를 알고 있는 AuthenticationFilter 선에서 예외 핸들링을 마치려고 했던 것도 있습니다..!

(질문 주신 의도에 맞는 답변일지 궁금하네요..!)

체크드 익셉션을 사용할 경우 의도하지 않은 곳에서도 throw를 통해 넘겨주어야 하기 때문에 가급적이면 언체크드 익셉션을 사용하기를 권장합니다.

우선 실제 시큐리티 코드에서는 예외 처리를 별도의 필터에서 진행하기 때문에 중간 단계에서는 예외 처리에 대해 고민하지 않도록 하기 위해서라도 언체크드 익셉션을 활용한다고 볼 수 있습니다!

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.


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

if (!userDetails.matchesPassword(password)) { throw new AuthenticationException("username or password invalid"); } // matches password. authenticated.

'비밀번호에 대한 검증을 끝냈으니, 이제 비밀번호는 일치하고 인증이 된 것이다. 그러니까 인증된 Authentication을 리턴하면 된다.' 라는 의미를 남기고 싶었는데, 지금 보니 컨텍스트가 생략되었네요. 주석을 달더라도 조금 더 컨텍스트를 전달하는, 의미 있는 주석을 달 수 있도록 해봐야겠습니다!

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의 동작 방식을 같이 확인해보면 좋겠네요!

OncePerRequestFilter의 동작 방식을 확인해보고, OncePerRequestFilter와 GenericFilterBean을 각각 언제 사용할지 나름의 기준을 잡아봤습니다..!

AuthenticationFilter

한 번의 요청에서, 사용자의 신원은 변경되지 않으므로 OncePerRequestFilter 사용.

AuthorizationFilter

같은 요청이더라도 내부적으로 포워딩 되는 경우 다른 리소스에 접근 가능해질 수 있으므로 항상 인가를 해야한다. 👉 GenericFilterBean 사용

위 UsernamePasswordAuthenticationFilter의 경우 역시 별도의 인가처리는 진행하지 않으므로 OncePerRequestFilter를 사용해도 괜찮을 것 같은데, 혹시 브라운님의 의견은 어떨지 궁금합니다!!

요청에 따라 다를 수 있을 것 같은데 일반적인 요청에서는 큰 차이는 없으나 포워딩이나 특수한 상황에서는 모든 경우에 대한 식별이 필요할 수 있을 것 같아요.

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

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

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


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

SecurityContextRepository를 생성자에서 주입받는 것의 장점은 각 AuthenticationFilter마다 필요한 SecurityContextRepository를 AppSecurityConfig에서 주입해줄 수 있다 정도일 것 같은데, 이것은 사실 'SecurityContextRepository'를 생성자로 주입해줄 때의 장점이라기보다는 그냥 생성자에서 의존성 주입을 받는 모든 스프링 빈이 동일할 것 같아요.

그래서 가만 생각해보니, 다음의 근거를 통해 생성자가 아닌 AuthenticationFilter 스스로 SecurityContextRepository를 생성하는게 나은 것 같다고 생각이 바뀌었습니다.

SecurityContextHolderFilter가 SecurityContextPersistenceFilter의 securityContextRepository에 항상 context를 저장하는 불필요한 작업 때문에 나오게 된 건데, 그 말인 즉슨 각 AuthenticationFilter에서 securityContextRepository를 쓸지 말지를 자율적으로 결정하는 거고, 어떤 securityContextRepository를 쓸지도 알고 있어도 된다는 의미로 해석되더라고요.

각 AuthenticationFilter가 어떤 securityContextRepository를 쓸지 미리 알고 있어도 된다. 그러니까 AuthenticationFilter 스스로 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를 넘기는 방법도 있을 것 같은데
둘의 차이는 어떻게 생각하시나요?

userDetailsService 의존성 주입

UsernamePasswordAuthenticationFilter, BasicAuthenticationFilter 둘 다 생성자에서 ProviderManager를 생성해주고 있고 그 결과 동일한 역할을 하는 AuthenticationManager 인스턴스가 2개 생기고 있음.

userDetailsService에 대한 의존성이 생기므로, userDetailsService의 변화에 영향을 받는다.

authenticationManager 의존성 주입

동일한 역할을 하는 AuthenticationManager가 싱글톤으로 관리된다.

userDetailsService를 의존하지 않게 되므로, userDetailsService의 변화에 영향을 받지 않고 authenticationManager에게 인증을 맡기는 책임에 집중할 수 있게된다.

이 정도 차이가 있을 것 같은데 혹시 의도하신게 이 부분일까요..!

네, userDetailsService 대신 AuthenticationManager를 주입해주면 어떨까 하는 코멘트였습니다 :)

boorownie

남겨주신 답변 잘 확인했습니다!
충분히 고민하신 부분이 느껴져서 추가 코멘트 몇가지 남긴 후 머지하도록 하겠습니다!
이어서 2주차 미션도 화이팅입니다!

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

boorownie approved these changes Feb 20, 2025

View reviewed changes

boorownie merged commit 37f07e3 into next-step:haero77 Feb 20, 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

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

Uh oh!

Conversation

haero77 commented Feb 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

haero77 Feb 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

haero77 Feb 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

boorownie left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

haero77 Feb 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

haero77 Feb 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

haero77 Feb 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

haero77 Feb 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

boorownie left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

haero77 commented Feb 13, 2025 •

edited

Loading

haero77 Feb 13, 2025 •

edited

Loading

haero77 Feb 13, 2025 •

edited

Loading

haero77 Feb 16, 2025 •

edited

Loading

haero77 Feb 16, 2025 •

edited

Loading

haero77 Feb 19, 2025 •

edited

Loading

haero77 Feb 16, 2025 •

edited

Loading