Refactor: 예외 처리 관련 리팩토링

[zzoe2346]

#️⃣ 연관된 이슈

ex) #이슈번호, #이슈번호

#78

📝 작업 내용

이번 PR에서 작업한 내용을 간략히 설명해주세요.(이미지 첨부 가능)

• 각 도메인의 Advice 를 참고하여 예외 교체
• 꼼꼼히 하려 노력하였고 여러번 확인하였지만 실수한 부분이 있을 수 있으니 각자 맡으신 도메인에서 던져지는 예외 확인 부탁드립니다
• 수정요청 환영👍🙇🏻

스크린샷 (선택)

💬 리뷰 요구사항(선택)

리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요

ex) 메서드 XXX의 이름을 더 잘 짓고 싶은데 혹시 좋은 명칭이 있을까요?

⏰ 현재 버그

✏ Git Close

close #이슈번호

close #78

[2iedo]

Exception 관련 수정 부분 확인했습니다! 다른 부분은 괜찮은 것 같은데, jwt와 관련해서 코멘트 하나 남겼습니다! 고생하셨어요~

[2iedo]

요부분이 쪼금 걸리네요... 프론트엔드 기준으로 생각했을 때
1. accessToken 만료 2. UnAuthorizedException 발생 3. UnAuthorizedException에 해당하는 httpStatus 전송 4. 프론트엔드에서 해당 httpStatus 받을 경우 refreshToken 전송하여 accessToken 재발급 요청
이런 과정이 될 것 같은데, 지금은 accessToken 만료될 때와 refreshToken 만료될 때 둘 다 UnAuthorizedException이 발생되어서, refreshToken이 만료되었을 때에도 계속 만료된 refreshToken을 요청하여 무한루프가 될 것 같습니다.
refreshToken이 만료될 경우에는 다른 Exception으로 처리하면 좋을 것 같습니다!

[zzoe2346]

그러네요! 말씀대로 무한루프가 발생이 되겠네요. 중요한 이슈;;; 이런건 어떻게 보셨는지 대단하시네요 👍 Forbidden 이나 BadReqeust 둘 중 하나가 로 정하면 적절할까 싶네요? (Forbidden이 좀더 이 상황에서 맞는거 같기도합니다)

아직 이 이슈 못보신 분들 보시고 이견없으시면 수정 바로 진행하겠습니다!

[2iedo]

네네 forbidden이나 bad request로 해도 될 것 같긴 한데, refreshToken에 오류가 있는 경우 로그아웃과 똑같은 로직이 될 것 같아서(프론트엔드에서는 세션 저장소 등에 저장되어 있는 jwt 삭제, 백엔드는 redis에서 제거) 아에 custom http status로 해도 괜찮을 것 같습니다!

[zzoe2346]

ac2b414

수정사항 위 커밋에 있습니다. 간단한 코멘트도 달아놓았는데 천천히 확인해주시면 감사하겠습니다! 지호님이 디테일하게 수정사항 제안주신걸 토대로 수정하였습니다.

[eunsoni]

예외 처리 메서드 적절하게 작성해주신 것 같아요!
회원가입 쪽 메서드 부분에서 궁금한 점이 있어서 리뷰남깁니다

[eunsoni]

예외의 목적이 이메일이 이미 존재함을 알리는 것이라서 "고유하지 않다"는 의미의 NotUniqueException은 적절한 것 같은데 혹시 수정하신 이유가 따로 있으신가요?
MultiStatusException은 여러 상태와 관련된 문제처럼 느껴질 수 있어 상황에 대한 구체성을 잘 전달하지 못하지 않을까 싶어서요!

[zzoe2346]

감사합니다! 은선님 말씀대로 MultiStatusException가 좀 넓은 의미를 내포하는거 같네요. 지호님과도 의논하여 MultiStatusException -> ConflictException 으로 변경하였습니다!

[GitJIHO]

엄청 많았는데 통합하고 리팩토링 해주셔서 감사드립니다 :)
고생하셨습니다 👍

Unauthorized 상태코드 분리 필요성 관련하여 코멘트 남겼습니다!

[GitJIHO]

