Skip to content

[신성수] 인가(Authorization) 리뷰 요청 드립니다. #28

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

Open
wants to merge 20 commits into
base: shinseongsu
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
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
16 changes: 16 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -1 +1,17 @@
# spring-security-authorization

# 🚀 1단계 - AuthorizationManager를 활용

- [x] AuthorizationManager를 활용하여 인가 과정 추상화
- [x] SecuredAuthorizationManager 구현
- [x] RequestAuthorizationManager 구현
- [x] SecuredAuthorizationManager 구현
Comment on lines +6 to +8

Choose a reason for hiding this comment

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

Suggested change
- [x] SecuredAuthorizationManager 구현
- [x] RequestAuthorizationManager 구현
- [x] SecuredAuthorizationManager 구현
- [x] SecuredAuthorizationManager 구현
- [x] RequestAuthorizationManager 구현

중복되어있네요 :)


# 🚀 2단계 - 요청별 권한 검증 정보 분리

- [x] 요청별 권한 검증 정보를 별도의 객체로 분리하여 관리
- [x] RequestMatcher 작성
- [x] AnyRequestMatcher 구현
- [x] MvcRequestMatcher 구현
- [x] RequestMatcherEntry 작성

18 changes: 13 additions & 5 deletions src/main/java/nextstep/app/SecurityConfig.java
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
package nextstep.app;

import jakarta.servlet.http.HttpServletRequest;
import nextstep.app.domain.Member;
import nextstep.app.domain.MemberRepository;
import nextstep.security.authentication.AuthenticationException;
import nextstep.security.authentication.BasicAuthenticationFilter;
import nextstep.security.authentication.UsernamePasswordAuthenticationFilter;
import nextstep.security.authorization.AuthorizationFilter;
import nextstep.security.authorization.CheckAuthenticationFilter;
import nextstep.security.authorization.SecuredAspect;
import nextstep.security.authorization.SecuredMethodInterceptor;
import nextstep.security.authorization.manager.*;
import nextstep.security.config.DefaultSecurityFilterChain;
import nextstep.security.config.DelegatingFilterProxy;
import nextstep.security.config.FilterChainProxy;
Expand All @@ -22,6 +25,10 @@
import java.util.List;
import java.util.Set;

import static nextstep.security.authorization.matcher.RequestMatcherEntry.createDefaultMatcher;
import static nextstep.security.authorization.matcher.RequestMatcherEntry.createMvcMatcher;
import static org.springframework.http.HttpMethod.GET;

