Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
@@ -0,0 +1,26 @@
package knu_chatbot.application.dto.response;

import lombok.Builder;
import lombok.Getter;
import lombok.NoArgsConstructor;
import lombok.ToString;

@Getter
@NoArgsConstructor
@ToString
public class ChangePasswordResponse {

private String message;

@Builder
public ChangePasswordResponse(String message) {
this.message = message;
}

public static ChangePasswordResponse of(String message) {
return ChangePasswordResponse.builder()
.message(message)
.build();
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package knu_chatbot.application.dto.response;

Choose a reason for hiding this comment

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

medium

이 클래스는 서비스 계층에 대한 요청 데이터를 담는 DTO이므로, knu_chatbot.application.dto.request 패키지로 이동하는 것이 패키지 구조의 일관성 측면에서 더 적절해 보입니다. 현재는 response 패키지에 위치해 있어 혼란을 줄 수 있습니다.


import lombok.Builder;
import lombok.Getter;

@Getter
public class ChangePasswordServiceRequest {

private String oldPassword;
private String newPassword;

@Builder
public ChangePasswordServiceRequest(String oldPassword, String newPassword) {
this.oldPassword = oldPassword;
this.newPassword = newPassword;
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package knu_chatbot.application.dto.response;

import lombok.Builder;
import lombok.Getter;
import lombok.NoArgsConstructor;

@Getter
@NoArgsConstructor
public class LogoutResponse {

private String message;

@Builder
public LogoutResponse(String message) {
this.message = message;
}

public static LogoutResponse of(String message) {
return LogoutResponse.builder()
.message(message)
.build();
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package knu_chatbot.application.dto.response;

import lombok.Builder;
import lombok.Getter;
import lombok.NoArgsConstructor;

@Getter
@NoArgsConstructor
public class WithdrawResponse {

private String message;

@Builder
public WithdrawResponse(String message) {
this.message = message;
}

public static WithdrawResponse of(String message) {
return WithdrawResponse.builder()
.message(message)
.build();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,7 @@ public interface MemberRepository {

void deleteRefreshToken(String refreshToken);

void deleteMemberByEmail(String email);

void updatePasswordByEmail(String email, String newEncryptedPassword);
}
36 changes: 31 additions & 5 deletions src/main/java/knu_chatbot/application/service/MemberService.java
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
package knu_chatbot.application.service;

import knu_chatbot.application.dto.AuthUser;
import knu_chatbot.application.dto.MemberDto;
import knu_chatbot.application.dto.request.CheckEmailServiceRequest;
import knu_chatbot.application.dto.request.LoginServiceRequest;
import knu_chatbot.application.dto.request.SignupServiceRequest;
import knu_chatbot.application.dto.response.CheckEmailResponse;
import knu_chatbot.application.dto.response.LoginResponse;
import knu_chatbot.application.dto.response.ReissueTokensResponse;
import knu_chatbot.application.dto.response.SignupResponse;
import knu_chatbot.application.dto.response.*;
import knu_chatbot.application.error.ErrorType;
import knu_chatbot.application.error.KnuChatbotException;
import knu_chatbot.application.repository.MemberRepository;
Expand Down Expand Up @@ -96,11 +94,39 @@ public MyPageResponse getMyPage(String email) {
return MyPageResponse.from(memberDto);
}

public LogoutResponse logout(String refreshToken) {
memberRepository.deleteRefreshToken(refreshToken);
return LogoutResponse.of("회원가입이 완료되었습니다.");

Choose a reason for hiding this comment

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

high

로그아웃 성공 메시지가 "회원가입이 완료되었습니다."로 잘못 설정되어 있습니다. "로그아웃이 완료되었습니다."와 같이 올바른 메시지로 수정해야 합니다.

Suggested change
return LogoutResponse.of("회원가입이 완료되었습니다.");
return LogoutResponse.of("로그아웃이 완료되었습니다.");

}

@Transactional
public WithdrawResponse withdraw(String email) {
memberRepository.deleteMemberByEmail(email);

// TODO : 히스토리 및 채팅 삭제

return WithdrawResponse.of("회원탈퇴가 완료되었습니다.");
}
Comment on lines +106 to +112

Choose a reason for hiding this comment

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

high

회원 탈퇴 시 member만 삭제하고 관련 데이터(히스토리, 채팅)는 삭제하지 않고 있습니다. TODO로 명시되어 있지만, 이로 인해 고아 데이터(orphaned data)가 발생하여 데이터 무결성을 해칠 수 있습니다. 회원과 연관된 모든 데이터를 함께 삭제하거나, soft delete를 적용하는 것을 고려해야 합니다. 예를 들어, Member 엔티티에 deleted 플래그를 추가하고 회원 탈퇴 시 이 플래그를 업데이트하는 방식이 있습니다.


@Transactional
public ChangePasswordResponse changePassword(AuthUser authUser, ChangePasswordServiceRequest request) {
MemberDto memberDto = memberRepository.findByEmail(authUser.getEmail());

// 비밀번호 검증
if (!passwordEncryptor.verifyPassword(request.getOldPassword(), memberDto.getPassword())) {
throw new KnuChatbotException(ErrorType.USER_LOGIN_ERROR);

Choose a reason for hiding this comment

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

medium

비밀번호 변경 시 현재 비밀번호가 일치하지 않을 때 USER_LOGIN_ERROR를 사용하고 있습니다. 이 오류 유형은 로그인 실패를 위해 만들어진 것으로, "아이디 또는 비밀번호가 맞지 않습니다."라는 메시지를 반환하여 사용자에게 혼란을 줄 수 있습니다. "현재 비밀번호가 일치하지 않습니다."와 같은 명확한 메시지를 제공하는 새로운 오류 유형(예: CURRENT_PASSWORD_MISMATCH_ERROR)을 정의하여 사용하는 것이 좋습니다.

}

String newEncryptedPassword = passwordEncryptor.encryptPassword(request.getNewPassword());
memberRepository.updatePasswordByEmail(authUser.getEmail(),newEncryptedPassword);

return ChangePasswordResponse.of("비밀번호가 변경되었습니다.");
}

private void validatePasswordMatch(SignupServiceRequest request) {
if (!request.getPassword().equals(request.getConfirmPassword())) {
throw new KnuChatbotException(ErrorType.USER_CONFIRM_PASSWORD_ERROR, request);
}
}

}

8 changes: 8 additions & 0 deletions src/main/java/knu_chatbot/infrastructure/entity/Member.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,13 @@ public Member(String email, String password) {
this.email = email;
this.password = password;
}

private void setPassword(String password) {
this.password = password;
}

public void changePassword(String newEncryptedPassword) {
setPassword(newEncryptedPassword);
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,5 @@ public interface MemberJpaRepository extends JpaRepository<Member, Long> {

Optional<Member> findByEmail(String email);

void deleteByEmail(String email);
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@ public void save(Member member) {

@Override
public MemberDto findByEmail(String email) {
Member member = memberJpaRepository.findByEmail(email)
.orElseThrow(() -> new KnuChatbotException(ErrorType.USER_INVALID_ERROR));
Member member = getOrThrowMemberByEmail(email);
return memberMapper.convert(member);
}

Expand All @@ -56,4 +55,23 @@ public void deleteRefreshToken(String refreshToken) {
redisTemplate.delete(refreshToken);
}

@Override
public void deleteMemberByEmail(String email) {
memberJpaRepository.deleteByEmail(email);
}

@Override
public void updatePasswordByEmail(String email, String newEncryptedPassword) {
Member member = getOrThrowMemberByEmail(email);

member.changePassword(newEncryptedPassword);

memberJpaRepository.save(member);

Choose a reason for hiding this comment

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

medium

updatePasswordByEmail 메서드는 서비스 계층의 @Transactional 컨텍스트 내에서 호출됩니다. 따라서 getOrThrowMemberByEmail을 통해 조회된 Member 엔티티는 영속성 컨텍스트에 의해 관리되는 상태(managed state)입니다. member.changePassword()를 통해 엔티티의 상태를 변경하면, 트랜잭션이 커밋될 때 변경 감지(dirty checking)에 의해 자동으로 UPDATE 쿼리가 실행됩니다. 따라서 이 save 호출은 불필요하며, 제거하여 코드를 더 간결하게 만들 수 있습니다.

}

private Member getOrThrowMemberByEmail(String email) {
return memberJpaRepository.findByEmail(email)
.orElseThrow(() -> new KnuChatbotException(ErrorType.USER_INVALID_ERROR));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,14 @@
import jakarta.servlet.http.HttpServletResponse;
import jakarta.validation.Valid;
import knu_chatbot.application.dto.AuthUser;
import knu_chatbot.application.dto.response.CheckEmailResponse;
import knu_chatbot.application.dto.response.LoginResponse;
import knu_chatbot.application.dto.response.ReissueTokensResponse;
import knu_chatbot.application.dto.response.SignupResponse;
import knu_chatbot.application.dto.response.*;
import knu_chatbot.application.error.ErrorType;
import knu_chatbot.application.error.KnuChatbotException;
import knu_chatbot.application.service.MemberService;
import knu_chatbot.application.util.JwtProvider;
import knu_chatbot.presentation.ApiResponse;
import knu_chatbot.presentation.annotation.Login;
import knu_chatbot.presentation.request.ChangePasswordRequest;
import knu_chatbot.presentation.request.CheckEmailRequest;
import knu_chatbot.presentation.request.LoginRequest;
import knu_chatbot.presentation.request.SignupRequest;
Expand Down Expand Up @@ -91,7 +89,6 @@ public ApiResponse<AccessTokenResponse> reissueTokens(HttpServletRequest request
return ApiResponse.success(reissueTokensResponse.toAccessTokenResponse());
}

// TODO : 마이페이지
@Operation(
summary = "마이페이지",
description = "마이페이지 정보를 보여준다."
Expand All @@ -101,10 +98,37 @@ public ApiResponse<MyPageResponse> getMyPage(@Parameter(hidden = true) @Login Au
return ApiResponse.success(memberService.getMyPage(authUser.getEmail()));
}

// TODO : 로그아웃
// TODO : 회원 탈퇴
// TODO : 회원 정보 수정
// TODO : 비밀번호 수정
@Operation(
summary = "로그아웃",
description = "로그아웃 한다."
)
@PostMapping("/logout")
public ApiResponse<LogoutResponse> logout(HttpServletRequest request) {
String refreshToken = getRefreshTokenFromCookie(request);
return ApiResponse.success(memberService.logout(refreshToken));
}

@Operation(
summary = "회원 탈퇴"
)
@DeleteMapping
public ApiResponse<WithdrawResponse> withdraw(@Parameter(hidden = true) @Login AuthUser authUser) {
return ApiResponse.success(memberService.withdraw(authUser.getEmail()));
}

@Operation(
summary = "비밀번호 변경",
description = "비밀번호를 수정한다."
)
@PatchMapping
public ApiResponse<ChangePasswordResponse> changePassword(
@Parameter(hidden = true) @Login AuthUser authUser,
@Valid @RequestBody ChangePasswordRequest request
) {
return ApiResponse.success(memberService.changePassword(authUser, request.toServiceRequest()));
}

// TODO : 회원 정보 수정 (현재는 비즈니스 상 수정할 정보가 없음)

private ResponseCookie createCookie(String refreshToken) {
ResponseCookie cookie = ResponseCookie.from(REFRESH_TOKEN, refreshToken)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package knu_chatbot.presentation.request;

import jakarta.validation.constraints.NotBlank;
import knu_chatbot.application.dto.response.ChangePasswordServiceRequest;
import lombok.Builder;
import lombok.Getter;
import lombok.NoArgsConstructor;
import org.hibernate.validator.constraints.Length;

@Getter
@NoArgsConstructor
public class ChangePasswordRequest {

@NotBlank(message = "이전 비밀번호는 필수입니다.")
private String oldPassword;

@NotBlank(message = "새로운 비밀번호는 필수입니다.")
@Length(min = 8, max = 20, message = "길이는 최소 8글자, 최대 20글자 입니다.")
private String newPassword;

@Builder
public ChangePasswordRequest(String oldPassword, String newPassword) {
this.oldPassword = oldPassword;
this.newPassword = newPassword;
}

public ChangePasswordServiceRequest toServiceRequest() {
return ChangePasswordServiceRequest.builder()
.oldPassword(oldPassword)
.newPassword(newPassword)
.build();
}
}
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
package knu_chatbot.application.service;

import knu_chatbot.application.dto.AuthUser;
import knu_chatbot.application.dto.request.CheckEmailServiceRequest;
import knu_chatbot.application.dto.request.LoginServiceRequest;
import knu_chatbot.application.dto.request.SignupServiceRequest;
import knu_chatbot.application.dto.response.CheckEmailResponse;
import knu_chatbot.application.dto.response.LoginResponse;
import knu_chatbot.application.dto.response.ReissueTokensResponse;
import knu_chatbot.application.dto.response.SignupResponse;
import knu_chatbot.application.dto.response.*;
import knu_chatbot.application.error.ErrorType;
import knu_chatbot.application.error.KnuChatbotException;
import knu_chatbot.application.util.JwtProvider;
Expand Down Expand Up @@ -204,4 +202,36 @@ void reissueTokensWithInvalidRefreshToken() {
.isEqualTo(ErrorType.USER_INVALID_REFRESH_TOKEN_ERROR);
}

@DisplayName("비밀번호를 변경한다.")
@Test
void changePassword() {
// given
String email = "test@test.com";
String oldPassword = "oldPassword";

SignupServiceRequest signupRequest = SignupServiceRequest.builder()
.email(email)
.password(oldPassword)
.confirmPassword(oldPassword)
.build();

memberService.signup(signupRequest);

AuthUser authUser = AuthUser.builder()
.email(email)
.build();

String newPassword = "newPassword";
ChangePasswordServiceRequest request = ChangePasswordServiceRequest.builder()
.oldPassword(oldPassword)
.newPassword(newPassword)
.build();

// when
ChangePasswordResponse response = memberService.changePassword(authUser, request);

// then
assertThat(response).isNotNull();

Choose a reason for hiding this comment

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

medium

테스트의 then 절에서 responsenull이 아닌지만 검증하고 있습니다. 이는 비밀번호가 실제로 변경되었는지 보장하지 못합니다. 테스트의 신뢰도를 높이기 위해, 변경된 비밀번호로 로그인이 성공하는지, 그리고 이전 비밀번호로는 로그인이 실패하는지 등을 검증하는 로직을 추가하는 것이 좋습니다.

// then
assertThat(response).isNotNull();

// 새 비밀번호로 로그인 시도
LoginServiceRequest loginRequest = LoginServiceRequest.builder()
        .email(email)
        .password(newPassword)
        .build();
// 예외가 발생하지 않아야 함
assertThatCode(() -> memberService.login(loginRequest)).doesNotThrowAnyException();

// 이전 비밀번호로 로그인 시도
LoginServiceRequest oldLoginRequest = LoginServiceRequest.builder()
        .email(email)
        .password(oldPassword)
        .build();
// 예외가 발생해야 함
assertThatThrownBy(() -> memberService.login(oldLoginRequest))
        .isInstanceOf(KnuChatbotException.class);

}

}
Loading
Loading