-
Notifications
You must be signed in to change notification settings - Fork 97
[그리디] 김민욱 Spring Data JPA 4, 5단계 제출합니다. #263
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
hapdaypy
wants to merge
61
commits into
next-step:hapdaypy
Choose a base branch
from
hapdaypy:hapdaypy3
base: hapdaypy
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
Changes from 49 commits
Commits
Show all changes
61 commits
Select commit
Hold shift + click to select a range
89c3a69
feat: 토큰 생성 및 검증 기능 추가
mgim9316-a11y 1e6f4cf
feat: 로그인 API 구현
mgim9316-a11y c348327
feat: 서버의 토큰 발급 확인 테스트 코드
mgim9316-a11y 29152d3
refactor: resorver가 토큰을 받는 형태로 수정
mgim9316-a11y f084866
refactor: resorver가 토큰을 받는 형태로 수정
mgim9316-a11y da4f4ec
refactor: 예약 생성 시 ReservationReqeust의 name이 없는 경우 Cookie에 담긴 정보를 활용
mgim9316-a11y 0853489
fix: 2단계 테스트 코드 오류 수정
mgim9316-a11y 40bb963
커밋테스트
mgim9316-a11y 0c8710c
커밋테스트
mgim9316-a11y 0a5c76b
커밋테스트
mgim9316-a11y cdaddb5
커밋테스트
mgim9316-a11y 50c51c3
커밋테스트
hapdaypy 76ce709
refactor: JWT 생성/검증 책임 클래스 분리
hapdaypy 5b27027
refactor: 네이밍 수정 및 로그인한 회원의 정보를 담는 객체 분리
hapdaypy 7479e06
refactor: 네이밍 수정 및 로그인한 회원의 정보를 담는 객체 분리
hapdaypy bd2b014
refactor: 쿠키 관련 로직을 분리
hapdaypy 4aff64a
refactor: simpledbcInsert로 변경
hapdaypy b2e568f
refactor: ReservationDao의 조건 조회를 NamedParameterJdbcTemplate 로 바꿈
hapdaypy 66ce159
feat: “내 예약” 화면이 호출하는 API
hapdaypy 506eceb
feat: 공통 mapper 메서드를 추가
hapdaypy fbadb3f
refactor: 로그인 실패 조건식 수정
hapdaypy 446bd84
feat: 에러 응답 메시지 추가
hapdaypy 5870f17
refactor: 애노테이션을 옵션으로 분기하지 않고, Optional로 선택 가능성 표현
hapdaypy a862274
refactor: 인증 패키지 분리
hapdaypy d3c5540
refactor: 가독성을 위해 코드 분리
hapdaypy 68ab241
refactor: AuthUser, AdminOnly 권한 분리
hapdaypy 765d7a4
refactor: 인텁센터 로직 수정
hapdaypy b207c49
feat: 예외 처리를 상태코드로 나눔
hapdaypy ce3992d
refactor: DB 조회 데이터가 0개일 경우 에러를 NotFoundException 로 반환
hapdaypy cc91e2a
feat: 생성 멱등성을 검증하는 로직 추가
hapdaypy dbf7630
feat: 일반 사용자 기능과 관리자 기능을 인텁셉터를 도입하여 분리
hapdaypy 6a6e810
refactor: controller의 예외 처리 로직을 service로 반경
hapdaypy da6a387
refactor: jjwt의 JwtParser 사용
hapdaypy 654fff7
refactor: 사용자의 role을 enum으로 관리
hapdaypy 319ac52
feat: JWT access/refresh token 발급 흐름 추가
hapdaypy c9f3185
refactor: DTO 성격을 record 형태로 변경 및 record 호출 형태로 수정
hapdaypy 24416b3
refactor: raw type 을 구체적 타입으로 수정
hapdaypy bace53d
refactor: record 형태로 수정
hapdaypy 87aca69
fix: cherry-pick conflict cleanup
hapdaypy 41e1d3a
refactor:수동 getter를 @Getter 로 정리
hapdaypy 8374a94
feat: @Transactional 사용
hapdaypy 8c10705
feat: @Entity로 바꿈
hapdaypy 737af23
refactor: EntityManager 기반으로 교체
hapdaypy f6b104e
feat: 테마 기능 서비스 생성
hapdaypy 74b7d5e
refactor: 컨트롤러에서 DAO, 서비스 의존성 제거
hapdaypy 8aebdb7
refactor: optional 추가
hapdaypy 34599e8
refactor: 예약과 회원을 JPA 연관관계로 매핑
hapdaypy e0ac584
refactor: 상속하는 파일들 패키지로 분리
hapdaypy e0bca6d
refactor: 에러메시지를 enum으로 관리
hapdaypy 8a2b3fd
refactor: 사용되지 않은 생성자 삭제
hapdaypy 2387c0f
refactor: RuntimeException 전체를 잡아 AuthenticationException으로 변환하는 부분 수정
hapdaypy 45bb709
feat: 토큰 파싱 실패시, AuthenticationException으로 반환
hapdaypy 815c598
refactor: @Value로 secret 주입받아 key를 만들도록 변경
hapdaypy d914306
refactor: isValidSignature()를 제거
hapdaypy 255dcae
feat: 이메일 컬럼 매핑 추가
hapdaypy 0c00db4
refactor: getSingleResult()에서 발생하는 예외 사용
hapdaypy f764dd3
refactor: 회원이 없을 때 예외를 던지지 않고Optional<Member>를 반환
hapdaypy 6759fac
refactor: 서비스에서 실제 엔티티를 조회한 뒤 DAO에 넘기도록 변경
hapdaypy b45a1b0
refactor: join과 DTO projection 을 사용하도록 리팩토링
hapdaypy 39fa068
refactor: id를 받는 생성자 삭제
hapdaypy c6d7687
merge: sync hapdaypy base
hapdaypy 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 hidden or 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 hidden or 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 |
|---|---|---|
| @@ -1,14 +1,75 @@ | ||
| package roomescape; | ||
|
|
||
| import lombok.Getter; | ||
| import org.springframework.dao.DataIntegrityViolationException; | ||
| import org.springframework.http.HttpStatus; | ||
| import org.springframework.http.ResponseEntity; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
| import roomescape.exception.AuthenticationException; | ||
| import roomescape.exception.AuthorizationException; | ||
| import roomescape.exception.ConflictException; | ||
| import roomescape.exception.ErrorCode; | ||
| import roomescape.exception.NotFoundException; | ||
| import org.springframework.web.bind.annotation.ControllerAdvice; | ||
| import org.springframework.web.bind.annotation.ExceptionHandler; | ||
|
|
||
| @ControllerAdvice | ||
| @ControllerAdvice // 아 컨트롤러에서 발생한 예외를 가로챔 ~ ㅋ | ||
| public class ExceptionController { | ||
| private static final Logger log = LoggerFactory.getLogger(ExceptionController.class); | ||
|
|
||
| @ExceptionHandler(AuthenticationException.class) | ||
| public ResponseEntity<ErrorResponse> handleAuthenticationException(AuthenticationException e) { | ||
| return error(e.getErrorCode()); | ||
| } | ||
|
|
||
| @ExceptionHandler(AuthorizationException.class) | ||
| public ResponseEntity<ErrorResponse> handleAuthorizationException(AuthorizationException e) { | ||
| return error(e.getErrorCode()); | ||
| } | ||
|
|
||
| @ExceptionHandler(NotFoundException.class) | ||
| public ResponseEntity<ErrorResponse> handleNotFoundException(NotFoundException e) { | ||
| log.warn("Resource not found: {}", e.getMessage()); | ||
| return error(e.getErrorCode()); | ||
| } | ||
|
|
||
| @ExceptionHandler({ConflictException.class, DataIntegrityViolationException.class}) | ||
| public ResponseEntity<ErrorResponse> handleConflictException(Exception e) { | ||
| if (e instanceof ConflictException conflictException) { | ||
| return error(conflictException.getErrorCode()); | ||
| } | ||
| return error(ErrorCode.CONFLICT_REQUEST); | ||
| } | ||
|
|
||
| @ExceptionHandler(IllegalArgumentException.class) | ||
| public ResponseEntity<ErrorResponse> handleIllegalArgumentException(IllegalArgumentException e) { | ||
| return error(HttpStatus.BAD_REQUEST, e.getMessage()); | ||
| } | ||
|
|
||
| @ExceptionHandler(Exception.class) | ||
| public ResponseEntity<Void> handleRuntimeException(Exception e) { | ||
| public ResponseEntity<ErrorResponse> handleException(Exception e) { | ||
| e.printStackTrace(); | ||
| return ResponseEntity.badRequest().build(); | ||
| return error(ErrorCode.INTERNAL_SERVER_ERROR); | ||
| } | ||
|
|
||
| private ResponseEntity<ErrorResponse> error(ErrorCode errorCode) { | ||
| return error(errorCode.getStatus(), errorCode.getMessage()); | ||
| } | ||
|
|
||
| private ResponseEntity<ErrorResponse> error(HttpStatus status, String message) { | ||
| return ResponseEntity.status(status) | ||
| .body(new ErrorResponse(status.value(), message)); | ||
| } | ||
|
|
||
| @Getter | ||
| public static class ErrorResponse { | ||
| private final int statusCode; | ||
| private final String message; | ||
|
|
||
| public ErrorResponse(int statusCode, String message) { | ||
| this.statusCode = statusCode; | ||
| this.message = message; | ||
| } | ||
| } | ||
| } |
This file contains hidden or 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 hidden or 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,31 @@ | ||
| package roomescape; | ||
|
|
||
| import org.springframework.context.annotation.Configuration; | ||
| import org.springframework.web.method.support.HandlerMethodArgumentResolver; | ||
| import org.springframework.web.servlet.config.annotation.InterceptorRegistry; | ||
| import org.springframework.web.servlet.config.annotation.WebMvcConfigurer; | ||
| import roomescape.auth.AdminInterceptor; | ||
| import roomescape.auth.AuthUserArgumentResolver; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| @Configuration // Spring MVC 설정을 추가함 | ||
| public class WebConfig implements WebMvcConfigurer { | ||
| private final AuthUserArgumentResolver authUserArgumentResolver; | ||
| private final AdminInterceptor adminInterceptor; | ||
|
|
||
| public WebConfig(AuthUserArgumentResolver authUserArgumentResolver, AdminInterceptor adminInterceptor) { | ||
| this.authUserArgumentResolver = authUserArgumentResolver; | ||
| this.adminInterceptor = adminInterceptor; | ||
| } | ||
|
|
||
| @Override | ||
| public void addArgumentResolvers(List<HandlerMethodArgumentResolver> resolvers) { | ||
| resolvers.add(authUserArgumentResolver); | ||
| } | ||
|
|
||
| @Override | ||
| public void addInterceptors(InterceptorRegistry registry) { | ||
| registry.addInterceptor(adminInterceptor); | ||
| } | ||
| } |
This file contains hidden or 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,47 @@ | ||
| package roomescape.auth; | ||
|
|
||
| import jakarta.servlet.http.HttpServletRequest; | ||
| import jakarta.servlet.http.HttpServletResponse; | ||
| import org.springframework.stereotype.Component; | ||
| import org.springframework.web.method.HandlerMethod; | ||
| import org.springframework.web.servlet.HandlerInterceptor; | ||
| import roomescape.exception.AuthenticationException; | ||
| import roomescape.exception.AuthorizationException; | ||
| import roomescape.member.MemberService; | ||
| import roomescape.member.MemberRole; | ||
|
|
||
| @Component | ||
| public class AdminInterceptor implements HandlerInterceptor { | ||
| private final AuthCookieProvider authCookieProvider; | ||
| private final MemberService memberService; | ||
|
|
||
| public AdminInterceptor(AuthCookieProvider authCookieProvider, MemberService memberService) { | ||
| this.authCookieProvider = authCookieProvider; | ||
| this.memberService = memberService; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) { | ||
| if (!(handler instanceof HandlerMethod handlerMethod) || !hasAdminOnly(handlerMethod)) { | ||
| return true; | ||
| } | ||
|
|
||
| try { | ||
| String token = authCookieProvider.extractAccessToken(request); | ||
| LoginMemberInfo loginMember = memberService.checkLogin(token); | ||
| if (MemberRole.ADMIN == loginMember.role()) { | ||
| return true; | ||
| } | ||
| throw new AuthorizationException(); | ||
| } catch (AuthorizationException e) { | ||
| throw e; | ||
| } catch (RuntimeException e) { | ||
| throw new AuthenticationException(); | ||
| } | ||
| } | ||
|
|
||
| private boolean hasAdminOnly(HandlerMethod handlerMethod) { | ||
| return handlerMethod.hasMethodAnnotation(AdminOnly.class) | ||
| || handlerMethod.getBeanType().isAnnotationPresent(AdminOnly.class); | ||
| } | ||
| } | ||
This file contains hidden or 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 roomescape.auth; | ||
|
|
||
| import java.lang.annotation.ElementType; | ||
| import java.lang.annotation.Retention; | ||
| import java.lang.annotation.RetentionPolicy; | ||
| import java.lang.annotation.Target; | ||
|
|
||
| @Target({ElementType.METHOD, ElementType.TYPE}) | ||
| @Retention(RetentionPolicy.RUNTIME) | ||
| public @interface AdminOnly { | ||
| } |
This file contains hidden or 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,88 @@ | ||
| package roomescape.auth; | ||
|
|
||
| import jakarta.servlet.http.Cookie; | ||
| import jakarta.servlet.http.HttpServletRequest; | ||
| import org.springframework.stereotype.Component; | ||
| import roomescape.exception.AuthenticationException; | ||
|
|
||
| @Component | ||
| public class AuthCookieProvider { | ||
| private static final String ACCESS_TOKEN_COOKIE_NAME = "accessToken"; | ||
| private static final String REFRESH_TOKEN_COOKIE_NAME = "refreshToken"; | ||
| private static final String LEGACY_TOKEN_COOKIE_NAME = "token"; | ||
| private static final String COOKIE_PATH = "/"; | ||
|
|
||
| public Cookie createLoginCookie(String token) { | ||
| return createAccessTokenCookie(token); | ||
| } | ||
|
|
||
| public Cookie createAccessTokenCookie(String token) { | ||
| return createCookie(ACCESS_TOKEN_COOKIE_NAME, token); | ||
| } | ||
|
|
||
| public Cookie createRefreshTokenCookie(String token) { | ||
| return createCookie(REFRESH_TOKEN_COOKIE_NAME, token); | ||
| } | ||
|
|
||
| private Cookie createCookie(String name, String value) { | ||
| Cookie cookie = new Cookie(name, value); | ||
| cookie.setHttpOnly(true); | ||
| cookie.setPath(COOKIE_PATH); | ||
| return cookie; | ||
| } | ||
|
|
||
| public Cookie createLogoutCookie() { | ||
| return createLogoutCookie(LEGACY_TOKEN_COOKIE_NAME); | ||
| } | ||
|
|
||
| public Cookie createLogoutAccessTokenCookie() { | ||
| return createLogoutCookie(ACCESS_TOKEN_COOKIE_NAME); | ||
| } | ||
|
|
||
| public Cookie createLogoutRefreshTokenCookie() { | ||
| return createLogoutCookie(REFRESH_TOKEN_COOKIE_NAME); | ||
| } | ||
|
|
||
| private Cookie createLogoutCookie(String name) { | ||
| Cookie cookie = new Cookie(name, ""); | ||
| cookie.setHttpOnly(true); | ||
| cookie.setPath(COOKIE_PATH); | ||
| cookie.setMaxAge(0); | ||
| return cookie; | ||
| } | ||
|
|
||
| public String extractToken(HttpServletRequest request) { | ||
| return extractAccessToken(request); | ||
| } | ||
|
|
||
| public String extractAccessToken(HttpServletRequest request) { | ||
| return extractToken(request, ACCESS_TOKEN_COOKIE_NAME, LEGACY_TOKEN_COOKIE_NAME); | ||
| } | ||
|
|
||
| public String extractRefreshToken(HttpServletRequest request) { | ||
| return extractToken(request, REFRESH_TOKEN_COOKIE_NAME); | ||
| } | ||
|
|
||
| private String extractToken(HttpServletRequest request, String... cookieNames) { | ||
| Cookie[] cookies = request.getCookies(); | ||
| if (cookies == null) { | ||
| throw new AuthenticationException(); | ||
| } | ||
|
|
||
| for (Cookie cookie : cookies) { | ||
| if (contains(cookieNames, cookie.getName())) { | ||
| return cookie.getValue(); | ||
| } | ||
| } | ||
| throw new AuthenticationException(); | ||
| } | ||
|
|
||
| private boolean contains(String[] values, String target) { | ||
| for (String value : values) { | ||
| if (value.equals(target)) { | ||
| return true; | ||
| } | ||
| } | ||
| return false; | ||
| } | ||
| } |
This file contains hidden or 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 roomescape.auth; | ||
|
|
||
| import java.lang.annotation.ElementType; | ||
| import java.lang.annotation.Retention; | ||
| import java.lang.annotation.RetentionPolicy; | ||
| import java.lang.annotation.Target; | ||
|
|
||
| @Target(ElementType.PARAMETER) | ||
| @Retention(RetentionPolicy.RUNTIME) | ||
| public @interface AuthUser { | ||
| } |
78 changes: 78 additions & 0 deletions
78
src/main/java/roomescape/auth/AuthUserArgumentResolver.java
This file contains hidden or 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,78 @@ | ||
| package roomescape.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 roomescape.exception.AuthenticationException; | ||
| import roomescape.member.MemberService; | ||
|
|
||
| import java.lang.reflect.ParameterizedType; | ||
| import java.lang.reflect.Type; | ||
| import java.util.Optional; | ||
|
|
||
| @Component | ||
| public class AuthUserArgumentResolver implements HandlerMethodArgumentResolver { | ||
| private final MemberService memberService; | ||
| private final AuthCookieProvider authCookieProvider; | ||
|
|
||
| public AuthUserArgumentResolver(MemberService memberService, AuthCookieProvider authCookieProvider) { | ||
| this.memberService = memberService; | ||
| this.authCookieProvider = authCookieProvider; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean supportsParameter(MethodParameter parameter) { | ||
| return parameter.hasParameterAnnotation(AuthUser.class) | ||
| && (LoginMemberInfo.class.isAssignableFrom(parameter.getParameterType()) | ||
| || isOptionalLoginMemberInfo(parameter)); | ||
| } | ||
|
|
||
|
|
||
| @Override | ||
| public Object resolveArgument( | ||
| MethodParameter parameter, | ||
| ModelAndViewContainer mavContainer, | ||
| NativeWebRequest webRequest, | ||
| WebDataBinderFactory binderFactory | ||
| ) { | ||
| boolean optional = isOptionalLoginMemberInfo(parameter); | ||
| HttpServletRequest request = webRequest.getNativeRequest(HttpServletRequest.class); | ||
| if (request == null) { | ||
| return handleAuthenticationFailure(optional); | ||
| } | ||
|
|
||
| try { | ||
| String token = authCookieProvider.extractAccessToken(request); | ||
| LoginMemberInfo loginMember = memberService.checkLogin(token); | ||
| if (optional) { | ||
| return Optional.of(loginMember); | ||
| } | ||
| return loginMember; | ||
| } catch (RuntimeException e) { | ||
| return handleAuthenticationFailure(optional); | ||
| } | ||
|
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. 이 부분도 마찬가지로 모든 RuntimeException를 감싸버리면 장애 원인을 찾기 어려울 것 같아요
Author
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. 해당 부분을 위의 답변에 따라 수정해 보겠습니다! |
||
| } | ||
|
|
||
| private Object handleAuthenticationFailure(boolean optional) { | ||
| if (optional) { | ||
| return Optional.empty(); | ||
| } | ||
| throw new AuthenticationException(); | ||
| } | ||
|
|
||
| private boolean isOptionalLoginMemberInfo(MethodParameter parameter) { | ||
| if (!Optional.class.isAssignableFrom(parameter.getParameterType())) { | ||
| return false; | ||
| } | ||
| Type genericParameterType = parameter.getGenericParameterType(); | ||
| if (!(genericParameterType instanceof ParameterizedType parameterizedType)) { | ||
| return false; | ||
| } | ||
| Type actualType = parameterizedType.getActualTypeArguments()[0]; | ||
| return LoginMemberInfo.class.getName().equals(actualType.getTypeName()); | ||
| } | ||
| } | ||
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.
RuntimeException을 모두 AuthenticationException으로 변환하면 인증 실패와 서버 내부 오류를 구분하기 어려워질 것 같습니다.
인증 실패에 해당하는 예외만 변환하고, 다른 예외는 그대로 전파하는 편이 장애 원인을 파악하기 쉬울 것 같아요!
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.
RumtimException 으로 인증을 실패를 했을 경우도, 결과적으로는 인증에 실패한 것이기 때문에AuthenticationException 를 반환을 해야한다고 생각했었습니다!
하지만 서버의 오류, 잘못된 enum 반환, 예상하지 못한 데이터 형태를 처리해야할 경우 모두 401로 처리하는 것은 잘못되었다고 생각합니다 !
애초에 에러를 반환을 해야하는 이유가, 개발자가 원인을 파악하여 해결하기 위함인데 서비스 장애를 가지고 AuthenticationException 에러를 발생시키는 것은 본질적인 원인을 알려주지 못한 것이 때문에 해당 부분을 수정해야 한다고 생각합니다 !
올바른 에러 "원인" 을 반환할 수 있도록(서비스 vs 로그인) 리팩토링 해보겠습니다!
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.
추가로 궁금한 사항이 있는데 쿠키가 없거나 토큰이 없을 경우에는 로그인이 실패 RuntimeException 인지 AuthenticationException 인지 어떻게 정할 수 있을까요 ? 현실세계의 상황을 고려해서 개발자들끼리 정하면 될까요?
쿠키가 없는 경우에 서비스가 진행되지 않아서 RuntimeException 일 수 있고 인증이 되지 않았기 때문에 AuthenticationException 일 수 있기 때문에 궁금증이 생겨서 여쭤봅니다 !
상준 리뷰어님은 어떤 기준으로 판단하시나요 !?
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.
저는 쿠키가 없는 경우는, AuthenticationException으로 처리할 것 같습니다. 제가 말씀드렸던 RuntimeException을 모두 AuthenticationException로 감싸는 방식이 위험하다고 했던 이유는 "예상하지 못했던 예외 상황"이 AuthenticationException로 감싸져 정상적인 예외 후속 처리로 넘어갈 가능성이 있기 때문입니다.
쿠키가 없는 것은 저희가 예상 가능한 예외 상황입니다. 이 예외 상황이 발생했을 때 저희는 클라이언트에게 "쿠키가 없다"는 예외 상황을 적절한 예외 코드와 메시지로 전달할 필요가 있습니다. 이렇게 예상 가능한 예외는 일반적으로 후속처리를 하곤합니다. 하지만 예상치 못하게 DB와의 IO작업에서 네트워크 오류와 같은 예외 상황이 발생했는데, 이때 이 예외를 AuthenticationException로 처리한다면 어플리케이션은 정상적인 흐름인데도 불구하고 예외상황이라고 감지하고 "쿠키가 없다"와 같은 예외와 같은 후속처리로 퉁쳐질 가능성이 있습니다.
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.
예상할 수 있는 예외와 예상할 수 없는 예외라는 판단 기준을 가지고 예외를 처리하면 되겠군요 ! ..
구체적인 설명 감사합니다 !
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.
반영커밋: 2387c0f