-
Notifications
You must be signed in to change notification settings - Fork 8
refactor: accessToken 재발급을 refreshToken 쿠키를 통해 하도록 #452
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 all commits
a642f6b
011d855
7019c6a
2920527
3e20ee0
81ab68f
0a460e9
728e792
1e7db3c
e851aa2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3,7 +3,6 @@ | |||||||||||||||||||||||||||||||||||||||||||||
| import static com.example.solidconnection.common.exception.ErrorCode.REFRESH_TOKEN_EXPIRED; | ||||||||||||||||||||||||||||||||||||||||||||||
| import static com.example.solidconnection.common.exception.ErrorCode.USER_NOT_FOUND; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| import com.example.solidconnection.auth.dto.ReissueRequest; | ||||||||||||||||||||||||||||||||||||||||||||||
| import com.example.solidconnection.auth.dto.ReissueResponse; | ||||||||||||||||||||||||||||||||||||||||||||||
| import com.example.solidconnection.auth.token.TokenBlackListService; | ||||||||||||||||||||||||||||||||||||||||||||||
| import com.example.solidconnection.common.exception.CustomException; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -28,12 +27,8 @@ public class AuthService { | |||||||||||||||||||||||||||||||||||||||||||||
| * - 리프레시 토큰을 삭제한다. | ||||||||||||||||||||||||||||||||||||||||||||||
| * */ | ||||||||||||||||||||||||||||||||||||||||||||||
| public void signOut(String token) { | ||||||||||||||||||||||||||||||||||||||||||||||
| Subject subject = authTokenProvider.parseSubject(token); | ||||||||||||||||||||||||||||||||||||||||||||||
| long siteUserId = Long.parseLong(subject.value()); | ||||||||||||||||||||||||||||||||||||||||||||||
| SiteUser siteUser = siteUserRepository.findById(siteUserId) | ||||||||||||||||||||||||||||||||||||||||||||||
| .orElseThrow(() -> new CustomException(USER_NOT_FOUND)); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| AccessToken accessToken = authTokenProvider.generateAccessToken(subject, siteUser.getRole()); | ||||||||||||||||||||||||||||||||||||||||||||||
| SiteUser siteUser = authTokenProvider.parseSiteUser(token); | ||||||||||||||||||||||||||||||||||||||||||||||
| AccessToken accessToken = authTokenProvider.generateAccessToken(siteUser); | ||||||||||||||||||||||||||||||||||||||||||||||
| authTokenProvider.deleteRefreshTokenByAccessToken(accessToken); | ||||||||||||||||||||||||||||||||||||||||||||||
| tokenBlackListService.addToBlacklist(accessToken); | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
29
to
34
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. 1) signOut에서 새 토큰을 생성해 블랙리스트에 올리는 버그가 있습니다. 현재 코드는 신규 AccessToken을 만들어 블랙리스트에 올려 기존 토큰이 유효한 상태로 남습니다. 전달받은 실제 토큰 문자열을 블랙리스트에 올려야 합니다. 아래와 같이 원본 토큰 문자열로 AccessToken 객체를 구성해 사용하도록 수정해 주세요. public void signOut(String token) {
- SiteUser siteUser = authTokenProvider.parseSiteUser(token);
- AccessToken accessToken = authTokenProvider.generateAccessToken(siteUser);
+ SiteUser siteUser = authTokenProvider.parseSiteUser(token);
+ AccessToken accessToken = new AccessToken(
+ authTokenProvider.toSubject(siteUser),
+ siteUser.getRole(),
+ token
+ );
authTokenProvider.deleteRefreshTokenByAccessToken(accessToken);
tokenBlackListService.addToBlacklist(accessToken);
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -58,17 +53,14 @@ public void quit(long siteUserId, String token) { | |||||||||||||||||||||||||||||||||||||||||||||
| * - 유효한 리프레시토큰이면, 액세스 토큰을 재발급한다. | ||||||||||||||||||||||||||||||||||||||||||||||
| * - 그렇지 않으면 예외를 발생시킨다. | ||||||||||||||||||||||||||||||||||||||||||||||
| * */ | ||||||||||||||||||||||||||||||||||||||||||||||
| public ReissueResponse reissue(long siteUserId, ReissueRequest reissueRequest) { | ||||||||||||||||||||||||||||||||||||||||||||||
| public ReissueResponse reissue(String requestedRefreshToken) { | ||||||||||||||||||||||||||||||||||||||||||||||
| // 리프레시 토큰 확인 | ||||||||||||||||||||||||||||||||||||||||||||||
| String requestedRefreshToken = reissueRequest.refreshToken(); | ||||||||||||||||||||||||||||||||||||||||||||||
| if (!authTokenProvider.isValidRefreshToken(requestedRefreshToken)) { | ||||||||||||||||||||||||||||||||||||||||||||||
| throw new CustomException(REFRESH_TOKEN_EXPIRED); | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| // 액세스 토큰 재발급 | ||||||||||||||||||||||||||||||||||||||||||||||
| SiteUser siteUser = siteUserRepository.findById(siteUserId) | ||||||||||||||||||||||||||||||||||||||||||||||
| .orElseThrow(() -> new CustomException(USER_NOT_FOUND)); | ||||||||||||||||||||||||||||||||||||||||||||||
| Subject subject = authTokenProvider.parseSubject(requestedRefreshToken); | ||||||||||||||||||||||||||||||||||||||||||||||
| AccessToken newAccessToken = authTokenProvider.generateAccessToken(subject, siteUser.getRole()); | ||||||||||||||||||||||||||||||||||||||||||||||
| SiteUser siteUser = authTokenProvider.parseSiteUser(requestedRefreshToken); | ||||||||||||||||||||||||||||||||||||||||||||||
| AccessToken newAccessToken = authTokenProvider.generateAccessToken(siteUser); | ||||||||||||||||||||||||||||||||||||||||||||||
|
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.
|
||||||||||||||||||||||||||||||||||||||||||||||
| return ReissueResponse.from(newAccessToken); | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,8 +1,12 @@ | ||||||||||||||||||||||||||||||||||||||||
| package com.example.solidconnection.auth.service; | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| import static com.example.solidconnection.common.exception.ErrorCode.USER_NOT_FOUND; | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| import com.example.solidconnection.auth.domain.TokenType; | ||||||||||||||||||||||||||||||||||||||||
| import com.example.solidconnection.common.exception.CustomException; | ||||||||||||||||||||||||||||||||||||||||
| import com.example.solidconnection.siteuser.domain.Role; | ||||||||||||||||||||||||||||||||||||||||
| import com.example.solidconnection.siteuser.domain.SiteUser; | ||||||||||||||||||||||||||||||||||||||||
| import com.example.solidconnection.siteuser.repository.SiteUserRepository; | ||||||||||||||||||||||||||||||||||||||||
| import java.util.Map; | ||||||||||||||||||||||||||||||||||||||||
| import java.util.Objects; | ||||||||||||||||||||||||||||||||||||||||
| import lombok.RequiredArgsConstructor; | ||||||||||||||||||||||||||||||||||||||||
|
|
@@ -17,15 +21,21 @@ public class AuthTokenProvider { | |||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| private final RedisTemplate<String, String> redisTemplate; | ||||||||||||||||||||||||||||||||||||||||
| private final TokenProvider tokenProvider; | ||||||||||||||||||||||||||||||||||||||||
| private final SiteUserRepository siteUserRepository; | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| public AccessToken generateAccessToken(Subject subject, Role role) { | ||||||||||||||||||||||||||||||||||||||||
| public AccessToken generateAccessToken(SiteUser siteUser) { | ||||||||||||||||||||||||||||||||||||||||
| Subject subject = toSubject(siteUser); | ||||||||||||||||||||||||||||||||||||||||
| Role role = siteUser.getRole(); | ||||||||||||||||||||||||||||||||||||||||
| String token = tokenProvider.generateToken( | ||||||||||||||||||||||||||||||||||||||||
| subject.value(), Map.of(ROLE_CLAIM_KEY, role.name()), TokenType.ACCESS | ||||||||||||||||||||||||||||||||||||||||
| subject.value(), | ||||||||||||||||||||||||||||||||||||||||
| Map.of(ROLE_CLAIM_KEY, role.name()), | ||||||||||||||||||||||||||||||||||||||||
| TokenType.ACCESS | ||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||
| return new AccessToken(subject, role, token); | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| public RefreshToken generateAndSaveRefreshToken(Subject subject) { | ||||||||||||||||||||||||||||||||||||||||
| public RefreshToken generateAndSaveRefreshToken(SiteUser siteUser) { | ||||||||||||||||||||||||||||||||||||||||
| Subject subject = toSubject(siteUser); | ||||||||||||||||||||||||||||||||||||||||
| String token = tokenProvider.generateToken(subject.value(), TokenType.REFRESH); | ||||||||||||||||||||||||||||||||||||||||
| tokenProvider.saveToken(token, TokenType.REFRESH); | ||||||||||||||||||||||||||||||||||||||||
| return new RefreshToken(subject, token); | ||||||||||||||||||||||||||||||||||||||||
|
|
@@ -49,9 +59,11 @@ public void deleteRefreshTokenByAccessToken(AccessToken accessToken) { | |||||||||||||||||||||||||||||||||||||||
| redisTemplate.delete(refreshTokenKey); | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| public Subject parseSubject(String token) { | ||||||||||||||||||||||||||||||||||||||||
| public SiteUser parseSiteUser(String token) { | ||||||||||||||||||||||||||||||||||||||||
| String subject = tokenProvider.parseSubject(token); | ||||||||||||||||||||||||||||||||||||||||
| return new Subject(subject); | ||||||||||||||||||||||||||||||||||||||||
| long siteUserId = Long.parseLong(subject); | ||||||||||||||||||||||||||||||||||||||||
| return siteUserRepository.findById(siteUserId) | ||||||||||||||||||||||||||||||||||||||||
| .orElseThrow(() -> new CustomException(USER_NOT_FOUND)); | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+62
to
67
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. 🛠️ Refactor suggestion 1) parseSiteUser에서 NumberFormatException을 처리해 500을 방지해 주세요. 비정상 토큰의 subject가 숫자가 아니면 런타임 예외로 500이 날 수 있습니다. 커스텀 예외로 변환해 4xx로 응답하는 게 안전합니다. 다음과 같이 보강을 제안합니다. public SiteUser parseSiteUser(String token) {
String subject = tokenProvider.parseSubject(token);
- long siteUserId = Long.parseLong(subject);
+ long siteUserId;
+ try {
+ siteUserId = Long.parseLong(subject);
+ } catch (NumberFormatException e) {
+ // 토큰 변조/형식 오류를 서버 에러로 노출하지 않도록 처리
+ throw new CustomException(USER_NOT_FOUND);
+ }
return siteUserRepository.findById(siteUserId)
.orElseThrow(() -> new CustomException(USER_NOT_FOUND));
}원하시면, 이 경로에 대한 단위 테스트(비숫자 subject 케이스) 추가도 도와드리겠습니다. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| public Subject toSubject(SiteUser siteUser) { | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
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.
💡 Verification agent
🧩 Analysis chain
3) CSRF/CORS 관점에서 재발급 엔드포인트 보호 상태를 확인해 주세요.
SameSite=None + Secure로 운영 시, CORS 정책이 빈틈없이 설정되었는지와 CSRF 방어(예: Double Submit, 전용 헤더) 전략이 적절한지 재확인하는 게 안전합니다.
아래 스크립트로 보안 설정 위치를 빠르게 조회해 보세요.
🏁 Script executed:
Length of output: 7034
조치 권고 — /reissue 엔드포인트의 CSRF/CORS 보호 재검증 필요
검증 요약: SecurityConfiguration에서 CSRF가 비활성화(.csrf(AbstractHttpConfigurer::disable))되어 있고 CORS는 corsProperties.allowedOrigins()를 사용해 구성하며 configuration.setAllowCredentials(true)가 설정되어 있습니다.
확인한 위치:
권장 변경(간단 워크스루):
한 줄 결론: 전역 CSRF 비활성화 + credentials 허용 조합은 리프레시 토큰을 쿠키로 읽는 상황에서 위험하므로 위 권장사항 중 최소 하나 이상을 적용해 주세요.
🤖 Prompt for AI Agents