[JWT 인증 서버] 이민구 제출합니다.#45
Conversation
Donghwan814
left a comment
Donghwan814
left a comment
There was a problem hiding this comment.
안녕하세요 민구님! 시험은 잘 보셨을까요? 혹시 월요일까지 시험이 있으시다면 남은 시험도 파이팅입니다!
시험 기간이 겹치다 보니 코드 리뷰가 조금 늦어진 점 양해 부탁드립니다 🥲
전체적으로 코드는 잘 작성해주신 것 같습니다. 제가 평소 작성해오던 방식과는 다른 부분들도 있어서, 저도 새로운 방식으로 접근해볼 수 있는 좋은 기회가 되었습니다.
이번 코드 리뷰는 마지막 과제인 인증/인가와 시큐리티에서 중요하게 다루는 JWT 관련 내용입니다. 보안과 직접적으로 연결되는 부분이다 보니 신경 써야 할 내용도 많고, 헷갈리는 개념도 많았을 것이라고 생각합니다. 처음부터 완벽하게 구현하기에는 분명 어려움이 있었을 것이라 예상됩니다.
그래서 이번 리뷰에서는 민구님께서 코드를 이렇게 작성하신 의도를 먼저 확인하고, 제가 작성했던 방식과 비교해보면서 서로의 접근 방식을 공유하는 방향으로 진행해보려고 합니다.
과제 제출하시느라 정말 고생 많으셨습니다!
There was a problem hiding this comment.
이미지 파일명은 token인데 테스트 결과 화면은 조회 결과 완료된 이미지 내용인 부분을 확인했습니다. 혹시 파일 명을 token으로 지정해주신 이유에 대해서 설명해주실 수 있을까요?
There was a problem hiding this comment.
password 값을 encoder 사용해서 암호화 걸어주신 부분 잘 처리해주셨네요! 😊
There was a problem hiding this comment.
AuthController의 의미를 살펴봤을 때 회원의 계정과 관련된 요청을 처리해주는 부분에서 지금은 회원가입 관련 로직밖에 보이지 않습니다. 그렇다면 로그인은 Controller의 요청을 받아서 처리가 되지 않는 것인지 그렇다면 로그인이 이뤄져야 token이 발급되고 인증을 통해 그 뒤에 서비스들을 처리할 수 있을텐데 어떻게 구현을 하셨는지 말씀해주세요!
| public class CustomUserDetails implements UserDetails { | ||
| private final UserEntity userEntity; | ||
|
|
||
| public CustomUserDetails(UserEntity userEntity) { | ||
| this.userEntity = userEntity; | ||
| } | ||
|
|
||
| @Override | ||
| public Collection<? extends GrantedAuthority> getAuthorities() { | ||
|
|
||
| Collection<GrantedAuthority> collection = new ArrayList<>(); | ||
|
|
||
| collection.add(new GrantedAuthority() { | ||
|
|
||
| @Override | ||
| public String getAuthority() { | ||
|
|
||
| return userEntity.getRole(); | ||
| } | ||
| }); | ||
|
|
||
| return collection; | ||
| } | ||
|
|
||
| @Override | ||
| public String getPassword() { | ||
|
|
||
| return userEntity.getPassword(); | ||
| } | ||
|
|
||
| @Override | ||
| public String getUsername() { | ||
|
|
||
| return userEntity.getUsername(); | ||
| } | ||
|
|
||
| @Override | ||
| public boolean isAccountNonExpired() { | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean isAccountNonLocked() { | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean isCredentialsNonExpired() { | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean isEnabled() { | ||
|
|
||
| return true; | ||
| } |
There was a problem hiding this comment.
각 메서드별 구현을 잘 해주셨네요! 👍
스프링 시큐리티는 우리가 직접 만든 UserEntity의 구조를 알지 못하기 때문에, 이를 UserDetails 형태로 감싸서 전달하려고 하신 의도로 이해했습니다. 어댑터 역할을 정확히 잡아주신 것 같아요.
다만 한 가지 여쭤보고 싶은 부분이 있는데요, 유저의 권한을 반환하는 getAuthorities() 메서드입니다. 다른 메서드들은 깔끔하게 위임 호출로 구현해주셨는데, 이 부분만 익명 클래스로 GrantedAuthority를 직접 구현하신 특별한 이유가 있으실까요?
| String username, | ||
| String password |
There was a problem hiding this comment.
이 부분에는 로그인할 때 필요한 DTO값을 작성해주셨는데 이 부분이 어디에서 사용이 되고 있는지 알 수 있을까요?
| @RestControllerAdvice | ||
| public class GlobalExceptionHandler { | ||
|
|
||
| @ExceptionHandler(IllegalArgumentException.class) | ||
| public ResponseEntity<RsData<Void>> handleIllegalArgumentException(IllegalArgumentException e) { | ||
|
|
||
| RsData<Void> rsData = new RsData<>("404-1", e.getMessage()); // 메시지 그대로 사용 | ||
|
|
||
| return ResponseEntity | ||
| .status(rsData.statusCode()) | ||
| .body(rsData); | ||
| } | ||
| // 중복 회원 | ||
| @ExceptionHandler(IllegalStateException.class) | ||
| public ResponseEntity<RsData<Void>> handleIllegalStateException(IllegalStateException e) { | ||
| RsData<Void> rsData = new RsData<>("409-1", e.getMessage()); | ||
| return ResponseEntity.status(rsData.statusCode()).body(rsData); | ||
| } | ||
| // 찾을 수 없음 | ||
| @ExceptionHandler(NoSuchElementException.class) | ||
| public ResponseEntity<RsData<Void>> handleNoSuchElementException(NoSuchElementException e) { | ||
| RsData<Void> rsData = new RsData<>("404-1", e.getMessage()); | ||
| return ResponseEntity.status(rsData.statusCode()).body(rsData); | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
GlobalExceptionHandler 파일 내용 잘 봤습니다!
여러 곳에서 중복으로 발생할 수 있는 예외들을 이곳에서 공통으로 처리하시려는 의도로 이해했습니다. IllegalArgumentException, IllegalStateException, NoSuchElementException 별로 적절한 응답 코드를 매핑해주신 점 좋네요. 👍
다만 한 가지 여쭤보고 싶은 부분이 있습니다. 이번 과제가 JWT 인증·인가에 관한 내용인 만큼, 인증·인가 과정에서 필연적으로 발생하게 되는 401 Unauthorized(인증 실패)와 403 Forbidden(권한 부족)에 대한 공통 예외 처리는 별도로 구현하지 않으신 특별한 이유가 있으실까요?
There was a problem hiding this comment.
BaseTimeEntity 잘 봤습니다!
여러 엔티티에서 공통으로 쓰이는 시간 컬럼을 별도 클래스로 분리한 뒤, @MappedSuperclass로 상속받아 재사용하도록 구성해주셨네요. 중복을 줄이는 좋은 접근인 것 같습니다.
한 가지 여쭤보고 싶은 부분이 있는데요, 보통 이렇게 시간 정보를 공통 처리할 때는 생성일시(@CreatedDate)와 함께 수정일시(@LastModifiedDate)도 같이 두는 경우가 많은데, 이번에는 생성일시만 두신 특별한 이유가 있으실까요?
이번 과제에 게시물 수정(PUT /post/{id}) 기능이 포함되어 있는 만큼, 수정 시각을 함께 기록해두면 추후 정렬이나 변경 이력 추적에 유용할 것 같아 여쭤봅니다.
|
안녕하세요 민구님! 과제 수행하시느라 고생 많으셨습니다. 이번 리뷰에서는 JWT 토큰의 생성·검증 로직이 안전하게 구현되었는지, 예외 처리는 꼼꼼하게 되었는지를 중심으로 살펴보려고 합니다. 정답을 정해두고 하는 리뷰라기보다는, 더 좋은 코드를 함께 고민하는 자리이니 편하게 이야기 나눴으면 좋겠습니다. |
| http | ||
| .csrf((auth) -> auth.disable()); | ||
|
|
||
| //From 로그인 방식 disable | ||
| http | ||
| .formLogin((auth) -> auth.disable()); | ||
|
|
||
| //http basic 인증 방식 disable | ||
| http | ||
| .httpBasic((auth) -> auth.disable()); | ||
|
|
There was a problem hiding this comment.
csrf, session, formLogin, httpBasic을 비활성화 해두셨는데 비활성화를 사용하시는 이유를 설명해주시면 좋을 것 같습니다!
There was a problem hiding this comment.
민구님, User 엔티티를 보면 id, username, password, role 같은 기본적인 컬럼들을 깔끔하게 잘 구성해 주셨습니다!
한 가지 논의해보고 싶은 점은, 현재 **Refresh Token(RT)**을 저장하는 구조가 빠져 있다는 점입니다. Access Token은 무상태(Stateless)로 검증되므로 DB 저장 컬럼이 필수가 아니지만, Refresh Token은 토큰 갱신 및 보안 제어를 위해 서버 측 저장소(테이블이나 Redis 등)가 꼭 필요하다고 생각합니다.
만약 Access Token이 만료되었을 때, 프론트엔드가 보낸 Refresh Token이 서버에 저장된 값과 일치하는지 확인하고 검증하는 과정이 있어야 안전하게 Access Token을 재발급해줄 수 있거든요. (서버에 저장해두지 않으면 토큰이 탈취당했을 때 강제 로그아웃 등의 제어가 불가능해지는 문제도 있습니다.)
혹시 정훈님이 이번 로컬 로그인을 구현하시면서 구상하셨던 Access Token과 Refresh Token의 구체적인 관리 전략이나 각각의 역할에 대해 피드백으로 공유해 주시면 감사하겠습니다!
| if (authorization == null || !authorization.startsWith("Bearer ")) { | ||
| filterChain.doFilter(request, response); | ||
|
|
||
| return; | ||
| } |
There was a problem hiding this comment.
이 부분을 보면 Authorization헤더가 아니거나 토큰 타입이 Bearer가 아니면 다음 필터를 실행시키는 로직인 것 같습니다!
토큰이 없는데 에러메세지를 출력하지않고 다음 필터로 넘기셨는지 의도가 궁금합니다!
3 participants
과제명
JWT 인증 서버
💡 작업 내용
🔗 참고 링크
🤔 느낀 점 / 어려웠던 점
기본적인 회원가입/ 로그인 을 해보면서 전체적인 흐름에 대해 집중하면서 작성해보았습니다.
소셜로그인 , 보안, 예외처리에 대해 더욱 학습해야 할거같습니다.