-
Notifications
You must be signed in to change notification settings - Fork 0
#1 인증 토큰 spring security, JWT #5
base: master
Are you sure you want to change the base?
Conversation
junshock5
commented
Dec 11, 2020
- profiles add
- project build, report Encoding=UTF8 add
- Swagger UI add
- h2databse, spring jdbc add
- spring-security add
- jpa , modelmapper add
- jwt add
- JWT 인증 요약 (핵심 코드)
- profiles add - project build, report Encoding=UTF8 add - Swagger UI add - h2databse, spring jdbc add - spring-security add - jpa , modelmapper add - jwt add - JWT 인증 요약 (핵심 코드) JwtTokenFilter (JwtTokenFilter필터는 각각의 API (인가된다 /**로그인의 토큰 끝점의 예외 (포함) /users/signin)과 끝점 singup ( /users/signup).) JwtTokenFilterConfigurer (스프링 부트 보안 JwtTokenFilter을 추가합니다 DefaultSecurityFilterChain) JwtTokenProvider (액세스 토큰의 서명 확인, 액세스 토큰에서 ID 및 권한 부여 클레임을 추출하고이를 사용하여 UserContext를 만듭니다., 잘못되었거나 만료되었거나 예외발생) MyUserDetails (UserDetailsService사용자 정의 loadUserbyUsername 함수 를 정의하기 위해 구현, DaoAuthenticationProvider인증 중에 사용자에 대한 세부 정보를로드) WebSecurityConfig (WebSecurityConfig클래스 WebSecurityConfigurerAdapter는 사용자 지정 보안 구성을 제공하도록 확장, JwtTokenFilter, PasswordEncoder 인스턴스화) 1. 사용자는 권한 부여 서버에 자격 증명을 제공하여 새로 고침 및 액세스 토큰을 얻습니다. 2. 사용자는 보호 된 API 리소스에 액세스하기 위해 각 요청과 함께 액세스 토큰을 보냅니다. 3. 액세스 토큰은 서명되고 사용자 ID (예 : 사용자 ID) 및 권한 부여 클레임을 포함합니다.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기서도 다른 소스코드를 가져온 느낌이 많이 나서 리뷰를 하는 의미가 없을 것 같습니다.
어떤 부분에서 티가 많이 나는지 몇개는 언급해드렸지만 직접 찾아보시면서 고민하시는게 나을 것 같네요.
습관은 고치려고 노력하면 티가 나는데 여기서는 그 노력이 보이지가 않습니다
} | ||
|
||
@Override | ||
public void run(String... params) throws Exception { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이거는 왜 넣어주신건가요?
@Bean | ||
public Docket swaggerApi() { | ||
return new Docket(DocumentationType.SWAGGER_2)// | ||
.select()// |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기 주석은 복붙해온 소스의 주석을 지우다가 남아있는걸까요?
} | ||
|
||
private ApiKey apiKey() { | ||
return new ApiKey("Authorization", "Authorization", "header"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이런것들은 어떤 의도로 추가하신건지 주석이 필요하네요
private JwtTokenProvider jwtTokenProvider; | ||
|
||
@Override | ||
protected void configure(HttpSecurity http) throws Exception { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 메소드에 있는 전체 설정 한줄한줄에 대한 의도를 설명이 가능하실까요?
|
||
import org.springframework.http.HttpStatus; | ||
|
||
public class CustomException extends RuntimeException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CustomException
이라는 이름은 어떤곳에 쓰이는지 명확하게 보이지 않습니다.
} | ||
} catch (CustomException ex) { | ||
//this is very important, since it guarantees the user is not authenticated at all | ||
SecurityContextHolder.clearContext(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기서 clearContext
를 해줘야하는 이유가 무엇인가요?
|
||
@Override | ||
@Bean | ||
public AuthenticationManager authenticationManagerBean() throws Exception { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기 들여쓰기가 다른 클래스들의 들여쓰기와 다른 것을 보니 이것 또한 베껴온 소스의 느낌이 나네요. 스스로 작성한 코드가 아니면 리뷰를 받아도 그 리뷰내용을 암기만 할뿐 이해가 되지 않기 때문에 리뷰의 의미가 없어집니다
- 주요 메서드 1. String token = jwtTokenProvider.resolveToken(httpServletRequest); // 토큰 확인 2. Jwts.parser().setSigningKey(secretKey).parseClaimsJws(token); // 권한 맞는지 확인 3. org.springframework.security.core.userdetails.User // 로그인한 유저의 정보 찾기 - JwtTokenFilter 1. Authorization 헤더에서 액세스 토큰을 확인한다. 2. 헤더에 액세스 토큰이있는 경우 인증을 위임한다. 3. JwtTokenProvider은 인증 프로세스의 결과에 따라 미충족시 인증 예외를 발생 시킵니다. - JwtTokenFilterConfigurer 1. 스프링 부트 security JwtTokenFilter을 추가합니다 - JwtTokenProvider 1. 액세스 토큰 서명 확인 2. 액세스 토큰 에서 ID 및 권한 부여 클레임(사용자가 누구인지 URI, 무엇에 접근 할 수 있는지, 토큰 만료 시간 확인) 을 추출하고 이를 사용하여 UserContext를 만듭니다. 3. 액세스 토큰의 형식이 잘못 되었거나 만료 되었거나 토큰이 적절한 서명 키로 서명되지 않은 경우 인증 예외가 발생시킵니다. - MyUserDetails 1. UserDetails 인터페이스는 사용자 관련 데이터를 검색하는데 사용한다. 2. 사용자 이름을 기반으로 사용자 엔터티를 찾는 loadUserByUsername() 이라는 하나의 메서드가 있으며 이를 재정 의하여 사용자를 찾는 프로세스를 사용자 지정 - WebSecurityConfig 1. WebSecurityConfigurerAdapter는 사용자 지정 보안 구성을 제공하도록 확장 됩니다. 2. 이 클래스에서 아래 Bean이 구성되고 인스턴스화됩니다. - JwtTokenFilter - PasswordEncoder
피드백 주신 전체 불필요한 주석, 코드는 삭제하였고 |
- test User add 코드 삭제 - 유저, 인증 예외 구체적이게 변경 - 설명이 필요한 어노테이션 주석 추가 - 들여쓰기 통일 - 불필요한 주석 삭제
일단 커밋 메세지에 주석을 넣은것도 커밋메세지 원래의 용도와 맞지 않는 것 같네요~ |
pom.xml
Outdated
<!-- JSON Web Token Support --> | ||
<!-- JSON Web Token Support | ||
Spring Boot 및 Spring Security 덕분에 | ||
JWT 인증 서비스를 기록적인 시간 내에 가동 및 실행 --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이것도 이 라이브러리의 용도와 다른 주석 같네요
.anyRequest().authenticated(); | ||
|
||
// If a user try to access a resource without having enough permissions | ||
// 요구사항을 만족하지 않는다면 예외(exceptionHandling)를 발생 시킨다. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이건 이 코드의 동작에 대해 설명하는게 아니라 단순 번역같네요. 내용도 다릅니다~ 서비스개발자는 꼼꼼함이 중요합니다. 그 꼼꼼함이 제일 치명적인 약점으로 보입니다.
} | ||
} catch (CustomException ex) { | ||
// 인증 실패시 호출 | ||
SecurityContextHolder.clearContext(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
실패시에 이 코드를 왜 호출해야할까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Security에서 인증이 완료되고 인증 매니저가 해당 유저의 세션 생성 후 인가를 하는과정에서
인메모리 세션저장소인 SecurityContextHolder에 저장되는데, 해당 인가과정에서 유저 정보가 요청 권한이
부족할때 위 에러가 발생하는데, 처리 과정의 SecurityContextHolder 데이터가 threadlocal에 저장 되기 떄문에
clearContext(); 메서드를 통해 쓰레드 변수를 삭제해야 합니다.
protected void init() { | ||
secretKey = Base64.getEncoder().encodeToString(secretKey.getBytes()); | ||
} | ||
@PostConstruct |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
생성자 주입을 사용하면 이걸 실행 안시켜줘도 됩니다. 이 코드도 인터넷 어딘가에서 본 기억이 있네요. 자신의 것으로 만드셔야합니다. 가져온 코드를 고민 없이 그대로 사용하신 것 같습니다. 여태까지 공부했던 내용들이 녹아있지가 않네요
- 의존성 주석 변경 - 초기 구동시 admin 계정 추가 - secretKey PostConstruct 삭제 - formLogin() defaultSuccessUrl() 추가 및 accessDeniedPage /login?error 로 변경
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
제가 리뷰드린것은 리뷰받은 내용만 고치라는게 아니라 전체코드를 다시 바꿔야할 것 같다는 얘기입니다. 코드의 전반적인 부분에서 다른 코드를 고민없이 가져온 흔적이 아주 많습니다.
그렇게 되면 멘티의 코드가 아니라 남의 블로그 코드를 리뷰하는게 되고, 피드백을 받더라도 그냥 고치게만 되지 본인의 것이 잘 되지 않습니다. 전반적으로 코드 참고하지마시고 다시 작성해주세요
UserService userService; | ||
|
||
@Override | ||
public void run(String... params) throws Exception { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
특정 환경에 종속적인 이 코드가 왜 필요한지 모르겠네요~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
테스트 시에만 넣고 배포전에 빼려고 하는데 안좋은 습관일까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아니면 초기 db insert 스크립트에 넣는게 더 좋은 방식일까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
말씀하신대로 진행하게되면 테스트환경과 배포환경의 소스코드가 달라질텐데 좋은 방식일까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
심지어 관리자 비밀번호까지 소스코드에 노출되고있네요~