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 (인증)] 이준민 미션 제출합니다. #100

Open
wants to merge 3 commits into
base: jermany17
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
55 changes: 55 additions & 0 deletions src/main/java/roomescape/AdminInterceptor.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package roomescape;

import io.jsonwebtoken.Jwts;
import io.jsonwebtoken.security.Keys;
import jakarta.servlet.http.Cookie;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import org.springframework.stereotype.Component;
import org.springframework.web.servlet.HandlerInterceptor;
import roomescape.member.MemberService;

@Component
public class AdminInterceptor implements HandlerInterceptor {
private final String SECRET_KEY = "Yn2kjibddFAWtnPJ2AFlL8WXmohJMCvigQggaEypa5E=";

Choose a reason for hiding this comment

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

[Request Change]

이 시크릿 키로 jwt 토큰의 서명정보를 입력하였군요!
이 시크릿 키를 모르면 임의로 jwt를 만들 수가 없겠네요!!
이 파일 line30에서 디코딩되는 토큰은 이 시크릿키를 알고 만들었다는 확실한 믿음이 생기네요.

시크릿키의 일치를 통해 토큰의 신뢰를 판단하는 만큼 해당 정보는 매우 중요한 정보입니다.
다른 멤버들의 PR을 둘러보시면, properties 파일에 시크릿 키를 선언해두고
실제 코드에는 @Value, @ConfigurationProperties 같은 어노테이션을 이용해 빈에 주입받아 사용하는 경우가 많습니다.

그렇게 하는 이유는 여러가지가 있겠죠.

  • 이 파일이 공개된 곳에 올라오면 안된다
    • 이미 준민님의 방탈출 어플 인증은 우리 모두가 다 뚫을 수 있게 됐네요.
  • 프로덕트 코드에 클래스 변수는 변경하기 쉽다.
    • 준민님과 같이 협업하는 루카라는 개발자가 이 코드를 실수로 이코드를 바꿔버리면 심각한 버그와 보안상 문제가 생기겠어요.

이러한 이유들로 따로 변수로 관리하는 경우가 많은데요.
그렇다면 그런 궁금증이 있을 수 있겠네요.

Q) properties 파일도 깃헙에 올라오는거 아닌가요?

A) 이는 여러 방식으로 특정 properties파일을 공개 레포에서 숨기거나 할 수 있습니다. 프로젝트 내부에서 설정해주지 않고 빌드, 배포 스크립트에서 주입해도 되겠죠.

방법적인 부분은 추후에 고민해도 좋을 것 같아요. 다만, 로직적으로 관리할 영역과 프로젝트 설정 영역에 쓰는 변수들을 구분해서 관리하는 습관을 가져보면 좋겠어요.

Copy link
Author

Choose a reason for hiding this comment

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

넵 감사합니다. 더 공부해보고 고민해보겠습니다.

private final MemberService memberService;

public AdminInterceptor(MemberService memberService) {
this.memberService = memberService;
}

@Override
public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) throws Exception {
String token = extractTokenFromCookie(request.getCookies());
if (token.isEmpty()) {
response.setStatus(401); // Unauthorized
return false;
}

// JWT 토큰에서 사용자 정보 추출
String role = Jwts.parserBuilder()
.setSigningKey(Keys.hmacShaKeyFor(SECRET_KEY.getBytes()))
.build()
.parseClaimsJws(token)
.getBody()
.get("role", String.class);

if (role == null || !role.equals("ADMIN")) {

Choose a reason for hiding this comment

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

[Request Change]

누가 토큰을 만들 때 role을 admin이라고 소문자로 입력하면 실패하겠군요.
role 같은 중요한 정보를 String 같은 자유로운 타입에 담는 것은 여러 문제를 야기할 수 있겠네요.
enum을 활용해서 바꿔볼까요?

response.setStatus(401); // Unauthorized
return false;
}

return true; // ADMIN 권한이 있는 경우만 true 반환
}

private String extractTokenFromCookie(Cookie[] cookies) {
if (cookies != null) {
for (Cookie cookie : cookies) {
if (cookie.getName().equals("token")) {
return cookie.getValue();
}
}
}
return "";
}
}
31 changes: 31 additions & 0 deletions src/main/java/roomescape/WebConfig.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package roomescape;

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 roomescape.member.LoginMemberArgumentResolver;

import java.util.List;

@Configuration
public class WebConfig implements WebMvcConfigurer {
private final LoginMemberArgumentResolver loginMemberArgumentResolver;
private final AdminInterceptor adminInterceptor;

public WebConfig(AdminInterceptor adminInterceptor, LoginMemberArgumentResolver loginMemberArgumentResolver) {
this.loginMemberArgumentResolver = loginMemberArgumentResolver;
this.adminInterceptor = adminInterceptor;
}

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

@Override
public void addInterceptors(InterceptorRegistry registry) {
registry.addInterceptor(adminInterceptor)
.addPathPatterns("/admin"); // /admin 경로에만 적용
}
Comment on lines +26 to +30

Choose a reason for hiding this comment

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

[Request Change]

엥?
image
저 로그인 않안했는데 /admin/reservation 페이지에 진입했어요...
보안이 뚤렸네요!!

Path를 설정할 때, 접근하려는 리소스를 계층적으로 나타내는 경우가 많습니다.
이 경우에도

