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

탈퇴 기능 추가 #378

Merged
merged 25 commits into from
Sep 15, 2023
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
854c900
feat: 카카오 연결 끊기 기능 추가
JJ503 Sep 11, 2023
fa53da7
feat: 탈퇴 기능 서비스 추가
JJ503 Sep 12, 2023
692abb0
feat: oauth id에 대한 unique 속성 제거
JJ503 Sep 12, 2023
4436508
refactor: 회원과 사용자라는 용어 통일
JJ503 Sep 12, 2023
ed70865
feat: 이름이 이미 존재하는지에 대한 확인 쿼리 메서드 추가
JJ503 Sep 14, 2023
8635682
feat: 랜덤 이름을 생성하는 util 클래스 추가
JJ503 Sep 14, 2023
5aff1f4
feat: 재가입 시 이름에 대한 중복 문제 해결을 위한 로직 추가
JJ503 Sep 14, 2023
e20b6e4
feat: 탈퇴한 회원의 이름을 가져오는 경우에 대한 로직 추가
JJ503 Sep 14, 2023
621590a
feat: 탈퇴 컨트롤러 기능 추가
JJ503 Sep 15, 2023
059f81c
feat: 예외처리 추가
JJ503 Sep 15, 2023
6cbadb8
test: 테스트 실패 문제 해결
JJ503 Sep 15, 2023
01cccdc
refactor: flyway 버전 수정
JJ503 Sep 15, 2023
ea5d5f1
refactor: 탈퇴한 사용자 이름 변경 로직 위치 수정
JJ503 Sep 15, 2023
e63f83c
docs: 문서 최신화
JJ503 Sep 15, 2023
72ed2e1
refactor: 예외 메시지 클래스명 수정
JJ503 Sep 15, 2023
94433a3
refactor: do-while문을 while문으로 수정
JJ503 Sep 15, 2023
898cefb
refactor: 탈퇴 시 로직 순서 변경
JJ503 Sep 15, 2023
797ceca
style: 해결된 todo 제거
JJ503 Sep 15, 2023
ba0ab60
ci: flyway 버전 수정
JJ503 Sep 15, 2023
75fd058
ci: 충돌 문제 해결
JJ503 Sep 15, 2023
559c0b7
ci: 충돌 문제 해결
JJ503 Sep 15, 2023
af0f74e
refactor: 이미지가 null인 경우에 대한 예외처리 추가
JJ503 Sep 15, 2023
f70c71a
refactor: util 클래스에 final 추가
JJ503 Sep 15, 2023
de24d84
ci: 충돌 문제 해결
JJ503 Sep 15, 2023
662b4f7
fix: 경매 이미지 url 경로 누락 문제 해결
JJ503 Sep 15, 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
Original file line number Diff line number Diff line change
@@ -1,23 +1,26 @@
package com.ddang.ddang.authentication.application;

import com.ddang.ddang.authentication.application.dto.TokenDto;
import com.ddang.ddang.authentication.domain.exception.InvalidTokenException;
import com.ddang.ddang.authentication.application.exception.InaccessibleWithdrawalException;
import com.ddang.ddang.authentication.application.util.RandomNameGenerator;
import com.ddang.ddang.authentication.domain.Oauth2UserInformationProviderComposite;
import com.ddang.ddang.authentication.infrastructure.jwt.PrivateClaims;
import com.ddang.ddang.authentication.domain.TokenDecoder;
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.domain.exception.InvalidTokenException;
import com.ddang.ddang.authentication.infrastructure.jwt.PrivateClaims;
import com.ddang.ddang.authentication.infrastructure.oauth2.OAuth2UserInformationProvider;
import com.ddang.ddang.authentication.infrastructure.oauth2.Oauth2Type;
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;