1. TokenService 작성할 때 토큰 형식에 대한 유효성 처리는 제외했었습니다. 단, resolver에서

    @Override
    public Object resolveArgument(MethodParameter parameter, ModelAndViewContainer mavContainer, NativeWebRequest webRequest, WebDataBinderFactory binderFactory) throws Exception {
        HttpServletRequest request = (HttpServletRequest) webRequest.getNativeRequest();

        String authorizationHeader = request.getHeader("Authorization");
        if (authorizationHeader == null || !authorizationHeader.startsWith("Bearer ")) {
            throw new UnauthorizedException("토큰이 없거나, 헤더 형식에 맞지 않습니다.");
        }

        String token = authorizationHeader.substring(7);
        return memberIdProvider.getMemberIdByToken(token);
    }

위와 같은 로직이 존재하여 만약 프론트에서 Bearer를 붙이지 않거나, authorizationHeader에 토큰 자체를 넣지 않는다면 로직에 따라 현재는 UnauthorizedException이 발생할 것입니다!

그러나, 만약 토큰을 넣었는데 그 토큰 형식이 잘못되는 경우

ERROR 79658 --- [sinnitto-example] [nio-8080-exec-3] o.a.c.c.C.[.[.[/].[dispatcherServlet]    : 
Servlet.service() for servlet [dispatcherServlet] in context with path [] threw exception [Request processing failed: io.jsonwebtoken.MalformedJwtException: 
JWT strings must contain exactly 2 period characters. Found: 0] with root cause

위와 같은 오류가 뜨는데, 실제로 지금 가장 많이 백로그에 찍히는 에러입니다
해당 오류는 말그대로 Bearer도 잘 붙였고, authorizationHeader에도 토큰을 넣었지만 토큰 형식이 맞지 않아 생기는 오류입니다.

또한 자주뜨는 에러 로그가

ERROR 79658 --- [sinnitto-example] [nio-8080-exec-4] o.a.c.c.C.[.[.[/].[dispatcherServlet]    : Servlet.service() for servlet [dispatcherServlet] in context with path [] threw exception [Request processing failed: io.jsonwebtoken.security.SignatureException: 
JWT signature does not match locally computed signature. JWT validity cannot be asserted and should not be trusted.] with root cause

위 에러 로그가 있는데 이건 Bearer, 헤더, 형식은 다 맞고, 토큰 값이 잘못된 경우입니다.

지금 아마 서버에서 위와 같은 상황에 대해서는 swagger로 볼 때 500에러를 내고 있는걸로 알고 있는데
위 두 상황에 대한 구체적인 조건문을 통한 상태코드를 지정하거나,
try-catch문을 통해 Unauthorized가 아닌 상태코드를 지정해야 할 것 같습니다.

물론 실 서비스에서는 아마 토큰을 잘못 넣거나 예전 토큰을 넣는 일이 없을 것 같긴 한데, 동시성 문제나 여러 엣지케이스 상황에서 혹시나 위 상황과 동일한 오류가 생길수도 있지 않나 하는 마음에,, ㅎㅎ

만약 위와같은 상황이 나타났을 경우에, 오류 페이지를 띄우는 다른 일반 에러와 같이 처리해도 괜찮을 것 같아서 일반적인 상태코드를 보내도 괜찮을 것 같긴 한데, 단순 재로그인을 유도하기만 하면 해결될 법 하지 않나..? 하는 생각에 다른 상태코드를 넣는게 좋지 않을까 싶어 말씀드렸습니다! <- 다른 상태코드와 관련해서는 2. 에서 추가로 말씀드리겠습니다!

reslover에 조건을 추가하거나 인터셉터나 필터를 추가하는 리팩토링 과정에서 추가해도 괜찮을 것 같습니다!

2. 위의 내용에서 이어지는 내용인데, 지금 상태코드를 통해 저희가 프론트에서 단순 오류 페이지를 띄울지 추가 행동을 유도하는 로직을 실행시킬지 결정하기로 정했기 때문에 Unathorization exception을 분리하는게 어떨까 싶습니다.

위에서 도훈님께서 말씀하신 내용처럼

• accessToken의 유효시간이 끝나는 에러 -> 리프레쉬를 유도하는 상태코드 A
• 만약 accessToken자체에서 오류가 나는 에러 -> 로그아웃(or 로그아웃 이후 재로그인창까지 페이지 이동시키는 로직)을 유도하는 상태코드 B
• 리프레쉬토큰이 만료된 경우 -> 로그아웃(or 로그아웃 이후 재로그인창까지 페이지 이동시키는 로직)을 유도하는 상태코드 B
  가 있을 수 있고

추가적인 예시로,

• 안부전화를 수정하려고 하는데 해당 시니어의 보호자가 아닐 때 에러(다른 이외의 기존 Unauthorized에러를 내보내는 상황도 마찬가지)-> 단순 권한이 없다 오류페이지 내보내는 상태코드 Unauthorized 그대로 유지
• 이미 한번 사용되거나 만료된 refreshToken으로 리프레쉬를 시도할 때 에러 -> (일반적인 Unauthorized 경고창이 아닌) 리프레쉬 토큰이 탈취당했다는 경고창과 함께 관리자에게 문의하라는 메시지를 띄우는 페이지를 유도하는 상태코드 C
• 위 1. 에서 말한 '알수없는 이유로 토큰이 잘못 넣어져 전달된 상황' 에러 -> 로그아웃(or 로그아웃 이후 재로그인창까지 페이지 이동시키는 로직)을 유도하는 상태코드 B

정도가 있을 것 같습니다.

위에 말씀드린 예시들이 지금 모두 하나의 동일한 Unauthorized 상태 코드로 처리 되고 있기 때문에 단순 경고창을 띄울 Unauthorized와 추가 행동 유도 or 다른 경고창 띄울 Unauthorized가 다른 상태코드로 분리가 필요할 것 같습니다!

[zzoe2346]

지호님! 정성스러운 글 감사드립니다!!! ㅎ.ㅎ 꼼꼼히 적어주시고 예시까지 만들어주셔서 이해도 잘되고 리팩토링에 도움이 많이 되네요 👍👍👍👍

ac2b414 이 커밋에 지호님이 말씀하신거 반영해보았습니다. 간단한 코멘트도 달아놓았는데 천천히 확인 해주시면 좋겠습니다!

제가 지호님 의도를 잘 파악하고 코드로 만들었는지 모르겠네요 ㅜ 이상하면 걍 롤백해도 됩니다!! 편히 봐주세욤

[GitJIHO]

엇 너무 완벽한 것 같습니다!
바로 해주시다니 ,, 대박
감사드립니다! 😄

[zzoe2346]

ForceLogoutException(460)

원인: '알수없는 이유로 토큰이 잘못 넣어져 전달된 상황' 에러
클라이언트 유도: 로그아웃 이후 재로그인창까지 페이지 이동시키는 로직

[zzoe2346]

500번대 응답코드가 안나오게 수정해보았습니다. 혹시 try-catch 하는 위치가 별로라서 resolver 등 다른곳으로 옮기고 싶으시면 말씀주세요!

[GitJIHO]

제 생각에도 아마 여기가 베스트가 아닌가 싶네요...! 좋은 것 같습니다 ㅎㅎ

[zzoe2346]

ForceLogoutException(460)

원인: '리프레쉬토큰이 만료된 경우' 에러
클라이언트 유도: 로그아웃 이후 재로그인창까지 페이지 이동시키는 로직

[zzoe2346]

RefreshTokenStolen(462)

원인: '리프레쉬토큰이 이미 사용된 경우' 에러
클라이언트 유도: 리프레쉬 토큰이 탈취당했다는 경고창과 함께 관리자에게 문의하라는 메시지를 띄우는 페이지를 유도

[GitJIHO]

반영된 부분 확인했습니다!
작업량이 많았을 텐데 고민하신 흔적이 보입니다 😭 👍 👍

[2iedo]

auth관련 응답 코드 다앙하게 나눈 부분 확인했습니다! 잘 만드신 것 같구 해당 상태 코드는 프론트 분들에게 공유하면 좋을 것 같아요! 고생하셨습니다~

[eunsoni]

피드백 반영된 것 확인했습니다~!
고생하셨어요:>