Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Spring MVC(인증)] 정상희 미션 제출합니다. #103

Open
wants to merge 13 commits into
base: sangheejeong
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
# 🌀 Spring MVC (인증)

# 1단계
___
## 로그인
#### 로그인 페이지
+ 이메일, 비밀번호 입력
#### 로그인 요청
+ 이메일, 비밀번호 -> 멤버 조회
+ 조회한 멤버로 토큰 발급
+ Cookie를 만들어 응답
#### 인증 정보 조회
+ Cookie -> 토큰 정보 추출
+ 멤버를 찾아서 응답

로그인 관련 API
+ GET/login : 로그인 페이지 호출
+ POST/login : 로그인 요청
+ GET/login/check : 인증 정보 조회

# 2단계
___
## 로그인 리팩터링
#### HandlerMethodArgumentResolver
+ 컨트롤러 메서드 파라미터로 자동 주입

## 예약 생성 기능 변경
+ 예약 : ReservationRequest(요청 DTO)
-> name이 있으면 name으로 Member 찾기
-> name이 없으면 Cookie에 담긴 정보 활용

# 3단계
___
## 관리자 기능
+ admin 페이지 진입 (HandlerInterceptor 이용)
-> 관리자 : 진입 가능
-> 관리자 X : 401 코드 응답
Comment on lines +1 to +37

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Approve]

깔끔한 요구사항 정리 감사합니다.

5 changes: 5 additions & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ dependencies {
implementation 'io.jsonwebtoken:jjwt-impl:0.11.2'
implementation 'io.jsonwebtoken:jjwt-gson:0.11.2'

compileOnly 'org.projectlombok:lombok'
annotationProcessor 'org.projectlombok:lombok'
testCompileOnly 'org.projectlombok:lombok'
testAnnotationProcessor 'org.projectlombok:lombok'

testImplementation 'org.springframework.boot:spring-boot-starter-test'
testImplementation 'io.rest-assured:rest-assured:5.3.1'

Expand Down
7 changes: 7 additions & 0 deletions src/main/java/roomescape/auth/AuthClaims.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package roomescape.auth;

public record AuthClaims(
String name,
String role
) {
}
Comment on lines +1 to +7

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Comment]

🤔 AuthClaims는 어떤 역할을 하는 객체인가요?

이름으로 유추해봤을 땐, token payload에 들어갈 인증 정보로 판단이 되는데요.

지금 해당 객체가 사용되는 목적은 3가지 정도로 보여집니다.

  1. GET /login/check 로그인 정보 조회 API 응답하는 객체
  2. POST /reservations 쿠키의 토큰에 있는 회원 정보를 담는 객체
  3. class JwtUtils 토큰에서 꺼낸 내용을 래핑하는 객체

각 역할이 충돌하는 바가 있다고 봐요.

2, 3번은 서버에서 회원정보를 파악하기 위해 만든 것입니다.
1번은 요구사항에도 있듯이 회원정보를 띄워주기 위해 존재하는 조금 더 클라이언트가 원하는 정보입니다.

저는 1, 2, 3이 각각 다른 목적으로 사용된다고 보여져요. (최소한 1번과 3번은 달라야한다고 생각합니다.)

이 서로 다른 상황에서 같은 객체로 해결하려 했을 때 어떤 점이 문제가 생길지 고려해보면 좋겠습니다.

중복을 줄이는 것은 개발자라면 누구나 느끼는 마음일텐데요.
SOLID 원칙의 SRP 같은 내용이나, 클린 아키텍처의 독립성(우발적 중복)같은 키워드를 살펴보시면서, 필드가 유사한 객체를 어디까지 분리할지 고려해보면 좋겠어요.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

미션을 진행할 때 1번에서 사용할 때 name만 반환하면 되는데 AuthClaims를 모두 반환하는 것이 맞는지에 대해 살짝 고민을 하긴 했었는데요 ㅎㅎ.. 이렇게 사용되는 목적을 직접 보니까 확실히 사용되는 부분이 다른 것이 느껴졌습니다.

