[Spring MVC (인증)] 홍석주 미션 제출합니다. #150

somefood · 2025-04-13T11:10:31Z

안녕하세요 석환님,

MVC (인증) 부분 작성해서 PR 올립니다!
간만에 미션을 진행하다보니 미흡한게 많은데,
천천히 봐주시면 감사하겠습니다~! ㅎㅎ

seokhwan-an

spring mvc(인증) 미션 구현해주시느라 고생하셨습니다.
석주님이 구현하신 것 기반으로 리뷰 남겼는데 확인 부탁드립니다!

seokhwan-an · 2025-04-14T07:57:42Z

src/main/java/roomescape/member/MemberDao.java

+    @NotNull
+    private static RowMapper<Member> getMemberRowMapper() {
+        return (rs, rowNum) -> new Member(
+                rs.getLong("id"),
+                rs.getString("name"),
+                rs.getString("email"),
+                rs.getString("role")
+        );
+    }
+


혹시 rowmapper에 @NotNull를 붙이신 이유가 있을까요?
더불어서 static은 필요가 없을 것 같은데 아마 인텔리제이에서 메소드를 추출하는 과정에서 발생한 것 같습니다. static 키워드는 제거해도 좋을 것 같습니다.

저는 공통으로 쓰는 private 메소드는 가장 아래에 두는 편인데 석주님은 어떻게 private 메소드를 관리하시나요?

어 그러네요 왜 NotNull이 붙어있지..! 빼두겠습니다 ㅎㅎ
static도 계속 메서드 추출하다 생긴건데 놓치게 되네요 감사합니다!

음 저도 private 메서드는 아래로 빼두는 편인데 이번에 추출하면서 그냥 바로 아래에 둬버렸군요 ㅎㅎ

seokhwan-an · 2025-04-14T08:18:15Z

src/main/java/roomescape/member/infrastructure/JwtTokenProvider.java

+    public String createToken(Member member) {
+        return Jwts.builder()
+                .setSubject(member.getId().toString())
+                .claim("name", member.getName())
+                .claim("role", member.getRole())
+                .signWith(Keys.hmacShaKeyFor(secretKey.getBytes()))
+                .compact();
+    }


현재 해당 코드 부분은 lms에서 힌트 코드를 이용하신 것 같습니다.
JWT의 경우 payload 부분은 누구나 접근이 가능할 수 있는데 "이름", "권한" 정보를 노출시켜도 괜찮을 지 한번 고민해보면 좋을 것 같습니다.

오 좋은 고민인거 같습니다.
이번엔 그냥 힌트에 있는거 고대로 가져다 쓰긴 했는데,
사실 권한은 리퀘스트가 서버에 도달했을 때 그때 내부적에서 체크하면 된다고 생각을 하거든요.

석환님은 payload에 보통 어떤 데이터들을 넣으시는지 궁금합니다!

저는 주로 외부에 노출되어도 의미가 없는 정보인 id를 payload로 넣고 서버 내부에서 member를 조회해서 인증 및 인가를 체크해서 이용합니다. 아무래도 payload는 외부에 노출되는 정보이다 보니 서버를 파악할 수 있는 정보를 포함하지 않는 것이 중요하다고 생각합니다.

그러면 더 이야기를 나누고 싶은 부분이 있을 것 같은데 만약 토큰이 탈취되면 어떻게 처리할 수 있을까요?

의견 감사합니다.
id 정도만 넣어도 충분히 서버에서 다 검사할 수 있을거 같네요 ㅎㅎ

음 토큰 탈취가 되면 곤란한 상황이 될거 같습니다.
토큰의 유효기간이 짧아서 만료되면 다행일거 같지만, 발급 직후에 탈취당한다면 쉽지 않겠군요..!
만약 사용자의 토큰을 DB에서 관리한다면, 블랙리스트를 만들어서 해당 토큰으로 들어오면 차단할 수 있을거 같습니다.
블랙리스트를 만들기 어렵다면, 단순히 하나의 토큰만 각 멤버의 컬럼으로 저장해서 비교해보지 않을 까 싶습니다
그 전에 신규 토큰들을 먼저 생성하구요.

석환님의 의견도 궁금합니다 ㅎㅎ

seokhwan-an · 2025-04-14T08:26:30Z

