Skip to content

Conversation

@jeonghyeokSim
Copy link
Collaborator

@jeonghyeokSim jeonghyeokSim commented Sep 25, 2025

📝 작업 내용

  1. AuthService

    • changePassword 메서드 구현
    • 새 비밀번호와 확인 비밀번호 불일치 시 PasswordMismatchException 발생
    • 현재 비밀번호 검증 실패 시 InvalidPasswordException 발생
    • 비밀번호 변경 성공 시 해싱 후 DB 업데이트
  2. Exception

    • PasswordMismatchException, InvalidPasswordException 추가
    • GlobalExceptionHandler에 Password 관련 예외 처리 로직 추가 (400 Bad Request 반환)
  3. ApiResponse

    • 예외 발생 시도 동일한 응답 포맷(success:false, status:BAD_REQUEST) 유지
  4. Integration Test

    • AuthApiIntegrationTest에 changePassword 성공/실패 케이스 추가
    • 잘못된 현재 비밀번호 입력 시 400 Bad Request 검증
    • 새 비밀번호/확인 비밀번호 불일치 시 400 Bad Request 검증

🔗 관련 이슈

  • Closes #이슈번호
  • Related to #이슈번호

💬 추가 요청사항


✅ 체크리스트

코드 품질

  • 커밋 컨벤션 준수 (feat/fix/docs/refactor 등)
  • 불필요한 코드/주석 제거

테스트

  • 로컬 환경에서 동작 확인 완료
  • 기존 기능에 영향 없음 확인

배포 준비

  • 환경변수 추가/변경사항 문서화
  • DB 마이그레이션 필요 여부 확인
  • 배포 시 주의사항 없음

# Conflicts:
#	apps/user-service/src/test/java/site/icebang/integration/tests/auth/AuthApiIntegrationTest.java
@jeonghyeokSim jeonghyeokSim self-assigned this Sep 25, 2025
Comment on lines +20 to +22
String findPasswordByEmail(String email);

int updatePassword(String email, String newPassword);
Copy link
Collaborator

@can019 can019 Sep 25, 2025

Choose a reason for hiding this comment

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

Pk(BigInteger)로 찾는게 더 빨라 보일 것 같습니다.

Controller에서

void method(@AuthenticationPrincipal AuthCredential user) {

    BigInteger id = user.getId();
}

이렇게 꺼낼 수 있습니다

Comment on lines +58 to +63
public void changePassword(String email, ChangePasswordRequestDto request) {
// 1. 새 비밀번호와 확인 비밀번호 일치 검증
if (!request.getNewPassword().equals(request.getConfirmPassword())) {
throw new PasswordMismatchException("새 비밀번호가 일치하지 않습니다");
}

Copy link
Collaborator

@can019 can019 Sep 25, 2025

Choose a reason for hiding this comment

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

이 부분은 비즈니스 로직이 아니라 단순한 입력값 검증(validation) 으로 보는 게 맞습니다.
따라서 아래 세 가지 방향 중 하나로 하면 좋을 것 같습니다.

  1. MethodArgumentNotValidException를 상속한 커스텀 예외 사용

    • 기존 Bean Validation 흐름을 그대로 활용하면서도, 비밀번호 검증 전용 예외를 구분할 수 있습니다.
  2. 커스텀 @Validate 어노테이션 작성 후 @Valid에게 완전 위임

    • 컨트롤러 단에서 DTO에 어노테이션을 붙이는 것만으로 검증이 가능합니다.
  3. 검증 로직을 Controller 단으로 이동

    • 서비스 레이어에서 검증을 제거하고, 요청 파라미터 검증 책임을 명확히 컨트롤러에 둡니다.

PasswordMismatch라는 이름은 “DB에 저장된 비밀번호와 입력 비밀번호가 일치하지 않는 경우”를 연상시키므로,
단순히 새 비밀번호와 새 비밀번호 확인값이 다른 케이스를 표현하기에는 좋지 않다고 생각됩니다.

GPT는 DB에 저장된 비밀번호와 입력 비밀번호가 일치하지 않는 경우로 알아들어 비지니스 로직인데 왜 MethodArgumentNotValidException를 발생시키냐고 한 것 같습니다.

또한 새 비밀번호와 새 비밀번호 확인은 실제 서비스에서도 프론트엔드에서만 체크하고

백엔드에서 별도 검증을 생략하는 경우가 흔합니다.
이 점도 고려해, 백엔드에서 검증을 유지할지 여부부터 명확히 결정하는 게 좋을 것 같습니다.

비즈니스 로직이란 컴퓨터 프로그램이 현실 세계의 비즈니스 규칙에 따라 데이터를 생성, 표시, 저장, 변경하는 핵심 로직입니다. by Google

Comment on lines +10 to +28
@Configuration
public class MailConfig {

@Bean
public JavaMailSender javaMailSender() {
JavaMailSenderImpl mailSender = new JavaMailSenderImpl();
mailSender.setHost("smtp.gmail.com");
mailSender.setPort(587);
mailSender.setUsername("");
mailSender.setPassword("");

Properties props = mailSender.getJavaMailProperties();
props.put("mail.transport.protocol", "smtp");
props.put("mail.smtp.auth", "true");
props.put("mail.smtp.starttls.enable", "true");
props.put("mail.debug", "true");

return mailSender;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

저희는 JavaMailSender Bean 설정을 application.yml에서 관리하고 있습니다.
아마 테스트 환경 때문에 Bean을 직접 등록하신 것으로 보입니다.

따라서 다음 두 가지 중 하나로 맞춰 주시면 좋겠습니다.

  1. 각 환경별 application-*.yml에 동일한 설정 정의

    • 테스트, 스테이징, 프로덕션 등 모든 프로필에 동일하게 mail 설정을 작성합니다.
  2. 프로덕션 환경도 Bean 등록 방식으로 통일

    • 테스트처럼 @Configuration 클래스에서 JavaMailSender Bean을 생성하도록 변경합니다.

두 방식 중 어떤 것을 선택하든,
환경별 설정 관리가 일관되도록 해 주시면 유지보수가 훨씬 수월해집니다.

Comment on lines +121 to +126

@ExceptionHandler({PasswordMismatchException.class, InvalidPasswordException.class}) // 추가
@ResponseStatus(HttpStatus.BAD_REQUEST)
public ApiResponse<String> handlePasswordException(RuntimeException ex) {
return ApiResponse.error(ex.getMessage(), HttpStatus.BAD_REQUEST);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

각 도메인에서 400 응답을 내려야 하는 경우마다 CustomException을 새로 생성하고,
그때마다 GlobalExceptionHandler에 일일이 등록하게 되면
핸들러 클래스는 금세 100줄, 200줄, 300줄로 커질 위험이 있습니다.

Comment on lines +626 to +635
mockMvc
.perform(
patch(getApiUrlForDocs("/v0/auth/change-password"))
.contentType(MediaType.APPLICATION_JSON)
.content(objectMapper.writeValueAsString(changePasswordRequest)))
.andExpect(status().isUnauthorized())
.andExpect(jsonPath("$.success").value(false))
.andExpect(jsonPath("$.status").value("UNAUTHORIZED"))
.andExpect(jsonPath("$.data").doesNotExist());
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Spring rest api documentation을 위한 처리가 되어있지 않습니다. 확인해주세요

Copy link
Collaborator

@can019 can019 left a comment

Choose a reason for hiding this comment

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

전체적으로 확인해주시면 감사하겠습니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants