Conversation
|
|
||
| log.info("RefreshToken 발급 완료: {}", dto.email()); | ||
| log.info("RefreshToken 발급 완료: {}", token); |
There was a problem hiding this comment.
RefreshToken이 발급되는 로직을 확인하기 위해 log를 사용하신걸로 생각했습니다.
토큰 값과같이 민감한 정보는 출력되지 않도록하고 token.substring(0, 10) 이런식으로 일부만 확인하는건 어떨까요 ?
There was a problem hiding this comment.
개발상 용이하게 확인 목적으로 로그를 찍어놨습니당 외부로 노출되는 부분이 아니고, 현재 개발 상태니까 별도의 로직을 넣기보단 그냥 두는 것은 어떨까요??
| redisTemplate.opsForHash().put(key, "token", refreshToken); | ||
| redisTemplate.opsForHash().put(key, "role", role.toString()); | ||
| redisTemplate.opsForHash().put(key, "email", email); |
There was a problem hiding this comment.
Hash 타입을 사용해서 여러개 필드를 그룹화해서 관리한 것이 좋아보입니다
| return Optional.ofNullable(roleValue) | ||
| .map(Role::valueOf) | ||
| .orElseThrow(RoleNotFoundException::new); |
There was a problem hiding this comment.
두개의 예외처리 발생시 RoleNotFoundException로 한번에 처리해주는 것도 좋지만,
null인 경우와 잘못된 Role 값이 저장된 경우의 예외처리를 나눠서 서비스단에서 처리해주는 방식은 어떨까요 ?
There was a problem hiding this comment.
유저의 역할이 저장되기 때문에 레디스에 저장될 때 잘못된 값이 저장될 일이 없다고 생각합니다.
이전 로직에서 Role에 대한 검증은 이루어진 후에 저장이 된다고 생각해요!
그렇다면 조회의 책임을 지는 메서드에서 잘못된 값을 검증할 필요는 없지 않을까요?? 어떻게 생각하시는지 궁금합니다!
There was a problem hiding this comment.
넵 이중으로 검증을 해줄 필요는 없으니까 해당 방식으로 간결하게 구현하는게 좋을거같습니다 !
| public void set(String email, String refreshToken) { | ||
| String key = getKey(email); | ||
| redisTemplate.opsForValue().set(key, refreshToken, expirationTime, TimeUnit.MILLISECONDS); | ||
| public void set(long userId, String refreshToken, Role role, String email) { |
There was a problem hiding this comment.
set 메서드가 expirationTime, email, role, refreshToken 등의 여러 역할을 한번에 처리해주고 있는데
setRefreshToken, setUserMetadata, setExpiration 이런식으로 분리를 해주는게
유지보수 측면에서 좋을거같다는 생각이 있습니다
There was a problem hiding this comment.
메서드로 분리해두는게 확장성이 좋을 것 같아요!
수정하겠습니당
| String accessToken = jwtProvider.createAccessToken(id, email); | ||
| String refreshToken = jwtProvider.createRefreshToken(id); | ||
| public JwtDto create(Long userId, String email, Role role){ | ||
| String accessToken = jwtProvider.createAccessToken(userId, email, role); |
| String key = getKey(userId); | ||
|
|
||
| if (Boolean.TRUE.equals(redisTemplate.hasKey(key))) { | ||
| redisTemplate.opsForHash().put(key, "role", role); |
There was a problem hiding this comment.
"role"로 겹치는데 JwtProvider의 private static final String ROLE_CLAIM = "role";처럼 상수화 해주시면 좋을거같습니다!
PR 내용
PR 세부사항
관련 스크린샷
주의사항
체크 리스트