src/main/java/roomescape/member/infrastructure/JwtTokenProvider.java

+    public Long parse(String token) {
+        String subject = Jwts.parserBuilder()
+                .setSigningKey(Keys.hmacShaKeyFor(secretKey.getBytes()))
+                .build()
+                .parseClaimsJws(token)
+                .getBody().getSubject();
+        return Long.valueOf(subject);
+    }


jwt 토큰을 파싱하는 과정에서 여러 예외 상황이 발생할 수 있는데 이 부분은 어떻게 처리할 수 있을지 한번 고민해보면 좋을 것 같아요

토큰 기간이 만료되었을 때

토큰의 secret key가 잘못된 경우 일 때(잘못된 토큰이 넘어왔을 때)

현재는 이 정도 예외가 떠오르는데요 각각 어떻게 처리하면 좋을지 한번 고민해보시면 좋을 것 같습니다.

seokhwan-an · 2025-04-14T08:31:36Z

src/main/java/roomescape/common/config/WebConfig.java

+    @Override
+    public void addInterceptors(InterceptorRegistry registry) {
+        registry
+                .addInterceptor(authorizationInterceptor)
+                .addPathPatterns("/admin/**");
+    }


이번에 인가를 처리하기 위해서 인터셉터를 이용하셨네요!
spring에서 인가를 처리하는 방안에서는 여러가지가 있는데 인터셉터를 이용하신 이유는 무엇인가요? (filter, interceptor, aop를 이용해서 인가 처리를 할 수 있을 것 같네요)

가장 큰 이유는 미션 내용에 있는 인터셉터를 참고했기 때문입니다 ㅎㅎ..!
추가적으로 찾아보았을 때,
AOP의 경우 컨트롤러의 타입과 실행 메서드가 모두 제각각이면 포인트컷의 작성이 번거롭고, request, response 객체를 얻기 불편하기에 인가 처리에는 조금 부적합하다고 합니다.

필터의 경우로도 빈으로 등록해서 똑같이 로직을 태워도 될거 같습니다. 근데 아래 링크를 참고해 보니 필터의 경우는 서블릿 밖에서 적용되다보니 스프링의 예외처리(ControllerAdvice)를 사용하기 어려워진다고 하니 인증/인가 용으로 인터셉터를 사용하는게 괜찮아 보입니다!
물론 필터에서도 response를 객체를 조작해서 예외응답을 내려주면 될거 같긴 하네요 ㅎㅎ

참고 - https://mangkyu.tistory.com/173

석주님이 답변주신 것처럼 인터셉터의 가장 큰 장점은 스프링의 예외처리(ControllerAdvice)를 이용할 수 있다는 것입니다. 또한 인터셉터는 필터와 다르게 메소드에 Object handler를 파라미터를 가지고 있어서 요청을 처리하는 handler를 다룰 수 있다는 특징이 있네요.

필터의 경우 이용해보면서 느꼈던 장점은 url뿐만 아니라 http method 정보를 통해서 필터 요청을 구분할 수 있다는 것이 인터셉터에 비해서 인증 및 인가를 상세하게 처리할 수 있다는 것이 가장 큰 장점이라고 느꼈습니다. 인터셉터에서 http method를 통해 구분할려고 하면 추가적인 구현이 필요하더라구요

아하 그렇군요.
궁금한 것이 인터셉터의 preHandle에서도 필터와 똑같이 request, response 객체들은 받을 수 있는거 같은데,
Http 메서드 구분할 때 필터랑 어떻게 다른 방식으로 구분하시는 지 궁금합니다!

seokhwan-an · 2025-04-14T08:35:11Z

src/main/java/roomescape/common/interceptor/AuthorizationInterceptor.java

+        if (member == null || !member.getRole().equals("ADMIN")) {
+            response.setStatus(401);
+            return false;
+        }


현재 존재하지 않는 사용자와 "ADMIN" 권한이 없는 사용자 일 때 모두 401(unauthorized) 응답 값을 보내주고 있네요
이는 인증과 인가가 모두 401로 처리되는 것 같은데 이 둘을 분리하면 어떨까요? 권한이 허용되지 않는 요청에 대한 상태 코드도 한번 보고 분리하는 것이 좋을 것 같습니다.

더 상세히 분리하는 것도 좋아보이네요!
401과 403으로 더 분리해보도록 하겠습니다~!

