Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
Expand Up @@ -6,6 +6,7 @@
import life.mosu.mosuserver.domain.user.repository.UserJpaRepository;
import life.mosu.mosuserver.global.exception.CustomRuntimeException;
import life.mosu.mosuserver.global.exception.ErrorCode;
import life.mosu.mosuserver.global.util.PhoneNumberUtil;
import life.mosu.mosuserver.presentation.user.dto.request.ChangePasswordRequest;
import life.mosu.mosuserver.presentation.user.dto.request.FindLoginIdRequest;
import life.mosu.mosuserver.presentation.user.dto.request.FindPasswordRequest;
Expand All @@ -29,20 +30,13 @@ public class MyUserService {
@Transactional
public ChangePasswordResponse changePassword(ChangePasswordRequest request, String phoneNumber) {

log.info("loginId: {}, phoneNumber: {}", request.loginId(), phoneNumber);
log.info("비밀번호 요청 하는 phoneNumber: {}", phoneNumber);

Choose a reason for hiding this comment

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

medium

The log message is in Korean. For consistency, it's better to use English. This improves maintainability, especially in a diverse team.

Suggested change
log.info("비밀번호 요청 하는 phoneNumber: {}", phoneNumber);
log.info("Request to change password for phoneNumber: {}", phoneNumber);


String loginId = request.loginId();
UserJpaEntity user = userJpaRepository.findByLoginId(loginId)
.orElseThrow(() -> new CustomRuntimeException(ErrorCode.USER_NOT_FOUND));
String rgxPhone = PhoneNumberUtil.formatPhoneNumberWithHyphen(phoneNumber);

Choose a reason for hiding this comment

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

medium

The variable name rgxPhone can be misleading. A more descriptive name like formattedPhoneNumber would improve code readability.

Suggested change
String rgxPhone = PhoneNumberUtil.formatPhoneNumberWithHyphen(phoneNumber);
String formattedPhoneNumber = PhoneNumberUtil.formatPhoneNumberWithHyphen(phoneNumber);


// DB에서 가져온 번호에서 하이픈 제거
String userPhone = user.getPhoneNumber().replace("-", "");
UserJpaEntity user = userJpaRepository.findByPhoneNumber(rgxPhone)
.orElseThrow(() -> new CustomRuntimeException(ErrorCode.USER_NOT_FOUND));

// 전화번호 일치 검증
if (!userPhone.equals(phoneNumber)) {
throw new CustomRuntimeException(ErrorCode.USER_INFO_INVALID);
}

user.changePassword(passwordEncode(encoder, request.newPassword()));

return ChangePasswordResponse.from(Boolean.TRUE);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,6 @@ public interface UserJpaRepository extends JpaRepository<UserJpaEntity, Long> {


Optional<UserJpaEntity> findByNameAndPhoneNumber(String name, String phoneNumber);

Optional<UserJpaEntity> findByPhoneNumber(String phoneNumber);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package life.mosu.mosuserver.global.annotation;

import java.lang.annotation.Documented;
import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

@Target(ElementType.PARAMETER)
@Retention(RetentionPolicy.RUNTIME)
@Documented
public @interface PhoneNumber {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package life.mosu.mosuserver.global.resolver;

import life.mosu.mosuserver.global.annotation.PhoneNumber;
import life.mosu.mosuserver.global.filter.KmcAuthenticationToken;
import org.springframework.core.MethodParameter;
import org.springframework.core.io.support.SpringFactoriesLoader.ArgumentResolver;

Choose a reason for hiding this comment

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

medium

This import org.springframework.core.io.support.SpringFactoriesLoader.ArgumentResolver is unused and should be removed.

import org.springframework.security.core.Authentication;
import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.stereotype.Component;
import org.springframework.web.bind.support.WebDataBinderFactory;
import org.springframework.web.context.request.NativeWebRequest;
import org.springframework.web.method.support.HandlerMethodArgumentResolver;
import org.springframework.web.method.support.ModelAndViewContainer;

@Component
public class PhoneNumberArgumentResolver implements HandlerMethodArgumentResolver {

@Override
public boolean supportsParameter(MethodParameter parameter) {
return parameter.hasParameterAnnotation(PhoneNumber.class)
&& parameter.getParameterType().equals(String.class);
}

@Override
public Object resolveArgument(MethodParameter parameter, ModelAndViewContainer mavContainer,
NativeWebRequest webRequest, WebDataBinderFactory binderFactory) {
Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
if (authentication instanceof KmcAuthenticationToken kmcAuth) {
return kmcAuth.getPhoneNumber();
}
throw new IllegalStateException("KmcAuthenticationToken not found in SecurityContext");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package life.mosu.mosuserver.global.util;

import org.springframework.stereotype.Component;

@Component

Choose a reason for hiding this comment

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

medium

The @Component annotation is unnecessary for a utility class that only contains static methods. Consider making the class final and providing a private constructor to prevent instantiation.

public class PhoneNumberUtil {

public static String formatPhoneNumberWithHyphen(String phoneNumber) {
if (phoneNumber == null || phoneNumber.length() != 11) {
throw new IllegalArgumentException("Invalid phone number format");
}

return String.format("%s-%s-%s",
phoneNumber.substring(0, 3),
phoneNumber.substring(3, 7),
phoneNumber.substring(7));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import jakarta.validation.Valid;
import life.mosu.mosuserver.application.user.MyUserService;
import life.mosu.mosuserver.global.annotation.PhoneNumber;
import life.mosu.mosuserver.global.annotation.UserId;
import life.mosu.mosuserver.global.filter.KmcAuthenticationToken;
import life.mosu.mosuserver.global.util.ApiResponseWrapper;
Expand Down Expand Up @@ -51,18 +52,12 @@ public ResponseEntity<ApiResponseWrapper<Void>> findPassword(

@PostMapping("/password")
public ResponseEntity<ApiResponseWrapper<ChangePasswordResponse>> changePassword(
@PhoneNumber final String phoneNumber,
@RequestBody @Valid ChangePasswordRequest request
) {

Authentication authentication = SecurityContextHolder.getContext().getAuthentication();

Comment on lines 59 to 60

Choose a reason for hiding this comment

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

medium

The authentication variable is initialized but is no longer used after the refactoring. This unused code should be removed.

String phoneNumber;
if (authentication instanceof KmcAuthenticationToken kmcAuth) {
phoneNumber = kmcAuth.getPhoneNumber();
log.info("phoneNumber: {}", phoneNumber);
} else {
throw new IllegalStateException("KmcAuthenticationToken not found in SecurityContext for password change.");
}
ChangePasswordResponse response = myUserService.changePassword(request, phoneNumber);

return ResponseEntity.ok(ApiResponseWrapper.success(HttpStatus.OK, "비밀번호 변경 성공", response));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import io.swagger.v3.oas.annotations.Operation;
import io.swagger.v3.oas.annotations.tags.Tag;
import life.mosu.mosuserver.global.annotation.PhoneNumber;
import life.mosu.mosuserver.global.annotation.UserId;
import life.mosu.mosuserver.global.util.ApiResponseWrapper;
import life.mosu.mosuserver.presentation.user.dto.request.ChangePasswordRequest;
Expand All @@ -21,6 +22,7 @@ public ResponseEntity<ApiResponseWrapper<Void>> findPassword(

@Operation(summary = "비밀번호 변경", description = "현재 로그인한 사용자의 비밀번호를 변경합니다. 새 비밀번호와 새 비밀번호 확인이 일치해야 합니다.(입력값으로 valificationCode 필드 추가 예정)")
public ResponseEntity<ApiResponseWrapper<ChangePasswordResponse>> changePassword(
@PhoneNumber final String phoneNumber,
@RequestBody ChangePasswordRequest request
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import life.mosu.mosuserver.global.annotation.PasswordPattern;

public record ChangePasswordRequest(
String loginId,
@Schema(

Choose a reason for hiding this comment

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

medium

The loginId field is no longer needed in this DTO, as the phone number is now used for identification. Remove this field to simplify the request object.

description = "새로운 비밀번호는 8~20자의 영문 대/소문자, 숫자, 특수문자를 모두 포함해야 합니다.",
example = "Mosu!1234"
Expand Down