  • /admin => 어드민 페이지
  • /admin/reservation => 어드민 페이지의 예약 관리
  • /admin/theme => 어드민 페이지의 테마 관리
  • /admin/time => 어드민 페이지의 시간 관리

이런식으로 맨 첫 계층에는 /admin으로 어드민 페이지라는 리소스를 정의했고
그 뒤로 각 기능 페이지에 대해서 정의하고 있군요.
그렇다면 어드민 페이지의 기능에 접근 제한을 생각하면, 저 페이지들을 모두 제한해야겠군요. (그리고 앞으로 생길 수 잇는 어드민 기능에 대해서두요)

그렇게 하기 위해서 스프링에서는 패스를 패턴으로 등록하는 것이 일반적입니다.

}
31 changes: 31 additions & 0 deletions src/main/java/roomescape/member/LoginMember.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package roomescape.member;

public class LoginMember {
private Long id;
private String name;
private String email;
private String role;

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

public Long getId() {
return id;
}

public String getName() {
return name;
}

public String getEmail() {
return email;
}

public String getRole() {
return role;
}
}
58 changes: 58 additions & 0 deletions src/main/java/roomescape/member/LoginMemberArgumentResolver.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
package roomescape.member;

import io.jsonwebtoken.Jwts;
import io.jsonwebtoken.security.Keys;
import jakarta.servlet.http.Cookie;
import jakarta.servlet.http.HttpServletRequest;
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.method.support.HandlerMethodArgumentResolver;
import org.springframework.web.method.support.ModelAndViewContainer;

@Component
public class LoginMemberArgumentResolver implements HandlerMethodArgumentResolver {
private final String secretKey = "Yn2kjibddFAWtnPJ2AFlL8WXmohJMCvigQggaEypa5E=";
private final MemberService memberService;

public LoginMemberArgumentResolver(MemberService memberService) {
this.memberService = memberService;
}

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

@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());
Comment on lines +37 to +42

Choose a reason for hiding this comment

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

[Request Change]

JWT에서 사용자 인증 정보를 추출하는 과정이 여러곳에 존재할 수 있는데요.
만약 이 로직이 변경된다거나, 인증 정보를 담는 방식을 JWT를 하지 않으면 이 부분이 변경되겠네요.
위의 변경사항이 ArgumentResolver와 Interceptor 둘다 영향을 받는다니,
저희가 지금까지 챙긴 객체지향이 깨지는 느낌이 드네요.

  • ArgumentResolver와 Interceptor는 요청 응답 흐름에 있어 인증 정보를 적절한 시기에 로직을 처리하기 위해서 존재하는 Spring의 기능이라고 생각할 수 있겠죠.

  • Jwt는 요청 응답 흐름 보다는 인증정보를 담기위한 토큰 방식이라고 볼 수 있습니다.

이 두가지 역할을 확실히 구분하고,
중복을 없애,
유연한 프로그램을 짜면 좋겠어요.


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 "";
}
Comment on lines +28 to +57

Choose a reason for hiding this comment

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

[Request Change]

인증 정보를 추출하는 곳이 ArgumentResolver와 Interceptor 두군데가 있는데요.
각각 인증정보가 옳지 않을 때 이렇게 처리하고 있어요.

  • Interceptor
    • 응답에 상태코드를 401로 주고, false를 반환해서 컨트롤러로 진입하지 않고 바로 응답하도록 한다
  • ArgumentResolver
    • 해당 아규먼트를 null로 내려준다. 컨트롤러에서 NPE가 터지도록...

[고민 1] 두 방식을 비교하며 ArgumentResolver에서 인증정보가 옳지 않을 때 어떤 문제가 있을지?

컨트롤러에서 LoginMember를 argument로 받겠다는것은 인증정보가 꼭 필요하다는 것입니다.
근데, 해당 정보가 없다는 것은 부적절한 접근이에요.
하지만 그 상황에서 NPE 같은 일반적인 예외가 터지면, 아무도 뭐가 문젠지 파악하기 어려워요.

[고민 2] 과연 인증 정보가 옳지 않은 모든 상황을 커버하고 있는가?

제가 생각했을 땐, 인증정보가 옳지 않다고 생각할만한 지점은 여러가지 있어요.

  • 토큰을 담은 쿠키가 없을 때 (만료되었을 때)
  • 토큰이 디코딩이 안될 때 (시크릿키가 불일치하거나 만료되었거나)
  • 인증 정보에 있는 회원 정보가 현재 DB에 존재하지 않을 때
  • ...

