[Spring MVC (인증)] 이준민 미션 제출합니다. #100
Conversation
jermany17
changed the title
There was a problem hiding this comment.
안녕하세요 준민님
리뷰어 루카입니다!! 🐳
이번 미션이 쉽지 않으셨을거라 생각이 듭니다.
새로운 Spring 개념도 많이 나오고, 인증 인가라는 것이 쉬운 개념은 아니라서 생각할 포인트가 많았을 것 같아요.
일단, 이번 리뷰는 이번 학습에서 꼭 가져갔으면 하는 내용을 우선적으로 리뷰를 달아보았습니다.
💡 리뷰포인트
- 시크릿키 관리
- 인터셉터 경로 등록
- 인터셉터와 아규먼트 리졸버 역할
- 인증 실패 시 예외 처리
사실 질문이 없으셔가지고 제가 포인트를 준민님이 원하는바에 맞게 잡았는지 모르겠는데요.
너무 막막해서 질문을 못하는것일수도 있겠단 생각도 드네요.
이 시간이 조금 더 의미 있으려면 준민님의 여러 상태를 공유해주시면 좋겠어요.
일단 RC 드리겠습니다. 파이팅
|
|
||
| @Component | ||
| public class AdminInterceptor implements HandlerInterceptor { | ||
| private final String SECRET_KEY = "Yn2kjibddFAWtnPJ2AFlL8WXmohJMCvigQggaEypa5E="; |
There was a problem hiding this comment.
[Request Change]
이 시크릿 키로 jwt 토큰의 서명정보를 입력하였군요!
이 시크릿 키를 모르면 임의로 jwt를 만들 수가 없겠네요!!
이 파일 line30에서 디코딩되는 토큰은 이 시크릿키를 알고 만들었다는 확실한 믿음이 생기네요.
시크릿키의 일치를 통해 토큰의 신뢰를 판단하는 만큼 해당 정보는 매우 중요한 정보입니다.
다른 멤버들의 PR을 둘러보시면, properties 파일에 시크릿 키를 선언해두고
실제 코드에는 @Value, @ConfigurationProperties 같은 어노테이션을 이용해 빈에 주입받아 사용하는 경우가 많습니다.
그렇게 하는 이유는 여러가지가 있겠죠.
- 이 파일이 공개된 곳에 올라오면 안된다
- 이미 준민님의 방탈출 어플 인증은 우리 모두가 다 뚫을 수 있게 됐네요.
- 프로덕트 코드에 클래스 변수는 변경하기 쉽다.
- 준민님과 같이 협업하는 루카라는 개발자가 이 코드를 실수로 이코드를 바꿔버리면 심각한 버그와 보안상 문제가 생기겠어요.
이러한 이유들로 따로 변수로 관리하는 경우가 많은데요.
그렇다면 그런 궁금증이 있을 수 있겠네요.
Q) properties 파일도 깃헙에 올라오는거 아닌가요?
A) 이는 여러 방식으로 특정 properties파일을 공개 레포에서 숨기거나 할 수 있습니다. 프로젝트 내부에서 설정해주지 않고 빌드, 배포 스크립트에서 주입해도 되겠죠.
방법적인 부분은 추후에 고민해도 좋을 것 같아요. 다만, 로직적으로 관리할 영역과 프로젝트 설정 영역에 쓰는 변수들을 구분해서 관리하는 습관을 가져보면 좋겠어요.
| @Override | ||
| public Object resolveArgument(MethodParameter parameter, ModelAndViewContainer mavContainer, NativeWebRequest webRequest, WebDataBinderFactory binderFactory) { | ||
| HttpServletRequest request = (HttpServletRequest) webRequest.getNativeRequest(); | ||
| String token = extractTokenFromCookie(request.getCookies()); | ||
| if (token.isEmpty()) { | ||
| return null; // 로그인되지 않은 사용자 | ||
| } | ||
|
|
||
| // JWT에서 사용자 정보 추출 | ||
| Long memberId = Long.valueOf(Jwts.parserBuilder() | ||
| .setSigningKey(Keys.hmacShaKeyFor(secretKey.getBytes())) | ||
| .build() | ||
| .parseClaimsJws(token) | ||
| .getBody() | ||
| .getSubject()); | ||
|
|
||
| Member member = memberService.findById(memberId); | ||
| return new LoginMember(member.getId(), member.getName(), member.getEmail(), member.getRole()); | ||
| } | ||
|
|
||
| private String extractTokenFromCookie(Cookie[] cookies) { | ||
| if (cookies != null) { | ||
| for (Cookie cookie : cookies) { | ||
| if (cookie.getName().equals("token")) { | ||
| return cookie.getValue(); | ||
| } | ||
| } | ||
| } | ||
| return ""; | ||
| } |
There was a problem hiding this comment.
[Request Change]
인증 정보를 추출하는 곳이 ArgumentResolver와 Interceptor 두군데가 있는데요.
각각 인증정보가 옳지 않을 때 이렇게 처리하고 있어요.
- Interceptor
- 응답에 상태코드를 401로 주고, false를 반환해서 컨트롤러로 진입하지 않고 바로 응답하도록 한다
- ArgumentResolver
- 해당 아규먼트를 null로 내려준다. 컨트롤러에서 NPE가 터지도록...
[고민 1] 두 방식을 비교하며 ArgumentResolver에서 인증정보가 옳지 않을 때 어떤 문제가 있을지?
컨트롤러에서 LoginMember를 argument로 받겠다는것은 인증정보가 꼭 필요하다는 것입니다.
근데, 해당 정보가 없다는 것은 부적절한 접근이에요.
하지만 그 상황에서 NPE 같은 일반적인 예외가 터지면, 아무도 뭐가 문젠지 파악하기 어려워요.
[고민 2] 과연 인증 정보가 옳지 않은 모든 상황을 커버하고 있는가?
제가 생각했을 땐, 인증정보가 옳지 않다고 생각할만한 지점은 여러가지 있어요.
- 토큰을 담은 쿠키가 없을 때 (만료되었을 때)
- 토큰이 디코딩이 안될 때 (시크릿키가 불일치하거나 만료되었거나)
- 인증 정보에 있는 회원 정보가 현재 DB에 존재하지 않을 때
- ...
이런 여러 상황에서 발생할 수 있는 문제들(혹은 예외)을 어떻게 하면 잘 관리해서, 인증 실패라고 잘 클라이언트에게 전달할 수 있을까요?
| @Override | ||
| public void addInterceptors(InterceptorRegistry registry) { | ||
| registry.addInterceptor(adminInterceptor) | ||
| .addPathPatterns("/admin"); // /admin 경로에만 적용 | ||
| } |
There was a problem hiding this comment.
[Request Change]
엥?
저 로그인 않안했는데 /admin/reservation 페이지에 진입했어요...
보안이 뚤렸네요!!
Path를 설정할 때, 접근하려는 리소스를 계층적으로 나타내는 경우가 많습니다.
이 경우에도
- /admin => 어드민 페이지
- /admin/reservation => 어드민 페이지의 예약 관리
- /admin/theme => 어드민 페이지의 테마 관리
- /admin/time => 어드민 페이지의 시간 관리
이런식으로 맨 첫 계층에는 /admin으로 어드민 페이지라는 리소스를 정의했고
그 뒤로 각 기능 페이지에 대해서 정의하고 있군요.
그렇다면 어드민 페이지의 기능에 접근 제한을 생각하면, 저 페이지들을 모두 제한해야겠군요. (그리고 앞으로 생길 수 잇는 어드민 기능에 대해서두요)
그렇게 하기 위해서 스프링에서는 패스를 패턴으로 등록하는 것이 일반적입니다.
| Long memberId = Long.valueOf(Jwts.parserBuilder() | ||
| .setSigningKey(Keys.hmacShaKeyFor(secretKey.getBytes())) | ||
| .build() | ||
| .parseClaimsJws(token) | ||
| .getBody() | ||
| .getSubject()); |
There was a problem hiding this comment.
[Request Change]
JWT에서 사용자 인증 정보를 추출하는 과정이 여러곳에 존재할 수 있는데요.
만약 이 로직이 변경된다거나, 인증 정보를 담는 방식을 JWT를 하지 않으면 이 부분이 변경되겠네요.
위의 변경사항이 ArgumentResolver와 Interceptor 둘다 영향을 받는다니,
저희가 지금까지 챙긴 객체지향이 깨지는 느낌이 드네요.
-
ArgumentResolver와 Interceptor는 요청 응답 흐름에 있어 인증 정보를 적절한 시기에 로직을 처리하기 위해서 존재하는 Spring의 기능이라고 생각할 수 있겠죠.
-
Jwt는 요청 응답 흐름 보다는 인증정보를 담기위한 토큰 방식이라고 볼 수 있습니다.
이 두가지 역할을 확실히 구분하고,
중복을 없애,
유연한 프로그램을 짜면 좋겠어요.
| .getBody() | ||
| .get("role", String.class); | ||
|
|
||
| if (role == null || !role.equals("ADMIN")) { |
There was a problem hiding this comment.
[Request Change]
누가 토큰을 만들 때 role을 admin이라고 소문자로 입력하면 실패하겠군요.
role 같은 중요한 정보를 String 같은 자유로운 타입에 담는 것은 여러 문제를 야기할 수 있겠네요.
enum을 활용해서 바꿔볼까요?
No description provided.