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

Security 관련 코드 리팩토링 & 카카오 로그인 방식 제거 #20

Merged
merged 13 commits into from
Jul 10, 2022
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
4 changes: 2 additions & 2 deletions module-api/src/main/java/com/zoopi/controller/ResultCode.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
public enum ResultCode {

// Member
EMAIL_AVAILABLE(200, "R-M001", "사용 가능한 이메일입니다."),
EMAIL_DUPLICATE(200, "R-M002", "이미 사용 중인 이메일입니다."),
USERNAME_AVAILABLE(200, "R-M001", "사용 가능한 아이디입니다."),
USERNAME_DUPLICATE(200, "R-M002", "이미 사용 중인 아이디입니다."),
PHONE_AVAILABLE(200, "R-M003", "사용 가능한 휴대폰 번호입니다."),
PHONE_DUPLICATE(200, "R-M004", "이미 사용 중인 휴대폰 번호입니다."),
SIGN_UP_SUCCESS(200, "R-M005", "회원 가입에 성공하였습니다."),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import static com.zoopi.domain.member.dto.SigninResponse.SigninResult.*;

import javax.validation.Valid;
import javax.validation.constraints.Email;
import javax.validation.constraints.Pattern;
import javax.validation.constraints.Size;

Expand All @@ -28,7 +27,6 @@
import com.zoopi.domain.authentication.service.AuthenticationService;
import com.zoopi.domain.authentication.service.BanService;
import com.zoopi.domain.member.dto.SigninResponse;
import com.zoopi.domain.member.entity.JoinType;
import com.zoopi.domain.member.service.MemberService;
import com.zoopi.util.AuthenticationCodeUtils;

Expand All @@ -53,19 +51,19 @@ public class MemberAuthController {

// TODO: @APiResponse 추가

@ApiOperation(value = "이메일 유효성 검사")
@ApiImplicitParam(name = "email", value = "이메일", required = true, example = "zoopi@gmail.com")
@PostMapping("/email/validate")
public ResponseEntity<ResultResponse> validateEmail(@RequestParam @Size(max = 30) @Email String email) {
final boolean isValidated = memberService.validateEmail(email);
@ApiOperation(value = "아이디 유효성 검사")
@ApiImplicitParam(name = "username", value = "아이디", required = true, example = "zoopi")
@PostMapping("/username/validate")
public ResponseEntity<ResultResponse> validateUsername(@RequestParam @Size(max = 30) String username) {
final boolean isValidated = memberService.validateUsername(username);
final ResultCode resultCode;
if (isValidated) {
resultCode = EMAIL_AVAILABLE;
resultCode = USERNAME_AVAILABLE;
} else {
resultCode = EMAIL_DUPLICATE;
resultCode = USERNAME_DUPLICATE;
}

final ValidationResponse response = new ValidationResponse(email, isValidated);
final ValidationResponse response = new ValidationResponse(username, isValidated);
return ResponseEntity.ok(ResultResponse.of(resultCode, response));
}

Expand Down Expand Up @@ -139,13 +137,13 @@ public ResponseEntity<ResultResponse> deleteExpiredAuthenticationCodes() {
return ResponseEntity.ok(ResultResponse.of(DELETE_ALL_EXPIRED_AUTHENTICATION_CODES));
}

@ApiOperation(value = "이메일 회원 가입")
@PostMapping("/signup/email")
public ResponseEntity<ResultResponse> signupByEmail(@Valid @RequestBody SignupRequest request) {
@ApiOperation(value = "일반 회원 가입")
@PostMapping("/signup")
public ResponseEntity<ResultResponse> signup(@Valid @RequestBody SignupRequest request) {
authenticationService.validatePassword(request.getPassword(), request.getPasswordCheck());
if (!memberService.validateEmail(request.getEmail())) {
final ValidationResponse response = new ValidationResponse(request.getEmail(), false);
return ResponseEntity.ok(ResultResponse.of(EMAIL_DUPLICATE, response));
if (!memberService.validateUsername(request.getUsername())) {
final ValidationResponse response = new ValidationResponse(request.getUsername(), false);
return ResponseEntity.ok(ResultResponse.of(USERNAME_DUPLICATE, response));
}
if (!memberService.validatePhone(request.getPhone())) {
final ValidationResponse response = new ValidationResponse(request.getPhone(), false);
Expand All @@ -162,16 +160,16 @@ public ResponseEntity<ResultResponse> signupByEmail(@Valid @RequestBody SignupRe
return ResponseEntity.ok(ResultResponse.of(AUTHENTICATION_KEY_NOT_AUTHENTICATED, response));
}

memberService.createMember(request.getEmail(), request.getPhone(), request.getName(), request.getPassword(), JoinType.EMAIL);
memberService.createMember(request.getUsername(), request.getPhone(), "", request.getPassword(), "");

return ResponseEntity.ok(ResultResponse.of(SIGN_UP_SUCCESS));
}

// TODO: RefreshToken -> Cookie 저장
@ApiOperation(value = "이메일 로그인")
@PostMapping("/signin/email")
@ApiOperation(value = "일반 로그인")
@PostMapping("/signin")
public ResponseEntity<ResultResponse> signinByEmail(@Valid @RequestBody SigninRequest request) {
final SigninResponse response = memberService.signin(request.getEmail(), request.getPassword());
final SigninResponse response = memberService.signin(request.getUsername(), request.getPassword());
final ResultCode resultCode;
if (response.getResult().equals(NONEXISTENT_USERNAME)) {
resultCode = MEMBER_USERNAME_NONEXISTENT;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package com.zoopi.controller.member.request;

import javax.validation.constraints.Email;
import javax.validation.constraints.NotBlank;
import javax.validation.constraints.NotNull;
import javax.validation.constraints.Size;

import lombok.AccessLevel;
Expand All @@ -17,10 +15,9 @@
@NoArgsConstructor(access = AccessLevel.PROTECTED)
public class SigninRequest {

@NotNull
@NotBlank
@Size(max = 30)
@Email
private String email;
private String username;

@Size(max = 30)
@NotBlank
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package com.zoopi.controller.member.request;

import javax.validation.constraints.Email;
import javax.validation.constraints.NotBlank;
import javax.validation.constraints.NotNull;
import javax.validation.constraints.Pattern;
Expand All @@ -18,10 +17,9 @@
@NoArgsConstructor(access = AccessLevel.PROTECTED)
public class SignupRequest {

@NotNull
@NotBlank
@Size(max = 30)
@Email
private String email;
private String username;

@NotNull
@Pattern(regexp = "^.*(?=^.{10,20}$)(?=.*\\d)(?=.*[a-zA-Z])(?=.*[`~!@#$%^&*()+=]).*$")
Expand All @@ -35,10 +33,6 @@ public class SignupRequest {
@Pattern(regexp = "^01(?:0|1|[6-9])(?:\\d{3}|\\d{4})\\d{4}$")
private String phone;

@NotNull
@Pattern(regexp = "^[\\da-zA-Z가-힣]{1,10}$")
private String name;

@NotBlank
@Size(max = 36)
private String authenticationKey;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package com.zoopi.config;

import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;

import com.fasterxml.jackson.databind.ObjectMapper;

@Configuration
public class ObjectMapperConfig {

@Bean
public ObjectMapper objectMapper() {
return new ObjectMapper();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter;
import org.springframework.security.config.http.SessionCreationPolicy;
import org.springframework.security.web.authentication.AuthenticationEntryPointFailureHandler;
import org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter;
import org.springframework.security.web.util.matcher.RequestMatcher;
import org.springframework.web.cors.CorsConfiguration;
Expand Down Expand Up @@ -60,6 +61,11 @@ public void configure(WebSecurity web) {
web.ignoring().requestMatchers(PathRequest.toStaticResources().atCommonLocations());
}

@Bean
public AuthenticationEntryPointFailureHandler authenticationEntryPointFailureHandler() {
return new AuthenticationEntryPointFailureHandler(authenticationEntryPoint);
}

@Override
protected void configure(HttpSecurity http) throws Exception {
http
Expand Down Expand Up @@ -90,6 +96,7 @@ protected void configure(HttpSecurity http) throws Exception {
.and()

.successHandler(oAuth2SuccessHandler)
.failureHandler(authenticationEntryPointFailureHandler())
.userInfoEndpoint().userService(oAuth2UserService)
;

Expand All @@ -104,6 +111,7 @@ public JwtAuthenticationFilter jwtAuthenticationFilter() throws Exception {
final RequestMatcher matcher = new SkipPathRequestMatcher(skipPaths);
final JwtAuthenticationFilter filter = new JwtAuthenticationFilter(matcher, jwtUtils);
filter.setAuthenticationManager(super.authenticationManager());
filter.setAuthenticationFailureHandler(authenticationEntryPointFailureHandler());
return filter;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package com.zoopi.config.security.exception;

import java.util.ArrayList;
import java.util.List;

import org.springframework.security.core.AuthenticationException;

import com.zoopi.exception.response.ErrorCode;
import com.zoopi.exception.response.ErrorResponse;

import lombok.Getter;

@Getter
public abstract class CustomAuthenticationException extends AuthenticationException {

private final ErrorCode errorCode;
private final List<ErrorResponse.FieldError> errors;

public CustomAuthenticationException(ErrorCode errorCode, List<ErrorResponse.FieldError> errors) {
super(errorCode.getMessage());
this.errorCode = errorCode;
this.errors = errors;
}

public CustomAuthenticationException(ErrorCode errorCode) {
super(errorCode.getMessage());
this.errorCode = errorCode;
this.errors = new ArrayList<>();
}

}
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
package com.zoopi.config.security.handler;

import static com.zoopi.exception.response.ErrorCode.*;

import java.io.OutputStream;
import java.util.List;

import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
Expand All @@ -15,23 +12,23 @@
import org.springframework.stereotype.Component;

import com.fasterxml.jackson.databind.ObjectMapper;
import com.zoopi.config.security.jwt.exception.JwtAuthenticationException;
import com.zoopi.config.security.exception.CustomAuthenticationException;
import com.zoopi.exception.response.ErrorResponse;

import lombok.RequiredArgsConstructor;

@Component
@RequiredArgsConstructor
public class CustomAuthenticationEntryPoint implements AuthenticationEntryPoint {

private final ObjectMapper objectMapper;

@Override
public void commence(HttpServletRequest request, HttpServletResponse response,
AuthenticationException authException) {
final JwtAuthenticationException exception = (JwtAuthenticationException)authException;
final int status = AUTHENTICATION_FAILURE.getStatus();
final String code = AUTHENTICATION_FAILURE.getCode();
final String message = AUTHENTICATION_FAILURE.getMessage();
final List<ErrorResponse.FieldError> errors = exception.getErrors();
final ErrorResponse errorResponse = ErrorResponse.of(status, code, message, errors);

final ObjectMapper objectMapper = new ObjectMapper();
final CustomAuthenticationException exception = (CustomAuthenticationException)authException;
final ErrorResponse errorResponse = ErrorResponse.of(exception.getErrorCode(), exception.getErrors());

try (OutputStream os = response.getOutputStream()) {
objectMapper.writeValue(os, errorResponse);
os.flush();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,10 @@
public class JwtAuthenticationFilter extends AbstractAuthenticationProcessingFilter {

private final JwtUtils jwtUtils;
private final RequestMatcher matcher;

public JwtAuthenticationFilter(RequestMatcher matcher, JwtUtils jwtUtils) {
super(ALL_PATTERN);
super(matcher);
this.jwtUtils = jwtUtils;
this.matcher = matcher;
}

@Override
protected boolean requiresAuthentication(HttpServletRequest request, HttpServletResponse response) {
return matcher.matches(request);
}

@Override
Expand All @@ -54,14 +47,14 @@ public Authentication attemptAuthentication(HttpServletRequest request, HttpServ
protected void successfulAuthentication(HttpServletRequest request, HttpServletResponse response, FilterChain chain,
Authentication authResult) throws IOException, ServletException {
SecurityContextHolder.getContext().setAuthentication(authResult);
super.successfulAuthentication(request, response, chain, authResult);
chain.doFilter(request, response);
Copy link
Member

Choose a reason for hiding this comment

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

👍

}

@Override
protected void unsuccessfulAuthentication(HttpServletRequest request, HttpServletResponse response,
AuthenticationException failed) throws IOException, ServletException {
SecurityContextHolder.clearContext();
super.unsuccessfulAuthentication(request, response, failed);
super.getFailureHandler().onAuthenticationFailure(request, response, failed);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,18 @@

import java.util.List;

import org.springframework.security.core.AuthenticationException;

import com.zoopi.config.security.exception.CustomAuthenticationException;
import com.zoopi.exception.response.ErrorCode;
import com.zoopi.exception.response.ErrorResponse;

import lombok.Getter;

@Getter
public class JwtAuthenticationException extends AuthenticationException {
public class JwtAuthenticationException extends CustomAuthenticationException {
Copy link
Contributor

Choose a reason for hiding this comment

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

이런식으로 인증쪽 관련된 별도의 클래스를 만들어 사용하는 것도 굉장히 깔끔하고 좋은 방법인 것 같네요 👍

다만, 제가 Spring Security 를 사용하면서 느낀 부분이 굉장히 Spring Security 의존적으로 개발하게 되는 것 같다는 것인데요,
만약 기존에 활용되고 있는 최상위 Exception 클래스인 BusinessException 을 활용하는 방식에 대해서는 어떻게 생각하시는지 의견이 궁금합니다!!

제가 생각한 방법은 따로 Spring Security 에서 제공해주는 CustomAuthenticationEntryPoint, CustomAccessDeniedHandler 를 사용하지 않고, JwtAuthenticationFilter.unsuccessfulAuthentication() 메소드에서 BusinessException 을 상속한 별도의 Exception 으로 throw 하게 되면 에러 처리에 대한 포인트를 하나로 모을 수 있을것 같다는 생각이 문득 들어서요!!

@seonpilKim @dongkyunkimdev 이 부분에 대해서 함께 논의해보고 싶습니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

엇 혹시 filter에서 발생한 예외도 Advice가 처리할 수 있을까요?
저는 Advice를 이용한 예외 처리는 DispatcherServlet에서 담당하는 것으로 알고 있었는데, 제가 잘못 알고 있는 부분이 있다면 지적 부탁드려도 괜찮을까요?!

Copy link
Member

@dongkyunkimdev dongkyunkimdev Jul 2, 2022

Choose a reason for hiding this comment

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

제가 알기로 Filter는 Web Application Context 상에서 동작해서 선필님이 말씀해주신 것처럼 context가 달라 advice에서 처리하는 에러 핸들링이 불가능한 것으로 알고 있습니다!
찬준님이 말씀해주신 것처럼 unsuccessfulAuthentication 에서 CustomException을 던져보았는데, advice에서 받지 못하고 기본으로 제공되는 error response가 발생하는 것 같더라구요
filter에서 발생한 예외를 advice에서 받을 수 있게 하는 어떤 방법이 있는걸까요??
image

Copy link
Contributor

Choose a reason for hiding this comment

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

훔... 그렇군요!! 왜 된다고 생각했는지 모르겠는데 좀더 알아볼걸 그랬네요 ㅠ__ㅠ
감사합니다 ㅎㅎㅎ

Copy link
Member

Choose a reason for hiding this comment

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

저도 개념, 기술적으로 모르거나 애매하게 아는 부분이 엄청 많아서, 이런 논의들을 하면서 모르는 것도 알아가고, 애매하게 아는 것은 더 확실하게 알게 되니까 되게 좋은 것 같습니다!!
감사합니다 찬준님 선필님 👍👍


private final List<ErrorResponse.FieldError> errors;

public JwtAuthenticationException(List<ErrorResponse.FieldError> errors) {
super(ErrorCode.AUTHENTICATION_FAILURE.getMessage());
this.errors = errors;
super(ErrorCode.AUTHENTICATION_FAILURE, errors);
}

}
Loading