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.

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

사용자 로그아웃, 탈퇴 기능 구현 #48

Merged
merged 11 commits into from
Feb 7, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.backend.blooming.authentication.application.dto.LoginDto;
import com.backend.blooming.authentication.application.dto.LoginInformationDto;
import com.backend.blooming.authentication.application.dto.LoginUserInformationDto;
import com.backend.blooming.authentication.application.dto.LogoutDto;
import com.backend.blooming.authentication.application.dto.TokenDto;
import com.backend.blooming.authentication.application.util.OAuthClientComposite;
import com.backend.blooming.authentication.infrastructure.exception.InvalidTokenException;
Expand All @@ -13,6 +14,7 @@
import com.backend.blooming.authentication.infrastructure.oauth.OAuthType;
import com.backend.blooming.authentication.infrastructure.oauth.dto.UserInformationDto;
import com.backend.blooming.devicetoken.application.service.DeviceTokenService;
import com.backend.blooming.user.application.exception.NotFoundUserException;
import com.backend.blooming.user.domain.Email;
import com.backend.blooming.user.domain.Name;
import com.backend.blooming.user.domain.User;
Expand All @@ -33,6 +35,7 @@ public class AuthenticationService {
private final OAuthClientComposite oAuthClientComposite;
private final TokenProvider tokenProvider;
private final UserRepository userRepository;
private final BlackListTokenService blackListTokenService;
private final DeviceTokenService deviceTokenService;

public LoginInformationDto login(final OAuthType oAuthType, final LoginDto loginDto) {
Expand Down Expand Up @@ -88,7 +91,7 @@ private TokenDto convertToTokenDto(final User user) {

private void saveOrActiveToken(final User user, final String deviceToken) {
if (deviceToken != null && !deviceToken.isEmpty()) {
deviceTokenService.saveOrActive(user.getId(), deviceToken);
deviceTokenService.saveOrActivate(user.getId(), deviceToken);
}
}

Expand All @@ -107,4 +110,30 @@ private void validateUser(final Long userId) {
throw new InvalidTokenException();
}
}

public void logout(final Long userId, final LogoutDto logoutDto) {
final AuthClaims authClaims = tokenProvider.parseToken(TokenType.REFRESH, logoutDto.refreshToken());
final User user = validateAndGetUser(userId, authClaims);

blackListTokenService.register(logoutDto.refreshToken());
deviceTokenService.deactivate(user.getId(), logoutDto.deviceToken());
}

private User validateAndGetUser(final Long userId, final AuthClaims authClaims) {
if (!userId.equals(authClaims.userId())) {
throw new InvalidTokenException();
}

return userRepository.findById(userId)
.orElseThrow(NotFoundUserException::new);
}
Comment on lines +122 to +129
Copy link
Member

Choose a reason for hiding this comment

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

별건 아닌데 저희 메서드 정렬 기준이 필요할까요?
지금까지는 구현 순서대로 정렬하기로 해서 그렇게 진행하고 있는데 코드가 점점 길어지다보니 메서드 정렬도 뭔가 기준이 필요한가? 하는 생각이 들더라구요. 그래서 클래스 내 메서드 정렬 기준을 만드는 건 어떻게 생각하시는지 궁금합니다!

Copy link
Member Author

@JJ503 JJ503 Feb 6, 2024

Choose a reason for hiding this comment

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

저는 보통 public 메서드는 C -> R -> U -> D의 순서로 작성하고, 그 외에는 호출 순서로 작성하고 있습니다.
저도 제안드릴까 고민을 하다, 오히려 익숙해진 메서드 작성 방식이 있으시다면 불편함을 드릴까 해서 제안드리지 못했습니다.
그래서 메서드 정렬 기준을 만드는 것에 대해선 찬성합니다!
혹시 생각해 보시거나 적용하신 메서드 정렬 기준이 있으실까요?

Copy link
Member

Choose a reason for hiding this comment

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

저는 이번에 서비스 클래스에서 CRUD 순서대로 메서드 정렬 후에 검증 메서드들을 아예 crud 메서드 아래로 다 빼서 정리를 해봤습니다. 순서는 자주 호출되는 순서대로 정렬했습니다. 그런데 구글링 해보니 읽는 사람이 편하게 메서드끼리 논리적 응집도를 높이는 방법도 있더라구요!
어떤 방법이 더 좋을 것 같으신가요?

Copy link
Member Author

@JJ503 JJ503 Feb 6, 2024

Choose a reason for hiding this comment

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

저는 굳이 검증 메서드를 하위로 빼는 것을 선호하지 않긴 합니다.
왜냐하면, 보통 코드리뷰를 할 때 위에서부터 코드를 읽게 되는데, 말씀해 주신 방식대로 한다면 계속 스크롤을 왔다 갔다 해야 하는 번거로움이 있으며 기존 있던 메서들를 새로운 메서드에서 사용하게 된다면 이에 대한 순서를 계속해 고려해야 하기 때문입니다.
그래서 저는 보통 호출되는 순서대로 작성했었습니다. 하단에서 상단의 메서드를 사용하더라도 이미 본 코드기에 이해하기 편하기도 하고요!

그런데, 메서드 정렬을 논리적 응집도를 사용하게 된다면 어떤 형식으로 되는 것인지 잘 모르겠는데 설명을 좀 더 해주실 수 있을까요?

Copy link
Member

Choose a reason for hiding this comment

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

논리적 응집도를 높이는 방식이 호출 순서대로 작성하는 방식입니다. 예를 들어 update 메서드에서 차례대로 getUser, getGoal, validateTeamsToUpdate 함수를 호출한다면 update 메서드에서 아래에 세가지 함수를 순서대로 정렬하는 것입니다. 가독성을 위해 하나의 큰 메서드를 기준으로 해서 호출되는 메서드를 주위에 정렬하는 방식으로 이해했습니다.
정리를 깔끔하게 하고자 하는 마음에 검증 메서드를 아예 하위로 빼봤는데 가독성이 더 중요하니 하던대로 호출 순서대로 정렬을 할까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

저희가 슬랙에서 이야기한 결과 메서드 작성 순서는 큰 형식은 CRUD 순으로 작성하는 것으로 결정되었습니다.
또한, private과 같이 public 메서드들에서 사용되는 메서드들은 모두 호출 순서로 public 메서드 사이에 위치하도록 결정되었습니다.


public void withdraw(final Long userId, final String refreshToken) {
final AuthClaims authClaims = tokenProvider.parseToken(TokenType.REFRESH, refreshToken);
final User user = validateAndGetUser(userId, authClaims);

user.delete();
blackListTokenService.register(refreshToken);
deviceTokenService.deactivateAllByUserId(user.getId());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package com.backend.blooming.authentication.application;

import com.backend.blooming.authentication.application.exception.AlreadyRegisterBlackListTokenException;
import com.backend.blooming.authentication.domain.BlackListToken;
import com.backend.blooming.authentication.infrastructure.blacklist.BlackListTokenRepository;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@Service
@Transactional
@RequiredArgsConstructor
public class BlackListTokenService {

private final BlackListTokenRepository blackListTokenRepository;

public Long register(final String token) {
validateToken(token);
final BlackListToken blackListToken = new BlackListToken(token);

return blackListTokenRepository.save(blackListToken)
.getId();
}

private void validateToken(String token) {
if (blackListTokenRepository.existsByToken(token)) {
throw new AlreadyRegisterBlackListTokenException();
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package com.backend.blooming.authentication.application.dto;

import com.backend.blooming.authentication.presentation.dto.LogoutRequest;

public record LogoutDto(String refreshToken, String deviceToken) {

public static LogoutDto from(final LogoutRequest logoutRequest) {
return new LogoutDto(logoutRequest.refreshToken(), logoutRequest.deviceToken());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package com.backend.blooming.authentication.application.exception;

import com.backend.blooming.exception.BloomingException;
import com.backend.blooming.exception.ExceptionMessage;

public class AlreadyRegisterBlackListTokenException extends BloomingException {

public AlreadyRegisterBlackListTokenException() {
super(ExceptionMessage.ALREADY_REGISTER_BLACK_LIST_TOKEN);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ public class AuthenticationWebMvcConfiguration implements WebMvcConfigurer {
@Override
public void addInterceptors(final InterceptorRegistry registry) {
registry.addInterceptor(authenticationInterceptor)
.addPathPatterns("/**")
.excludePathPatterns("/auth/**");
.addPathPatterns("/**");
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package com.backend.blooming.authentication.domain;

import jakarta.persistence.Entity;
import jakarta.persistence.GeneratedValue;
import jakarta.persistence.GenerationType;
import jakarta.persistence.Id;
import jakarta.persistence.Table;
import lombok.AccessLevel;
import lombok.EqualsAndHashCode;
import lombok.Getter;
import lombok.NoArgsConstructor;
import lombok.ToString;

@Entity
@NoArgsConstructor(access = AccessLevel.PROTECTED)
@Getter
@EqualsAndHashCode(of = "id", callSuper = false)
@ToString
@Table
public class BlackListToken {

@Id
Copy link
Member

Choose a reason for hiding this comment

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

개행이 필요할 것 같네요!

@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;

private String token;

public BlackListToken(final String token) {
this.token = token;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package com.backend.blooming.authentication.infrastructure.blacklist;

import com.backend.blooming.authentication.domain.BlackListToken;
import org.springframework.data.jpa.repository.JpaRepository;

import java.util.Optional;

public interface BlackListTokenRepository extends JpaRepository<BlackListToken, Long> {

Optional<BlackListToken> findByToken(final String token);

boolean existsByToken(final String token);
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,21 @@
import com.backend.blooming.authentication.application.AuthenticationService;
import com.backend.blooming.authentication.application.dto.LoginDto;
import com.backend.blooming.authentication.application.dto.LoginInformationDto;
import com.backend.blooming.authentication.application.dto.LogoutDto;
import com.backend.blooming.authentication.application.dto.TokenDto;
import com.backend.blooming.authentication.infrastructure.oauth.OAuthType;
import com.backend.blooming.authentication.presentation.anotaion.Authenticated;
import com.backend.blooming.authentication.presentation.argumentresolver.AuthenticatedUser;
import com.backend.blooming.authentication.presentation.dto.LogoutRequest;
import com.backend.blooming.authentication.presentation.dto.WithdrawRequest;
import com.backend.blooming.authentication.presentation.dto.request.ReissueAccessTokenRequest;
import com.backend.blooming.authentication.presentation.dto.response.LoginInformationResponse;
import com.backend.blooming.authentication.presentation.dto.response.SocialLoginRequest;
import com.backend.blooming.authentication.presentation.dto.response.TokenResponse;
import lombok.RequiredArgsConstructor;
import org.springframework.http.ResponseEntity;
import org.springframework.stereotype.Controller;
import org.springframework.web.bind.annotation.DeleteMapping;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestBody;
Expand Down Expand Up @@ -45,4 +51,26 @@ public ResponseEntity<TokenResponse> reissueAccessToken(

return ResponseEntity.ok(TokenResponse.from(tokenDto));
}

@PostMapping(value = "/logout", headers = "X-API-VERSION=1")
public ResponseEntity<Void> logout(
@Authenticated final AuthenticatedUser authenticatedUser,
@RequestBody final LogoutRequest logoutRequest
) {
authenticationService.logout(authenticatedUser.userId(), LogoutDto.from(logoutRequest));

return ResponseEntity.noContent()
.build();
}

@DeleteMapping(headers = "X-API-VERSION=1")
public ResponseEntity<Void> withdraw(
@Authenticated final AuthenticatedUser authenticatedUser,
@RequestBody final WithdrawRequest withdrawRequest
) {
authenticationService.withdraw(authenticatedUser.userId(), withdrawRequest.refreshToken());

return ResponseEntity.noContent()
.build();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
package com.backend.blooming.authentication.presentation.dto;

public record LogoutRequest(String refreshToken, String deviceToken) {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
package com.backend.blooming.authentication.presentation.dto;

public record WithdrawRequest(String refreshToken) {
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import org.springframework.transaction.annotation.Transactional;

import java.util.List;
import java.util.Optional;

@Service
@Transactional
Expand All @@ -16,7 +17,7 @@ public class DeviceTokenService {

private final DeviceTokenRepository deviceTokenRepository;

public Long saveOrActive(final Long userId, final String token) {
public Long saveOrActivate(final Long userId, final String token) {
final DeviceToken deviceToken = findOrPersistDeviceToken(userId, token);
activateIfInactive(deviceToken);

Expand Down Expand Up @@ -46,4 +47,14 @@ public ReadDeviceTokensDto readAllByUserId(final Long userId) {

return ReadDeviceTokensDto.from(deviceTokens);
}

public void deactivate(final Long userId, final String token) {
final Optional<DeviceToken> deviceToken = deviceTokenRepository.findByUserIdAndToken(userId, token);
deviceToken.ifPresent(DeviceToken::deactivate);
}

public void deactivateAllByUserId(final Long userId) {
final List<DeviceToken> deviceTokens = deviceTokenRepository.findAllByUserIdAndActiveIsTrue(userId);
deviceTokens.forEach(DeviceToken::deactivate);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ public enum ExceptionMessage {
EXPIRED_TOKEN("기한이 만료된 토큰입니다."),
UNSUPPORTED_OAUTH_TYPE("지원하지 않는 소셜 로그인 방식입니다."),

// 블랙 리스트 토큰
ALREADY_REGISTER_BLACK_LIST_TOKEN("이미 등록된 블랙 리스트 토큰입니다."),

// 사용자
NOT_FOUND_USER("사용자를 조회할 수 없습니다."),
NULL_OR_EMPTY_EMAIL("이메일은 비어있을 수 없습니다."),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.backend.blooming.exception;

import com.backend.blooming.authentication.application.exception.AlreadyRegisterBlackListTokenException;
import com.backend.blooming.authentication.infrastructure.exception.InvalidTokenException;
import com.backend.blooming.authentication.infrastructure.exception.OAuthException;
import com.backend.blooming.authentication.infrastructure.exception.UnsupportedOAuthTypeException;
Expand Down Expand Up @@ -170,4 +171,14 @@ public ResponseEntity<ExceptionResponse> handleDeleteFriendForbiddenException(
return ResponseEntity.status(HttpStatus.FORBIDDEN)
.body(new ExceptionResponse(exception.getMessage()));
}

@ExceptionHandler(AlreadyRegisterBlackListTokenException.class)
public ResponseEntity<ExceptionResponse> handleAlreadyRegisterBlackListTokenException(
final AlreadyRegisterBlackListTokenException exception
) {
logger.warn(String.format(LOG_MESSAGE_FORMAT, exception.getClass().getSimpleName(), exception.getMessage()));

return ResponseEntity.status(HttpStatus.BAD_REQUEST)
.body(new ExceptionResponse(exception.getMessage()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,26 @@

import com.backend.blooming.authentication.application.dto.LoginInformationDto;
import com.backend.blooming.authentication.application.dto.TokenDto;
import com.backend.blooming.authentication.domain.BlackListToken;
import com.backend.blooming.authentication.infrastructure.blacklist.BlackListTokenRepository;
import com.backend.blooming.authentication.infrastructure.exception.InvalidTokenException;
import com.backend.blooming.authentication.infrastructure.exception.OAuthException;
import com.backend.blooming.authentication.infrastructure.exception.UnsupportedOAuthTypeException;
import com.backend.blooming.authentication.infrastructure.oauth.OAuthClient;
import com.backend.blooming.configuration.IsolateDatabase;
import com.backend.blooming.devicetoken.domain.DeviceToken;
import com.backend.blooming.devicetoken.infrastructure.repository.DeviceTokenRepository;
import com.backend.blooming.user.domain.User;
import com.backend.blooming.user.infrastructure.repository.UserRepository;
import org.assertj.core.api.SoftAssertions;
import org.junit.jupiter.api.DisplayNameGeneration;
import org.junit.jupiter.api.DisplayNameGenerator;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.mock.mockito.SpyBean;

import java.util.List;

import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.assertj.core.api.SoftAssertions.assertSoftly;
import static org.mockito.BDDMockito.willReturn;
Expand All @@ -34,6 +41,12 @@ class AuthenticationServiceTest extends AuthenticationServiceTestFixture {
@Autowired
private UserRepository userRepository;

@Autowired
private DeviceTokenRepository deviceTokenRepository;

@Autowired
private BlackListTokenRepository blackListTokenRepository;

@Test
void 로그인시_존재하지_않는_사용자인_경우_해당_사용자를_저장후_토큰_정보를_반환한다() {
// given
Expand Down Expand Up @@ -155,4 +168,51 @@ class AuthenticationServiceTest extends AuthenticationServiceTestFixture {
assertThatThrownBy(() -> authenticationService.reissueAccessToken(유효하지_않는_타입의_refresh_token))
.isInstanceOf(InvalidTokenException.class);
}

@Test
void 로그아웃시_디바이스_토큰과_액세스_토큰을_비활성화한다() {
// when
authenticationService.logout(기존_사용자.getId(), 로그아웃_dto);

// then
final BlackListToken blackListToken = blackListTokenRepository.findByToken(로그아웃_dto.refreshToken()).get();
final DeviceToken deviceToken = deviceTokenRepository.findByUserIdAndToken(
기존_사용자.getId(),
로그아웃_dto.deviceToken()
).get();
SoftAssertions.assertSoftly(softAssertions -> {
softAssertions.assertThat(blackListToken.getToken()).isEqualTo(로그아웃_dto.refreshToken());
softAssertions.assertThat(deviceToken.isActive()).isFalse();
});
}

@Test
void 로그아웃시_리프레시_토큰이_유효하지_않다면_예외를_발생시킨다() {
// when & then
assertThatThrownBy(() -> authenticationService.logout(기존_사용자.getId(), 유효하지_않은_리프레시_토큰을_갖는_로그아웃_dto))
.isInstanceOf(InvalidTokenException.class);
}

@Test
void 로그아웃시_디바이스_토큰과_액세스_토큰을_비활성화하고_사용자의_삭제_여부를_참으로_변경한다() {
// when
authenticationService.withdraw(기존_사용자.getId(), 유효한_refresh_token);

// then
final User user = userRepository.findById(기존_사용자.getId()).get();
final BlackListToken blackListToken = blackListTokenRepository.findByToken(유효한_refresh_token).get();
final List<DeviceToken> deviceTokens = deviceTokenRepository.findAllByUserIdAndActiveIsTrue(기존_사용자.getId());
SoftAssertions.assertSoftly(softAssertions -> {
softAssertions.assertThat(user.isDeleted()).isTrue();
softAssertions.assertThat(blackListToken.getToken()).isEqualTo(유효한_refresh_token);
softAssertions.assertThat(deviceTokens).isEmpty();
});
}

@Test
void 탈퇴시_리프레시_토큰이_유효하지_않다면_예외를_발생시킨다() {
// when & then
assertThatThrownBy(() -> authenticationService.withdraw(기존_사용자.getId(), 유효하지_않는_refresh_token))
.isInstanceOf(InvalidTokenException.class);
}
}
Loading
Loading