사실 지금 JPA 미션 도중에 이 리뷰를 보러 왔는데 JPA 미션이 MVC 미션에서 변경을 하면서 진행을 하는 과정이다보니까 이전 미션의 코드에 영향을 받을 수 밖에 없더라구요..
요구사항이 변경되었을 때 AuthClaims의 경우 다양한 목적을 가진 DTO라서 하나를 변경하면 여러 곳에서 수정을 해야하는 상황이 발생한다는 것을 깨닫고 루카님의 리뷰가 이 지점에서 있었던 것 같아서 다시 읽으러 왔습니다,, ㅎ
중복을 피하려고만 하다가 미션 도중에 직접 문제를 겪어보니 확실히 알겠네요 !

빠른 시일 내에 수정해서 올리겠습니다.

40 changes: 40 additions & 0 deletions src/main/java/roomescape/auth/AuthClaimsArgumentResolver.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package roomescape.auth;

import jakarta.servlet.http.Cookie;
import jakarta.servlet.http.HttpServletRequest;
import lombok.RequiredArgsConstructor;
import org.springframework.core.MethodParameter;
import org.springframework.stereotype.Component;
import org.springframework.web.bind.support.WebDataBinderFactory;
import org.springframework.web.context.request.NativeWebRequest;
import org.springframework.web.context.request.ServletWebRequest;
import org.springframework.web.method.support.HandlerMethodArgumentResolver;
import org.springframework.web.method.support.ModelAndViewContainer;
import roomescape.member.MemberService;

import java.util.Arrays;

@Component
@RequiredArgsConstructor
public class AuthClaimsArgumentResolver implements HandlerMethodArgumentResolver {

private final MemberService memberService;

@Override
public boolean supportsParameter(MethodParameter parameter) {
return parameter.getParameterType().equals(AuthClaims.class);
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Approve]

Spring을 학습하며 미션에 적용하는게 쉽지 않으실텐데 이렇게 특정 타입의 Argument를 처리할 수 있도록 등록하시다니 대단합니다.

이 방법도 좋은 방법이라고 생각하는데요. 보편적인 방법?전략?을 설명 드리면 ArgumentResolver를 등록할 때 커스텀 어노테이션을 정의하고 해당 어노테이션이 있는지 없는지 체크하는 경우가 많아요.

제 생각엔 특정 객체로 일치를 시키면 해당 객체는 컨트롤러에 아규먼트로 오게되면 그 리졸버를 무조건 타게 되겠지만,
근데 어노테이션으로 목적을 더 명식적으로 표현해서 의도에 더 맞게 사용할 수 있는 것이 장점 같아요.

저희가 애용하는 @PathVariable, @RequestBody 같은 아이들도 스프링에서 기본적으로 등록한 ArgumentResolver에 의해서 매개변수로 들어올 수 있답니다.🥰

Suggested change
@Override
public boolean supportsParameter(MethodParameter parameter) {
return parameter.getParameterType().equals(AuthClaims.class);
}
@Override
public boolean supportsParameter(MethodParameter parameter) {
return parameter.hasParameterAnnotation(�AuthCustomAnnotation.class) &&
parameter.getParameterType().equals(AuthClaims.class);
}


@Override
public Object resolveArgument(MethodParameter parameter, ModelAndViewContainer mavContainer, NativeWebRequest webRequest, WebDataBinderFactory binderFactory) throws Exception {
HttpServletRequest request = ((ServletWebRequest) webRequest).getRequest();
String token = Arrays.stream(request.getCookies())
.filter(cookie -> "token".equals(cookie.getName()))
.findFirst()
.map(Cookie::getValue)
.orElseThrow(() -> new IllegalArgumentException("토큰을 찾을 수 없습니다."));

return memberService.checkLogin(new AuthToken(token));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Comment]

HttpRequest -> Cookie -> Jwt -> 인증정보 이렇게 꺼내는 동작이 AutInterceptor에도 동일하게 존재하는데요.
이런 중복은 한번 줄여볼 수 있지 않을까요?

}
}

34 changes: 34 additions & 0 deletions src/main/java/roomescape/auth/AuthRoleInterceptor.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package roomescape.auth;

import jakarta.servlet.http.Cookie;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Component;
import org.springframework.web.servlet.HandlerInterceptor;
import roomescape.member.MemberService;

import java.util.Arrays;