@Service
@Transactional(readOnly = true)
@RequiredArgsConstructor
Expand All @@ -31,7 +34,7 @@ public class AuthenticationService {
private final TokenDecoder tokenDecoder;

@Transactional
public TokenDto login(final Oauth2Type oauth2Type, final String oauth2AccessToken) {
public TokenDto login(final Oauth2Type oauth2Type, final String oauth2AccessToken){
final OAuth2UserInformationProvider provider = providerComposite.findProvider(oauth2Type);
final UserInformationDto userInformationDto = provider.findUserInformation(oauth2AccessToken);
final User persistUser = findOrPersistUser(oauth2Type, userInformationDto);
Expand All @@ -40,10 +43,10 @@ public TokenDto login(final Oauth2Type oauth2Type, final String oauth2AccessToke
}

private User findOrPersistUser(final Oauth2Type oauth2Type, final UserInformationDto userInformationDto) {
return userRepository.findByOauthId(userInformationDto.findUserId())
return userRepository.findByOauthIdAndDeletedIsFalse(userInformationDto.findUserId())
.orElseGet(() -> {
final User user = User.builder()
.name(oauth2Type.calculateNickname(userInformationDto))
.name(oauth2Type.calculateNickname(calculateRandomNumber()))
.profileImage(null)
.reliability(0.0d)
.oauthId(userInformationDto.findUserId())
Expand All @@ -53,6 +56,20 @@ private User findOrPersistUser(final Oauth2Type oauth2Type, final UserInformatio
});
}

private String calculateRandomNumber() {
String name;

do {
name = RandomNameGenerator.generate();
} while (isAlreadyExist(name));
Copy link
Collaborator

Choose a reason for hiding this comment

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

do-while문은 가독성에 좋지 않다고 생각합니다
그리고 calculateRandomNumber()라는 메서드 네이밍에 비해 반환하는건 name이라 좀 애매한 것 같습니다
name도 값을 초기화하지 않는건 위험하다고 생각합니다

하지만 이건 다른 분들의 의견도 궁금하네요

Copy link
Collaborator

Choose a reason for hiding this comment

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

대충 전 이렇게 할 것 같기는 합니다

String name = RandomNameGenerator.generate();

while (isAlreadyExist(name)) {
    name = RandomNameGenerator.generate();
}

return name;


return name;
}

private boolean isAlreadyExist(final String name) {
return userRepository.existsByNameEndingWith(name);
}

private TokenDto convertTokenDto(final User persistUser) {
final String accessToken = tokenEncoder.encode(
LocalDateTime.now(),
Expand Down Expand Up @@ -86,4 +103,18 @@ public boolean validateToken(final String accessToken) {
return tokenDecoder.decode(TokenType.ACCESS, accessToken)
.isPresent();
}

@Transactional
public void withdrawal(
final Oauth2Type oauth2Type,
final String oauth2AccessToken
) throws InaccessibleWithdrawalException {
final OAuth2UserInformationProvider provider = providerComposite.findProvider(oauth2Type);
final UserInformationDto userInformationDto = provider.findUserInformation(oauth2AccessToken);
final User user = userRepository.findByOauthIdAndDeletedIsFalse(userInformationDto.findUserId())
.orElseThrow(() -> new InaccessibleWithdrawalException("탈퇴에 대한 권한 없습니다."));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Inaccessible보다 Invalid라는 네이밍이 많아서 변경을 부탁드리고 싶습니다
예외 메세지도 탈퇴에 대한 권한이 없습니다. 정도로..?

Copy link
Member Author

Choose a reason for hiding this comment

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

저 몰래 Invalid로 바꾸시면 안 됩니다... 속상

Copy link
Collaborator

Choose a reason for hiding this comment

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

일단 저는 아닙니다


provider.unlinkUserBy(oauth2AccessToken, user.getOauthId());
user.withdrawal();
Copy link
Collaborator

Choose a reason for hiding this comment

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

근데 순서를 탈퇴 -> 탈퇴로 인한 사용자 레코드 DB 변경
DB 변경에 성공하면 토큰 비활성화 -> 블랙리스트 토큰쪽 DB에 저장
사용자 레코드 DB 변경 + 블랙리스트 토큰 DB 추가가 되면 unlink를 해야 할 것 같습니다

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
@@ -0,0 +1,8 @@
package com.ddang.ddang.authentication.application.exception;

public class InaccessibleWithdrawalException extends IllegalArgumentException {

public InaccessibleWithdrawalException(final String message) {
super(message);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package com.ddang.ddang.authentication.application.util;

import java.util.Random;

public class RandomNameGenerator {

private static final int NAME_LENGTH = 10;

private static final Random random = new Random();

private RandomNameGenerator() {
}

public static String generate() {
StringBuilder name = new StringBuilder();
Copy link
Collaborator

Choose a reason for hiding this comment

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

final이 누락되었습니다!

Copy link
Collaborator

Choose a reason for hiding this comment

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

해당 클래스에 전체적으로 final이 누락되었습니다


for (int i = 0; i < NAME_LENGTH; i++) {
int digit = random.nextInt(10);
name.append(digit);
}

return name.toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@
import org.springframework.boot.context.properties.ConfigurationProperties;

@ConfigurationProperties("oauth2.client.providers.kakao")
public record KakaoProvidersConfigurationProperties(String userInfoUri) {
public record KakaoProvidersConfigurationProperties(String userInfoUri, String userUnlinkUri) {
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,6 @@ public interface OAuth2UserInformationProvider {
Oauth2Type supportsOauth2Type();

UserInformationDto findUserInformation(final String accessToken);

UserInformationDto unlinkUserBy(final String accessToken, final String oauthId);
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package com.ddang.ddang.authentication.infrastructure.oauth2;

import com.ddang.ddang.authentication.domain.dto.UserInformationDto;
import com.ddang.ddang.authentication.domain.exception.UnsupportedSocialLoginException;

import java.util.Locale;

public enum Oauth2Type {
Expand All @@ -16,9 +16,9 @@ public static Oauth2Type from(final String typeName) {
}
}

public String calculateNickname(final UserInformationDto dto) {
public String calculateNickname(final String name) {
return this.name()
.toLowerCase(Locale.ENGLISH)
.concat(String.valueOf(dto.id()));
.concat(name);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
import org.springframework.http.MediaType;
import org.springframework.http.ResponseEntity;
import org.springframework.stereotype.Component;
import org.springframework.util.LinkedMultiValueMap;
import org.springframework.util.MultiValueMap;
import org.springframework.web.client.HttpClientErrorException;
import org.springframework.web.client.RestTemplate;

Expand Down Expand Up @@ -54,4 +56,32 @@ public UserInformationDto findUserInformation(final String accessToken) {
throw new InvalidTokenException(message, ex);
}
}

@Override
public UserInformationDto unlinkUserBy(final String accessToken, final String oauthId) {
final HttpHeaders headers = new HttpHeaders();
headers.setContentType(MediaType.APPLICATION_JSON);
headers.set(HttpHeaders.AUTHORIZATION, TOKEN_TYPE + accessToken);

final MultiValueMap<String, String> parameters = new LinkedMultiValueMap<>();
parameters.add("target_id_type", "user_id");
parameters.add("target_id", oauthId);

final HttpEntity<MultiValueMap<String, String>> request = new HttpEntity<>(parameters, headers);

try {
final ResponseEntity<UserInformationDto> response = restTemplate.exchange(
providersConfigurationProperties.userUnlinkUri(),
HttpMethod.POST,
request,
UserInformationDto.class
);

return response.getBody();
} catch (final HttpClientErrorException ex) {
final String message = ex.getMessage().split(REST_TEMPLATE_MESSAGE_SEPARATOR)[MESSAGE_INDEX];

throw new InvalidTokenException(message, ex);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,4 @@ public ReadUserDto readById(final Long userId) {

return ReadUserDto.from(user);
}

@Transactional
public void deleteById(final Long userId) {
final User user = userRepository.findByIdAndDeletedIsFalse(userId)
.orElseThrow(() -> new UserNotFoundException("사용자 정보를 사용할 수 없습니다."));

user.withdrawal();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
public class User extends BaseTimeEntity {

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

@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
Expand All @@ -35,7 +36,6 @@ public class User extends BaseTimeEntity {

private double reliability;

@Column(unique = true)
private String oauthId;

@Column(name = "is_deleted")
Expand All @@ -57,4 +57,14 @@ private User(
public void withdrawal() {
this.deleted = DELETED_STATUS;
}

// TODO: 2023/09/15 [고민] getName() 메서드를 통해 가져오게 된다면 헷갈릴까요?
// TODO: 2023/09/15 [고민] 출력을 위한 로직이 여기 있다면 어색할까요? 사실 귀찮아서 여기 두긴 했습니다.
Copy link
Collaborator

Choose a reason for hiding this comment

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

getName() 메서드를 통해 가져오게 된다면 헷갈릴까요?
-> name을 가져오는 것이라면 getter는 괜찮을 것 같습니다

출력을 위한 로직이 여기 있다면 어색할까요? 사실 귀찮아서 여기 두긴 했습니다.
-> 저는 도메인에 출력 관련 로직이 있으면 안된다고 생각합니다
응답을 위한 반환 로직이라면 컨트롤러나 DTO에 있어야 할 것 같다고 생각합니다

public String getName() {
if (isDeleted()) {
return UNKOWN_NAME;
}

return name;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

저는 사실 User 클래스에 unknownUser를 반환하는 static 메서드가 있고, 레포지토리에서 findById()시 deleted가 true면 unknownUser를 반환하는 형태를 생각하긴 했는데, 이렇게 해도 괜찮을 것 같아요. 어차피 dto에 넣을 때 getName() 하니까 로직상 문제는 없을 것 같고.. 출력을 위한 로직이 있는게 어색하다면 아예 delete할 때 유저 이름을 바꾸는 것은 별로일까요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

엔초가 말씀하신 부분도 결국 비즈니스 로직에 출력 관련 로직이 포함된다고 생각합니다. 알 수 없음이 다른 값으로 바뀌는 경우 비즈니스 로직을 수정해야 하기 때문입니다. 따라서 기존 코드의 통일성일 지켜 DTO에서 해당 로직을 수행하는 것이 자연스럽다고 생각합니다.

}
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@

public interface JpaUserRepository extends JpaRepository<User, Long> {

Optional<User> findByOauthId(final String oauthId);
Optional<User> findByOauthIdAndDeletedIsFalse(final String oauthId);

Optional<User> findByIdAndDeletedIsFalse(final Long id);

boolean existsByIdAndDeletedIsTrue(final Long id);

boolean existsByNameEndingWith(final String name);
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import com.ddang.ddang.user.presentation.dto.ReadUserResponse;
import lombok.RequiredArgsConstructor;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.DeleteMapping;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RestController;
Expand All @@ -26,12 +25,4 @@ public ResponseEntity<ReadUserResponse> readById(@AuthenticateUser final Authent

return ResponseEntity.ok(response);
}

@DeleteMapping("/withdrawal")
public ResponseEntity<Void> delete(@AuthenticateUser final AuthenticationUserInfo userInfo) {
Comment on lines -51 to -52
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
Member Author

Choose a reason for hiding this comment

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

뭐야... 왜 자꾸 컨트롤러 안 만드는 거야...

userService.deleteById(userInfo.userId());

return ResponseEntity.noContent()
.build();
}
}
1 change: 1 addition & 0 deletions backend/ddang/src/main/resources/application-local.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,4 @@ oauth2:
providers:
kakao:
user-info-uri: https://kapi.kakao.com/v2/user/me
user-unlink-uri: https://kapi.kakao.com/v1/user/unlink
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE users DROP INDEX uq_oauth_id;
Loading