Bbaemin spring security, jwt #33

CHANEE-personal · 2023-03-01T08:09:10Z

spring securitry 적용
jwt와 spring security를 활용한 로그인 기능 구현
로그인 test case 구현 예정

f-lab-lyan · 2023-03-01T08:28:57Z

bbaemin-api/src/main/java/org/bbaemin/jwt/JwtTokenProvider.java

+ public JwtTokenProvider(@Value("${spring.jwt.secret}") String secret,
+ @Value("${spring.jwt.token-validity-in-seconds}") long tokenValidityInMilliseconds,


@value 대신에 ConfigurationProperties를 쓰는게 더 좋을 것 같습니다. 왜 좋은지 생각해 보시겠어요?

아 넵 한 번 알아보겠습니다!

f-lab-lyan · 2023-03-01T08:30:12Z

bbaemin-api/src/main/java/org/bbaemin/user/controller/UserController.java

@@ -16,6 +18,7 @@
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RestController;

+import javax.servlet.http.HttpServletResponse;


이건 필요한 것인가요?

삭제 완료했습니다!

f-lab-lyan · 2023-03-01T08:31:41Z

bbaemin-api/src/test/java/org/bbaemin/user/service/UserServiceTest.java

 import org.bbaemin.user.repository.UserRepository;
 import org.bbaemin.user.vo.User;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.Test;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.test.autoconfigure.jdbc.AutoConfigureTestDatabase;
 import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest;
+import org.springframework.security.authentication.*;


nit: Change it to actual import statements

yeonkyungJoo · 2023-03-01T10:40:19Z

bbaemin-api/src/main/java/org/bbaemin/api/user/service/UserService.java

 @Transactional(readOnly = true)
 @Service
 @RequiredArgsConstructor
 public class UserService {

 private final UserRepository userRepository;
+ private final PasswordEncoder passwordEncoder;
+ private final AuthenticationManager authenticationManager;
+ private final JwtTokenProvider jwtTokenProvider;


@f-lab-lyan UserService가 위와 같은 시큐리티 관련 클래스에 의존성을 가지는 부분에 대해 어떻게 생각하시는지 궁금합니다!

제 생각은 당연히 의존하지 않는 것이 좋을 것 같습니다. AuthenricationService등을 만들어서 처리해야하지 않을까싶은 생각이 듭니다.

yeonkyungJoo · 2023-03-01T10:43:14Z

bbaemin-api/src/main/java/org/bbaemin/api/user/service/UserService.java

+ }
+
+ private Authentication authenticate(String userEmail, String password) {
+ try {


위 로그인 처리나 인증 관련 메소드는 이왕이면 Spring Security의 handler나 filter들을 구현해서 처리하는 게 더 좋지 않을까요?

yeonkyungJoo · 2023-03-01T10:44:09Z

bbaemin-api/src/main/java/org/bbaemin/api/user/vo/User.java


 @Getter
 @NoArgsConstructor(access = AccessLevel.PROTECTED)
 @Entity
-public class User {
+public class User implements UserDetails {


@f-lab-lyan User 엔티티를 UserDetails의 구현체로 사용하는 것에 대해 어떻게 생각하시나요?

이건 저도 궁금한 부분인데요. 왜 UserDetails란 구현체가 User와 묶여야 하는지 정확히 모르겠어요. 아, 설명이 너무 대충인 것 같아서 잠깐 추가하면, Security에서 필요한 부분을 서비스에 녹여내면, 구현할 때는 편하겠지만, 실제로 확장하거나 수정할 때, 상관없는 시큐리티 이슈를 확인해야할 수도 있어요. 그렇게 하는 게 좋은 것인지 모르겠다는 말이었습니다.

yeonkyungJoo · 2023-03-01T10:46:16Z

bbaemin-api/src/main/java/org/bbaemin/config/SecurityConfig.java

+@Configuration
+@EnableWebSecurity
+@RequiredArgsConstructor
+public class SecurityConfig extends WebSecurityConfigurerAdapter {


WebSecurityConfigurerAdapter를 상속해서 설정하는 방법은 현재 deprecated된 것으로 아는데요! 현재 권장되는 방법으로 설정해보는 게 좋을 것 같습니다.

yeonkyungJoo · 2023-03-01T10:50:33Z

bbaemin-api/src/main/java/org/bbaemin/api/user/controller/UserController.java

@@ -71,4 +72,9 @@ public ApiResult<Void> quit(@PathVariable Long userId) {
 userService.quit(userId);
 return ApiResult.ok();
 }
+
+ @PostMapping("/login")
+ public JwtResponse login(@Validated @RequestBody LoginRequest loginRequest) {


이건 ApiResult로 return해주지 않는 건가요?

yeonkyungJoo · 2023-03-01T11:02:37Z

bbaemin-api/src/main/java/org/bbaemin/config/filter/JwtAuthorizationFilter.java

+
+ @Override
+ public Authentication attemptAuthentication(HttpServletRequest request, HttpServletResponse response) throws AuthenticationException {
+ log.info("UsernameFilter");


클래스명은 AuthorizationFilter인데 권한을 체크하고 있는 것 같지는 않아요!

yeonkyungJoo · 2023-03-01T11:20:20Z

bbaemin-api/src/main/java/org/bbaemin/config/filter/JwtAuthenticationFilter.java

+ } else {
+ Authentication authentication = jwtTokenProvider.getAuthentication(accessToken);
+ User getUser = userRepository.findByEmail(authentication.getName())
+ .orElseThrow(() -> new NoSuchElementException("email : " + authentication.getName()));


JwtAuthenticationFilter, JwtAuthorizationFilter에 대한 의문인데요!
Spring Security 프로세스가
UsernamePasswordAuthenticationToken 같은 인증 전 토큰을 AuthenticationManager에 넘기면,
각 Provider들이 UserDetailsService에서 loadByUsername을 통해 조회하고
조회 성공 시 권한이 담긴 토큰으로 리턴해주는 방식이잖아요.

Authentication authentication = jwtTokenProvider.getAuthentication(accessToken);
여기서 이걸 왜 실행하는 건가요? 그리고 JwtTokenProvider에서 getAuthentication(accessToken)을 하면 권한이 담긴 UsernamePasswordAuthenticationToken을 넘겨주는 게 맞는 건가요? 아직 인증 전인데요!

여기서 user를 조회하는 게 맞는 건가요? 지금 JwtAuthorizationFilter에서 authenticate()하면 조회를 할 텐데요. 바로 위에 있는 getAuthentication(accessToken)에서도 user를 조회하시더라구요!

시큐리티 설정이 메인은 아니지만,
프로세스의 각 필터들을 알맞게 쓰고 계신 건가에 대한 의문이 들었어요!
물론 제가 잘못 알고 있는 걸 수도, 찬희님 코드를 놓친 걸 수도 있습니다 :-)

yeonkyungJoo · 2023-03-01T11:23:32Z

bbaemin-api/src/main/java/org/bbaemin/config/SecurityConfig.java

+ .formLogin().disable()
+ .httpBasic().disable()
+ .csrf().disable()
+ .sessionManagement().sessionCreationPolicy(SessionCreationPolicy.STATELESS)


@f-lab-lyan frontend, backend 나눠질 때 STATELESS로 안 할 수도 있나요? 그냥 세션을 사용하는 거요!

이부분은 제가 아니라 찬희님이 대답해 주셔야할 것 같은 느낌이 드네요. :-)

yeonkyungJoo

전체적인 느낌은
인증을 어느 클래스에서 하는 건지 파악이 어렵다는 거였습니다.
JwtAuthenticationFilter인지, JwtAuthorizationFilter인지
아니면 UserService의 login(), authenticate() 메소드인지요!
같이 정리해보는 게 좋을 것 같아요!

CHANEE-personal · 2023-03-01T11:37:47Z

전체적인 느낌은 인증을 어느 클래스에서 하는 건지 파악이 어렵다는 거였습니다. JwtAuthenticationFilter인지, JwtAuthorizationFilter인지 아니면 UserService의 login(), authenticate() 메소드인지요! 같이 정리해보는 게 좋을 것 같아요!

아무래도 저도 처음 적용을 시켜보다보니 놓친 부분이 많은거 같습니다. 좀 더 알아보도록 하겠습니다!

CHANEE-personal self-assigned this Mar 1, 2023

f-lab-lyan reviewed Mar 1, 2023

View reviewed changes

CHANEE-personal requested a review from yeonkyungJoo March 1, 2023 08:29

f-lab-lyan reviewed Mar 1, 2023

View reviewed changes

CHANEE-personal added 5 commits March 1, 2023 18:00

spring security 및 jwt 관련 dependency 추가

a814884

jwt 관련 설정 추가

1e2b1bf

conflict 해결

324ac4a

수정

46d1568

수정

1ab6fa4

CHANEE-personal force-pushed the bbaemin-spring-security branch from 1cc3d1f to 1ab6fa4 Compare March 1, 2023 09:06

yeonkyungJoo requested changes Mar 1, 2023

View reviewed changes

CHANEE-personal marked this pull request as draft March 6, 2023 14:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bbaemin spring security, jwt #33

Bbaemin spring security, jwt #33

CHANEE-personal commented Mar 1, 2023 •

edited

Loading

f-lab-lyan Mar 1, 2023

CHANEE-personal Mar 1, 2023

f-lab-lyan Mar 1, 2023

CHANEE-personal Mar 1, 2023

f-lab-lyan Mar 1, 2023

yeonkyungJoo Mar 1, 2023

f-lab-lyan Mar 6, 2023

yeonkyungJoo Mar 1, 2023

yeonkyungJoo Mar 1, 2023

f-lab-lyan Mar 6, 2023 •

edited

Loading

yeonkyungJoo Mar 1, 2023

yeonkyungJoo Mar 1, 2023

yeonkyungJoo Mar 1, 2023

yeonkyungJoo Mar 1, 2023 •

edited

Loading

yeonkyungJoo Mar 1, 2023

f-lab-lyan Mar 6, 2023

yeonkyungJoo left a comment

CHANEE-personal commented Mar 1, 2023

		public JwtTokenProvider(@Value("${spring.jwt.secret}") String secret,
		@Value("${spring.jwt.token-validity-in-seconds}") long tokenValidityInMilliseconds,

Bbaemin spring security, jwt #33

Are you sure you want to change the base?

Bbaemin spring security, jwt #33

Conversation

CHANEE-personal commented Mar 1, 2023 • edited Loading

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

f-lab-lyan Mar 6, 2023 • edited Loading

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

yeonkyungJoo Mar 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yeonkyungJoo left a comment

Choose a reason for hiding this comment

CHANEE-personal commented Mar 1, 2023

CHANEE-personal commented Mar 1, 2023 •

edited

Loading

f-lab-lyan Mar 6, 2023 •

edited

Loading

yeonkyungJoo Mar 1, 2023 •

edited

Loading