-
Notifications
You must be signed in to change notification settings - Fork 8
refactor: 문자열 토큰을 Token 클래스로 #297
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
Changes from 10 commits
66e47ed
f27233c
71a1ba0
af06df8
3f94b96
d33933e
dbf5d72
b09c584
dd91537
29ffd67
afd7db5
0794878
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 |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ | |
| import com.example.solidconnection.auth.dto.SignUpRequest; | ||
| import com.example.solidconnection.auth.dto.oauth.OAuthCodeRequest; | ||
| import com.example.solidconnection.auth.dto.oauth.OAuthResponse; | ||
| import com.example.solidconnection.auth.service.AccessToken; | ||
| import com.example.solidconnection.auth.service.AuthService; | ||
| import com.example.solidconnection.auth.service.CommonSignUpTokenProvider; | ||
| import com.example.solidconnection.auth.service.EmailSignInService; | ||
|
|
@@ -97,11 +98,10 @@ public ResponseEntity<SignInResponse> signUp( | |
| public ResponseEntity<Void> signOut( | ||
| Authentication authentication | ||
| ) { | ||
| String token = authentication.getCredentials().toString(); | ||
| if (token == null) { | ||
| if (!(authentication.getCredentials() instanceof AccessToken accessToken)) { // null or AccessToken 로 형변환 실패 | ||
| throw new CustomException(ErrorCode.AUTHENTICATION_FAILED, "토큰이 없습니다."); | ||
| } | ||
| authService.signOut(token); | ||
| authService.signOut(accessToken); | ||
|
Member
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. 현재 로그아웃시에 refreshToken은 무효화가 안되고 있는 것으로 보았는데 맞을까요?
Collaborator
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. |
||
| return ResponseEntity.ok().build(); | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,12 @@ | ||
| package com.example.solidconnection.auth.dto; | ||
|
|
||
| import com.example.solidconnection.auth.service.AccessToken; | ||
|
|
||
| public record ReissueResponse( | ||
| String accessToken) { | ||
| String accessToken | ||
| ) { | ||
|
|
||
| public static ReissueResponse from(AccessToken accessToken) { | ||
| return new ReissueResponse(accessToken.token()); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,14 @@ | ||
| package com.example.solidconnection.auth.dto; | ||
|
|
||
| import com.example.solidconnection.auth.service.AccessToken; | ||
| import com.example.solidconnection.auth.service.RefreshToken; | ||
|
|
||
| public record SignInResponse( | ||
| String accessToken, | ||
| String refreshToken | ||
| ) { | ||
|
|
||
| public static SignInResponse of(AccessToken accessToken, RefreshToken refreshToken) { | ||
| return new SignInResponse(accessToken.token(), refreshToken.token()); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| package com.example.solidconnection.auth.service; | ||
|
|
||
| public record AccessToken( | ||
| Subject subject, | ||
| String token | ||
| ) { | ||
|
|
||
| public AccessToken(String subject, String token) { | ||
| this(new Subject(subject), token); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,35 +1,29 @@ | ||
| package com.example.solidconnection.auth.service; | ||
|
|
||
|
|
||
| import com.example.solidconnection.auth.dto.ReissueRequest; | ||
| import com.example.solidconnection.auth.dto.ReissueResponse; | ||
| import com.example.solidconnection.config.security.JwtProperties; | ||
| import com.example.solidconnection.custom.exception.CustomException; | ||
| import com.example.solidconnection.siteuser.domain.SiteUser; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.stereotype.Service; | ||
| import org.springframework.transaction.annotation.Transactional; | ||
|
|
||
| import java.time.LocalDate; | ||
| import java.util.Objects; | ||
| import java.util.Optional; | ||
|
|
||
| import static com.example.solidconnection.custom.exception.ErrorCode.REFRESH_TOKEN_EXPIRED; | ||
| import static com.example.solidconnection.util.JwtUtils.parseSubject; | ||
|
|
||
| @RequiredArgsConstructor | ||
| @Service | ||
| public class AuthService { | ||
|
|
||
| private final AuthTokenProvider authTokenProvider; | ||
| private final JwtProperties jwtProperties; | ||
|
|
||
| /* | ||
| * 로그아웃 한다. | ||
| * - 엑세스 토큰을 블랙리스트에 추가한다. | ||
| * */ | ||
| public void signOut(String accessToken) { | ||
| authTokenProvider.generateAndSaveBlackListToken(accessToken); | ||
| public void signOut(AccessToken accessToken) { | ||
| authTokenProvider.addToBlacklist(accessToken); | ||
| } | ||
|
|
||
| /* | ||
|
|
@@ -45,19 +39,18 @@ public void quit(SiteUser siteUser) { | |
|
|
||
| /* | ||
| * 액세스 토큰을 재발급한다. | ||
| * - 요청된 리프레시 토큰과 동일한 subject 의 토큰이 저장되어 있으며 값이 일치할 경우, 액세스 토큰을 재발급한다. | ||
| * - 그렇지 않으면 예외를 반환한다. | ||
| * - 유효한 리프레시토큰이면, 액세스 토큰을 재발급한다. | ||
| * - 그렇지 않으면 예외를 발생시킨다. | ||
| * */ | ||
| public ReissueResponse reissue(ReissueRequest reissueRequest) { | ||
| // 리프레시 토큰 확인 | ||
| String requestedRefreshToken = reissueRequest.refreshToken(); | ||
|
Comment on lines
45
to
48
Contributor
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. 토큰 객체를 만들었는데 결국 바디로 String refreshToken을 받아서 사용하고 있는게 쪼끔 일관성이 떨어지는 거 같다는 생각이 들었는데 나중 pr에서 클라이언트는 그대로 작성해놓고보니 좀 비효율적인 거 같다는 생각도 드네요 이건 😅 |
||
| String subject = parseSubject(requestedRefreshToken, jwtProperties.secret()); | ||
| Optional<String> savedRefreshToken = authTokenProvider.findRefreshToken(subject); | ||
| if (!Objects.equals(requestedRefreshToken, savedRefreshToken.orElse(null))) { | ||
| if (!authTokenProvider.isValidRefreshToken(requestedRefreshToken)) { | ||
| throw new CustomException(REFRESH_TOKEN_EXPIRED); | ||
| } | ||
| // 액세스 토큰 재발급 | ||
| String newAccessToken = authTokenProvider.generateAccessToken(subject); | ||
| return new ReissueResponse(newAccessToken); | ||
| Subject subject = authTokenProvider.parseSubject(requestedRefreshToken); | ||
| AccessToken newAccessToken = authTokenProvider.generateAccessToken(subject); | ||
| return ReissueResponse.from(newAccessToken); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| package com.example.solidconnection.auth.service; | ||
|
|
||
| public interface BlacklistChecker { | ||
|
|
||
| boolean isTokenBlacklisted(String token); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| package com.example.solidconnection.auth.service; | ||
|
|
||
| public record RefreshToken( | ||
| Subject subject, | ||
| String token | ||
| ) { | ||
|
|
||
| RefreshToken(String subject, String token) { | ||
| this(new Subject(subject), token); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| package com.example.solidconnection.auth.service; | ||
|
|
||
| public record Subject( | ||
| String value | ||
| ) { | ||
| } |
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.
authentication.getCredentials() 여기에는 String type의 엑세스 토큰이 들어있지 않나요?
지금 방식이면 모든 토큰이 여기서 걸려서 예외가 터질 거 같아서요!
한 번 로그 찍어봤는데
Credentials Type: java.lang.String
Is String: true
Is AccessToken: false
이러고 바로 커스텀 예외 터지네요!
Uh oh!
There was an error while loading. Please reload this page.
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.
제가 커밋은 했었는데 푸쉬를 안했었네요😭
꼼꼼한 확인 감사합니다!
Uh oh!
There was an error while loading. Please reload this page.
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.
이 코드에 대해 추가 설명드리자면,
이 코드는 제가 “시큐리티에서 사용되는 JwtAuthentication 자체가 AccessToken 을 가지고 있게 하면 어떻게 될까?”
를 생각하며 작성했던 코드입니다. 😅
그런데 그렇게 하지 않은 이유는,
filter 레이어가 auth의 도메인격인 AccessToken을 직접 참조하는 것은 좋은 구조가 아니라고 판단했기 때문입니다.
(물론 지금도 JwtAuthentication가 SiteUser를 직접 참조하고있긴 한데, 짜피 이건 리팩러팅 대상이니깐요🤫 #299)
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.
이해했습니다!