Review to 서용현

[jy-b]

용현님 구현 파트에 대한 리뷰입니다
코드에 한줄씩 달면 알람이 왕창 갈꺼 같아서 요런식으로 남겨봅니다
백엔드 코드만 보았고 제가 짠 코드가 아니라서 내용이 틀릴수도 있어요
미리 미안합니다 우리 조장님 （；´д｀）ゞ 😸😸😸😸

CorsConfig URL과 Cookie Domain URL이 같은 값이라면 CONST나 환경변수로 같이 묶어 주면 좋을것 같아요

MemberService 로그아웃 시 한번 더 검증 하는 이유가 있을까요?

JwtAuthenticationFilter

AT = Access RT = Refresh

• case 1. 쿠키 없이 /api/token에 접근 하면 exception 터지지 않을까요?
  
  
• case 2. AT가 비정상(조작됨), 만료, RT존재, requset uri = /api/token
  -> AT에 대한 검증을 먼저 하면 좋을것 같아요 extractExpiredCheck에서 사용하는 decode는 토큰의 정상여부를 보장 하지 않는다고 하네요
  
  
• case 3. AT 만료, 쿠키 존재, RT null
  -> 다음 필터로 넘어가나요?
  
  
• case 4. AT 만료, 쿠키 존재, RT가 존재하나 비정상 or 만료
  tokenService.createNewTokens에서 예외 처리 없음
  
  EX) RT가 비정상이면 NPE
  RT 만료 체크 없음
  
  대부분 정상적인 접근의 경우에는 일어나지 않을 일 이지만 혹시 모르니...
  
  RT에 대한 검증만 추가하면 대부분 막힐것 같아용
  
  java-jwt 라이브러리는 JWTVerifier 클래스가 검증 메소드를 제공한다고 하네요 DOCS

getAuthentication != null : Line 79에서 set하기 전까지는 null일것 같은데 null이 아닌 경우가 있나요?

UsernamePasswordAuthenticationToken userDetails 전체를 넣을 필요가 있을까요?
Authentication의 principal을 email로
Password를 사용 하는 로직이 없다면 credentials를 null로 넣어 줘도 괜찮을 것 같아요


궁금한 점

Spring Security의 Login, Logout logic대신 커스텀 컨트롤러로 구현하신 이유가 있으실까요?

🦖 🦖 🦖 🦖

[seonghye0n]

와... 자세한 코드리뷰 감사합니다👍

[zjdtm]

🙌 꼼꼼한 코드 리뷰 넘나 감사합니다.

• MemberService 로그아웃에 검증 로직은 잘못 작성하였네요 리펙터링 할 때 삭제하겠습니다 !!

• token 처리 부분 case 1, 3 다 403Forbidden 뜨네요 수정하겠습니다 !!

• token 처리 부분 case 2, 4 정보 감사합니다 다시 공부하겠습니다!!

• getAuthentication != null 인 경우가 로그인을 하지 않은 상태로 보고 작성을 했는데 한 번 다시 공부해보고 확인해보겠습니다 !!

• 헉 거창한 이유는 없습니다. 사실 Spring Security 개념이 부족해서 Login, Logout 을 security 의 handling 설정하는 것도 처음 알아서 공부해보고 한 번 해보겠습니다!!

시큐리티 공부가 아직 덜 된 상태라서 코드가 많이 더러운 상태인데도 봐주셔서 감사합니다 종윤님 !!

[jy-b]

UsernamePasswordAuthenticationToken userDetails 전체를 넣을 필요가 있을까요?
Authentication의 principal을 email로
Password를 사용 하는 로직이 없다면 credentials를 null로 넣어 줘도 괜찮을 것 같아요

--> 저렇게 하면 Role이 업데이트 될때 추가 로직이 필요 할것 같네요
저는 DB IO를 최소화 하려고 토큰으로 id와 role을 Authentication에 set했는데
확장성을 고려한다면 UserDetails로 처리하는것도 괜찮을것 같습니다