-
Notifications
You must be signed in to change notification settings - Fork 74
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
7주차 : 보안 #86
Open
tmxhsk99
wants to merge
30
commits into
CodeSoom:tmxhsk99
Choose a base branch
from
tmxhsk99:spring-security-apply
base: tmxhsk99
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
7주차 : 보안 #86
Changes from all commits
Commits
Show all changes
30 commits
Select commit
Hold shift + click to select a range
6290b43
build : SpringSecurity 의존성 추가
tmxhsk99 d0243f6
feat : SpringSecurity 설정 및 필터 추가
tmxhsk99 e3d1669
refactor : 인증 인터셉터 관련 로직 삭제
tmxhsk99 0974cbf
7-1 Spring security, 7-2 인증 까지 보고 진행
tmxhsk99 f92be3b
기존 TestHelper를 사용하는 계층형 테스트 구조로 수정
tmxhsk99 ac873fb
fix : 잘못된 테스트 이전 로직 제거
tmxhsk99 efd50a3
feat : claims 가 null인 경우 에러 처리 로직 추가
tmxhsk99 a3750b6
feat : 수정 삭제 요청시 Authentication 받도록 매개변수 추가
tmxhsk99 6178cae
fix : 기존에 하드 코딩되어있던 부분을 수정
tmxhsk99 6bb268a
feat : 메서드와 경로에 맞게 인증 로직을 수행하도록 관련 로직 추가
tmxhsk99 4cce0b9
refactor : 불필요한 주석 제거
tmxhsk99 79bc305
test : 유저 패스워드 기능 테스트 추가 및 해당 변경에 따른 테스트 수정
tmxhsk99 7e94f4b
feat : 유저 패스워드 암호화 기능 추가
tmxhsk99 58215a5
build : aop 관련 의존성 추가
tmxhsk99 3a54892
test : 다른 사용자 토큰 픽스쳐추가
tmxhsk99 9c49112
fix : 주석 직관적으로 수정, 경로 접근권한 수정
tmxhsk99 cbdcc48
chore : 불필요한 주석 삭제
tmxhsk99 1beee7a
test : 상품수정 권한 체크 테스트 추가
tmxhsk99 887bfb2
feat : 본인권한 관련 AOP 및 어노테이션 추가
tmxhsk99 9c3aa22
feat : 유저 권한 없음 에러 로직 추가
tmxhsk99 90903bb
test : secret 값이 서로 다르게 사용하던 것을 통일
tmxhsk99 3576adf
test : 상품 수정 삭제 권한 테스트 추가
tmxhsk99 1c516dd
feat : 상품정보에 추가한 유저아이디 속성 추가
tmxhsk99 d1a71ee
fix : 상품정보의 등록자정보를 가져와 비교하도록 로직 수정, 상품아이디 매개변수식 , 여러개 매개변수있어도 적용되도록 수정
tmxhsk99 b6eb6eb
feat : 상품 삭제, 상품 수정 시 권한 관련 로직 추가
tmxhsk99 e1d9b4d
chore : 상품생성 시 메서드 인터페이스 변화에 따른 수정
tmxhsk99 bf63a41
feat : 상품생성 시 생성자 정보도 추가하도록 로직 수정
tmxhsk99 29f2d12
chore : 상품생성 시 인터페이스 변경에 따른 수정
tmxhsk99 17b2f05
chore : 상품생성 시 인터페이스 변경에 따른 수정
tmxhsk99 5cb56c2
test : 테스트 전용 설정 파일 추가
tmxhsk99 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
50 changes: 50 additions & 0 deletions
50
app/src/main/java/com/codesoom/assignment/aop/OwnerCheckAspect.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
package com.codesoom.assignment.aop; | ||
|
||
import com.codesoom.assignment.application.ProductService; | ||
import com.codesoom.assignment.domain.Product; | ||
import com.codesoom.assignment.errors.InvalidTokenException; | ||
import com.codesoom.assignment.errors.ProductNotFoundException; | ||
import com.codesoom.assignment.errors.UserNoPermission; | ||
import org.aspectj.lang.annotation.Aspect; | ||
import org.aspectj.lang.annotation.Before; | ||
import org.springframework.security.access.AccessDeniedException; | ||
import org.springframework.security.core.Authentication; | ||
import org.springframework.security.core.context.SecurityContextHolder; | ||
import org.springframework.stereotype.Component; | ||
|
||
@Aspect | ||
@Component | ||
public class OwnerCheckAspect { | ||
private final ProductService productService; | ||
|
||
public OwnerCheckAspect(ProductService productService) { | ||
this.productService = productService; | ||
} | ||
|
||
@Before("@annotation(com.codesoom.assignment.aop.annotation.CheckOwner) && args(id,..)") | ||
public void checkOwner(Long id) throws InvalidTokenException, UserNoPermission, ProductNotFoundException { | ||
Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); | ||
|
||
authenticationValidate(authentication); | ||
|
||
Long loginUserId = (Long) authentication.getPrincipal(); | ||
|
||
Product product = productService.getProduct(id); | ||
|
||
Long createdUserId = product.getCreateUserId(); | ||
|
||
if (loginUserId != createdUserId) { | ||
throw new UserNoPermission("You do not have permission."); | ||
} | ||
} | ||
|
||
private void authenticationValidate(Authentication authentication) { | ||
if (authentication == null) { | ||
throw new InvalidTokenException("AccessToken is Invalid."); | ||
} | ||
if (authentication.getPrincipal().equals("anonymousUser")) { | ||
throw new AccessDeniedException("You do not have permission."); | ||
} | ||
} | ||
} | ||
|
11 changes: 11 additions & 0 deletions
11
app/src/main/java/com/codesoom/assignment/aop/annotation/CheckOwner.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
package com.codesoom.assignment.aop.annotation; | ||
|
||
import java.lang.annotation.ElementType; | ||
import java.lang.annotation.Retention; | ||
import java.lang.annotation.RetentionPolicy; | ||
import java.lang.annotation.Target; | ||
|
||
@Retention(RetentionPolicy.RUNTIME) | ||
@Target(ElementType.METHOD) | ||
public @interface CheckOwner { | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
120 changes: 120 additions & 0 deletions
120
app/src/main/java/com/codesoom/assignment/config/SecurityJavaConfig.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,120 @@ | ||
package com.codesoom.assignment.config; | ||
|
||
import com.codesoom.assignment.filters.AuthenticationErrorFilter; | ||
import com.codesoom.assignment.filters.JwtAuthenticationFilter; | ||
import com.codesoom.assignment.utils.JwtUtil; | ||
import org.springframework.context.annotation.Bean; | ||
import org.springframework.context.annotation.Configuration; | ||
import org.springframework.http.HttpStatus; | ||
import org.springframework.security.config.annotation.method.configuration.EnableGlobalMethodSecurity; | ||
import org.springframework.security.config.annotation.web.builders.HttpSecurity; | ||
import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter; | ||
import org.springframework.security.config.http.SessionCreationPolicy; | ||
import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder; | ||
import org.springframework.security.crypto.password.PasswordEncoder; | ||
import org.springframework.security.web.authentication.HttpStatusEntryPoint; | ||
|
||
import javax.servlet.Filter; | ||
import javax.servlet.http.HttpServletRequest; | ||
|
||
@Configuration | ||
@EnableGlobalMethodSecurity(prePostEnabled = true) | ||
public class SecurityJavaConfig extends WebSecurityConfigurerAdapter { | ||
private final JwtUtil jwtUtil; | ||
public SecurityJavaConfig(JwtUtil jwtUtil) { | ||
this.jwtUtil = jwtUtil; | ||
} | ||
|
||
|
||
@Override | ||
protected void configure(HttpSecurity http) throws Exception { | ||
|
||
Filter authenticationFilter = new JwtAuthenticationFilter(authenticationManager(), jwtUtil); | ||
Filter authenticationErrorFilter = new AuthenticationErrorFilter(); | ||
|
||
http | ||
.csrf().disable() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 여기서 csrf는 왜 disable하는지 이번 기회에 알아보면 좋습니다 |
||
.addFilter(authenticationFilter) | ||
.addFilterBefore(authenticationErrorFilter, JwtAuthenticationFilter.class) | ||
.sessionManagement() | ||
.sessionCreationPolicy(SessionCreationPolicy.STATELESS) | ||
.and(); | ||
|
||
configureAuthorizations(http); | ||
|
||
http | ||
.authorizeRequests() | ||
.anyRequest().permitAll() | ||
.and() | ||
.exceptionHandling() | ||
.authenticationEntryPoint(new HttpStatusEntryPoint(HttpStatus.UNAUTHORIZED)); | ||
} | ||
|
||
private void configureAuthorizations(HttpSecurity http) throws Exception { | ||
http.authorizeRequests() | ||
.requestMatchers(this::matchesPostProductRequest).authenticated() | ||
.and() | ||
.authorizeRequests() | ||
.requestMatchers(this::matchesPatchProductRequest).authenticated() | ||
.and() | ||
.authorizeRequests() | ||
.requestMatchers(this::matchesDeleteProductRequest).authenticated() | ||
.and() | ||
.authorizeRequests() | ||
.requestMatchers(this::matchesPostUserRequest).authenticated() | ||
.and() | ||
.authorizeRequests() | ||
.requestMatchers(this::matchesDeleteUserRequest).authenticated(); | ||
} | ||
|
||
/** | ||
* 요청이 상품 생성 요청인 경우 true를 아닌경우 false를 반환합니다. | ||
* @param req | ||
* @return | ||
*/ | ||
private boolean matchesPostProductRequest(HttpServletRequest req) { | ||
return req.getMethod().equals("POST") && | ||
(req.getRequestURI().matches("^/products$") || | ||
req.getRequestURI().matches("^/products/[0-9]+$")); | ||
} | ||
|
||
/** | ||
* 요청이 상품 수정 요청인 경우 true를 아닌경우 false를 반환합니다. | ||
* @param req | ||
* @return | ||
*/ | ||
private boolean matchesPatchProductRequest(HttpServletRequest req) { | ||
return req.getMethod().equals("PATCH") && | ||
req.getRequestURI().matches("^/products/[0-9]+$"); | ||
} | ||
|
||
/** | ||
* 요청이 상품 삭제 요청인 경우 true를 아닌경우 false를 반환합니다. | ||
* @param req | ||
* @return | ||
*/ | ||
private boolean matchesDeleteProductRequest(HttpServletRequest req) { | ||
return req.getMethod().equals("DELETE") && | ||
req.getRequestURI().matches("^/products/[0-9]+$"); | ||
} | ||
|
||
/** | ||
* 요청이 유저 생성 요청인 경우 true를 아닌경우 false를 반환합니다. | ||
* @param req | ||
* @return | ||
*/ | ||
private boolean matchesPostUserRequest(HttpServletRequest req) { | ||
return req.getMethod().equals("POST") && req.getRequestURI().matches("^/users/[0-9]+$"); | ||
} | ||
|
||
/** | ||
* 요청이 유저 삭제 요청인 경우 true를 아닌경우 false를 반환합니다. | ||
* @param req | ||
* @return | ||
*/ | ||
private boolean matchesDeleteUserRequest(HttpServletRequest req) { | ||
return req.getMethod().equals("DELETE") && req.getRequestURI().matches("^/users/[0-9]+$"); | ||
} | ||
|
||
|
||
} |
18 changes: 0 additions & 18 deletions
18
app/src/main/java/com/codesoom/assignment/config/WebJavaConfig.java
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AOP를 사용하셨군요. 팀원들이랑 협업한다고 가정했을 때, 이러한 기술 도입에 대해서 충분한 설명이 있어야 될 것 같아요.
만약에 팀원들에게 이 기술 도입에 대해서 설득하려고 할 때 뭐라고 하는게 좋을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
일단 현재 상황을 도입 할 만한 상황인지 고려합니다
프로젝트 일정 및 현재 팀원들의 기술수준이 해당 기술을 소화할만큼의 역량이 되는지
팀원들이 긍정적으로 받아들일 사람인지도 고려합니다.
만약에 프로젝트 일정이 충분하고 팀원들의 상태도
해당기술을 사용할 역량 혹은 긍정적으로 받아들일 여부가 된다면
이런 권한 체크에 관한 로직같은경우 비즈니스로직에 공통으로 필요하지만
해당 기능자체가 핵심 비즈니스로직은 아니니 비즈니스로직은 비즈니스로직만 볼수 있게
AOP를 써서 분리하자고 이야기합니다.
그리고 동시에 관련 지식 혹은
실제 적용할 AOP 코드 부분을 현재 적용하려면 이렇게 적용할것이다 및 주의점을 문서로 작성하여 배포합니다
그리고 커피도 한잔씩 사줍니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
사람과 사람과의 관계가 더 중요하다고 생각하시는군요 ㅎㅎ 저도 동의합니다