seokhwan-an · 2025-04-14T08:41:41Z

src/main/java/roomescape/common/resolver/LoginMemberArgumentResolver.java

+                return new LoginMember(member.getId(), member.getName(), member.getEmail(), member.getRole());
+            }
+        }
+        return null;


HandlerMethodArgumentResolver 호출 순서가 언제 일까요? 현재 인터셉터 보다 우선적으로 동작할까요 아니면 인터셉터가 동작한 이후에 동작을 할까요?
만약 인터셉터가 동작한 이후에 동작을 한다면 해당 요청에서 존재하지 않는 유저가 반환되는 상황은 없을 것 같네요

저는 이런 상황에서는 null을 반환하기 보다는 존재하지 않은 ANONYMOUS라는 member를 두고 이를 반환해 줍니다.

테코블에 정리된 내용을 발췌해보면
preHandle()이 먼저 작동을 하는군요~
석환님 말씀대로 ANONYMOUS 같은 멤버를 돌려주는 방법을 쓰면 NPE를 줄일 수 있겠군요!

요청이 들어온다.

filter가 작동한다. 이와 관련한 부분은 spring-security에서 자세히 확인할 수 있다.

DispatcherServlet에 전달된다. DispatcherServlet이란, Spring의 핵심 객체로, Client의 요청을 받고 응답을 주기까지의 모든 역할을 담당한다.

DispatcherServlet은 HandlerMapping을 통해 요청을 처리할 Controller를 찾는다.
이 때, Controller를 찾고 Interceptor가 확인할 url과 일치하면 Interceptor의 preHandle이 실행된다.

DispatcherServlet은 Controller를 실행해줄 HandlerAdapter를 찾는다.
이 때, Adapter를 찾고 handle을 실행하기 위해 필요한 파라미터를 생성하기 위해 Resolver는 실행된다.

HandlerAdapter는 Controller를 실행한다.
이 때, Interceptor의 postHandle이 실행된다.

DispatcherServlet은 실행한 결과를 ViewResolver에게 전달한다.

ViewResolver는 View에 전달한다.
이 때, Interceptor의 afterCompletion 실행된다.

DispatcherServletdms View로부터 받은 정보를 Client에 전달한다.

응답을 반환한다.

seokhwan-an · 2025-04-14T08:45:12Z

src/main/java/roomescape/member/MemberService.java

+        Member findMember = memberDao.findByEmailAndPassword(loginRequest.getEmail(), loginRequest.getPassword());
+
+        if (findMember == null) {
+            throw new IllegalArgumentException("Invalid email or password");
+        }


memberDao의 findByEmailAndPassword 메서드에서 member가 존재하지 않는 경우에 null을 반환하는 대신에 Optional을 반환하는 것은 어떤가요? 한번 Optional이 왜 등장했는지 찾아보면 좋을 것 같습니다.

넵 Optional로 적용해보겠습니다!

seokhwan-an · 2025-04-14T08:45:38Z

src/main/java/roomescape/member/MemberService.java

+        Member findMember = memberDao.findById(memberId);
+
+        if (findMember == null) {
+            throw new IllegalArgumentException("Invalid email or password");
+        }


여기도 위와 같은 리뷰입니다!

seokhwan-an · 2025-04-14T08:49:06Z

src/main/java/roomescape/member/TokenProvider.java

+public interface TokenProvider {
+
+    String createToken(Member member);
+
+    Long parse(String token);
+}


이번에 인증 인가를 처리하기 위해서 token을 활용하셨네요
인증 인가에서는 session을 이용할 수 있을 것 같은데 혹시 token을 이용한 인증 인가를 하신 이유가 있으실까요?

엄 미션의 내용이 토큰을 활용하는걸로 보여서 토큰을 활용했습니다!
석환님 말씀대로 세션을 활용해서 인증/인가를 처리할 수 있을거 같습니다 ㅎㅎ

저도 항상 고민하고 있는 부분인데요
토큰의 장점은 stateless하게 서버를 확장해도 인증 인가에 문제가 발생하지 않는다는 것이라고 합니다. 하지만 제가 느끼기에는 토큰의 탈취나 변조 토큰(secretKey는 같고 payload 값만 바뀐 토큰)의 우려로 refreshToken을 도입하고 이를 저장소(redis)에서 관리하는 상황이 발생하다보니 점점 세션과 비슷해진다고 생각이 되는데 석주님의 생각은 어떤거요?