@Component
@RequiredArgsConstructor
public class AuthRoleInterceptor implements HandlerInterceptor {
private final MemberService memberService;

@Override
public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) throws Exception {
String token = Arrays.stream(request.getCookies())
.filter(cookie -> "token".equals(cookie.getName()))
.findFirst()
.map(Cookie::getValue)
.orElseThrow(() -> new IllegalArgumentException("토큰을 찾을 수 없습니다."));

AuthClaims userClaims = memberService.checkLogin(new AuthToken(token));
if (!userClaims.role().equals("ADMIN")) {
response.setStatus(401);
return false;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Request Change]

Oh NO...
만약 토큰을 생성할 때, 실수로 "admin"이라고 소문자로 작성하면 인증이 실패하겠어요...
과연 role이 string 타입이여도 괜찮을까요...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오 찾아보니까 말씀해주신 문제(ex : 역할 변경 request에서 잘못된 역할값이 들어왔을 때)를 방지하기 위해서 enum 타입을 활용하는 방법이 있네요


return true;
}
}
6 changes: 6 additions & 0 deletions src/main/java/roomescape/auth/AuthToken.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package roomescape.auth;

public record AuthToken(
String token
) {
}
27 changes: 27 additions & 0 deletions src/main/java/roomescape/auth/AuthWebConfig.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package roomescape.auth;

import lombok.RequiredArgsConstructor;
import org.springframework.context.annotation.Configuration;
import org.springframework.web.method.support.HandlerMethodArgumentResolver;
import org.springframework.web.servlet.config.annotation.InterceptorRegistry;
import org.springframework.web.servlet.config.annotation.WebMvcConfigurer;

import java.util.List;

@Configuration
@RequiredArgsConstructor
public class AuthWebConfig implements WebMvcConfigurer {

private final AuthClaimsArgumentResolver loginMemberArgumentResolver;
private final AuthRoleInterceptor roleCheckInterceptor;

@Override
public void addArgumentResolvers(List<HandlerMethodArgumentResolver> resolvers) {
resolvers.add(loginMemberArgumentResolver);
}

@Override
public void addInterceptors(InterceptorRegistry registry) {
registry.addInterceptor(roleCheckInterceptor).addPathPatterns("/admin/**");
}
}
36 changes: 36 additions & 0 deletions src/main/java/roomescape/auth/JWTUtils.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package roomescape.auth;

import io.jsonwebtoken.Claims;
import io.jsonwebtoken.Jwts;
import io.jsonwebtoken.security.Keys;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.stereotype.Component;
import roomescape.member.Member;

@Component
public class JWTUtils {

@Value("${roomescape.auth.jwt.secret}")
private String secretKey;

public AuthToken createToken(Member member) {
String accessToken = Jwts.builder()
.setSubject(member.getId().toString())
.claim("name", member.getName())
.claim("role", member.getRole())
.signWith(Keys.hmacShaKeyFor(secretKey.getBytes()))
.compact();
Comment on lines +24 to +29

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Comment]

JWT 토큰을 인코딩할 때 설정하는 정보들은 보안에 매우 중요하다고 볼 수 있겠죠.
조금 더 고민해보면 좋을 것 같아요.

  1. claims 인증 정보의 어떤것을 넣어야할까? (해당 정보를 활용하는 주체가 누군지를 파악하고 고민해보면 좋겠어요.)
  2. token 만료 시간은 얼마나 설정해야할까? (지금 이 토큰은 만료시간이 어떻게 되지? 만약 그렇다면 어떤 위험성이 있지?)
  3. 만료된 토큰으로 인가 요청이 오면 어떻게 해야될까? (만료된 토큰은 비정상 접근일까? 이고민은 코드 반영은 말고 생각만 해보죠)
  4. subject에는 어떤 내용이 들어가야 자연스러운지? (중요하지 않음)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. 현재 claims는 서버가 인증 및 권한 검증을 위해 사용하고 있습니다. 인증을 위해서는 토큰 발급 시간과 만료 시간이 추가되면 좋을 것 같네요! 그리고 claims는 최소한의 정보만 넣는 것이 중요하므로 i사용자의 id만 포함하는 것이 보안상 맞는 것 같습니다.
    (사실 저는 미션에서 제공한 견본 코드대로 작성을 했는데요, 미션을 진행하다보니 name으로 찾으라는 요구사항이 많아서 견본 코드를 저렇게 제공해준 것 같아요. 약간 미션에서는 name이 id 같은 역할을 하는 것처럼 요구사항들이 주어진 것 같습니다.)

