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

회원 탈퇴 시 소셜 로그인 방식을 DB에서 조회하도록 변경 #626

Merged
merged 8 commits into from
Oct 13, 2023
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package com.ddang.ddang.authentication.application;

import static com.ddang.ddang.image.domain.ProfileImage.DEFAULT_PROFILE_IMAGE_STORE_NAME;

import com.ddang.ddang.authentication.application.dto.TokenDto;
import com.ddang.ddang.authentication.application.exception.InvalidWithdrawalException;
import com.ddang.ddang.authentication.application.util.RandomNameGenerator;
Expand All @@ -21,15 +23,12 @@
import com.ddang.ddang.user.domain.Reliability;
import com.ddang.ddang.user.domain.User;
import com.ddang.ddang.user.infrastructure.persistence.JpaUserRepository;
import java.time.LocalDateTime;
import java.util.Map;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

import java.time.LocalDateTime;
import java.util.Map;

import static com.ddang.ddang.image.domain.ProfileImage.DEFAULT_PROFILE_IMAGE_STORE_NAME;

@Service
@Transactional(readOnly = true)
@RequiredArgsConstructor
Expand Down Expand Up @@ -63,7 +62,7 @@ private void updateOrPersistDeviceToken(final String deviceToken, final User per
}

private User findOrPersistUser(final Oauth2Type oauth2Type, final UserInformationDto userInformationDto) {
return userRepository.findByOauthIdAndDeletedIsFalse(userInformationDto.findUserId())
return userRepository.findByOauthId(userInformationDto.findUserId())
.orElseGet(() -> {
final User user = User.builder()
.name(oauth2Type.calculateNickname(calculateRandomNumber()))
Expand Down Expand Up @@ -131,21 +130,21 @@ public boolean validateToken(final String accessToken) {

@Transactional
public void withdrawal(
final Oauth2Type oauth2Type,
final String accessToken,
final String refreshToken
) throws InvalidWithdrawalException {
final OAuth2UserInformationProvider provider = providerComposite.findProvider(oauth2Type);
final PrivateClaims privateClaims = tokenDecoder.decode(TokenType.ACCESS, accessToken)
.orElseThrow(() ->
new InvalidTokenException("유효한 토큰이 아닙니다.")
);
final User user = userRepository.findByIdAndDeletedIsFalse(privateClaims.userId())
.orElseThrow(() -> new InvalidWithdrawalException("탈퇴에 대한 권한 없습니다."));
.orElseThrow(() -> new InvalidWithdrawalException("탈퇴에 대한 권한이 없습니다."));
final OAuth2UserInformationProvider provider = providerComposite.findProvider(user.getOauthInformation()
.getOauth2Type());

user.withdrawal();
blackListTokenService.registerBlackListToken(accessToken, refreshToken);
deviceTokenRepository.deleteByUserId(user.getId());
provider.unlinkUserBy(user.getOauthId());
provider.unlinkUserBy(user.getOauthInformation().getOauthId());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,12 @@ public ResponseEntity<Void> logout(
.build();
}

@PostMapping("/withdrawal/{oauth2Type}")
@PostMapping("/withdrawal")
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
Collaborator Author

Choose a reason for hiding this comment

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

👍

public ResponseEntity<Void> withdrawal(
@PathVariable final Oauth2Type oauth2Type,
@RequestHeader(HttpHeaders.AUTHORIZATION) final String accessToken,
@RequestBody @Valid final WithdrawalRequest request
) {
authenticationService.withdrawal(oauth2Type, accessToken, request.refreshToken());
authenticationService.withdrawal(accessToken, request.refreshToken());

return ResponseEntity.noContent()
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public static ReadUserInQnaDto from(final User writer) {
writer.getName(),
writer.getProfileImage().getId(),
writer.getReliability().getValue(),
writer.getOauthId(),
writer.getOauthInformation().getOauthId(),
writer.isDeleted()
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public static ReadUserInReportDto from(final User user) {
user.getName(),
ImageIdProcessor.process(user.getProfileImage()),
user.getReliability().getValue(),
user.getOauthId(),
user.getOauthInformation().getOauthId(),
user.isDeleted()
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ public static ReadUserInReviewDto from(final User user) {
user.getName(),
user.getProfileImage().getId(),
user.getReliability().getValue(),
user.getOauthId()
user.getOauthInformation().getOauthId()
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public static ReadUserDto from(final User user) {
user.getName(),
ImageIdProcessor.process(user.getProfileImage()),
user.getReliability().getValue(),
user.getOauthId(),
user.getOauthInformation().getOauthId(),
user.isDeleted()
);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package com.ddang.ddang.user.domain;

import com.ddang.ddang.authentication.infrastructure.oauth2.Oauth2Type;
import jakarta.persistence.Column;
import jakarta.persistence.Embeddable;
import jakarta.persistence.EnumType;
import jakarta.persistence.Enumerated;
import lombok.AccessLevel;
import lombok.EqualsAndHashCode;
import lombok.Getter;
import lombok.NoArgsConstructor;
import lombok.ToString;

@Embeddable
@NoArgsConstructor(access = AccessLevel.PROTECTED)
@Getter
@EqualsAndHashCode
@ToString
public class OauthInformation {

private String oauthId;

@Column(name = "oauth2_type")
@Enumerated(EnumType.STRING)
private Oauth2Type oauth2Type;

public OauthInformation(final String oauthId, final Oauth2Type oauth2Type) {
this.oauthId = oauthId;
this.oauth2Type = oauth2Type;
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.ddang.ddang.user.domain;

import com.ddang.ddang.authentication.infrastructure.oauth2.Oauth2Type;
import com.ddang.ddang.common.entity.BaseTimeEntity;
import com.ddang.ddang.image.domain.ProfileImage;
import com.ddang.ddang.review.domain.Review;
Expand Down Expand Up @@ -29,12 +30,12 @@
@NoArgsConstructor(access = AccessLevel.PROTECTED)
@Getter
@EqualsAndHashCode(of = "id", callSuper = false)
@ToString(of = {"id", "name", "reliability", "oauthId", "deleted"})
@ToString(of = {"id", "name", "reliability", "oauthId", "deleted", "oauth2Type"})
@Table(name = "users")
public class User extends BaseTimeEntity {

private static final boolean DELETED_STATUS = true;
private static final String UNKOWN_NAME = "알 수 없음";
private static final String UNKNOWN_NAME = "알 수 없음";

@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
Expand All @@ -51,7 +52,8 @@ public class User extends BaseTimeEntity {
@AttributeOverride(name = "value", column = @Column(name = "reliability"))
private Reliability reliability;

private String oauthId;
@Embedded
private OauthInformation oauthInformation;

@Column(name = "is_deleted")
private boolean deleted = false;
Expand All @@ -61,12 +63,13 @@ private User(
final String name,
final ProfileImage profileImage,
final Reliability reliability,
final String oauthId
final String oauthId,
final Oauth2Type oauth2Type
) {
this.name = name;
this.profileImage = profileImage;
this.reliability = processReliability(reliability);
this.oauthId = oauthId;
this.oauthInformation = new OauthInformation(oauthId, oauth2Type);
}

private Reliability processReliability(final Reliability reliability) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,16 @@
import org.springframework.data.jpa.repository.JpaRepository;

import java.util.Optional;
import org.springframework.data.jpa.repository.Query;

public interface JpaUserRepository extends JpaRepository<User, Long> {

Optional<User> findByOauthIdAndDeletedIsFalse(final String oauthId);
@Query("""
SELECT u
FROM User u
WHERE u.deleted = false AND u.oauthInformation.oauthId = :oauthId
""")
Optional<User> findByOauthId(final String oauthId);
Copy link
Collaborator

Choose a reason for hiding this comment

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

질문

이건 제가 헷갈려서 드리는 질문입니다.
findPureUserByOauthId와 같이 변경해주지 않아도 괜찮은가요?
'경매 목록 조회 시 마감된 경매의 정렬 순서를 후순위로 변경' PR에서는 pureXX와 같이 변경해주셨길래 fetch join을 하는 항목이 없으면 pure를 무조건 붙이기로 했었는지 헷갈려서 질문 남깁니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

일단 그 부분은 정확한 논의가 없던 것으로 기억합니다

근데 일단...pure기는 한데 fetch join하는 구간이 없어서 안붙이기는 했습니다...
fetch join을 하지 않는 레포지토리에서도 Pure로 명시를 해주는게 좋을까요?


Optional<User> findByIdAndDeletedIsFalse(final Long id);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
ALTER TABLE users ADD oauth2_type VARCHAR(10);
UPDATE users SET oauth2_type = 'KAKAO' WHERE oauth2_type is null;
ALTER TABLE users MODIFY oauth2_type VARCHAR(10) NOT NULL;
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.

갑자기 불안해지네요

Copy link
Member

Choose a reason for hiding this comment

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

22 믿습니다 지토 🫡

Copy link
Collaborator

Choose a reason for hiding this comment

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

저도 믿습니다 🙏

Original file line number Diff line number Diff line change
Expand Up @@ -224,11 +224,12 @@ void setUp() {
@Test
void 가입한_회원이_탈퇴하는_경우_정상처리한다() throws InvalidWithdrawalException {
// given
//given(tokenDecoder.decode(TokenType.ACCESS, anyString())).willReturn(Optional.of(사용자_id_클레임));
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.

삭제하려고 했었는데 까먹고 안했네요 수정하도록 하겠습니다

given(providerComposite.findProvider(지원하는_소셜_로그인_타입)).willReturn(userInfoProvider);
given(userInfoProvider.findUserInformation(anyString())).willReturn(사용자_회원_정보);

// when
authenticationService.withdrawal(지원하는_소셜_로그인_타입, 유효한_액세스_토큰, 유효한_리프레시_토큰);
authenticationService.withdrawal(유효한_액세스_토큰, 유효한_리프레시_토큰);

// then
assertThat(사용자.isDeleted()).isTrue();
Expand All @@ -241,9 +242,9 @@ void setUp() {
given(userInfoProvider.unlinkUserBy(anyString())).willReturn(탈퇴한_사용자_회원_정보);

// when && then
assertThatThrownBy(() -> authenticationService.withdrawal(지원하는_소셜_로그인_타입, 탈퇴한_사용자_액세스_토큰, 유효한_리프레시_토큰))
assertThatThrownBy(() -> authenticationService.withdrawal(탈퇴한_사용자_액세스_토큰, 유효한_리프레시_토큰))
.isInstanceOf(InvalidWithdrawalException.class)
.hasMessage("탈퇴에 대한 권한 없습니다.");
.hasMessage("탈퇴에 대한 권한이 없습니다.");
}

@Test
Expand All @@ -253,15 +254,15 @@ void setUp() {
given(userInfoProvider.findUserInformation(anyString())).willThrow(new InvalidTokenException("401 Unauthorized"));

// when & then
assertThatThrownBy(() -> authenticationService.withdrawal(지원하는_소셜_로그인_타입, 존재하지_않는_사용자_액세스_토큰, 유효한_리프레시_토큰))
assertThatThrownBy(() -> authenticationService.withdrawal(존재하지_않는_사용자_액세스_토큰, 유효한_리프레시_토큰))
.isInstanceOf(InvalidWithdrawalException.class)
.hasMessage("탈퇴에 대한 권한 없습니다.");
.hasMessage("탈퇴에 대한 권한이 없습니다.");
}

@Test
void 탈퇴할_때_유효한_토큰이_아닌_경우_예외가_발생한다() {
// when & then
assertThatThrownBy(() -> authenticationService.withdrawal(지원하는_소셜_로그인_타입, 유효하지_않은_액세스_토큰, 유효한_리프레시_토큰))
assertThatThrownBy(() -> authenticationService.withdrawal(유효하지_않은_액세스_토큰, 유효한_리프레시_토큰))
.isInstanceOf(InvalidTokenException.class)
.hasMessage("유효한 토큰이 아닙니다.");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.ddang.ddang.authentication.domain.TokenEncoder;
import com.ddang.ddang.authentication.domain.TokenType;
import com.ddang.ddang.authentication.domain.dto.UserInformationDto;
import com.ddang.ddang.authentication.infrastructure.jwt.PrivateClaims;
import com.ddang.ddang.authentication.infrastructure.oauth2.Oauth2Type;
import com.ddang.ddang.device.domain.DeviceToken;
import com.ddang.ddang.device.infrastructure.persistence.JpaDeviceTokenRepository;
Expand Down Expand Up @@ -33,6 +34,8 @@ public class AuthenticationServiceFixture {
protected User 사용자;
protected User 탈퇴한_사용자;

protected PrivateClaims 사용자_id_클레임 = new PrivateClaims(1L);

protected UserInformationDto 사용자_회원_정보 = new UserInformationDto(12345L);
protected UserInformationDto 탈퇴한_사용자_회원_정보 = new UserInformationDto(54321L);
protected UserInformationDto 가입하지_않은_사용자_회원_정보 = new UserInformationDto(-99999L);
Expand Down Expand Up @@ -67,13 +70,15 @@ void fixtureSetUp() {
.profileImage(new ProfileImage("upload.png", "store.png"))
.reliability(new Reliability(0.0d))
.oauthId("12345")
.oauth2Type(Oauth2Type.KAKAO)
.build();

탈퇴한_사용자 = User.builder()
.name("kakao12346")
.profileImage(new ProfileImage("upload.png", "store.png"))
.reliability(new Reliability(0.0d))
.oauthId("12346")
.oauth2Type(Oauth2Type.KAKAO)
.build();

userRepository.save(사용자);
Expand All @@ -90,7 +95,7 @@ void fixtureSetUp() {
);

만료된_리프레시_토큰 = tokenEncoder.encode(
LocalDateTime.ofInstant(Instant.parse("2023-01-01T22:21:20Z"), ZoneId.of("UTC")),
LocalDateTime.ofInstant(Instant.parse("2023-01-01T22:21:20Z"), ZoneId.of("UTC")),
TokenType.REFRESH,
Map.of("userId", 1L)
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package com.ddang.ddang.authentication.presentation;

import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.BDDMockito.given;
Expand Down Expand Up @@ -201,11 +200,11 @@ void setUp() {
@Test
void ouath2Type과_accessToken과_refreshToken을_전달하면_탈퇴한다() throws Exception {
// given
willDoNothing().given(authenticationService).withdrawal(any(), anyString(), anyString());
willDoNothing().given(authenticationService).withdrawal(anyString(), anyString());

// when & then
final ResultActions resultActions =
mockMvc.perform(RestDocumentationRequestBuilders.post("/oauth2/withdrawal/{oauth2Type}", 소셜_로그인_타입)
mockMvc.perform(RestDocumentationRequestBuilders.post("/oauth2/withdrawal")
.contentType(MediaType.APPLICATION_JSON)
.content(objectMapper.writeValueAsString(유효한_회원탈퇴_요청))
.header(HttpHeaders.AUTHORIZATION, 유효한_액세스_토큰_내용)
Expand All @@ -221,10 +220,10 @@ void setUp() {
void ouath2Type과_accessToken과_refreshToken을_전달시_이미_탈퇴_혹은_존재하지_않아_권한이_없는_회원인_경우_403을_반환한다() throws Exception {
// given
willThrow(new InvalidWithdrawalException("탈퇴에 대한 권한 없습니다.")).given(authenticationService)
.withdrawal(any(), anyString(), anyString());
.withdrawal(anyString(), anyString());

// when & then
mockMvc.perform(RestDocumentationRequestBuilders.post("/oauth2/withdrawal/{oauth2Type}", 소셜_로그인_타입)
mockMvc.perform(RestDocumentationRequestBuilders.post("/oauth2/withdrawal")
.header(HttpHeaders.AUTHORIZATION, 유효한_액세스_토큰_내용)
.contentType(MediaType.APPLICATION_JSON)
.content(objectMapper.writeValueAsString(유효하지_않은_회원탈퇴_요청))
Expand Down Expand Up @@ -301,9 +300,6 @@ void setUp() {
private void withdrawal_문서화(final ResultActions resultActions) throws Exception {
resultActions.andDo(
restDocs.document(
pathParameters(
parameterWithName("oauth2Type").description("소셜 로그인을 할 서비스 선택(kakao로 고정)")
),
requestHeaders(
headerWithName("Authorization").description("회원 Bearer 인증 정보")
),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,23 +1,22 @@
package com.ddang.ddang.user.infrastructure.persistence;

import static org.assertj.core.api.Assertions.assertThat;

import com.ddang.ddang.configuration.JpaConfiguration;
import com.ddang.ddang.configuration.QuerydslConfiguration;
import com.ddang.ddang.user.domain.Reliability;
import com.ddang.ddang.user.domain.User;
import com.ddang.ddang.user.infrastructure.persistence.fixture.JpaUserRepositoryFixture;
import jakarta.persistence.EntityManager;
import jakarta.persistence.PersistenceContext;
import java.util.Optional;
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.autoconfigure.orm.jpa.DataJpaTest;
import org.springframework.context.annotation.Import;

import java.util.Optional;

import static org.assertj.core.api.Assertions.assertThat;

@DataJpaTest
@Import({JpaConfiguration.class, QuerydslConfiguration.class})
@DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class)
Expand Down Expand Up @@ -53,7 +52,7 @@ class JpaUserRepositoryTest extends JpaUserRepositoryFixture {
@Test
void 존재하는_oauthId를_전달하면_해당_회원을_Optional로_감싸_반환한다() {
// when
final Optional<User> actual = userRepository.findByOauthIdAndDeletedIsFalse(사용자.getOauthId());
final Optional<User> actual = userRepository.findByOauthId(사용자.getOauthInformation().getOauthId());

// then
assertThat(actual).contains(사용자);
Expand All @@ -62,7 +61,7 @@ class JpaUserRepositoryTest extends JpaUserRepositoryFixture {
@Test
void 존재하지_않는_oauthId를_전달하면_해당_회원을_빈_Optional로_반환한다() {
// when
final Optional<User> actual = userRepository.findByOauthIdAndDeletedIsFalse(존재하지_않는_oauth_아이디);
final Optional<User> actual = userRepository.findByOauthId(존재하지_않는_oauth_아이디);

// then
assertThat(actual).isEmpty();
Expand Down
Loading