석환님 말씀대로 그런 형태로 가면 레디스에서 토큰을 세션처럼 관리하게 될거 같습니다.
그래도 특정 서버에 종속되는 세션보다는 중앙 레디스에서 토큰이 관리되면 어떤 서버로 요청을 보내도 동일한 응답을 받을 수 있는 stateless한 환경을 구축할 수 있는 점에서 세션방식보다 괜찮은 선택일거 같습니다!
찾아보니 세션도 클러스터링해서 중앙에서 관리할 수 있는거 같긴한데, 석환님의 의견도 궁금합니다! ㅎㅎ

seokhwan-an · 2025-04-14T08:51:04Z

src/main/java/roomescape/reservation/ReservationService.java

+        if (reservationRequest.getName() == null) {
+            reservationRequest = new ReservationRequest(
+                    loginMember.getName(),
+                    reservationRequest.getDate(),
+                    reservationRequest.getTheme(),
+                    reservationRequest.getTime()
+            );
+        }


해당 책임은 서비스에서 처리하기 보다는 ReservationRequest에서 처리하는 것은 어떤 가요?

오호 그게 더 좋겠군요! 한 번 해보겠습니다!

seokhwan-an

석주님 리뷰 반영하신 것 모두 확인했고 답변 달아놓았습니다.
추가로 조금 더 반영하고 이야기 나누고 싶은 부분에 대해서도 코드 리뷰 추가했습니다
천천히 확인해주세요

seokhwan-an · 2025-04-28T01:32:18Z

src/main/java/roomescape/member/MemberController.java

+        Cookie cookie = new Cookie("token", token);
+        response.addCookie(cookie);


cookie 생성시 다양한 설정이 있는데 지금 처럼 cookie를 전달하게 되면 어떤 문제가 있을지 한번 고민해보시면 좋을것 같습니다!

넵 밑에 이미 제공해준 logout 코드를 보니 이런 코드들이 있군요!

cookie.setHttpOnly(true); cookie.setPath("/"); cookie.setMaxAge(0);

저처럼 단순하게 쿠키만 던져주면 유효기간도 없고, 자바스크립트에도 취약하게 되겠군요.
위와 같이 설정해서 자바스크립트 접근 및 유효기간을 설정하여 조금 더 보안에 신경쓰면 좋을거 같습니다!

없이 사용할 때

있이 사용할 때

seokhwan-an · 2025-04-28T01:33:54Z

src/main/java/roomescape/common/interceptor/AuthorizationInterceptor.java

+            return false;
+        }
+
+        if (!member.getRole().equals("ADMIN")) {


member의 상태를 불러와서 비교하는 것보다는 member에게 메세지를 던지는 것은 어떤가요?

메시지 아 자꾸 까먹네요 ㅎㅎ
감사합니다!

seokhwan-an · 2025-04-28T01:36:29Z

src/main/java/roomescape/common/interceptor/AuthorizationInterceptor.java

+    }
+
+    @Override
+    public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) throws Exception {


현재 preHandle에서는 인증 인가 체크와 더불어서 cookie들 중에서 이름이 "token"이 쿠키를 불러오는 책임을 가지고 있는데 이 부분을 분리하는 것은 어떤가요? 쿠키를 찾는 과정을 별도의 메소드로 관리하거나 혹은 cookie를 다루는 컴포넌트를 만드는 방법이 있을 것 같네요!

오우 좋은 의견이네요!
반영해보도록 하겠습니다! :)

seokhwan-an · 2025-04-28T01:37:28Z

src/main/java/roomescape/common/resolver/LoginMemberArgumentResolver.java

