[Spring MVC(인증)] 안금서 미션 제출합니다. #102
Conversation
There was a problem hiding this comment.
안녕하세요 금서님! 그리디에 처음 합류할 때부터 금서님께서 짱 잘하시고 또 열심히 임하신다는 이야기를 많이 들었습니다. 아니나 다를까 깔끔한 멋쟁이 코드를 작성해 주셨네요. :D
- 금서님께서 겪으신 WeakKeyException은 아쉽게도 제 노트북 환경에서 재현되지 않았습니다.
SHA 키를 만드는 데에 사용하는 hmacShaKeyFor(byte[] bytes) 메서드는 파라미터로 넘어온 키의 바이트 길이에 알맞은 암호화 알고리즘을 찾아 사용하는데, 기본으로 HS256, HS384, HS512 이렇게 세 개가 있습니다.
미션 예시에서 사용하는 "Yn2kjibddFAWtnPJ2AFlL8WXmohJMCvigQggaEypa5E=" 문자열의 바이트 길이가 352이기 때문에 그에 맞는 알고리즘인 HS256를 사용해 정상 작동이 되었어야 하지만
금서님 노트북의 라이브러리 버전 차이 등의 이유로 HS256이 없고 HS512만 있어 발생한 문제로 보입니다.
하지만 이 미션에서는 어떤 알고리즘을 쓰든 잘 작동하기만 하면 되니 금서님의 해결 방법은 멋쟁이 해결 방법입니다. 잘 하셨습니다. >:D
- 클라이언트의 의도를 파악할 수 있는 다른 수단이 있다면 클라이언트 코드를 보지 않아도 됩니다. 예를 들어 요구사항 문서가 지금보다 더 상세하게 적혀 있다던가, 어떤 API를 어떤 때에 호출한다는 내용이 상세히 적힌 API 문서가 있다면 더할 나위 없이 좋을 것입니다.
하지만 이번 미션을 포함하여 대부분의 상황에서 이렇게 친절한 문서는 존재하지 않을 것입니다. 문서로 얻을 수 있는 정보가 부족할 땐 클라이언트 코드를 직접 찾아보며 힌트를 얻는 것이 가장 정확하고 직접적인 방법입니다.
제가 일하는 회사에서도 요청/응답 포맷이나 예외 처리 방법을 참고하기 위해 프론트엔드 개발자분께 여쭈어보거나, 제가 직접 클라이언트 코드를 열어 분석하는 일을 엄청 자주 합니다.
- JWT는 인증/인가를 구현하는 여러 방법 중 하나입니다.
금서님께서 말씀하신 세션 기반의 인증, 또는 현업에서 잘 안 쓰지만 BASIC 인증방법도 인증 방법이 될 수 있습니다.
이러한 관점에서 보았을 때 JWT는 '변경 가능한 지점' 입니다. 인증방식으로 JWT를 쓰다가 하루 아침에 세션 인증 방식을 쓰자고 할 수도 있습니다.
만약 JWT에서 세션 인증을 쓰기로 했을 때, 지금의 코드에서는 얼마나 많은 곳이 변경되어야 할까요?
변경 지점을 최소화하고자 한다면 어떻게 개선해야 좋을까요?
리팩토링하는 과정에서 고민해보시면 좋을 것 같습니다. >:D
- 제가 생각하는 멋쟁이 예외처리의 기준은 다음과 같습니다.
[문제의 발생 원인을 정확하고 자세히 파악할 수 있도록 예외처리가 되어 있나요?]
금서님께서는 커스텀 예외를 활용해서 멋지게 예외처리를 해주셨지만, '문제의 발생 원인' 파악의 관점에서는 다소 덜 멋쟁이인 지점이 보입니다. 이건 리뷰를 통해 이야기를 나누면서 더 자세히 알아가면 좋을 것 같습니다 :)
제가 생각하는 개선 포인트를 앞으로의 일주일 동안 리뷰 코멘트로 남기겠습니다.
부담이 되지 않게 한 번에 남기는 코멘트의 수를 적게 하는 대신 빠르고 자주 남겨드릴게요.
답변이 급히 필요하신 경우에는 디스코드의 DM 등으로 연락주시면 더 빠르게 답변을 드릴게요.
함께 멋지게 개선해 보아요! :)
|
|
||
| @Component | ||
| @RequiredArgsConstructor | ||
| public class AuthAdminRoleInterceptor implements HandlerInterceptor { |
There was a problem hiding this comment.
금서님께서 'login', 'authentication', 'jwt' 패키지를 구분하신 기준이 궁금합니다.
기본 제공된 코드의 구조에 맞추어서 도메인 별로 패키지를 구분하신 것 같은데요,
얼핏 생각하기에 login과 authentication은 뜻 차이가 거의 없고,
jwt는 인증의 수단 중 하나이니 authentication에 포함되는 관계가 아닌가- 라는 생각이 들었어요.
| } catch (JwtException e) { | ||
| throw new JwtProviderException("JWT 생성에 실패하였습니다."); | ||
| } | ||
| } |
There was a problem hiding this comment.
- 이 메서드는 JwtUtils의 static 메서드로 만들지 않고 별도의 JwtProvider로 분리하셨습니다. 이유를 알 수 있을까요?
There was a problem hiding this comment.
} catch (JwtException e) {
throw new JwtProviderException("JWT 생성에 실패하였습니다.");
}
이 코드는 에러 추적의 관점에서 문제가 하나 있습니다. 어떤 문제가 있을까요?
| } | ||
|
|
||
| @GetMapping("/login/check") | ||
| public ResponseEntity<LoginCheckResponse> checkLogin(HttpServletRequest request) { |
There was a problem hiding this comment.
일반적으로 컨트롤러의 반환 타입으로 ResponseEntity<>를 사용하면
비즈니스 로직의 상황에 따라 다양한 httpStatus를 지정하여 응답할 수 있다는 장점이 있습니다.
예시)
try {
doSomethong();
return ResponseEntity.ok().build();
} catch (NotLoginException e) {
return ResponseEntity.status(401).build();
} catch (IllegalStateException e) {
return ResponseEntity.badRequest().build();
}
그런데 현재 컨트롤러 구조에서는
비즈니스 작업을 수행하다 예외가 발생했을 때
status를 401 또는 500으로 변환하여 응답하는 작업을 모두 ExceptionHandler에서 진행합니다.
그렇다면 LoginController 자체는 httpStatus를 분기하지 않고 언제나 ok를 응답하는데,
응답 타입으로 ResponseEntity를 사용할 필요가 있을까요?
| JwtUtils.secretKey = secretKey; | ||
| } | ||
|
|
||
| public static MemberAuthInfo extractMemberAuthInfoFromToken(String token) { |
There was a problem hiding this comment.
JwtUtils의 메서드는 static 메서드인데 secretKey라는 인스턴스 변수를 필요로 했습니다.
하지만 static 메서드는 인스턴스 변수에 접근할 수 없으니 secretKey 변수를 static한 클래스 변수로 만들었습니다.
스프링은 클래스 변수에 자동 주입을 해주지 않으니 인스턴스 메서드인 setScretKey()를 만들고 스프링이 사용하게끔 우회하셨습니다.
작성하시면서도 느끼셨겠지만 static 메서드에 주위 환경을 끼워맞추는 과정에서 점점 찜찜하고 부자연스러운 구조가 되었습니다.
이 클래스는 스프링이 없으면 단위 테스트를 하기도 어렵고, 동시성 측면에서 위험한 코드이기도 합니다.
자바의 정석 서적을 집필하신 멋쟁이 강사님께서 static 메서드에 대해 설명해주신 영상이 있습니다.
https://youtu.be/Fl4TzjPKAMU?si=JtddEqhCV7B127-A
위 영상을 참고해서 아래 두 질문의 답을 고민해주셨을 좋겠습니다.
- 자바에서 static 메서드는 어떤 경우에 사용하나요?
- JwtUtils의 메서드들은 static이어야 하는 메서드인가요? 그 이유는 무엇인가요?
| @NotEmpty | ||
| private String password; | ||
|
|
||
| } |
There was a problem hiding this comment.
이 친구는 record 대신 @DaTa를 사용하신 이유가 있나요? :D
안녕하세요 여우 리뷰어님! 🦊
처음 뵙네요 반갑습니다:)
이번 미션은 시행착오가 많았어서... 조금 오래 걸렸네요. 😵💫
개념은 얼추 알고 있었지만, 실제로 적용해보는 과정에서 진땀을 뺐던 것 같아요.
우선, 미션 진행하면서 겪었던 생겼던 시행착오에 대해 말씀드리고 조언을 구하고 싶습니다!
<겪었던 시행착오 및 해결방안>
"io.jsonwebtoken.security.WeakKeyException: The verification key's size is 352 bits which is not secure enough for the HS512 algorithm. The JWT JWA Specification (RFC 7518, Section 3.2) states that keys used with HS512 MUST have a size >= 512 bits (the key size must be greater than or equal to the hash output size)."와 같은 에러가 발생했었습니다. 이 에러가 계속 나서, 구글링 후 여러 방식으로 고쳐봤었지만 없어지질 않더라고요. 해결한 방법은 secret 키의 길이를 임의로 더 늘리는 방식이었습니다.
`
function updateUIBasedOnLogin() {
fetch('/login/check') // 로그인 상태 확인 API 호출
.then(response => {
}`
"//에러 처리 또는 로그아웃 상태일 때 UI 업데이트" 부분에서 에러가 발생하고 있었습니다. 400 Bad Request 애러가 발생했었기 때문에 이를 해결하기 위해 401 Unauthorized 상태 코드를 반환하도록 예외처리를 해주었습니다.
정리해보자면,
제가 이 과정에서 시행착오를 겪었던 부분은 메인페이지나, 로그인 페이지, 혹은 로그아웃 페이지를 각각 요청했을 때 원인 모를 "login/check"가 계속 호출이 되었고 그에 따른 error가 발생하였습니다. 에러가 발생하는 부분의 클라이언트 코드를 추적해보니, 이 error는 서버 응답이 200이 아닌 경우(즉, 토큰 추출에 실패하여 400 Bad Request의 상태 코드)에 발생하는 것으로 어떻게 보면 당연한 결과라고 말할 수 있습니다. 애초에 클라이언트의 의도를 잘못 파악하고 있었기 때문에 겪었던 시행착오라고 할 수 있겠네요. 클라이언트 측에서는 매번 새로운 페이지 요청이 올 때마다 "로그인 상태" 에 따른 UI를 업데이트 해줘야하기 때문에, 로그인 상태일 경우 json 형식으로 응답 데이터를 반환하고 로그인 상태가 아닌 경우 에러가 나도록 설계한 것이라고 조심스레 추측해봅니다. 저는 "에러가 왜 나지?", "왜 요청하지도 않은 페이지가 호출되는 거지?" 에 너무 초점을 맞추다 보니, 클라이언트의 의도와는 다르게 생각하여 삽질을 계속 했던 것 같습니다. 이렇게 매 요청마다 "/login/check"가 요청되는 게 아니라, 특정 상황에서만 호출될 것이라고 막연히 생각하고 있었던 것 같아요. "/login/check" 의 의도를 전혀 생각 안 하고 미션 구현에만 급급했던 저의 과거를 돌아봐야할 것 같네요..🥲
클라이언트의 의도를 파악한 후에, 로그인되지 않은 경우 반환되었던 400 Bad Request 상태 코드를 401 Unauthorized을 반환하도록 적절한 예외처리를 해주었습니다. 클라이언트는 이 상태 코드를 통해 적절한 UI 처리를 하겠죠?
여기서 궁금한 점은
위와 같은 상황이 발생했을 시, 클라이언트 코드를 까봐야 해결이 가능한 듯 한데 이렇게 직접 부딪혀보며 구현을 해야할까요? 혹은 다른 방법이 있을까요?
아니면 제가 위 내용 중 잘못 생각한 부분이 있을까요? 있다면 여우님의 의견이 궁금합니다!
세번째로, 토큰 생성 및 Utils를 활용하는 방식에 대한 내용입니다. 세션 기반으로 인증하는 방식은 알고 있었지만, 토큰 기반 인증 방식은 처음 접하는 거라 어떻게 토큰을 발급해야할지, 토큰을 발급해서 어떤식으로 사용해야하는지 등 가장 시간이 오래 걸렸던 구현 단계였습니다. 제가 구현한 코드 중에서 "이건 좀 성능에 문제가 있을 것 같다." 거나, 부적절해 보이는 부분이 있다거나, 좀 더 개선했으면 하는 사항이 있다면 해당 부분에 대한 코멘트 부탁드립니다!
네번째로, 예외처리에 대한 내용입니다. 저는 커스텀 예외를 적극 활용하여 @controllerAdvice와 @ExceptionHandler를 통해 예외처리를 하였습니다.
GeneralExceptionHandler : 전역적으로 발생한 예외에 따른 적절한 상태 코드 반환하여 예외 처리
PageExceptionHandler : 뷰 페이지를 랜더링 하는 과정에서 발생하는 예외를 잡아 적절한 예외 처리(해당 예외와 관련된 뷰 페이지로 랜더링하는 등)
제 생각에는 조금 더 구체적으로 예외처리를 더 해야만 할 것 같은 느낌이 드는데 리뷰어님이 생각하시기엔 어떤지 여쭤보고 싶습니다~!
날카롭고 아낌 없는 리뷰 주시면 감사하겠습니다 :)
잘 부탁드립니다 🙇🏻🙇🏻♀️
아 그리고 해피뉴이어입니다!