-
Notifications
You must be signed in to change notification settings - Fork 0
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
Refactor: 예외 처리 관련 리팩토링 #92
Changes from 1 commit
b1b53c9
0cc344e
ae3eb3f
0ba9287
3c5d0fb
9485078
1b47824
3680639
a52eb35
1f606ab
82deeaf
e73fa7b
079a7d3
b7fb8ae
34781b6
7700318
ac2b414
d4bd2c3
a377d8f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,10 @@ | ||
package com.example.sinitto.auth.service; | ||
|
||
import com.example.sinitto.auth.dto.TokenResponse; | ||
import com.example.sinitto.common.exception.UnauthorizedException; | ||
import com.example.sinitto.common.exception.AccessTokenExpired; | ||
import com.example.sinitto.common.exception.ForceLogoutException; | ||
import com.example.sinitto.common.exception.RefreshTokenStolen; | ||
import io.jsonwebtoken.Claims; | ||
import io.jsonwebtoken.Jwts; | ||
import io.jsonwebtoken.SignatureAlgorithm; | ||
import org.springframework.beans.factory.annotation.Value; | ||
|
@@ -52,14 +55,19 @@ public String generateRefreshToken(String email) { | |
|
||
|
||
public String extractEmail(String token) { | ||
var claims = Jwts.parserBuilder() | ||
.setSigningKey(secretKey) | ||
.build() | ||
.parseClaimsJws(token) | ||
.getBody(); | ||
Claims claims; | ||
try { | ||
claims = Jwts.parserBuilder() | ||
.setSigningKey(secretKey) | ||
.build() | ||
.parseClaimsJws(token) | ||
.getBody(); | ||
} catch (Exception e) { | ||
throw new ForceLogoutException(e.getMessage()); | ||
} | ||
|
||
if (claims.getExpiration().before(new Date())) { | ||
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. 요부분이 쪼금 걸리네요... 프론트엔드 기준으로 생각했을 때 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. 그러네요! 말씀대로 무한루프가 발생이 되겠네요. 중요한 이슈;;; 이런건 어떻게 보셨는지 대단하시네요 👍 Forbidden 이나 BadReqeust 둘 중 하나가 로 정하면 적절할까 싶네요? (Forbidden이 좀더 이 상황에서 맞는거 같기도합니다) 아직 이 이슈 못보신 분들 보시고 이견없으시면 수정 바로 진행하겠습니다! 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. 네네 forbidden이나 bad request로 해도 될 것 같긴 한데, refreshToken에 오류가 있는 경우 로그아웃과 똑같은 로직이 될 것 같아서(프론트엔드에서는 세션 저장소 등에 저장되어 있는 jwt 삭제, 백엔드는 redis에서 제거) 아에 custom http status로 해도 괜찮을 것 같습니다! 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. 수정사항 위 커밋에 있습니다. 간단한 코멘트도 달아놓았는데 천천히 확인해주시면 감사하겠습니다! 지호님이 디테일하게 수정사항 제안주신걸 토대로 수정하였습니다. |
||
throw new UnauthorizedException("토큰이 만료되었습니다. 재로그인이 필요합니다."); | ||
throw new AccessTokenExpired("액세스 토큰이 만료되었습니다. 리프레시 토큰으로 다시 액세스 토큰을 발급받으세요."); | ||
} | ||
|
||
return claims.getSubject(); | ||
|
@@ -69,8 +77,13 @@ public TokenResponse refreshAccessToken(String refreshToken) { | |
String email = extractEmail(refreshToken); | ||
|
||
String storedRefreshToken = redisTemplate.opsForValue().get(email); | ||
if (storedRefreshToken == null || !storedRefreshToken.equals(refreshToken)) { | ||
throw new UnauthorizedException("만료되거나 이미 한번 사용된 리프레쉬 토큰입니다. 재로그인이 필요합니다."); | ||
|
||
if (storedRefreshToken == null) { | ||
throw new ForceLogoutException("토큰이 만료되었습니다. 재로그인이 필요합니다."); | ||
} | ||
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. ForceLogoutException(460)원인: '리프레쉬토큰이 만료된 경우' 에러 |
||
|
||
if (!storedRefreshToken.equals(refreshToken)) { | ||
throw new RefreshTokenStolen("이미 한번 사용된 리프레시 토큰입니다. 리프레시 토큰이 탈취되었을 가능성이 있습니다."); | ||
} | ||
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. RefreshTokenStolen(462)원인: '리프레쉬토큰이 이미 사용된 경우' 에러 |
||
|
||
redisTemplate.delete(email); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
package com.example.sinitto.common.exception; | ||
|
||
public class AccessTokenExpired extends RuntimeException { | ||
|
||
public AccessTokenExpired(String message) { | ||
super(message); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
package com.example.sinitto.common.exception; | ||
|
||
public class ForceLogoutException extends RuntimeException { | ||
|
||
public ForceLogoutException(String message) { | ||
super(message); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
package com.example.sinitto.common.exception; | ||
|
||
public class RefreshTokenStolen extends RuntimeException { | ||
|
||
public RefreshTokenStolen(String message) { | ||
super(message); | ||
} | ||
} |
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.
ForceLogoutException(460)
원인: '알수없는 이유로 토큰이 잘못 넣어져 전달된 상황' 에러
클라이언트 유도: 로그아웃 이후 재로그인창까지 페이지 이동시키는 로직
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.
500번대 응답코드가 안나오게 수정해보았습니다. 혹시 try-catch 하는 위치가 별로라서 resolver 등 다른곳으로 옮기고 싶으시면 말씀주세요!
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.
제 생각에도 아마 여기가 베스트가 아닌가 싶네요...! 좋은 것 같습니다 ㅎㅎ