+        Cookie[] cookies = request.getCookies();
+        for (Cookie cookie : cookies) {
+            if (cookie.getName().equals("token")) {
+                Member member = authService.loginCheck(cookie.getValue());
+                return new LoginMember(member.getId(), member.getName(), member.getEmail(), member.getRole());
+            }


이 부분도 위의 인터셉터와 비슷하게 "token" 이름을 가지는 cookie를 찾아야 하는 과정이 발생하네요. 중복된 부분을 묶어서 관리하는 것이 좋을 것 같습니다!

seokhwan-an · 2025-04-28T04:34:40Z

src/test/java/roomescape/MissionStepTest.java

+                .cookie("token", brownToken)
+                .get("/admin")
+                .then().log().all()
+                .statusCode(403);


이 부분은 예시 코드라 숫자를 직접 이용하신 것 같은데 HttpStatus를 이용하면 더 가독성이 좋을 것 같아요

setHttpOnly, setPath, setMaxAge

somefood · 2025-05-02T14:52:10Z

HandlerMethodArgumentResolver 호출 순서가 언제 일까요? 현재 인터셉터 보다 우선적으로 동작할까요 아니면 인터셉터가 동작한 이후에 동작을 할까요? 만약 인터셉터가 동작한 이후에 동작을 한다면 해당 요청에서 존재하지 않는 유저가 반환되는 상황은 없을 것 같네요

저는 이런 상황에서는 null을 반환하기 보다는 존재하지 않은 ANONYMOUS라는 member를 두고 이를 반환해 줍니다.

석환님 말씀대로 Resolver에서 null 반환대신 ANONYMOUS를 넣어서 주도록 한 번 해보았습니다!

public static final LoginMember ANONYMOUS = new LoginMember(0L, "", "", "");

위와 같은 형태로 주었는데, 내부 데이터를 어떤 식으로 갖고 있는게 좋을지 궁금하네요 ㅎㅎ

seokhwan-an

석주님 리뷰 반영하신 것 모두 확인했습니다!
이번 단계에서는 충분히 이야기를 나눠본 것 같아서 다음 단계를 진행해보시면 좋을 것 같습니다.

석환님 말씀대로 Resolver에서 null 반환대신 ANONYMOUS를 넣어서 주도록 한 번 해보았습니다!
public static final LoginMember ANONYMOUS = new LoginMember(0L, "", "", "");
위와 같은 형태로 주었는데, 내부 데이터를 어떤 식으로 갖고 있는게 좋을지 궁금하네요 ㅎㅎ

이 부분에 대해서 답변을 드리자면 저도 석주님처럼 비인증 정보를 넘길 때에는 0L을 가지는 객체를 만들어서 처리했습니다. (id는 1L부터 생성되는 것을 이용)

somefood added 4 commits April 12, 2025 22:46

1단계 - 로그인 구현

8562507

2단계 - 로그인 리팩터링

8f90049

2단계 - 로그인 리팩터링 (예약 생성 기능 변경)

136c263

3단계 - 관리자 기능

e6ae0f2

seokhwan-an reviewed Apr 14, 2025

View reviewed changes

somefood added 5 commits April 27, 2025 12:12

메서드 불필요한 어노테이션 제거 및 위치 이동

22efde4

Optional 사용

3cf9d43

ReservationRequest 이름 설정 책임 부여

7418108

MemberService, AuthService 분리

037ae2e

인증/인가 에러 코드 분리

cd7761e

seokhwan-an reviewed Apr 28, 2025

View reviewed changes

somefood added 5 commits May 1, 2025 11:21

member 권한 묻는 메서드 작성

453269b

statusCode enum 사용

45ba6f7

cookie 객체 구체적으로 작성

5c0c15f

setHttpOnly, setPath, setMaxAge

쿠키로부터 토큰 갖고오는 책임 분리

e0ab3b5

ArgumentResolve에서 null 대신 LoginMember.ANONYMOUS 반환토록 설정

ac6e166

seokhwan-an approved these changes May 9, 2025

View reviewed changes

somefood changed the base branch from main to somefood May 9, 2025 07:00

boorownie merged commit fe0744c into next-step:somefood May 14, 2025

		Cookie cookie = new Cookie("token", token);
		response.addCookie(cookie);

[Spring MVC (인증)] 홍석주 미션 제출합니다. #150

[Spring MVC (인증)] 홍석주 미션 제출합니다. #150

Uh oh!

Conversation

somefood commented Apr 13, 2025

Uh oh!

seokhwan-an left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

seokhwan-an Apr 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

seokhwan-an left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

somefood May 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

없이 사용할 때

있이 사용할 때

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

seokhwan-an Apr 28, 2025 •

edited

Loading

somefood May 1, 2025 •

edited

Loading

seokhwan-an left a comment •

edited

Loading