이런 여러 상황에서 발생할 수 있는 문제들(혹은 예외)을 어떻게 하면 잘 관리해서, 인증 실패라고 잘 클라이언트에게 전달할 수 있을까요?

}
59 changes: 59 additions & 0 deletions src/main/java/roomescape/member/MemberController.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package roomescape.member;

import io.jsonwebtoken.Jwts;
import io.jsonwebtoken.security.Keys;
import jakarta.servlet.http.Cookie;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
Expand All @@ -14,6 +16,7 @@
@RestController
public class MemberController {
private MemberService memberService;
private final String secretKey = "Yn2kjibddFAWtnPJ2AFlL8WXmohJMCvigQggaEypa5E=";

public MemberController(MemberService memberService) {
this.memberService = memberService;
Expand All @@ -34,4 +37,60 @@ public ResponseEntity logout(HttpServletResponse response) {
response.addCookie(cookie);
return ResponseEntity.ok().build();
}

@PostMapping("/login")
public ResponseEntity<Void> login(@RequestBody MemberRequest memberRequest, HttpServletResponse response) {
try {
Member member = memberService.findByEmailAndPassword(memberRequest.getEmail(), memberRequest.getPassword());

// JWT 토큰 생성
String token = Jwts.builder()
.setSubject(member.getId().toString())
.claim("name", member.getName())
.claim("role", member.getRole())
.signWith(Keys.hmacShaKeyFor(secretKey.getBytes()))
.compact();

// 쿠키 생성
Cookie cookie = new Cookie("token", token);
cookie.setHttpOnly(true);
cookie.setPath("/");
response.addCookie(cookie);

return ResponseEntity.ok().build();
} catch (Exception e) {
return ResponseEntity.status(401).build(); // Unauthorized
}
}

@GetMapping("/login/check")
public ResponseEntity<MemberResponse> checkLogin(HttpServletRequest request) {
String token = extractTokenFromCookie(request.getCookies());
if (token.isEmpty()) {
return ResponseEntity.status(401).build(); // Unauthorized
}

// JWT 토큰에서 사용자 정보 추출
Long memberId = Long.valueOf(Jwts.parserBuilder()
.setSigningKey(Keys.hmacShaKeyFor(secretKey.getBytes()))
.build()
.parseClaimsJws(token)
.getBody()
.getSubject());

// ID로 사용자 조회
Member member = memberService.findById(memberId);
return ResponseEntity.ok(new MemberResponse(member.getId(), member.getName(), member.getEmail()));
}

private String extractTokenFromCookie(Cookie[] cookies) {
if (cookies != null) {
for (Cookie cookie : cookies) {
if (cookie.getName().equals("token")) {
return cookie.getValue();
}
}
}
return "";
}
}
13 changes: 13 additions & 0 deletions src/main/java/roomescape/member/MemberDao.java
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,17 @@ public Member findByName(String name) {
name
);
}

public Member findById(Long id) {
return jdbcTemplate.queryForObject(
"SELECT id, name, email, role FROM member WHERE id = ?",
(rs, rowNum) -> new Member(
rs.getLong("id"),
rs.getString("name"),
rs.getString("email"),
rs.getString("role")
),
id
);
}
}
8 changes: 8 additions & 0 deletions src/main/java/roomescape/member/MemberService.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,12 @@ public MemberResponse createMember(MemberRequest memberRequest) {
Member member = memberDao.save(new Member(memberRequest.getName(), memberRequest.getEmail(), memberRequest.getPassword(), "USER"));
return new MemberResponse(member.getId(), member.getName(), member.getEmail());
}

public Member findByEmailAndPassword(String email, String password) {
return memberDao.findByEmailAndPassword(email, password);
}

public Member findById(Long id) {
return memberDao.findById(id);
}
}
12 changes: 5 additions & 7 deletions src/main/java/roomescape/reservation/ReservationController.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RestController;
import roomescape.member.LoginMember;

import java.net.URI;
import java.util.List;
Expand All @@ -26,15 +27,12 @@ public List<ReservationResponse> list() {
}

@PostMapping("/reservations")
public ResponseEntity create(@RequestBody ReservationRequest reservationRequest) {
if (reservationRequest.getName() == null
|| reservationRequest.getDate() == null
|| reservationRequest.getTheme() == null
|| reservationRequest.getTime() == null) {
return ResponseEntity.badRequest().build();
public ResponseEntity create(@RequestBody ReservationRequest reservationRequest, LoginMember member) {
if (reservationRequest.getName() == null) {
reservationRequest.setName(member.getName());
}
ReservationResponse reservation = reservationService.save(reservationRequest);

ReservationResponse reservation = reservationService.save(reservationRequest);
return ResponseEntity.created(URI.create("/reservations/" + reservation.getId())).body(reservation);
}

Expand Down
2 changes: 2 additions & 0 deletions src/main/java/roomescape/reservation/ReservationRequest.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ public String getName() {
return name;
}

public void setName(String name) { this.name = name; }

public String getDate() {
return date;
}
Expand Down
Loading