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

[Feature] - 인가 구현 #79

Merged
merged 9 commits into from
Jul 30, 2024
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
package woowacourse.touroot.authentication.infrastructure;

import io.jsonwebtoken.ExpiredJwtException;
import io.jsonwebtoken.Jwts;
import io.jsonwebtoken.security.Keys;
import java.util.Date;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.stereotype.Component;
import woowacourse.touroot.global.exception.UnauthorizedException;
import woowacourse.touroot.member.domain.Member;

import java.nio.charset.StandardCharsets;
import java.util.Date;

@Component
public class JwtTokenProvider {

Expand Down Expand Up @@ -34,4 +38,20 @@ public String createToken(Member member) {
.signWith(Keys.hmacShaKeyFor(secretKey.getBytes()))
.compact();
}

public String decode(String token) {
try {
return Jwts.parserBuilder()
.setSigningKey(secretKey.getBytes(StandardCharsets.UTF_8))
.build()
.parseClaimsJws(token)
.getBody()
.get(MEMBER_ID_KEY)
.toString();
} catch (ExpiredJwtException exception) {
throw new UnauthorizedException("이미 만료된 토큰입니다.", exception.getCause());
} catch (Exception exception) {
throw new UnauthorizedException("유효하지 않은 토큰입니다.", exception.getCause());
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
package woowacourse.touroot.global.auth;

import com.fasterxml.jackson.databind.ObjectMapper;
import jakarta.servlet.FilterChain;
import jakarta.servlet.ServletException;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpMethod;
import org.springframework.http.HttpStatus;
import org.springframework.stereotype.Component;
import org.springframework.util.AntPathMatcher;
import org.springframework.web.filter.OncePerRequestFilter;
import woowacourse.touroot.authentication.infrastructure.JwtTokenProvider;
import woowacourse.touroot.global.auth.dto.HttpRequestInfo;
import woowacourse.touroot.global.exception.dto.ExceptionResponse;

import java.io.IOException;
import java.util.List;

@RequiredArgsConstructor
@Slf4j
@Component
public class JwtAuthFilter extends OncePerRequestFilter {

private final ObjectMapper objectMapper;
private final JwtTokenProvider tokenProvider;
Comment on lines +23 to +29
Copy link

Choose a reason for hiding this comment

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

필터를 운용할 필요가 있나 싶습니다.

ArgumentResolver를 운용하는 만큼 인가 과정을 ArgumentResolver에 맞기시는 것은 어떨까요?
이와 같이 구현하면 관리하기 힘든 화이트 리스트를 운용할 필요도 없고 서블릿 필터라는 굉장히 Low한 계층에 의존하고 있는 문제점을 제거할 수 있습니다!

ArgumentResolver가 JwtTokenProvider를 의존하도록 하고 인자가 resolve 될 때마다 인가하도록 처리하면 제가 말씀드린 방식의 구현이 됩니닷
인자가 붙어있는 컨트롤러 메서드에서만 인가가 이루어지도록 처리할 수 있쥬

클로버의 의견 들려주세요!

Copy link
Author

Choose a reason for hiding this comment

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

Interceptor로 변경하자는 의견이 아니라 ArgumentResolver에게 인가를 맡기자는거라면 반대입니당
인증되지 않은 사용자의 요청은 Handler까지 흘러들어와야 할 필요가 없다고 생각해요. 적어도 Interceptor를 통해 Handler가 불리기 이전에 blocking 되는게 리소스 낭비도 덜하고 보안적인 측면에서도 더 안전할 것 같습니다.
위의 이유로 MethodArguementResolver는 모든 메서드의 파라미터를 처리할 때 공통적으로 필요할 때 사용되는 기능이지 인증/인가를 위한 기능이라고 생각되지 않습니다. 🙇

Copy link
Author

Choose a reason for hiding this comment

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

+) 추가로 화이트 리스트는 이후 더 늘어날 곳은 없어보입니다. 유지보수가 힘들 것 같지는 않아용!

Copy link

Choose a reason for hiding this comment

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

오 .. 몰랐던 시각이에요
클로버의 말 이해했고 충분히 합리적이라는 생각이 드네요.

다만 인터셉터 혹은 필터로 분리할 경우 인가과정이 두개의 스텝으로 진행되면서 한눈에 들어오지 않는 것도 사실 인 것 같아요.
그리고 리소스 낭비를 이야기 하셨는데 트랜잭션이 열리기 전에 요청이 반려된다면 리소스 낭비가 그렇게 심할지도 의문이긴 합니다.

저는 개인적으로 argumentResolver에서 한큐에 처리하고 싶다는 생각이기는 한데요, 제가 일반적인 방법을 모르고 있어서 저도 제 방식에 확신을 하지는 못합니다.

다 같이 얘기해보면 좋을 것 같아유!

Copy link
Author

Choose a reason for hiding this comment

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

다만 인터셉터 혹은 필터로 분리할 경우 인가과정이 두개의 스텝으로 진행되면서 한눈에 들어오지 않는 것도 사실 인 것 같아요.

요 부분에 대해서는 filter에서는 JWT를 해독하는 인증 + 해당 요청은 로그인 된 유저만 접근할 수 있다는 인가를 진행하고 MethodArgumentResolver는 개발 편의를 위한 MemberAuth의 객체를 만드는 역할만 하기 때문에 저는 인증/인가가 두 가지 스텝이라고 생각하고 있지 않습니다!

다른 부분에 대해서는 다 같이 얘기해보면 좋을 것 같아요 좋은 의견 감삼둥

Choose a reason for hiding this comment

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

저는 Filter를 활용하지 않는다면 Interceptor가 좀 더 합리적이라고 생각됩니다. ArgumentResolver의 역할은 인증/인가라기 보단 인증/인가된 대상을 메소드 인자로 할당하는 것이라고 생각합니다. 클로버가 적어주신 Filter의 여러 단점(그 중에서도 전역 예외처리 불가)를 생각해보면 여러모로 굳이 도입할 이유가 없어보이긴 합니다.

Interceptor로의 리팩토링에 찬성합니다!

Copy link
Author

@eunjungL eunjungL Jul 22, 2024

Choose a reason for hiding this comment

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

filter가 early return이 안된다는건 저의 허위사실 유포였습니다 사죄드립니다 리건 :cry ealry return으로 수정해뒀습니다,,,


private static final List<HttpRequestInfo> WHITE_LIST = List.of(
new HttpRequestInfo(HttpMethod.GET, "/ping"),
new HttpRequestInfo(HttpMethod.GET, "/h2-console/**"),
new HttpRequestInfo(HttpMethod.POST, "/h2-console/**"),
new HttpRequestInfo(HttpMethod.GET, "/favicon/**"),
new HttpRequestInfo(HttpMethod.GET, "/swagger-ui/**"),
new HttpRequestInfo(HttpMethod.GET, "/swagger-resources/**"),
new HttpRequestInfo(HttpMethod.GET, "/v3/api-docs/**"),
new HttpRequestInfo(HttpMethod.GET, "/api/v1/travelogues/**"),
new HttpRequestInfo(HttpMethod.GET, "/api/v1/travel-plans/**"),
new HttpRequestInfo(HttpMethod.GET, "/api/v1/login/**")
);

@Override
protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain)
throws ServletException, IOException {
String token = request.getHeader(HttpHeaders.AUTHORIZATION);
if (token == null) {
sendUnauthorizedResponse(response, "로그인을 해주세요.");
return;
}

token = token.split("Bearer|bearer")[1];
try {
String memberId = tokenProvider.decode(token);
request.setAttribute("memberId", memberId);
filterChain.doFilter(request, response);
} catch (Exception e) {
sendUnauthorizedResponse(response, e.getMessage());
}
}

private void sendUnauthorizedResponse(HttpServletResponse response, String message) throws IOException {
ExceptionResponse errorResponse = new ExceptionResponse(message);

response.setStatus(HttpStatus.UNAUTHORIZED.value());
response.setContentType("application/json");
response.setCharacterEncoding("UTF-8");
response.getWriter()
.write(objectMapper.writeValueAsString(errorResponse));
}

@Override
protected boolean shouldNotFilter(HttpServletRequest request) throws ServletException {
AntPathMatcher antPathMatcher = new AntPathMatcher();

String url = request.getRequestURI();
String method = request.getMethod();

return WHITE_LIST.stream()
.anyMatch(white -> white.method().matches(method) && antPathMatcher.match(white.urlPattern(), url));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package woowacourse.touroot.global.auth;

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;
import woowacourse.touroot.global.auth.dto.MemberAuth;

@Component
public class MemberAuthMethodArgumentResolver implements HandlerMethodArgumentResolver {

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

@Override
public Object resolveArgument(
MethodParameter parameter,
ModelAndViewContainer mavContainer,
NativeWebRequest webRequest,
WebDataBinderFactory binderFactory
) throws Exception {
HttpServletRequest request = webRequest.getNativeRequest(HttpServletRequest.class);
String memberId = request.getAttribute("memberId").toString();
return new MemberAuth(Long.valueOf(memberId));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package woowacourse.touroot.global.auth.dto;

import org.springframework.http.HttpMethod;

public record HttpRequestInfo(HttpMethod method, String urlPattern) {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
package woowacourse.touroot.global.auth.dto;

public record MemberAuth(Long memberId) {
}
Original file line number Diff line number Diff line change
@@ -1,17 +1,23 @@
package woowacourse.touroot.global.config;

import io.swagger.v3.oas.models.Components;
import io.swagger.v3.oas.models.OpenAPI;
import io.swagger.v3.oas.models.info.Info;
import io.swagger.v3.oas.models.security.SecurityRequirement;
import io.swagger.v3.oas.models.security.SecurityScheme;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.http.HttpHeaders;

@Configuration
public class SwaggerConfig {

@Bean
public OpenAPI createOpenApi() {
return new OpenAPI()
.info(getInfo());
.info(getInfo())
.components(new Components().addSecuritySchemes("bearerAuth", getSecurityScheme()))
.addSecurityItem(new SecurityRequirement().addList("bearerAuth"));
}

private Info getInfo() {
Expand All @@ -20,4 +26,12 @@ private Info getInfo() {
.description("To your route, 투룻 API")
.version("0.1");
}

private SecurityScheme getSecurityScheme() {
return new SecurityScheme().type(SecurityScheme.Type.HTTP)
.scheme("bearer")
.bearerFormat("JWT")
.in(SecurityScheme.In.HEADER)
.name(HttpHeaders.AUTHORIZATION);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package woowacourse.touroot.global.config;

import lombok.RequiredArgsConstructor;
import org.springframework.context.annotation.Configuration;
import org.springframework.web.method.support.HandlerMethodArgumentResolver;
import org.springframework.web.servlet.config.annotation.WebMvcConfigurer;
import woowacourse.touroot.global.auth.MemberAuthMethodArgumentResolver;

import java.util.List;

@RequiredArgsConstructor
@Configuration
public class WebMvcConfig implements WebMvcConfigurer {

private final MemberAuthMethodArgumentResolver memberAuthMethodArgumentResolver;

@Override
public void addArgumentResolvers(List<HandlerMethodArgumentResolver> resolvers) {
resolvers.add(memberAuthMethodArgumentResolver);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package woowacourse.touroot.global.exception;

public class UnauthorizedException extends RuntimeException {

public UnauthorizedException(String message) {
super(message);
}

public UnauthorizedException(String message, Throwable cause) {
super(message, cause);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.web.server.LocalServerPort;
import org.springframework.http.HttpHeaders;
import woowacourse.touroot.authentication.infrastructure.JwtTokenProvider;
import woowacourse.touroot.global.AcceptanceTest;
import woowacourse.touroot.member.domain.Member;
import woowacourse.touroot.travelplan.dto.request.PlanDayCreateRequest;
import woowacourse.touroot.travelplan.dto.request.PlanLocationCreateRequest;
import woowacourse.touroot.travelplan.dto.request.PlanPlaceCreateRequest;
Expand All @@ -28,11 +31,17 @@ class TravelPlanControllerTest {
private int port;
private final DatabaseCleaner databaseCleaner;
private final TestFixture testFixture;
private final JwtTokenProvider jwtTokenProvider;

@Autowired
public TravelPlanControllerTest(DatabaseCleaner databaseCleaner, TestFixture testFixture) {
public TravelPlanControllerTest(
DatabaseCleaner databaseCleaner,
TestFixture testFixture,
JwtTokenProvider jwtTokenProvider
) {
this.databaseCleaner = databaseCleaner;
this.testFixture = testFixture;
this.jwtTokenProvider = jwtTokenProvider;
}

@BeforeEach
Expand All @@ -58,9 +67,13 @@ void createTravelPlan() {
.days(List.of(planDayCreateRequest))
.build();

Member member = testFixture.initMemberTestData();
String accessToken = jwtTokenProvider.createToken(member);

// when & then
RestAssured.given().log().all()
.contentType(ContentType.JSON)
.header(HttpHeaders.AUTHORIZATION, "Bearer " + accessToken)
.body(request)
.when().log().all()
.post("/api/v1/travel-plans")
Expand All @@ -86,9 +99,13 @@ void createTravelPlanWithInvalidStartDate() {
.days(List.of(planDayCreateRequest))
.build();

Member member = testFixture.initMemberTestData();
String accessToken = jwtTokenProvider.createToken(member);

// when & then
RestAssured.given().log().all()
.contentType(ContentType.JSON)
.header(HttpHeaders.AUTHORIZATION, "Bearer " + accessToken)
.body(request)
.when().log().all()
.post("/api/v1/travel-plans")
Expand Down
12 changes: 12 additions & 0 deletions backend/src/test/java/woowacourse/touroot/utils/TestFixture.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Component;
import woowacourse.touroot.member.domain.Member;
import woowacourse.touroot.member.repository.MemberRepository;
import woowacourse.touroot.place.domain.Place;
import woowacourse.touroot.place.repository.PlaceRepository;
import woowacourse.touroot.travelogue.domain.Travelogue;
Expand Down Expand Up @@ -45,7 +47,12 @@ public class TestFixture {
private TravelPlanDayRepository travelPlanDayRepository;
@Autowired
private TravelPlanPlaceRepository travelPlanPlaceRepository;
@Autowired
private MemberRepository memberRepository;

public static Member getMember(Long kakaoId, String nickname, String profileImageUri) {
return new Member(kakaoId, nickname, profileImageUri);
}

public static Travelogue getTravelogue(String name, String thumbnail) {
return new Travelogue(name, thumbnail);
Expand Down Expand Up @@ -105,4 +112,9 @@ public void initTravelPlanTestData() {
placeRepository.save(place);
travelPlanPlaceRepository.save(travelPlanPlace);
}

public Member initMemberTestData() {
Member member = getMember(1L, "tester", "image");
return memberRepository.save(member);
}
}
Loading