Skip to content

Commit

Permalink
토큰 디코딩 오류 해결 (#500)
Browse files Browse the repository at this point in the history 
* refactor: Bearer 헤더와 AccessToken 분리

* test: 깨지는 테스트 수정

* refactor: JwtDecoder 메서드 이름 변경

* feat: VO인 AuthorizationHeader 도입

* refactor: jwt 만료기간을 분 단위 기준으로 변경

* test: 잘못된 테스트 수정
  • Loading branch information
@cookienc
cookienc authored Sep 18, 2023
1 parent c113a4f commit 6be8648
Show file tree
Hide file tree
Showing 13 changed files with 148 additions and 44 deletions.
27 changes: 27 additions & 0 deletions backend/baton/src/main/java/touch/baton/domain/oauth/AuthorizationHeader.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package touch.baton.domain.oauth;

public class AuthorizationHeader {

private static final String BEARER = "Bearer ";

private final String value;

public AuthorizationHeader(final String value) {
validateNotNull(value);
this.value = value;
}

private void validateNotNull(final String value) {
if (value == null) {
throw new IllegalArgumentException("AuthorizationHeader 의 value 는 null 일 수 없습니다.");
}
}

public String parseBearerAccessToken() {
return value.substring(BEARER.length());
}

public boolean isNotBearerAuth() {
return !value.startsWith(BEARER);
}
}
8 changes: 3 additions & 5 deletions backend/baton/src/main/java/touch/baton/domain/oauth/controller/OauthController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import jakarta.servlet.http.HttpServletResponse;
import lombok.RequiredArgsConstructor;
import org.springframework.boot.web.server.Cookie.SameSite;
import org.springframework.http.HttpHeaders;
import org.springframework.http.ResponseCookie;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.CookieValue;
Expand All @@ -14,6 +13,7 @@
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RestController;
import touch.baton.domain.oauth.AuthorizationHeader;
import touch.baton.domain.oauth.OauthType;
import touch.baton.domain.oauth.service.OauthService;
import touch.baton.domain.oauth.token.RefreshToken;
Expand All @@ -22,7 +22,6 @@
import java.io.IOException;
import java.time.Duration;

import static org.springframework.boot.web.server.Cookie.SameSite.NONE;
import static org.springframework.http.HttpHeaders.AUTHORIZATION;
import static org.springframework.http.HttpStatus.FOUND;
import static touch.baton.domain.oauth.token.RefreshToken.REFRESH_TOKEN_LIFECYCLE;
Expand Down Expand Up @@ -65,9 +64,8 @@ public ResponseEntity<Void> refreshJwt(@CookieValue final String refreshToken,
final HttpServletRequest request,
final HttpServletResponse response
) {
final String jwtToken = request.getHeader(AUTHORIZATION);

final Tokens tokens = oauthService.reissueAccessToken(jwtToken, refreshToken);
final AuthorizationHeader authorizationHeader = new AuthorizationHeader(request.getHeader(AUTHORIZATION));
final Tokens tokens = oauthService.reissueAccessToken(authorizationHeader, refreshToken);

setCookie(response, tokens.refreshToken());

Expand Down
8 changes: 4 additions & 4 deletions ...main/java/touch/baton/domain/oauth/controller/resolver/UserPrincipalArgumentResolver.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import org.springframework.web.method.support.HandlerMethodArgumentResolver;
import org.springframework.web.method.support.ModelAndViewContainer;
import touch.baton.domain.common.exception.ClientErrorCode;
import touch.baton.domain.oauth.AuthorizationHeader;
import touch.baton.domain.oauth.exception.OauthRequestException;
import touch.baton.infra.auth.jwt.JwtDecoder;

Expand Down Expand Up @@ -37,13 +38,12 @@ public Object resolveArgument(final MethodParameter parameter,
return getGuest();
}

final String authHeader = webRequest.getHeader(AUTHORIZATION);
if (!authHeader.startsWith(BEARER)) {
final AuthorizationHeader authorization = new AuthorizationHeader(webRequest.getHeader(AUTHORIZATION));
if (authorization.isNotBearerAuth()) {
throw new OauthRequestException(ClientErrorCode.OAUTH_AUTHORIZATION_BEARER_TYPE_NOT_FOUND);
}

final String token = authHeader.substring(BEARER.length());
final Claims claims = jwtDecoder.parseJwtToken(token);
final Claims claims = jwtDecoder.parseAuthorizationHeader(authorization);
final String socialId = claims.get("socialId", String.class);

return getUser(socialId);
Expand Down
5 changes: 3 additions & 2 deletions backend/baton/src/main/java/touch/baton/domain/oauth/service/OauthService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import touch.baton.domain.member.Member;
import touch.baton.domain.member.vo.Company;
import touch.baton.domain.member.vo.SocialId;
import touch.baton.domain.oauth.AuthorizationHeader;
import touch.baton.domain.oauth.OauthInformation;
import touch.baton.domain.oauth.OauthType;
import touch.baton.domain.oauth.authcode.AuthCodeRequestUrlProviderComposite;
Expand Down Expand Up @@ -125,8 +126,8 @@ private Tokens createTokens(final SocialId socialId, final Member member) {
}

@Transactional
public Tokens reissueAccessToken(final String jwtToken, final String refreshToken) {
final Claims claims = jwtDecoder.parseExpiredJwtToken(jwtToken);
public Tokens reissueAccessToken(final AuthorizationHeader authHeader, final String refreshToken) {
final Claims claims = jwtDecoder.parseExpiredAuthorizationHeader(authHeader);
final SocialId socialId = new SocialId(claims.get("socialId", String.class));
final Member findMember = oauthMemberRepository.findBySocialId(socialId)
.orElseThrow(() -> new OauthRequestException(ClientErrorCode.JWT_CLAIM_SOCIAL_ID_IS_WRONG));
Expand Down
7 changes: 5 additions & 2 deletions backend/baton/src/main/java/touch/baton/infra/auth/jwt/JwtDecoder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.springframework.context.annotation.Profile;
import org.springframework.stereotype.Component;
import touch.baton.domain.common.exception.ClientErrorCode;
import touch.baton.domain.oauth.AuthorizationHeader;
import touch.baton.domain.oauth.exception.OauthRequestException;

@Profile("!test")
Expand All @@ -21,13 +22,14 @@ public class JwtDecoder {

private final JwtConfig jwtConfig;

public Claims parseJwtToken(final String token) {
public Claims parseAuthorizationHeader(final AuthorizationHeader authorizationHeader) {
try {
final JwtParser jwtParser = Jwts.parserBuilder()
.setSigningKey(jwtConfig.getSecretKey())
.requireIssuer(jwtConfig.getIssuer())
.build();

final String token = authorizationHeader.parseBearerAccessToken();
return jwtParser.parseClaimsJws(token).getBody();
} catch (final SignatureException e) {
throw new OauthRequestException(ClientErrorCode.JWT_SIGNATURE_IS_WRONG);
Expand All @@ -40,13 +42,14 @@ public Claims parseJwtToken(final String token) {
}
}

public Claims parseExpiredJwtToken(final String token) {
public Claims parseExpiredAuthorizationHeader(final AuthorizationHeader authorizationHeader) {
try {
final JwtParser jwtParser = Jwts.parserBuilder()
.setSigningKey(jwtConfig.getSecretKey())
.requireIssuer(jwtConfig.getIssuer())
.build();

final String token = authorizationHeader.parseBearerAccessToken();
jwtParser.parseClaimsJws(token).getBody();

throw new OauthRequestException(ClientErrorCode.JWT_CLAIM_IS_NOT_EXPIRED);
Expand Down
3 changes: 1 addition & 2 deletions backend/baton/src/test/java/touch/baton/assure/common/AssuredSupport.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import io.restassured.response.ExtractableResponse;
import io.restassured.response.Response;

import static org.springframework.http.HttpHeaders.AUTHORIZATION;
import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE;

public class AssuredSupport {
Expand All @@ -23,8 +22,8 @@ public static ExtractableResponse<Response> post(final String uri, final Object
public static ExtractableResponse<Response> post(final String uri, final String accessToken, final String refreshToken) {
return RestAssured
.given().log().ifValidationFails()
.auth().preemptive().oauth2(accessToken)
.cookie("refreshToken", refreshToken)
.header(AUTHORIZATION, accessToken)
.when().log().ifValidationFails()
.post(uri)
.then().log().ifError()
Expand Down
4 changes: 3 additions & 1 deletion backend/baton/src/test/java/touch/baton/assure/common/JwtTestManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
import touch.baton.domain.member.vo.SocialId;
import touch.baton.infra.auth.jwt.JwtDecoder;

import static touch.baton.fixture.vo.AuthorizationHeaderFixture.bearerAuthorizationHeader;

@Profile("test")
@TestComponent
public class JwtTestManager {
Expand All @@ -17,7 +19,7 @@ public JwtTestManager(final JwtDecoder jwtDecoder) {
}

public SocialId parseToSocialId(final String accessToken) {
final Claims claims = jwtDecoder.parseJwtToken(accessToken);
final Claims claims = jwtDecoder.parseAuthorizationHeader(bearerAuthorizationHeader(accessToken));
final String socialId = claims.get("socialId", String.class);

return new SocialId(socialId);
Expand Down
2 changes: 1 addition & 1 deletion backend/baton/src/test/java/touch/baton/config/RestdocsConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ protected String getAccessTokenBySocialId(final String socialId) {
final Claims claims = Jwts.claims();
claims.put("socialId", socialId);

when(jwtDecoder.parseJwtToken(any())).thenReturn(claims);
when(jwtDecoder.parseAuthorizationHeader(any())).thenReturn(claims);

return token;
}
Expand Down
3 changes: 2 additions & 1 deletion backend/baton/src/test/java/touch/baton/document/oauth/token/RefreshTokenApiTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest;
import org.springframework.boot.test.mock.mockito.MockBean;
import touch.baton.config.RestdocsConfig;
import touch.baton.domain.oauth.AuthorizationHeader;
import touch.baton.domain.oauth.controller.OauthController;
import touch.baton.domain.oauth.service.OauthService;
import touch.baton.domain.oauth.token.AccessToken;
Expand Down Expand Up @@ -56,7 +57,7 @@ void refresh() throws Exception {
final Tokens tokens = new Tokens(new AccessToken("renew access token"), refreshToken);
final Cookie cookie = createCookie();

given(oauthService.reissueAccessToken(any(String.class), any(String.class))).willReturn(tokens);
given(oauthService.reissueAccessToken(any(AuthorizationHeader.class), any(String.class))).willReturn(tokens);

// then
mockMvc.perform(post("/api/v1/oauth/refresh")
Expand Down
38 changes: 15 additions & 23 deletions backend/baton/src/test/java/touch/baton/domain/oauth/service/OauthServiceUpdateTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import org.mockito.junit.jupiter.MockitoExtension;
import touch.baton.domain.member.Member;
import touch.baton.domain.member.vo.SocialId;
import touch.baton.domain.oauth.AuthorizationHeader;
import touch.baton.domain.oauth.authcode.AuthCodeRequestUrlProviderComposite;
import touch.baton.domain.oauth.client.OauthInformationClientComposite;
import touch.baton.domain.oauth.exception.OauthRequestException;
Expand All @@ -32,6 +33,7 @@
import static org.assertj.core.api.SoftAssertions.assertSoftly;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.BDDMockito.given;
import static touch.baton.fixture.vo.AuthorizationHeaderFixture.bearerAuthorizationHeader;

@ExtendWith(MockitoExtension.class)
class OauthServiceUpdateTest {
Expand Down Expand Up @@ -70,7 +72,7 @@ void setUp() {
void success_reissueAccessToken() {
// given
final Member tokenOwner = MemberFixture.createEthan();
final String expiredJwtToken = expiredJwtEncoder.jwtToken(Map.of("socialId", tokenOwner.getSocialId().getValue()));
final AuthorizationHeader expiredAuthorizationHeader = bearerAuthorizationHeader(expiredJwtEncoder.jwtToken(Map.of("socialId", tokenOwner.getSocialId().getValue())));
final String refreshTokenValue = "ethan-refresh-token";
final RefreshToken beforeRefreshToken = RefreshToken.builder()
.member(tokenOwner)
Expand All @@ -82,14 +84,14 @@ void success_reissueAccessToken() {
given(refreshTokenRepository.findByToken(eq(new Token(refreshTokenValue)))).willReturn(Optional.of(beforeRefreshToken));

// when
final Tokens tokens = oauthService.reissueAccessToken(expiredJwtToken, refreshTokenValue);
final Tokens tokens = oauthService.reissueAccessToken(expiredAuthorizationHeader, refreshTokenValue);

// then
assertSoftly(softly -> {
softly.assertThat(tokens.accessToken()).isNotNull();
softly.assertThat(tokens.accessToken().getValue()).isNotEqualTo(expiredJwtToken);
softly.assertThat(tokens.accessToken().getValue()).isNotEqualTo(expiredAuthorizationHeader.parseBearerAccessToken());
softly.assertThat(tokens.refreshToken()).isNotNull();
softly.assertThat(tokens.refreshToken().getToken().getValue()).isNotEqualTo(beforeRefreshToken);
softly.assertThat(tokens.refreshToken().getToken().getValue()).isNotEqualTo(refreshTokenValue);
});
}

Expand All @@ -98,7 +100,7 @@ void success_reissueAccessToken() {
void fail_reissueAccessToken_when_jwt_is_not_expired() {
// given
final Member tokenOwner = MemberFixture.createEthan();
final String normalJwtToken = jwtEncoder.jwtToken(Map.of("socialId", tokenOwner.getSocialId().getValue()));
final AuthorizationHeader normalJwtToken = bearerAuthorizationHeader(jwtEncoder.jwtToken(Map.of("socialId", tokenOwner.getSocialId().getValue())));
final String refreshTokenValue = "ethan-refresh-token";
final RefreshToken beforeRefreshToken = RefreshToken.builder()
.member(tokenOwner)
Expand All @@ -115,46 +117,36 @@ void fail_reissueAccessToken_when_jwt_is_not_expired() {
void fail_reissueAccessToken_when_socialId_not_exists() {
// given
final Member tokenOwner = MemberFixture.createEthan();
final String expiredJwtToken = expiredJwtEncoder.jwtToken(Map.of("socialId", tokenOwner.getSocialId().getValue()));
final AuthorizationHeader expiredAuthorizationHeader = bearerAuthorizationHeader(expiredJwtEncoder.jwtToken(Map.of("socialId", tokenOwner.getSocialId().getValue())));
final String refreshTokenValue = "ethan-refresh-token";
final RefreshToken beforeRefreshToken = RefreshToken.builder()
.member(tokenOwner)
.token(new Token(refreshTokenValue))
.expireDate(new ExpireDate(LocalDateTime.now().plusDays(30)))
.build();

given(oauthMemberRepository.findBySocialId(eq(new SocialId(tokenOwner.getSocialId().getValue())))).willReturn(Optional.empty());

// when, then
assertThatThrownBy(() -> oauthService.reissueAccessToken(expiredJwtToken, refreshTokenValue)).isInstanceOf(OauthRequestException.class);
assertThatThrownBy(() -> oauthService.reissueAccessToken(expiredAuthorizationHeader, refreshTokenValue)).isInstanceOf(OauthRequestException.class);
}

@DisplayName("refreshToken 이 없으면 재발급 요청하면 오류가 발생한다.")
@Test
void fail_reissueAccessToken_when_refreshToken_not_exists() {
// given
final Member tokenOwner = MemberFixture.createEthan();
final String expiredJwtToken = expiredJwtEncoder.jwtToken(Map.of("socialId", tokenOwner.getSocialId().getValue()));
final AuthorizationHeader expiredAuthorizationHeader = bearerAuthorizationHeader(expiredJwtEncoder.jwtToken(Map.of("socialId", tokenOwner.getSocialId().getValue())));
final String refreshTokenValue = "ethan-refresh-token";
final RefreshToken beforeRefreshToken = RefreshToken.builder()
.member(tokenOwner)
.token(new Token(refreshTokenValue))
.expireDate(new ExpireDate(LocalDateTime.now().plusDays(30)))
.build();

given(oauthMemberRepository.findBySocialId(eq(new SocialId(tokenOwner.getSocialId().getValue())))).willReturn(Optional.of(tokenOwner));
given(refreshTokenRepository.findByToken(eq(new Token(refreshTokenValue)))).willReturn(Optional.empty());

// when, then
assertThatThrownBy(() -> oauthService.reissueAccessToken(expiredJwtToken, refreshTokenValue)).isInstanceOf(OauthRequestException.class);
assertThatThrownBy(() -> oauthService.reissueAccessToken(expiredAuthorizationHeader, refreshTokenValue)).isInstanceOf(OauthRequestException.class);
}

@DisplayName("주인이 아닌 accessToken 으로 재발급 요청하면 오류가 발생한다.")
@Test
void fail_reissueAccessToken_when_not_owner_of_accessToken() {
// given
final Member tokenOwner = MemberFixture.createEthan();
final String expiredJwtToken = expiredJwtEncoder.jwtToken(Map.of("socialId", tokenOwner.getSocialId().getValue()));
final AuthorizationHeader expiredAuthorizationHeader = bearerAuthorizationHeader(expiredJwtEncoder.jwtToken(Map.of("socialId", tokenOwner.getSocialId().getValue())));
final String refreshTokenValue = "ethan-refresh-token";
final RefreshToken beforeRefreshToken = RefreshToken.builder()
.member(tokenOwner)
Expand All @@ -167,15 +159,15 @@ void fail_reissueAccessToken_when_not_owner_of_accessToken() {
given(refreshTokenRepository.findByToken(eq(new Token(refreshTokenValue)))).willReturn(Optional.of(beforeRefreshToken));

// when, then
assertThatThrownBy(() -> oauthService.reissueAccessToken(expiredJwtToken, refreshTokenValue)).isInstanceOf(OauthRequestException.class);
assertThatThrownBy(() -> oauthService.reissueAccessToken(expiredAuthorizationHeader, refreshTokenValue)).isInstanceOf(OauthRequestException.class);
}

@DisplayName("만료된 refreshToken 으로 재발급 요청하면 오류가 발생한다.")
@Test
void fail_reissueAccessToken_when_refreshToken_is_expired() {
// given
final Member tokenOwner = MemberFixture.createEthan();
final String expiredJwtToken = expiredJwtEncoder.jwtToken(Map.of("socialId", tokenOwner.getSocialId().getValue()));
final AuthorizationHeader expiredAuthorizationHeader = bearerAuthorizationHeader(expiredJwtEncoder.jwtToken(Map.of("socialId", tokenOwner.getSocialId().getValue())));
final String refreshTokenValue = "ethan-refresh-token";
final RefreshToken beforeRefreshToken = RefreshToken.builder()
.member(tokenOwner)
Expand All @@ -187,6 +179,6 @@ void fail_reissueAccessToken_when_refreshToken_is_expired() {
given(refreshTokenRepository.findByToken(eq(new Token(refreshTokenValue)))).willReturn(Optional.of(beforeRefreshToken));

// when, then
assertThatThrownBy(() -> oauthService.reissueAccessToken(expiredJwtToken, refreshTokenValue)).isInstanceOf(OauthRequestException.class);
assertThatThrownBy(() -> oauthService.reissueAccessToken(expiredAuthorizationHeader, refreshTokenValue)).isInstanceOf(OauthRequestException.class);
}
}
Loading

0 comments on commit 6be8648

Please sign in to comment.