Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[BE] JWT 토큰 사용 제거, Interceptor와 ArgumentResolver 중복 로직 제거 #733

Merged
merged 10 commits into from
Nov 21, 2023
Merged

[BE] JWT 토큰 사용 제거, Interceptor와 ArgumentResolver 중복 로직 제거 #733

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
a071f21
feat: jwt 대신 aes를 사용하도록 변경
jaehee329 Nov 14, 2023
8becd6b
refactor: Interceptor와 ArgumentResolver의 중복 로직 제거
jaehee329 Nov 14, 2023
ebee485
refactor: 예외 처리 위치 수정
jaehee329 Nov 14, 2023
04595b3
chore: jwt 관련 의존성 build.gradle에서 제거
jaehee329 Nov 16, 2023
9d09e10
fix: submodule 관련 수정
jaehee329 Nov 16, 2023
9966cc0
refactor: parseSubject()의 예외 처리 로직 수정
jaehee329 Nov 16, 2023
7c1b8f4
fix: 누락된 예외 추가
jaehee329 Nov 16, 2023
f3a3dfe
refactor: 토큰 관련 로직을 별도의 클래스로 추상화
jaehee329 Nov 16, 2023
8893087
refactor: AccessTokenUtils 추가
jaehee329 Nov 16, 2023
8919c1e
fix: 잘못된 softAssertion 사용 수정
jaehee329 Nov 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions backend/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,6 @@ dependencies {
implementation 'org.flywaydb:flyway-core'
implementation 'org.flywaydb:flyway-mysql'

implementation 'io.jsonwebtoken:jjwt-api:0.11.5'
implementation 'io.jsonwebtoken:jjwt-impl:0.11.5'
implementation 'io.jsonwebtoken:jjwt-jackson:0.11.5'

implementation 'mysql:mysql-connector-java:8.0.33'

compileOnly 'org.projectlombok:lombok'
Expand Down
12 changes: 2 additions & 10 deletions backend/src/main/java/harustudy/backend/auth/AuthArgumentResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,25 +1,19 @@
package harustudy.backend.auth;

import harustudy.backend.auth.dto.AuthMember;
import harustudy.backend.auth.service.AuthService;
import harustudy.backend.auth.util.BearerAuthorizationParser;
import lombok.RequiredArgsConstructor;
import org.springframework.core.MethodParameter;
import org.springframework.http.HttpHeaders;
import org.springframework.stereotype.Component;
import org.springframework.web.bind.support.WebDataBinderFactory;
import org.springframework.web.context.request.NativeWebRequest;
import org.springframework.web.context.request.RequestAttributes;
import org.springframework.web.method.support.HandlerMethodArgumentResolver;
import org.springframework.web.method.support.ModelAndViewContainer;

@RequiredArgsConstructor
@Component
public class AuthArgumentResolver implements HandlerMethodArgumentResolver {

private final AuthService authService;

private final BearerAuthorizationParser bearerAuthorizationParser;

@Override
public boolean supportsParameter(MethodParameter parameter) {
return parameter.hasParameterAnnotation(Authenticated.class);
Expand All @@ -28,9 +22,7 @@ public boolean supportsParameter(MethodParameter parameter) {
@Override
public AuthMember resolveArgument(MethodParameter parameter, ModelAndViewContainer mavContainer,
NativeWebRequest webRequest, WebDataBinderFactory binderFactory) {
String authorizationHeader = webRequest.getHeader(HttpHeaders.AUTHORIZATION);
String accessToken = bearerAuthorizationParser.parse(authorizationHeader);
long memberId = Long.parseLong(authService.parseMemberId(accessToken));
Long memberId = (Long) webRequest.getAttribute("memberId", RequestAttributes.SCOPE_REQUEST);
return new AuthMember(memberId);
}
}
3 changes: 2 additions & 1 deletion backend/src/main/java/harustudy/backend/auth/AuthInterceptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ public boolean preHandle(HttpServletRequest request, HttpServletResponse respons
}
String authorizationHeader = request.getHeader(HttpHeaders.AUTHORIZATION);
String accessToken = bearerAuthorizationParser.parse(authorizationHeader);
authService.validateAccessToken(accessToken);
Long memberId = authService.parseMemberId(accessToken);
request.setAttribute("memberId", memberId);
return HandlerInterceptor.super.preHandle(request, response, handler);
}
}
6 changes: 3 additions & 3 deletions backend/src/main/java/harustudy/backend/auth/config/TokenConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@

@Component
public record TokenConfig(
@Value("${jwt.secret-key}") String secretKey,
@Value("${jwt.expire-length}") long accessTokenExpireLength,
@Value("${jwt.guest-expire-length}") long guestAccessTokenExpireLength,
@Value("${access-token.secret-key}") String secretKey,
@Value("${access-token.expire-length}") long accessTokenExpireLength,
@Value("${access-token.guest-expire-length}") long guestAccessTokenExpireLength,
@Value("${refresh-token.expire-length}") long refreshTokenExpireLength) {

}
10 changes: 8 additions & 2 deletions backend/src/main/java/harustudy/backend/auth/exception/InvalidAccessTokenException.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,13 @@

import harustudy.backend.common.exception.HaruStudyException;

public class InvalidAccessTokenException extends
HaruStudyException {
public class InvalidAccessTokenException extends HaruStudyException {

public InvalidAccessTokenException() {

}

public InvalidAccessTokenException(Exception e) {
super(e);
}
}
36 changes: 10 additions & 26 deletions backend/src/main/java/harustudy/backend/auth/service/AuthService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,14 @@
import harustudy.backend.auth.dto.OauthLoginRequest;
import harustudy.backend.auth.dto.TokenResponse;
import harustudy.backend.auth.dto.UserInfo;
import harustudy.backend.auth.exception.InvalidAccessTokenException;
import harustudy.backend.auth.exception.InvalidRefreshTokenException;
import harustudy.backend.auth.repository.RefreshTokenRepository;
import harustudy.backend.auth.util.JwtTokenProvider;
import harustudy.backend.auth.util.AesTokenProvider;
import harustudy.backend.member.domain.LoginType;
import harustudy.backend.member.domain.Member;
import harustudy.backend.member.repository.MemberRepository;
import io.jsonwebtoken.JwtException;
import java.util.UUID;
import lombok.RequiredArgsConstructor;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

Expand All @@ -24,7 +21,7 @@
@Service
public class AuthService {

private final JwtTokenProvider jwtTokenProvider;
private final AesTokenProvider aesTokenProvider;
private final TokenConfig tokenConfig;
private final MemberRepository memberRepository;
private final RefreshTokenRepository refreshTokenRepository;
Expand All @@ -44,11 +41,8 @@ private Member saveOrUpdateMember(String oauthProvider, UserInfo userInfo) {
}

private String generateAccessToken(Long memberId) {
return jwtTokenProvider.builder()
.subject(String.valueOf(memberId))
.accessTokenExpireLength(tokenConfig.accessTokenExpireLength())
.secretKey(tokenConfig.secretKey())
.build();
return aesTokenProvider.createAccessToken(memberId, tokenConfig.accessTokenExpireLength(),
tokenConfig.secretKey());
}

private RefreshToken saveRefreshTokenOf(Member member) {
Expand All @@ -67,11 +61,8 @@ public TokenResponse guestLogin() {
}

private String generateGuestAccessToken(Long memberId) {
return jwtTokenProvider.builder()
.subject(String.valueOf(memberId))
.accessTokenExpireLength(tokenConfig.guestAccessTokenExpireLength())
.secretKey(tokenConfig.secretKey())
.build();
return aesTokenProvider.createAccessToken(memberId,
tokenConfig.guestAccessTokenExpireLength(), tokenConfig.secretKey());
}

public TokenResponse refresh(String refreshTokenRequest) {
Expand All @@ -80,19 +71,12 @@ public TokenResponse refresh(String refreshTokenRequest) {
refreshToken.validateExpired();
refreshToken.updateUuidAndExpireDateTime(tokenConfig.refreshTokenExpireLength());
String accessToken = generateAccessToken(refreshToken.getMember().getId());
return TokenResponse.forLoggedIn(accessToken, refreshToken, tokenConfig.refreshTokenExpireLength());
}

public void validateAccessToken(String accessToken) {
try {
jwtTokenProvider.validateAccessToken(accessToken, tokenConfig.secretKey());
} catch (JwtException e) {
throw new InvalidAccessTokenException();
}
return TokenResponse.forLoggedIn(accessToken, refreshToken,
tokenConfig.refreshTokenExpireLength());
Comment on lines +74 to +75
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

요기 개행은 안해도 되지 않을까 싶습니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IDE 설정을 맞췄을텐데 저는 여기에서는 글자수가 길어서 그런지 자동으로 리포맷을 해주네요..😂

}

public String parseMemberId(String accessToken) {
return jwtTokenProvider.parseSubject(accessToken, tokenConfig.secretKey());
public Long parseMemberId(String accessToken) {
return aesTokenProvider.parseSubject(accessToken, tokenConfig.secretKey());
}

public void deleteStringifiedRefreshToken(String refreshToken) {
Expand Down
83 changes: 83 additions & 0 deletions backend/src/main/java/harustudy/backend/auth/util/AesTokenProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
package harustudy.backend.auth.util;

import harustudy.backend.auth.exception.InvalidAccessTokenException;
import java.nio.charset.StandardCharsets;
import java.security.GeneralSecurityException;
import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.util.Base64;
import java.util.Date;
import javax.crypto.Cipher;
import javax.crypto.spec.IvParameterSpec;
import javax.crypto.spec.SecretKeySpec;
import org.springframework.stereotype.Component;

@Component
public class AesTokenProvider {

private static final String alg = "AES/CBC/PKCS5Padding";
private static final String iv = "0123456789abcdef"; // 16byte
Comment on lines +18 to +19
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AES 암호화 방식에서는 어떤 알고리즘을 사용하는지, IV가 고정인지 유동인지 등의 정보가 외부로 노출되어도 큰 보안적 위험은 이론적으로 없다고는 하지만 그럼에도 최대한 정보를 노출하지 않는 편이 좋다고 생각합니다. 요 정보들을 submodule로 같이 분리해서 TokenConfig를 통해 접근하도록 통일하는 방향은 어떠신가요? 혹시 이렇게 구현하신 다른 이유가 있으셨다면 말씀 부탁드립니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오프라인으로 얘기하여 굳이 적용하지 않는 것으로...

private static final SimpleDateFormat DATE_FORMAT = new SimpleDateFormat("MMddHH:mm:ssyyyy");

public String createAccessToken(Long subject, Long accessTokenExpireLength, String secretKey) {
Date now = new Date();
Date expireAt = new Date(now.getTime() + accessTokenExpireLength);
String formatted = DATE_FORMAT.format(expireAt);
String text = subject + " " + formatted;
return encrypt(text, secretKey);
}

private String encrypt(String text, String secretKey) {
try {
Cipher cipher = Cipher.getInstance(alg);
SecretKeySpec keySpec = new SecretKeySpec(secretKey.getBytes(), "AES");
IvParameterSpec ivParamSpec = new IvParameterSpec(iv.getBytes());
cipher.init(Cipher.ENCRYPT_MODE, keySpec, ivParamSpec);

byte[] encrypted = cipher.doFinal(text.getBytes(StandardCharsets.UTF_8));
return Base64.getEncoder().encodeToString(encrypted);
} catch (GeneralSecurityException e) {
throw new InvalidAccessTokenException(e);
}
}

public Long parseSubject(String accessToken, String secretKey) {
try {
String[] splitted = decrypt(accessToken, secretKey);
validateLength(splitted);
validateExpiration(splitted);
return parseSubject(splitted);
} catch (Exception e) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기도 Exception을 catch하고 있군요

Copy link
Collaborator Author

@jaehee329 jaehee329 Nov 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기는 uncheckedException과 checkedException 모든 상황에 대한 예외가 다양해서 구체적인 예외를 모두 나열하는 것이 필요가 없을 것 같아서 Exception으로 처리했어요. 또 Exception으로 모아도 잃는 stacktrace 정보도 없으니 괜찮을 것 같고요.

내부에서는 CheckedException인 ParseException과 RuntimeException을 상속한 배열 크기 관련한 ArrayIndexOutOfBoundsException도 발생하고 상속한 저희의 커스텀 예외인 InvalidAccessTokenException 을 던지기도 하고요.

사실 private 메서드에서는 그냥 상위로 throw하고 가장 외부의 public 메서드에서 모두 catch하는 방법이 가장 좋을 것 같은데
이렇게 되면 private 메서드들에서 public으로 throw할 만한 적절한 exception이 없다 느껴졌습니다.
그래서 부분적으로 내부에서 InvalidAccessTokenException을 발생시키고 외부에서 다시 전환하게 했어요.

더 구체적으로 좋은 방법 제안해주시면 좋겠습니다~

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
} catch (Exception e) {
} catch (ParseException |
GeneralSecurityException|
RuntimeException e) {
throw new InvalidAccessTokenException(e);
}

조금 번거롭더라도 이 코드를 나중에 보게될 저희 스스로를 위해서 catch문을 분리하는 것도 하나의 방법이 될 수 있지 않을까 제안해봅니다.

그게 아니라면 개인적으로 private 검증 메서드마다 발생하는 checkedException에 대해서 예외 전환을 각각 해주는 편이 좋다고 생각이 들었어요. encrypt메서드의 경우는 이미 내부에서 예외를 런타임 예외로 전환해주고 있어서 크게 걸릴 부분은 없을 것 같은데 어떻게 생각하시나요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 부분은 별도 클래스에서 처리하도록 해서 예외들을 깔끔하게 처리했습니다!

추가로 확인해주세요~!

throw new InvalidAccessTokenException(e);
}
}

private String[] decrypt(String accessToken, String secretKey) throws GeneralSecurityException {
Cipher cipher = Cipher.getInstance(alg);
SecretKeySpec keySpec = new SecretKeySpec(secretKey.getBytes(), "AES");
IvParameterSpec ivParamSpec = new IvParameterSpec(iv.getBytes());
cipher.init(Cipher.DECRYPT_MODE, keySpec, ivParamSpec);

byte[] decodedBytes = Base64.getDecoder().decode(accessToken);
byte[] decrypted = cipher.doFinal(decodedBytes);
String string = new String(decrypted, StandardCharsets.UTF_8);
return string.split(" ");
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

encrypt 메서드는 내부에서 예외 전환을 해주는 방향으로 수정해주신 것 같아서 비슷한 방향으로 런타임 예외로 전환해주시면 좋을 것 같습니다. 😃


private void validateLength(String[] splitted) {
if (splitted.length != 2) {
throw new InvalidAccessTokenException();
}
}

private void validateExpiration(String[] splitted) throws ParseException {
Date expireAt = DATE_FORMAT.parse(splitted[1]);
if (expireAt.before(new Date())) {
throw new InvalidAccessTokenException();
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

리프레시 토큰은 Expired 된 상황에 대한 예외를 처리하고 있어서 accessToken에서도 통일성있게 AccessTokenExpiredException으로 처리하는 건 어떤가요?

(추가) 아 프론트랑 논의되지 않은 부분이라 섣불리 수정하기 어려울 수도 있겠네요...! 이 부분은 좀 더 얘기해보면 좋을 것 같습니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

말씀하신 예외가 더 깔끔할 것 같긴 한데 당장은 프론트랑 연계되는 내용이라 바로 반영은 어렵겠네요!😅


private Long parseSubject(String[] splitted) {
return Long.parseLong(splitted[0]);
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

개행 부탁드립니다 🙏

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

제 IDE와 깃헙에서는 괜찮아보이는데 다른 분들도 없는 것으로 보이면 말씀해주세요!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

제꺼에서는 일단 개행이 있는 것으로 표기되기는 하네요~

55 changes: 0 additions & 55 deletions backend/src/main/java/harustudy/backend/auth/util/JwtTokenProvider.java
View file
Open in desktop

This file was deleted.

4 changes: 4 additions & 0 deletions backend/src/main/java/harustudy/backend/common/exception/HaruStudyException.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ public abstract class HaruStudyException extends RuntimeException {
protected HaruStudyException() {
}

protected HaruStudyException(Exception e) {
super(e);
}

protected HaruStudyException(String message) {
super(message);
}
Expand Down
2 changes: 1 addition & 1 deletion backend/src/main/resources/submodule
Open in desktop
Submodule submodule updated from 0b9d86 to d56076
17 changes: 8 additions & 9 deletions backend/src/test/java/harustudy/backend/acceptance/AcceptanceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package harustudy.backend.acceptance;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.BDDMockito.given;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
Expand All @@ -13,7 +14,7 @@
import harustudy.backend.auth.dto.OauthLoginRequest;
import harustudy.backend.auth.dto.OauthTokenResponse;
import harustudy.backend.auth.dto.TokenResponse;
import harustudy.backend.auth.util.JwtTokenProvider;
import harustudy.backend.auth.util.AesTokenProvider;
import harustudy.backend.content.dto.WritePlanRequest;
import harustudy.backend.content.dto.WriteRetrospectRequest;
import harustudy.backend.integration.LoginResponse;
Expand All @@ -27,7 +28,6 @@
import java.nio.charset.StandardCharsets;
import java.util.List;
import java.util.Map;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.DisplayNameGeneration;
Expand Down Expand Up @@ -66,7 +66,7 @@ class AcceptanceTest {
private MockMvc mockMvc;

@Autowired
private JwtTokenProvider jwtTokenProvider;
private AesTokenProvider aesTokenProvider;

@Autowired
private TokenConfig tokenConfig;
Expand Down Expand Up @@ -127,8 +127,8 @@ void setUp() {

private List<StudyResponse> 회원으로_진행했던_모든_스터디_목록을_조회한다(LoginResponse 로그인_정보)
throws Exception {
long memberId = Long.parseLong(jwtTokenProvider
.parseSubject(로그인_정보.tokenResponse().accessToken(), tokenConfig.secretKey()));
Long memberId = aesTokenProvider.parseSubject(로그인_정보.tokenResponse().accessToken(),
tokenConfig.secretKey());

MvcResult result = mockMvc.perform(
get("/api/studies")
Expand Down Expand Up @@ -196,8 +196,8 @@ void setUp() {
}

private Long 스터디에_참여한다(LoginResponse 로그인_정보, Long 스터디_아이디) throws Exception {
Long memberId = Long.valueOf(jwtTokenProvider
.parseSubject(로그인_정보.tokenResponse().accessToken(), tokenConfig.secretKey()));
Long memberId = aesTokenProvider.parseSubject(로그인_정보.tokenResponse().accessToken(),
tokenConfig.secretKey());
ParticipateStudyRequest request = new ParticipateStudyRequest(memberId, "nickname");
String jsonRequest = objectMapper.writeValueAsString(request);

Expand Down Expand Up @@ -291,7 +291,6 @@ void setUp() {
.andExpect(status().isOk())
.andReturn();
String response = result.getResponse().getContentAsString(StandardCharsets.UTF_8);
Assertions.assertDoesNotThrow(() -> objectMapper.readValue(response,
StudyResponse.class));
assertDoesNotThrow(() -> objectMapper.readValue(response, StudyResponse.class));
}
}
Loading