@EnableAspectJAutoProxy
@Configuration
public class SecurityConfig {
Expand All @@ -46,19 +53,20 @@ public FilterChainProxy filterChainProxy(List<SecurityFilterChain> securityFilte
public SecuredMethodInterceptor securedMethodInterceptor() {
return new SecuredMethodInterceptor();
}
// @Bean
// public SecuredAspect securedAspect() {
// return new SecuredAspect();
// }

@Bean
public SecurityFilterChain securityFilterChain() {
final AuthorizationManager<HttpServletRequest> authorizationManager = new RequestAuthorizationManager(List.of(
createMvcMatcher(GET, "/members", new AuthorityAuthorizationManager<>("ADMIN")),
createMvcMatcher(GET, "/members/me", new AuthenticatedAuthorizationManager<>()),
createMvcMatcher(GET, "/search", new PermitAllAuthorizationManager<>())
), createDefaultMatcher(new DenyAllAuthorizationManager<>()));
Comment on lines +59 to +63

Choose a reason for hiding this comment

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

잘 추가해주셨네요 👍

return new DefaultSecurityFilterChain(
List.of(
new SecurityContextHolderFilter(),
new UsernamePasswordAuthenticationFilter(userDetailsService()),
new BasicAuthenticationFilter(userDetailsService()),
new CheckAuthenticationFilter()
new AuthorizationFilter(authorizationManager)
)
);
}
Expand Down
17 changes: 17 additions & 0 deletions src/main/java/nextstep/app/ui/MemberController.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,16 @@

import nextstep.app.domain.Member;
import nextstep.app.domain.MemberRepository;
import nextstep.security.authentication.Authentication;
import nextstep.security.authentication.AuthenticationException;
import nextstep.security.authorization.Secured;
import nextstep.security.context.SecurityContextHolder;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RestController;

import java.util.List;
import java.util.Optional;

@RestController
public class MemberController {
Expand All @@ -30,4 +34,17 @@ public ResponseEntity<List<Member>> search() {
List<Member> members = memberRepository.findAll();
return ResponseEntity.ok(members);
}

@Secured("USER")
@GetMapping("/members/me")
public ResponseEntity<Member> me() {
final Authentication authentication = SecurityContextHolder
.getContext().getAuthentication();

Member member = memberRepository.findByEmail(
authentication.getPrincipal().toString()
).orElseThrow(AuthenticationException::new);

return ResponseEntity.ok(member);
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package nextstep.security.authentication;

import jakarta.servlet.FilterChain;
import jakarta.servlet.ServletException;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import nextstep.security.context.SecurityContext;
Expand All @@ -10,6 +11,7 @@
import org.springframework.util.StringUtils;
import org.springframework.web.filter.OncePerRequestFilter;

import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.Base64;
import java.util.List;
Expand All @@ -26,23 +28,23 @@ public BasicAuthenticationFilter(UserDetailsService userDetailsService) {
}

@Override
protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) {
try {
Authentication authentication = convert(request);
if (authentication == null) {
filterChain.doFilter(request, response);
return;
}

Authentication authResult = this.authenticationManager.authenticate(authentication);
SecurityContext context = SecurityContextHolder.createEmptyContext();
context.setAuthentication(authResult);
SecurityContextHolder.setContext(context);

protected void doFilterInternal(
HttpServletRequest request,
HttpServletResponse response,
FilterChain filterChain
) throws ServletException, IOException {
Authentication authentication = convert(request);
if (authentication == null) {
filterChain.doFilter(request, response);
} catch (Exception e) {
response.setStatus(HttpServletResponse.SC_UNAUTHORIZED);
return;
}

Authentication authResult = this.authenticationManager.authenticate(authentication);
SecurityContext context = SecurityContextHolder.createEmptyContext();
context.setAuthentication(authResult);
SecurityContextHolder.setContext(context);

filterChain.doFilter(request, response);
}

private Authentication convert(HttpServletRequest request) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package nextstep.security.authorization;

import jakarta.servlet.FilterChain;
import jakarta.servlet.ServletException;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import nextstep.security.authentication.Authentication;
import nextstep.security.authentication.AuthenticationException;
import nextstep.security.authorization.manager.AuthorizationManager;
import nextstep.security.context.SecurityContextHolder;
import org.springframework.web.filter.OncePerRequestFilter;

import java.io.IOException;

public class AuthorizationFilter extends OncePerRequestFilter {
private final AuthorizationManager<HttpServletRequest> authorizationManager;

public AuthorizationFilter(AuthorizationManager<HttpServletRequest> authorizationManager) {
this.authorizationManager = authorizationManager;
}

@Override
protected void doFilterInternal(
HttpServletRequest request,
HttpServletResponse response,
FilterChain filterChain
) throws ServletException, IOException {
final Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
if (authentication == null || !authentication.isAuthenticated()) {
throw new AuthenticationException();
}
if (!authorizationManager.authorize(authentication, request).isGranted()) {
throw new ForbiddenException();
}
filterChain.doFilter(request, response);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import nextstep.security.authentication.Authentication;
import nextstep.security.authentication.AuthenticationException;
import nextstep.security.authorization.manager.SecuredAuthorizationManager;
import nextstep.security.context.SecurityContextHolder;
import org.aopalliance.aop.Advice;
import org.aopalliance.intercept.MethodInterceptor;
Expand All @@ -11,32 +12,42 @@
import org.springframework.aop.framework.AopInfrastructureBean;
import org.springframework.aop.support.annotation.AnnotationMatchingPointcut;

import java.lang.reflect.Method;

public class SecuredMethodInterceptor implements MethodInterceptor, PointcutAdvisor, AopInfrastructureBean {

private final SecuredAuthorizationManager securedAuthorizationManager;

private final Pointcut pointcut;

public SecuredMethodInterceptor() {
this.pointcut = new AnnotationMatchingPointcut(null, Secured.class);
this.securedAuthorizationManager = new SecuredAuthorizationManager();
}

@Override
public Object invoke(MethodInvocation invocation) throws Throwable {
Method method = invocation.getMethod();
if (method.isAnnotationPresent(Secured.class)) {
Secured secured = method.getAnnotation(Secured.class);
Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
if (authentication == null) {
throw new AuthenticationException();
}
if (!authentication.getAuthorities().contains(secured.value())) {
throw new ForbiddenException();
}
if (!invocation.getMethod().isAnnotationPresent(Secured.class)) {
return invocation.proceed();
}
final Authentication authentication = SecurityContextHolder.getContext().getAuthentication();

checkAuthenticated(authentication);
checkAuthorize(authentication, invocation);

return invocation.proceed();
}

private void checkAuthenticated(Authentication authentication) {
if (authentication == null || !authentication.isAuthenticated()) {
throw new AuthenticationException();
}
}

private void checkAuthorize(Authentication authentication, MethodInvocation invocation) {
if (!securedAuthorizationManager.authorize(authentication, invocation).isGranted()) {
throw new ForbiddenException();
}
}

@Override
public Pointcut getPointcut() {
return pointcut;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package nextstep.security.authorization.manager;

import nextstep.security.authentication.Authentication;

import static nextstep.security.authorization.manager.AuthorizationDecision.GRANTED;
import static nextstep.security.authorization.manager.AuthorizationDecision.NOT_GRANTED;

public class AuthenticatedAuthorizationManager<T> implements AuthorizationManager<T> {
@Override
public AuthorizationResult authorize(Authentication authentication, T target) {
if(authentication != null &&
authentication.isAuthenticated()
) {
return GRANTED;
}

return NOT_GRANTED;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package nextstep.security.authorization.manager;

import nextstep.security.authentication.Authentication;

import java.util.Set;

public class AuthorityAuthorizationManager<T> implements AuthorizationManager<T> {
private final Set<String> authorities;

public AuthorityAuthorizationManager(String... authorities) {
this.authorities = Set.of(authorities);
}

@Override
public AuthorizationResult authorize(Authentication authentication, T target) {
return AuthorizationDecision.from(isGranted(authentication));
}

private boolean isGranted(Authentication authentication) {
return authentication != null
&& authentication.isAuthenticated()
&& anyMatch(authentication);
}

private boolean anyMatch(Authentication authentication) {
return authentication.getAuthorities().stream()
.anyMatch(authorities::contains);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package nextstep.security.authorization.manager;

public enum AuthorizationDecision implements AuthorizationResult {

Choose a reason for hiding this comment

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

true와 false로 구성되어있어 enum으로 생각해볼 수도 있으나 실제 구현체에서는 AuthorizationDecision을 상속받아 구현하는 새로운 구현체들도 있어 enum으로 사용되지는 않습니다.

일반적으로 enum을 구성하는 것은 몇가지 고정값이 있기 때문에 enum으로 표현하지는 않고 정말 static한 정보들로 표기될때에만 사용되어요. 예를들면 HttpStatus는 400, 401과 같은 코드가 있기 때문에 각각을 표기하는 것처럼요.

그래서 사실은 AuthorizationDecision은 granted가 있다 없다로 생성되는 객체일뿐 enum에 적합하진 않습니다.

GRANTED(true),
NOT_GRANTED(false);

private final boolean granted;

AuthorizationDecision(final boolean granted) {
this.granted = granted;
}

public boolean isGranted() {
return this.granted;
}

public static AuthorizationResult from(boolean granted) {
return granted ? GRANTED : NOT_GRANTED;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package nextstep.security.authorization.manager;

import nextstep.security.authentication.Authentication;

@FunctionalInterface
public interface AuthorizationManager<T> {
AuthorizationResult authorize(Authentication authentication, T target);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package nextstep.security.authorization.manager;

public interface AuthorizationResult {

Choose a reason for hiding this comment

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

미션내의 요구사항은 AuthorizationDecision이었으나 최근 버전에서의 추상화를 잘 구현해주셨네요

boolean isGranted();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package nextstep.security.authorization.manager;

import nextstep.security.authentication.Authentication;

import static nextstep.security.authorization.manager.AuthorizationDecision.NOT_GRANTED;

public class DenyAllAuthorizationManager<T> implements AuthorizationManager<T> {
@Override
public AuthorizationResult authorize(Authentication authentication, T target) {
return NOT_GRANTED;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package nextstep.security.authorization.manager;

import nextstep.security.authentication.Authentication;

import static nextstep.security.authorization.manager.AuthorizationDecision.GRANTED;

public class PermitAllAuthorizationManager<T> implements AuthorizationManager<T> {
@Override
public AuthorizationResult authorize(Authentication authentication, T target) {
return GRANTED;
}
}
Loading