  2. 현재는 만료 시간이 설정되지 않았는데 이 경우 만약 토큰이 탈취가 되어도 평생 사용된다는 문제점이 있습니다.
    찾아보니까 Access 랑 Refresh 두 가지 토큰을 사용하여 이 문제를 해결할 수 있는 것 같습니다!

보통의 경우
Access Token 만료기간 : 30분 ~ 1시간
Refresh Token 만료기간 : 3일 ~ 1달으로 설정하는 듯 하다.

  1. 401 Unauthorized 응답을 보내는 것이 맞는 것 같습니다.
    또한 만료된 토큰은 세션 만료 상태에서 요청된 상황 (정상), 탈취된 상황 (비정상)
    두 경우 모두 가능하므로 모두 고려하여 대응해야 합니다.

  2. subject는 토큰의 주체를 나타내므로 사용자의 고유 식별자( id, email ) 등을 넣으면 좋을 것 같습니다.


return new AuthToken(accessToken);
}

public AuthClaims getClaimsFromToken(String token) {
Claims claims = Jwts.parserBuilder()
.setSigningKey(Keys.hmacShaKeyFor(secretKey.getBytes()))
.build()
.parseClaimsJws(token)
.getBody();

return new AuthClaims(claims.get("name", String.class), claims.get("role", String.class));
}
}
7 changes: 7 additions & 0 deletions src/main/java/roomescape/member/LoginRequest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package roomescape.member;

public record LoginRequest(
String email,
String password
) {
}
28 changes: 21 additions & 7 deletions src/main/java/roomescape/member/MemberController.java
Original file line number Diff line number Diff line change
@@ -1,28 +1,42 @@
package roomescape.member;

import jakarta.servlet.http.Cookie;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import lombok.RequiredArgsConstructor;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.CookieValue;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RestController;
import roomescape.auth.AuthClaims;
import roomescape.auth.AuthToken;

import java.net.URI;

@RestController
@RequiredArgsConstructor
public class MemberController {
private MemberService memberService;

public MemberController(MemberService memberService) {
this.memberService = memberService;
}
private final MemberService memberService;

@PostMapping("/members")
public ResponseEntity createMember(@RequestBody MemberRequest memberRequest) {
MemberResponse member = memberService.createMember(memberRequest);
return ResponseEntity.created(URI.create("/members/" + member.getId())).body(member);
return ResponseEntity.created(URI.create("/members/" + member.id())).body(member);
}

@PostMapping("/login")
public void login(@RequestBody LoginRequest loginRequest, HttpServletResponse response) {
String token = memberService.getToken(loginRequest).token();
Cookie cookie = new Cookie("token", token);
cookie.setHttpOnly(true);
cookie.setPath("/");
response.addCookie(cookie);
Comment on lines +29 to +33

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Approve]

이번 미션에서 아주 중요한 내용이라고 할 수 있겠죠.

쿠키가 무엇인지,
각각 옵션들이 갖는 의미가 무엇인지 한번 살펴보면 좋겠어요.

쿠키에 대해서 학습하시고, 각 옵션을 설정한 이유에 대해서 저한테 한번 설명해주시면 좋을 것 같아요!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

쿠키 :
사용자의 브라우저(로컬 환경)에 저장되는 데이터
ex) 로그인 상태, 사용자 설정, 장바구니 등을 저장하여 다음 요청에서도 이를 활용 가능

사용 이유 :
HTTP의 속성 중 비연결성, stateless를 해결하기 위해서 사용
-> HTTP 통신 간에 유지하려는 정보가 있는 경우 사용

31줄 : 쿠키 생성 key, value값 -> "token", 생성한 token값
32줄 : HTTP만 -> 브라우저(javascript) 접근을 차단함으로써 쿠키에 접근할 수 없도록 제한하는 역할
33줄 : 경로설정을 통해서 설정된 경로와 하위 경로에 대해서만 쿠키에 접근할 수 있게 함
34줄 : HttpServletResponse 객체인 response 즉, 응답에 쿠키를 넣어서 전송

오 사실 견본 코드의 내용이라서 한 번 검색만 해보고 넘겼었는데 이렇게 다시 정리하고 쿠키에 대해서 좀 더 자세히 찾아보니까 사용 목적에 대해 명확하게 알게 된 것 같아서 좋네요!

}

@GetMapping("/login/check")
public AuthClaims checkLogin(@CookieValue("token") String token) {
return memberService.checkLogin(new AuthToken(token));
}

