Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
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
Expand Up @@ -56,6 +56,9 @@ public enum ErrorCode {
ACCESS_TOKEN_EXPIRED(HttpStatus.UNAUTHORIZED.value(), "액세스 토큰이 만료되었습니다. 재발급 api를 호출해주세요."),
REFRESH_TOKEN_EXPIRED(HttpStatus.UNAUTHORIZED.value(), "리프레시 토큰이 만료되었습니다. 다시 로그인을 진행해주세요."),
ACCESS_DENIED(HttpStatus.FORBIDDEN.value(), "접근 권한이 없습니다."),
PASSWORD_MISMATCH(HttpStatus.BAD_REQUEST.value(), "비밀번호가 일치하지 않습니다."),
PASSWORD_NOT_CHANGED(HttpStatus.BAD_REQUEST.value(), "현재 비밀번호와 새 비밀번호가 동일합니다."),
PASSWORD_NOT_CONFIRMED(HttpStatus.BAD_REQUEST.value(), "새 비밀번호가 일치하지 않습니다."),

// s3
S3_SERVICE_EXCEPTION(HttpStatus.BAD_REQUEST.value(), "S3 서비스 에러 발생"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,14 @@

import com.example.solidconnection.common.resolver.AuthorizedUser;
import com.example.solidconnection.siteuser.dto.MyPageResponse;
import com.example.solidconnection.siteuser.dto.PasswordUpdateRequest;
import com.example.solidconnection.siteuser.service.MyPageService;
import jakarta.validation.Valid;
import lombok.RequiredArgsConstructor;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PatchMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RestController;
Expand Down Expand Up @@ -37,4 +40,13 @@ public ResponseEntity<Void> updateMyPageInfo(
myPageService.updateMyPageInfo(siteUserId, imageFile, nickname);
return ResponseEntity.ok().build();
}

@PatchMapping("/password")
public ResponseEntity<Void> updatePassword(
@AuthorizedUser long siteUserId,
@RequestBody @Valid PasswordUpdateRequest request
) {
myPageService.updatePassword(siteUserId, request);
return ResponseEntity.ok().build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -115,4 +115,8 @@ public SiteUser(
this.authType = authType;
this.password = password;
}

public void updatePassword(String newPassword) {
this.password = newPassword;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package com.example.solidconnection.siteuser.dto;

import com.example.solidconnection.siteuser.dto.validation.PasswordConfirmation;
import jakarta.validation.constraints.NotBlank;

@PasswordConfirmation
public record PasswordUpdateRequest(
@NotBlank(message = "현재 비밀번호를 입력해주세요.")
String currentPassword,

@NotBlank(message = "새 비밀번호를 입력해주세요.")
// @Password // todo: #435 merge 후
String newPassword,
Comment thread
whqtker marked this conversation as resolved.

@NotBlank(message = "새 비밀번호를 다시 한번 입력해주세요.")
String confirmNewPassword
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

정말 아주 사소한 부분이긴 한데요..! 😅

confirmNewPassword를 그대로 해석하자면 "새로운 비밀번호를 확인한다"라는 동사구이므로, 필드의 이름으로는 적합하지 않은 것 같아요.

  • 필드, 인자명은 명사구
  • 함수명은 동사구

로 알고 있습니다!

그래서 이 필드 이름을 newPasswordConfirmation 으로 하는게 어떨지 조심스럽게 제안드립니다..

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

앗 ... 변수명 고민하다가 동사형으로 써 버렸네요
제안하신 변수명 말고 좋은 이름은 없는 것 같아 그대로 사용하였습니다 ~!!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

) {

}
Comment thread
whqtker marked this conversation as resolved.
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package com.example.solidconnection.siteuser.dto.validation;

import jakarta.validation.Constraint;
import jakarta.validation.Payload;
import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

@Constraint(validatedBy = PasswordConfirmationValidator.class)
@Target({ElementType.TYPE})
@Retention(RetentionPolicy.RUNTIME)
public @interface PasswordConfirmation {

String message() default "비밀번호 변경 과정에서 오류가 발생했습니다.";

Class<?>[] groups() default {};

Class<? extends Payload>[] payload() default {};
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package com.example.solidconnection.siteuser.dto.validation;

import static com.example.solidconnection.common.exception.ErrorCode.PASSWORD_NOT_CHANGED;
import static com.example.solidconnection.common.exception.ErrorCode.PASSWORD_NOT_CONFIRMED;

import com.example.solidconnection.siteuser.dto.PasswordUpdateRequest;
import jakarta.validation.ConstraintValidator;
import jakarta.validation.ConstraintValidatorContext;
import java.util.Objects;

public class PasswordConfirmationValidator implements ConstraintValidator<PasswordConfirmation, PasswordUpdateRequest> {

@Override
public boolean isValid(PasswordUpdateRequest request, ConstraintValidatorContext context) {
context.disableDefaultConstraintViolation();

if (isNewPasswordNotConfirmed(request)) {
addConstraintViolation(context, PASSWORD_NOT_CONFIRMED.getMessage(), "confirmNewPassword");

return false;
}

if (isPasswordUnchanged(request)) {
addConstraintViolation(context, PASSWORD_NOT_CHANGED.getMessage(), "newPassword");

return false;
}

return true;
}

private boolean isNewPasswordNotConfirmed(PasswordUpdateRequest request) {
return !Objects.equals(request.newPassword(), request.confirmNewPassword());
}

private boolean isPasswordUnchanged(PasswordUpdateRequest request) {
return Objects.equals(request.currentPassword(), request.newPassword());
}

private void addConstraintViolation(ConstraintValidatorContext context, String message, String propertyName) {
context.buildConstraintViolationWithTemplate(message)
.addPropertyNode(propertyName)
.addConstraintViolation();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static com.example.solidconnection.common.exception.ErrorCode.CAN_NOT_CHANGE_NICKNAME_YET;
import static com.example.solidconnection.common.exception.ErrorCode.NICKNAME_ALREADY_EXISTED;
import static com.example.solidconnection.common.exception.ErrorCode.PASSWORD_MISMATCH;
import static com.example.solidconnection.common.exception.ErrorCode.USER_NOT_FOUND;

import com.example.solidconnection.common.exception.CustomException;
Expand All @@ -10,11 +11,13 @@
import com.example.solidconnection.s3.service.S3Service;
import com.example.solidconnection.siteuser.domain.SiteUser;
import com.example.solidconnection.siteuser.dto.MyPageResponse;
import com.example.solidconnection.siteuser.dto.PasswordUpdateRequest;
import com.example.solidconnection.siteuser.repository.SiteUserRepository;
import com.example.solidconnection.university.repository.LikedUnivApplyInfoRepository;
import java.time.LocalDateTime;
import java.time.format.DateTimeFormatter;
import lombok.RequiredArgsConstructor;
import org.springframework.security.crypto.password.PasswordEncoder;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.web.multipart.MultipartFile;
Expand All @@ -26,6 +29,7 @@ public class MyPageService {
public static final int MIN_DAYS_BETWEEN_NICKNAME_CHANGES = 7;
public static final DateTimeFormatter NICKNAME_LAST_CHANGE_DATE_FORMAT = DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm");

private final PasswordEncoder passwordEncoder;
private final SiteUserRepository siteUserRepository;
private final LikedUnivApplyInfoRepository likedUnivApplyInfoRepository;
private final S3Service s3Service;
Expand Down Expand Up @@ -87,4 +91,21 @@ private boolean isDefaultProfileImage(String profileImageUrl) {
String prefix = "profile/";
return profileImageUrl == null || !profileImageUrl.startsWith(prefix);
}

@Transactional
public void updatePassword(long siteUserId, PasswordUpdateRequest request) {
SiteUser user = siteUserRepository.findById(siteUserId)
.orElseThrow(() -> new CustomException(USER_NOT_FOUND));

// 사용자의 비밀번호와 request의 currentPassword가 동일한지 검증
validateCurrentPasswordSame(user.getPassword(), request.currentPassword());

user.updatePassword(passwordEncoder.encode(request.newPassword()));
}

private void validateCurrentPasswordSame(String userPassword, String currentPassword) {
if (!passwordEncoder.matches(userPassword, currentPassword)) {
throw new CustomException(PASSWORD_MISMATCH);
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

스크린샷 2025-08-12 오후 7 49 41

passwordEncoder.matches를 들어가보면 파라미터 순서가
(rawPassword(평문), encodedPassword(암호화))
입니다!
순서를 변경하는 게 좋을 거 같습니다~
validateCurrentPasswordSame의 인자 이름을 rawPassword, encodedPassword로 맞춰도 좋을 거 같아요!(이건 선택사항으로 해도 될 거 같아요!)

사실 동작을 할까?란 의문을 품고 bruno로 호출해봤는데 지금 상태에서도 작동이 되긴 합니다!

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

저도 규혁님 말씀에 동의합니다~

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

!! 이건 처음 알았네요 변경하였습니다

Copy link
Copy Markdown
Member Author

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,74 @@
package com.example.solidconnection.siteuser.dto.validation;

import static com.example.solidconnection.common.exception.ErrorCode.PASSWORD_NOT_CHANGED;
import static com.example.solidconnection.common.exception.ErrorCode.PASSWORD_NOT_CONFIRMED;
import static org.assertj.core.api.Assertions.assertThat;

import com.example.solidconnection.siteuser.dto.PasswordUpdateRequest;
import jakarta.validation.ConstraintViolation;
import jakarta.validation.Validation;
import jakarta.validation.Validator;
import jakarta.validation.ValidatorFactory;
import java.util.Set;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;

@DisplayName("비밀번호 변경 유효성 검사 테스트")
class PasswordConfirmationValidatorTest {

private Validator validator;

@BeforeEach
void setUp() {
ValidatorFactory factory = Validation.buildDefaultValidatorFactory();
validator = factory.getValidator();
}

@Test
void 유효한_비밀번호_변경_요청은_검증을_통과한다() {
// given
PasswordUpdateRequest request = new PasswordUpdateRequest("currentPassword123", "newPassword123!", "newPassword123!");

// when
Set<ConstraintViolation<PasswordUpdateRequest>> violations = validator.validate(request);

// then
assertThat(violations).isEmpty();
}

@Nested
class 유효하지_않은_비밀번호_변경_테스트 {

@Test
void 새로운_비밀번호와_확인_비밀번호가_일치하지_않으면_검증에_실패한다() {
// given
PasswordUpdateRequest request = new PasswordUpdateRequest("currentPassword123", "newPassword123!", "differentPassword123!");

// when
Set<ConstraintViolation<PasswordUpdateRequest>> violations = validator.validate(request);

// then
assertThat(violations).hasSize(1);
ConstraintViolation<PasswordUpdateRequest> violation = violations.iterator().next();
assertThat(violation.getMessage()).isEqualTo(PASSWORD_NOT_CONFIRMED.getMessage());
assertThat(violation.getPropertyPath().toString()).isEqualTo("confirmNewPassword");
}
Comment on lines +53 to +59
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"message"는 따로 상수로 빼도 좋겠네요!

추가로 저는 개인적으로

assertThat(violations)
              .isNotEmpty()
              .extracting("message")
              .contains(PASSWORD_NOT_CONFIRMED.getMessage()
                    

만 있어도 충분히 검증이 되는 거 같다는 생각이긴 한데 어떻게 생각하시나요?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

validator 역할을 테스트하는 것에 초점을 맞추자면 제거해도 무방할 것 같습니다 !
message도 상수로 분리하였습니다 ~

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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


@Test
void 현재_비밀번호와_새로운_비밀번호가_같으면_검증에_실패한다() {
// given
PasswordUpdateRequest request = new PasswordUpdateRequest("currentPassword123", "currentPassword123", "currentPassword123");

// when
Set<ConstraintViolation<PasswordUpdateRequest>> violations = validator.validate(request);

// then
assertThat(violations).hasSize(1);
ConstraintViolation<PasswordUpdateRequest> violation = violations.iterator().next();
assertThat(violation.getMessage()).isEqualTo(PASSWORD_NOT_CHANGED.getMessage());
assertThat(violation.getPropertyPath().toString()).isEqualTo("newPassword");
}
}
}
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
package com.example.solidconnection.siteuser.service;

import static com.example.solidconnection.common.exception.ErrorCode.CAN_NOT_CHANGE_NICKNAME_YET;
import static com.example.solidconnection.common.exception.ErrorCode.PASSWORD_MISMATCH;
import static com.example.solidconnection.siteuser.service.MyPageService.MIN_DAYS_BETWEEN_NICKNAME_CHANGES;
import static com.example.solidconnection.siteuser.service.MyPageService.NICKNAME_LAST_CHANGE_DATE_FORMAT;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.assertj.core.api.AssertionsForClassTypes.assertThatCode;
import static org.junit.jupiter.api.Assertions.assertAll;
import static org.mockito.BDDMockito.any;
import static org.mockito.BDDMockito.eq;
import static org.mockito.BDDMockito.given;
Expand All @@ -19,6 +22,7 @@
import com.example.solidconnection.siteuser.domain.Role;
import com.example.solidconnection.siteuser.domain.SiteUser;
import com.example.solidconnection.siteuser.dto.MyPageResponse;
import com.example.solidconnection.siteuser.dto.PasswordUpdateRequest;
import com.example.solidconnection.siteuser.fixture.SiteUserFixture;
import com.example.solidconnection.siteuser.fixture.SiteUserFixtureBuilder;
import com.example.solidconnection.siteuser.repository.SiteUserRepository;
Expand All @@ -27,14 +31,14 @@
import com.example.solidconnection.university.fixture.UnivApplyInfoFixture;
import com.example.solidconnection.university.repository.LikedUnivApplyInfoRepository;
import java.time.LocalDateTime;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.mock.web.MockMultipartFile;
import org.springframework.security.crypto.password.PasswordEncoder;

@TestContainerSpringBootTest
@DisplayName("마이페이지 서비스 테스트")
Expand All @@ -61,6 +65,9 @@ class MyPageServiceTest {
@Autowired
private SiteUserFixtureBuilder siteUserFixtureBuilder;

@Autowired
private PasswordEncoder passwordEncoder;

private SiteUser user;

@BeforeEach
Expand All @@ -77,7 +84,7 @@ void setUp() {
MyPageResponse response = myPageService.getMyPageInfo(user.getId());

// then
Assertions.assertAll(
assertAll(
() -> assertThat(response.nickname()).isEqualTo(user.getNickname()),
() -> assertThat(response.profileImageUrl()).isEqualTo(user.getProfileImageUrl()),
() -> assertThat(response.role()).isEqualTo(user.getRole()),
Expand Down Expand Up @@ -176,6 +183,47 @@ void setUp() {
}
}

@Nested
class 비밀번호_변경_테스트 {

private String currentPassword;
private String newPassword;

@BeforeEach
void setUp() {
currentPassword = passwordEncoder.encode(user.getPassword());
newPassword = "newPassword123";
}

@Test
void 비밀번호를_성공적으로_변경한다() {
// given
PasswordUpdateRequest request = new PasswordUpdateRequest(currentPassword, newPassword, newPassword);

// when
myPageService.updatePassword(user.getId(), request);

// then
SiteUser updatedUser = siteUserRepository.findById(user.getId()).get();
assertAll(
() -> assertThat(passwordEncoder.matches(newPassword, updatedUser.getPassword())).isTrue(),
() -> assertThat(passwordEncoder.matches(currentPassword, updatedUser.getPassword())).isFalse()
);
}

@Test
void 현재_비밀번호가_일치하지_않으면_예외가_발생한다() {
// given
String wrongPassword = "wrongPassword";
PasswordUpdateRequest request = new PasswordUpdateRequest(wrongPassword, newPassword, newPassword);

// when & then
assertThatThrownBy(() -> myPageService.updatePassword(user.getId(), request))
.isInstanceOf(CustomException.class)
.hasMessage(PASSWORD_MISMATCH.getMessage());
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

private int createLikedUnivApplyInfos(SiteUser testUser) {
LikedUnivApplyInfo likedUnivApplyInfo1 = new LikedUnivApplyInfo(null, univApplyInfoFixture.괌대학_A_지원_정보().getId(), testUser.getId());
LikedUnivApplyInfo likedUnivApplyInfo2 = new LikedUnivApplyInfo(null, univApplyInfoFixture.메이지대학_지원_정보().getId(), testUser.getId());
Expand Down
Loading