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

로그인 Interceptor 개선 #1616

Merged
merged 4 commits into from
Nov 23, 2023
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public class KeywordRecommendedPostStepDefinitions extends AcceptanceSteps {
public void 추천_포스트가_수정된다() {
int statusCode = context.response.statusCode();

assertThat(statusCode).isEqualTo(HttpStatus.OK.value());
assertThat(statusCode).isEqualTo(HttpStatus.NO_CONTENT.value());
Copy link
Contributor

Choose a reason for hiding this comment

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

오 상태코드도 적절하게 수정해주셨네요! 굿입니다^^*

}

@Then("추천 포스트가 삭제된다")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
@Profile("!docu")
public class LoginConfig implements WebMvcConfigurer {

private final static String BASE_PACKAGE = "wooteco.prolog";
private static final String BASE_PACKAGE = "wooteco.prolog";

private final GithubLoginService githubLoginService;
private final JwtTokenProvider jwtTokenProvider;
Expand All @@ -29,7 +29,7 @@ public void addInterceptors(InterceptorRegistry registry) {
AutoInterceptorPatternMaker mapper =
new AutoInterceptorPatternMaker(BASE_PACKAGE, AuthMemberPrincipal.class);

registry.addInterceptor(new LoginInterceptor(githubLoginService))
registry.addInterceptor(new LoginInterceptor(githubLoginService, mapper.extractLoginDetector()))
.addPathPatterns(mapper.extractPatterns());
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,58 +1,26 @@
package wooteco.prolog.login.ui;

import java.util.Objects;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import lombok.AllArgsConstructor;
import org.springframework.http.HttpMethod;
import org.springframework.web.servlet.HandlerInterceptor;
import wooteco.prolog.login.application.AuthorizationExtractor;
import wooteco.prolog.login.application.GithubLoginService;
import wooteco.support.autoceptor.AuthenticationDetector;

@AllArgsConstructor
public class LoginInterceptor implements HandlerInterceptor {

private static final String ORIGIN = "Origin";
private static final String ACCESS_REQUEST_METHOD = "Access-Control-Request-Method";
private static final String ACCESS_REQUEST_HEADERS = "Access-Control-Request-Headers";

private final GithubLoginService githubLoginService;
private final AuthenticationDetector authenticationDetector;

@Override
public boolean preHandle(HttpServletRequest request, HttpServletResponse response,
Object handler) {
if (isPreflighted(request)) {
return true;
}

if (HttpMethod.GET.matches(request.getMethod())) {
public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) {
if (!authenticationDetector.requireLogin(request)) {
return true;
}

githubLoginService.validateToken(AuthorizationExtractor.extract(request));
return true;
}

private boolean isPreflighted(HttpServletRequest request) {
return isOptionsMethod(request)
&& hasOrigin(request)
&& hasRequestHeaders(request)
&& hasRequestMethods(request);
}

public boolean isOptionsMethod(HttpServletRequest request) {
return HttpMethod.OPTIONS.matches(request.getMethod());
}

public boolean hasOrigin(HttpServletRequest request) {
return Objects.nonNull(request.getHeader(ORIGIN));
}

public boolean hasRequestMethods(HttpServletRequest request) {
return Objects.nonNull(request.getHeader(ACCESS_REQUEST_METHOD));
}

public boolean hasRequestHeaders(HttpServletRequest request) {
return Objects.nonNull(request.getHeader(ACCESS_REQUEST_HEADERS));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package wooteco.support.autoceptor;


import java.util.List;
import java.util.Objects;
import javax.servlet.http.HttpServletRequest;
import lombok.RequiredArgsConstructor;
import org.springframework.http.HttpMethod;

@RequiredArgsConstructor
public class AuthenticationDetector {

private static final String ORIGIN = "Origin";
private static final String ACCESS_REQUEST_METHOD = "Access-Control-Request-Method";
private static final String ACCESS_REQUEST_HEADERS = "Access-Control-Request-Headers";

private final List<MethodPattern> requireLoginPatterns;

public boolean requireLogin(HttpServletRequest request) {
if (isPreflighted(request)) {
return false;
}

if (HttpMethod.GET.matches(request.getMethod())) {
return false;
}

return requireLoginPatterns.stream()
.anyMatch(pattern -> pattern.match(request));
}

private boolean isPreflighted(HttpServletRequest request) {
return isOptionsMethod(request)
&& hasOrigin(request)
&& hasRequestHeaders(request)
&& hasRequestMethods(request);
}

private boolean isOptionsMethod(HttpServletRequest request) {
return HttpMethod.OPTIONS.matches(request.getMethod());
}

private boolean hasOrigin(HttpServletRequest request) {
return Objects.nonNull(request.getHeader(ORIGIN));
}

private boolean hasRequestMethods(HttpServletRequest request) {
return Objects.nonNull(request.getHeader(ACCESS_REQUEST_METHOD));
}

private boolean hasRequestHeaders(HttpServletRequest request) {
return Objects.nonNull(request.getHeader(ACCESS_REQUEST_HEADERS));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,8 @@ private MethodScanner createMethodScanner(List<Class<? extends Annotation>> targ
public List<String> extractPatterns() {
return uriScanner.extractUri();
}

public AuthenticationDetector extractLoginDetector() {
return uriScanner.extractLoginDetector();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package wooteco.support.autoceptor;

import java.util.regex.Pattern;
import javax.servlet.http.HttpServletRequest;
import org.springframework.http.HttpMethod;

public class MethodPattern {

private static final String INVALID_METHOD_PATTERN_MESSAGE = "uri 와 method 가 지정되지 않은 MethodPattern 이 있습니다. method : %s, uri : %s";

private final HttpMethod method;
private final Pattern pattern;

public MethodPattern(HttpMethod method, String uri) {
validate(uri, method);
this.method = method;
this.pattern = convertToPattern(uri);
}

private Pattern convertToPattern(String uri) {
String replace = uri.replace("*", "[^/]+");
String regex = "^" + replace + "$";
return Pattern.compile(regex);
}

private void validate(String method, HttpMethod uri) {
if (uri == null || method == null) {
throw new IllegalArgumentException(String.format(INVALID_METHOD_PATTERN_MESSAGE, method, uri));
}
}

public boolean match(HttpServletRequest request) {
if (!pattern.matcher(request.getRequestURI()).matches()) {
return false;
}
if (!method.matches(request.getMethod())) {
return false;
}
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,44 +3,68 @@
import java.lang.annotation.Annotation;
import java.lang.reflect.GenericDeclaration;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.function.Function;
import java.util.function.Supplier;
import org.springframework.http.HttpMethod;
import org.springframework.web.bind.annotation.DeleteMapping;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PatchMapping;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.PutMapping;
import org.springframework.web.bind.annotation.RequestMapping;

public enum MappingAnnotation {
REQUEST_MAPPING(RequestMapping.class,
declaration -> Arrays.asList(
declaration.getAnnotation(RequestMapping.class).value())),
declaration -> Arrays.asList(declaration.getAnnotation(RequestMapping.class).value()),
() -> {
throw new IllegalArgumentException(
"RequestMapping 을 통해 LoginInterceptor 를 등록할 수 없습니다. @GetMapping 과 같은 형식으로 사용해주세요.");
}),
GET(GetMapping.class,
declaration -> Arrays.asList(declaration.getAnnotation(GetMapping.class).value())),
declaration -> Arrays.asList(declaration.getAnnotation(GetMapping.class).value()),
() -> HttpMethod.GET),
POST(PostMapping.class,
declaration -> Arrays.asList(declaration.getAnnotation(PostMapping.class).value())),
declaration -> Arrays.asList(declaration.getAnnotation(PostMapping.class).value()),
() -> HttpMethod.POST),
DELETE(DeleteMapping.class,
declaration -> Arrays.asList(declaration.getAnnotation(DeleteMapping.class).value())),
declaration -> Arrays.asList(declaration.getAnnotation(DeleteMapping.class).value()),
() -> HttpMethod.DELETE),
PUT(PutMapping.class,
declaration -> Arrays.asList(declaration.getAnnotation(PutMapping.class).value()));
declaration -> Arrays.asList(declaration.getAnnotation(PutMapping.class).value()),
() -> HttpMethod.PUT),
PATCH(PatchMapping.class,
declaration -> Arrays.asList(declaration.getAnnotation(PatchMapping.class).value()),
() -> HttpMethod.PATCH);

private final Class<? extends Annotation> typeToken;
private final Function<GenericDeclaration, List<String>> values;
private final Supplier<HttpMethod> method;

MappingAnnotation(
Class<? extends Annotation> typeToken,
Function<GenericDeclaration, List<String>> values
) {
MappingAnnotation(Class<? extends Annotation> typeToken, Function<GenericDeclaration, List<String>> values,
Supplier<HttpMethod> method) {
this.typeToken = typeToken;
this.values = values;
this.method = method;
}

public static List<String> extractUriFrom(GenericDeclaration declaration) {
return Arrays.stream(values())
List<String> methodUris = Arrays.stream(values())
.filter(httpMethod -> declaration.isAnnotationPresent(httpMethod.typeToken))
.map(httpMethods -> httpMethods.values.apply(declaration))
.findAny()
.orElse(Collections.emptyList());
.map(httpMethod -> httpMethod.values.apply(declaration))
.orElseGet(() -> Arrays.asList(""));
if(methodUris.isEmpty()){
return Arrays.asList("");
}
return methodUris;
}

public static HttpMethod extractHttpMethod(GenericDeclaration declaration) {
MappingAnnotation annotation = Arrays.stream(values())
.filter(httpMethod -> declaration.isAnnotationPresent(httpMethod.typeToken))
.findAny()
.orElseThrow(() -> new IllegalArgumentException("해당 HttpMethod 는 로그인 처리에서 고려하지 않았습니다. 필요시 추가하시오"));
return annotation.method.get();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,15 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.stream.Stream;
import org.springframework.http.HttpMethod;
import org.springframework.util.StringUtils;
import wooteco.support.autoceptor.AuthenticationDetector;
import wooteco.support.autoceptor.MethodPattern;

public class URIScanner {

Expand Down Expand Up @@ -82,4 +86,38 @@ private String createUri(String controllerUri, String methodUri) {
.collect(joining("/"));

}

public AuthenticationDetector extractLoginDetector() {
List<Class<?>> controllers = controllerScanner.extractControllers();
List<MethodPattern> requireLoginMethods = new ArrayList<>();

for (Class<?> controller : controllers) {
List<String> controllerUris = extractControllerUri(controller);
List<Method> methods = methodScanner.extractMethodAnnotatedOnParameter(controller);
requireLoginMethods.addAll(extractRequireLogin(controllerUris, methods));
}

return new AuthenticationDetector(requireLoginMethods);
}

private List<MethodPattern> extractRequireLogin(List<String> controllerUris,
List<Method> methods) {
if (methods.isEmpty()) {
return Collections.emptyList();
}
return methods.stream()
.map(method -> extractRequireLoginFrom(method, controllerUris))
.reduce((patternList1, patternList2) -> {
patternList1.addAll(patternList2);
return patternList1;
})
.orElseThrow(() -> new IllegalArgumentException("해당 메서드로부터 uri 추출할 수 없습니다." + controllerUris));
}

private List<MethodPattern> extractRequireLoginFrom(Method method, List<String> controllerUris) {
HttpMethod httpMethod = MappingAnnotation.extractHttpMethod(method);
return createUris(controllerUris, MappingAnnotation.extractUriFrom(method)).stream()
.map(methodUri -> new MethodPattern(httpMethod, methodUri))
.collect(toList());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package wooteco.support.autoceptor;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.BDDMockito.given;

import javax.servlet.http.HttpServletRequest;
import org.junit.jupiter.api.DisplayNameGeneration;
import org.junit.jupiter.api.DisplayNameGenerator.ReplaceUnderscores;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.mockito.Mockito;
import org.mockito.junit.jupiter.MockitoExtension;
import org.springframework.http.HttpMethod;

@DisplayNameGeneration(ReplaceUnderscores.class)
@SuppressWarnings("NonAsciiCharacters")
@ExtendWith(MockitoExtension.class)
class MethodPatternTest {

@ParameterizedTest
@ValueSource(strings = {"/path1/param1/path2", "/path1/123123/path2"})
void matching(String requestUri) {
// given
MethodPattern methodPattern = new MethodPattern(HttpMethod.GET, "/path1/*/path2");
HttpServletRequest mockRequest = Mockito.mock(HttpServletRequest.class);
given(mockRequest.getMethod())
.willReturn(HttpMethod.GET.name());
given(mockRequest.getRequestURI())
.willReturn(requestUri);

// when
boolean mathcing = methodPattern.match(mockRequest);

// then
assertThat(mathcing).isTrue();
}

@ParameterizedTest
@ValueSource(strings = {"/path1/param1/path2/path3", "/path1/path2"})
void notMatching(String requestUri) {
// given
MethodPattern methodPattern = new MethodPattern(HttpMethod.GET, "/path1/*/path2");
HttpServletRequest mockRequest = Mockito.mock(HttpServletRequest.class);
given(mockRequest.getRequestURI())
.willReturn(requestUri);

// when
boolean mathcing = methodPattern.match(mockRequest);

// then
assertThat(mathcing).isFalse();
}
}
Loading
Loading