@PostMapping("/logout")
Expand Down
21 changes: 6 additions & 15 deletions src/main/java/roomescape/member/MemberRequest.java
Original file line number Diff line number Diff line change
@@ -1,19 +1,10 @@
package roomescape.member;

public class MemberRequest {
private String name;
private String email;
private String password;

public String getName() {
return name;
}

public String getEmail() {
return email;
}

public String getPassword() {
return password;
}
public record MemberRequest(
String name,
String email,
String password
) {
}

27 changes: 5 additions & 22 deletions src/main/java/roomescape/member/MemberResponse.java
Original file line number Diff line number Diff line change
@@ -1,25 +1,8 @@
package roomescape.member;

public class MemberResponse {
private Long id;
private String name;
private String email;

public MemberResponse(Long id, String name, String email) {
this.id = id;
this.name = name;
this.email = email;
}

public Long getId() {
return id;
}

public String getName() {
return name;
}

public String getEmail() {
return email;
}
public record MemberResponse(
Long id,
String name,
String email
) {
}
34 changes: 28 additions & 6 deletions src/main/java/roomescape/member/MemberService.java
Original file line number Diff line number Diff line change
@@ -1,17 +1,39 @@
package roomescape.member;

import lombok.RequiredArgsConstructor;
import org.springframework.dao.EmptyResultDataAccessException;
import org.springframework.stereotype.Service;
import roomescape.auth.AuthClaims;
import roomescape.auth.AuthToken;
import roomescape.auth.JWTUtils;

@Service
@RequiredArgsConstructor
public class MemberService {
private MemberDao memberDao;

public MemberService(MemberDao memberDao) {
this.memberDao = memberDao;
}
private final MemberDao memberDao;
private final JWTUtils jwtUtils;

public MemberResponse createMember(MemberRequest memberRequest) {
Member member = memberDao.save(new Member(memberRequest.getName(), memberRequest.getEmail(), memberRequest.getPassword(), "USER"));
Member member = memberDao.save(new Member(memberRequest.name(), memberRequest.email(), memberRequest.password(), "USER"));
return new MemberResponse(member.getId(), member.getName(), member.getEmail());
}

public AuthToken getToken(LoginRequest loginRequest) {
try {
Member member = memberDao.findByEmailAndPassword(loginRequest.email(), loginRequest.password());
return jwtUtils.createToken(member);
} catch (EmptyResultDataAccessException e) {
throw new IllegalArgumentException("로그인 정보가 잘못되었습니다.");
}
}

public AuthClaims checkLogin(AuthToken userToken) {
try {
AuthClaims userClaims = jwtUtils.getClaimsFromToken(userToken.token());
memberDao.findByName(userClaims.name());
return userClaims;
} catch (EmptyResultDataAccessException exception) {
throw new IllegalArgumentException("존재하지 않는 회원입니다.");
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Request Change]

이 두 로직에서 핸들링 되는 예외 상황은 아래와 같습니다.

  1. 로그인 실패 아이디나 비밀번호로 회원 조회가 안될 때
  2. 없는 회원의 인증정보 (jwt에서 내용이 잘 나왔다는 가정하에) 토큰에 있는 인증 정보로 회원 조회가 안될 때

IllegalArgumentException을 뱉어내고, 400 BAD REQUEST로 응답이 갈 것으로 보여집니다.


두가지 고민 포인트가 있겠어요.

  1. 저는 이 값이 처리할 수 없는 형식이라기보단, 부적절한 접근 시도, 혹은 로그인 실패 같은 인증과 관련된 예외상황이라고 생각해요. 그렇다면 어떤 응답이 적절할까요? 그리고 이런 인증관련된 예외는 이곳에서 말고 여러영역(예를 들면 인터셉터, 아규먼트리졸버, ...)에서 발생할 수 있는데요. 이런 아이들을 어떻게 하면 효율적으로 예외 핸들링 할 수 있을까요?
  2. 위의 상황 2번에서 토큰에서 내용이 잘 나왔다고 가정했지만, 서명정보가 불일치하거나 만료되거나 하였으면 예외를 발생할 수 있겠어요. 그 예외에 대해서는 지금 핸들링되고 있지 않아요.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. exceptionController를 활용하면 핸들링할 수 있을 것 같습니다.
    인터셉터, 아규먼트 리졸버에서 발생한 예외 -> 컨트롤러 전달 -> @ControllerAdvice로 처리

  2. 넵 처리 완료했습니다!

}
Loading