Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 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 @@ -23,13 +23,11 @@ public class SignUpService {
@Transactional
public Token signUp(final SignUpAccountRequest request) {
UserJpaEntity user = doAccountStep(request);
log.info("User signed up successfully: {}", user);
return authTokenManager.generateAuthToken(user);
}

private UserJpaEntity doAccountStep(SignUpAccountRequest request) {
UserJpaEntity user = request.toAuthEntity(passwordEncoder);
log.info("log : {}", user);
return signUpAccountStepProcessor.process(user);
}
}
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
package life.mosu.mosuserver.application.auth.kmc.tx;

import life.mosu.mosuserver.domain.auth.signup.Token;
import life.mosu.mosuserver.domain.auth.token.Token;

public record KmcContext(
String certNum,
Long expiration,
Boolean isSuccess
) {

public static KmcContext ofSuccess(String certNum, Long expiration) {
return new KmcContext(certNum, expiration, true);
}
Expand All @@ -17,8 +18,8 @@ public static KmcContext ofFailure(String certNum) {

public Token toToken() {
return Token.of(
this.certNum,
this.expiration
this.certNum,
this.expiration
);
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package life.mosu.mosuserver.application.auth.kmc.tx;

import life.mosu.mosuserver.domain.auth.signup.Token;
import life.mosu.mosuserver.domain.auth.signup.TokenRepository;
import life.mosu.mosuserver.domain.auth.token.Token;
import life.mosu.mosuserver.domain.auth.token.TokenRepository;
import life.mosu.mosuserver.global.tx.TxFailureHandler;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,28 @@ public class SignUpAccountStepProcessor implements StepProcessor<UserJpaEntity,
@Transactional
@Override
public UserJpaEntity process(UserJpaEntity user) {
if (userRepository.existsByPhoneNumber(user.getOriginPhoneNumber())) {
throw new CustomRuntimeException(ErrorCode.USER_ALREADY_EXISTS);
} else if (userRepository.existsByLoginId(user.getLoginId())) {

validateForNewSignUp(user);

return userRepository.save(user);
}

private void validateForNewSignUp(UserJpaEntity user) {

userRepository.findByPhoneNumber(user.getOriginPhoneNumber())
.ifPresent(existingUser -> {
if (existingUser.isPendingUser()) {
switch (existingUser.getProvider()) {
case MOSU:
throw new CustomRuntimeException(ErrorCode.USER_ALREADY_EXISTS);
case KAKAO:
throw new CustomRuntimeException(ErrorCode.KAKAO_DUPLICATED);
}
}
});
Comment on lines +31 to +41

Choose a reason for hiding this comment

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

critical

validateForNewSignUp 메서드의 중복 사용자 검증 로직이 모든 케이스를 처리하지 못하고 있습니다. 현재 코드는 PENDING 상태의 사용자에 대해서만 중복을 검사하고, 이미 가입이 완료된 사용자에 대해서는 아무런 처리를 하지 않아 DataIntegrityViolationException이 발생할 수 있습니다.

또한, switch 문에 default 케이스가 없어 새로운 AuthProvider가 추가될 경우를 대비하지 못하고 있습니다.

아래와 같이 수정하여 모든 중복 케이스를 처리하고 코드를 단순화하는 것을 제안합니다. 이 로직은 카카오 중복 가입 시도에 대해 특정 에러를 발생시키고, 그 외 모든 중복 상황에서는 일반적인 "사용자 존재" 에러를 발생시킵니다.

        userRepository.findByPhoneNumber(user.getOriginPhoneNumber())
                .ifPresent(existingUser -> {
                    if (existingUser.isPendingUser() && existingUser.getProvider() == life.mosu.mosuserver.domain.user.entity.AuthProvider.KAKAO) {
                        throw new CustomRuntimeException(ErrorCode.KAKAO_DUPLICATED);
                    }
                    throw new CustomRuntimeException(ErrorCode.USER_ALREADY_EXISTS);
                });

Comment on lines +31 to +41

Choose a reason for hiding this comment

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

high

회원가입 시 사용자를 검증하는 로직에 잠재적인 문제가 있습니다. 현재 코드는 전화번호로 사용자를 조회한 후, 해당 사용자가 isPendingUser()일 경우에만 중복 여부를 판단합니다. 만약 이미 가입이 완료된 사용자가 동일한 전화번호로 다시 회원가입을 시도할 경우, isPendingUser()false를 반환하여 아무런 예외도 발생시키지 않고 넘어가게 됩니다. 이 경우 이후 userRepository.save(user) 호출 시 데이터베이스의 유니크 제약 조건 위반으로 인해 처리되지 않은 예외가 발생하여 사용자에게 좋지 않은 경험을 줄 수 있습니다.

기존 사용자가 PENDING 상태가 아니더라도 이미 존재하는 사용자이므로, USER_ALREADY_EXISTS 예외를 발생시켜야 합니다.

        userRepository.findByPhoneNumber(user.getOriginPhoneNumber())
                .ifPresent(existingUser -> {
                    if (existingUser.isPendingUser()) {
                        switch (existingUser.getProvider()) {
                            case KAKAO:
                                throw new CustomRuntimeException(ErrorCode.KAKAO_DUPLICATED);
                            case MOSU:
                            default:
                                throw new CustomRuntimeException(ErrorCode.USER_ALREADY_EXISTS);
                        }
                    }
                    // 사용자가 존재하지만 PENDING 상태가 아닌 경우에도 중복으로 처리합니다.
                    throw new CustomRuntimeException(ErrorCode.USER_ALREADY_EXISTS);
                });


if (userRepository.existsByLoginId(user.getLoginId())) {
throw new CustomRuntimeException(ErrorCode.USER_ALREADY_EXISTS);
}
log.info("Processing user sign-up: {}", user);
return userRepository.save(user);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,20 @@
import io.jsonwebtoken.security.Keys;
import java.security.Key;
import java.util.Date;
import life.mosu.mosuserver.domain.auth.signup.Token;
import life.mosu.mosuserver.domain.auth.signup.TokenRepository;
import life.mosu.mosuserver.domain.auth.token.Token;
import life.mosu.mosuserver.domain.auth.token.TokenRepository;
import lombok.extern.slf4j.Slf4j;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.stereotype.Service;

@Slf4j
@Service
public class OneTimeTokenProvider {
private final TokenRepository tokenRepository;

private static final String tokenType = "ONE_TIME";
private static final String TOKEN_TYPE_KEY = "type";
private static final String PHONE_NUMBER_KEY = "phone";

private final TokenRepository tokenRepository;
private final Long expireTime;
private final Key key;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,10 @@ public void notify(LunaNotificationEvent event) {
NotifySender<LunaNotificationVariable> sender = senderResolver.resolve(event.status());

sender.send(phone, event, notifyVariable);
log.info("[NotifyService] 알림톡 전송 성공: userId={}, status={}", event.userId(), event.status());
}

@Recover
public void recover(CustomRuntimeException exception, LunaNotificationEvent event) {
log.warn(
"[NotifyService] 알림톡 전송 실패: userId={}, status={}, reason={}",
event.userId(), event.status(), exception.getMessage()
);
DiscordExceptionNotifyEventRequest request = DiscordExceptionNotifyEventRequest.of(
exception.getCause() != null ? exception.getCause().toString() : "N/A",
exception.getMessage(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,23 @@ public class OAuthUserPersistenceProcessor implements StepProcessor<OAuthUserInf

private final UserJpaRepository userRepository;

/**
* this method is processing OAuthUserInfo to UserJpaEntity that is Get or Create.
*
* @param info
* @return
*/
@Override
@Transactional
public UserJpaEntity process(final OAuthUserInfo info) {
return userRepository.findByPhoneNumber(
PhoneNumberUtil.formatPhoneNumber(info.phoneNumber()))
.map(existingUser -> {
if (existingUser.isMosuUser()) {
throw new OAuthException("DUPLICATE");
switch (existingUser.getProvider()) {
case MOSU:
if (existingUser.isPendingUser()) {
throw new OAuthException("DUPLICATE");
}
break;
case KAKAO:
if (existingUser.isPendingUser()) {
throw new OAuthException("KAKAO_DUPLICATE");
}
break;
}
Comment on lines +26 to 37

Choose a reason for hiding this comment

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

high

switch (existingUser.getProvider()) 문에 default 케이스가 없어 새로운 AuthProvider가 추가될 경우를 대비하지 못하고 있습니다. 새로운 Provider가 추가되면 아무런 검증 없이 updateOAuthUser가 호출될 수 있습니다.

향후 확장성을 고려하여 default 케이스를 추가하는 것이 좋습니다. 예를 들어, 지원하지 않는 Provider에 대해 예외를 발생시키거나 기본 동작을 정의할 수 있습니다.

                    switch (existingUser.getProvider()) {
                        case MOSU:
                            if (existingUser.isPendingUser()) {
                                throw new OAuthException("DUPLICATE");
                            }
                            break;
                        case KAKAO:
                            if (existingUser.isPendingUser()) {
                                throw new OAuthException("KAKAO_DUPLICATE");
                            }
                            break;
                        default:
                            // 새로운 Provider에 대한 처리를 명시적으로 하거나, 아무것도 하지 않음을 주석으로 남기는 것이 좋습니다.
                            break;
                    }

existingUser.updateOAuthUser(
info.gender(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,6 @@ public OAuth2User loadUser(final OAuth2UserRequest userRequest)
.orElse(false);
}

log.info("동의 여부{}", agreedToMarketing);

final String registrationId = userRequest.getClientRegistration().getRegistrationId();
final String userNameAttributeName = userRequest.getClientRegistration()
.getProviderDetails()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package life.mosu.mosuserver.application.user;

import life.mosu.mosuserver.domain.profile.entity.ProfileJpaEntity;
import life.mosu.mosuserver.domain.user.entity.UserJpaEntity;
import life.mosu.mosuserver.domain.user.repository.UserJpaRepository;
import life.mosu.mosuserver.global.exception.CustomRuntimeException;
Expand Down Expand Up @@ -31,13 +30,6 @@ public UserJpaEntity getUserOrThrow(Long userId) {
);
}

public void syncUserInfoFromProfile(UserJpaEntity user, ProfileJpaEntity profile) {
if (user.isMosuUser()) {
user.updateUserInfo(profile.getGender(), profile.getUserName(),
profile.getPhoneNumber(), profile.getBirth());
}
}

public Long saveOrGetUser(UserJpaEntity user) {
return userJpaRepository.findByPhoneNumber(
PhoneNumberUtil.formatGuestPhoneNumber(user.getPhoneNumber()))
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
package life.mosu.mosuserver.domain.auth.signup;
package life.mosu.mosuserver.domain.auth.token;

public record Token(
String certNum,
Long expiration
) {

public static Token of(String certNum, Long expiration) {
return new Token(certNum, expiration);
}

public static Token from(SignUpTokenRedisEntity signUpTokenRedisEntity) {
public static Token from(TokenRedisEntity tokenRedisEntity) {
return new Token(
signUpTokenRedisEntity.getCertNum(),
signUpTokenRedisEntity.getExpiration()
tokenRedisEntity.getCertNum(),
tokenRedisEntity.getExpiration()
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package life.mosu.mosuserver.domain.auth.token;

import org.springframework.data.keyvalue.repository.KeyValueRepository;

public interface TokenKeyValueRepository extends
KeyValueRepository<TokenRedisEntity, String> {

}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package life.mosu.mosuserver.domain.auth.signup;
package life.mosu.mosuserver.domain.auth.token;

import java.util.concurrent.TimeUnit;
import lombok.AllArgsConstructor;
Expand All @@ -13,7 +13,7 @@
@RedisHash(value = "token")
@AllArgsConstructor(access = lombok.AccessLevel.PRIVATE)
@NoArgsConstructor(access = lombok.AccessLevel.PROTECTED)
public class SignUpTokenRedisEntity {
public class TokenRedisEntity {

@Id
@Indexed
Expand All @@ -22,7 +22,7 @@ public class SignUpTokenRedisEntity {
@TimeToLive(unit = TimeUnit.MILLISECONDS)
private Long expiration;

public static SignUpTokenRedisEntity from(final Token token) {
return new SignUpTokenRedisEntity(token.certNum(), token.expiration());
public static TokenRedisEntity from(final Token token) {
return new TokenRedisEntity(token.certNum(), token.expiration());
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package life.mosu.mosuserver.domain.auth.signup;
package life.mosu.mosuserver.domain.auth.token;

import life.mosu.mosuserver.global.exception.CustomRuntimeException;
import life.mosu.mosuserver.global.exception.ErrorCode;
Expand All @@ -9,23 +9,23 @@
@Slf4j
@Repository
@RequiredArgsConstructor
public class SignUpTokenRedisRepository implements TokenRepository {
public class TokenRedisRepository implements TokenRepository {

private final SignUpTokenKeyValueRepository repository;
private final TokenKeyValueRepository repository;

@Override
public void save(Token signUpToken) {
SignUpTokenRedisEntity entity = SignUpTokenRedisEntity.from(signUpToken);
TokenRedisEntity entity = TokenRedisEntity.from(signUpToken);
repository.save(entity);
}

@Override
public Token findByCertNum(String certNum) {
log.info("findByCertNum certNum={}", certNum);
SignUpTokenRedisEntity signUpTokenRedisEntity = repository.findById(certNum).orElseThrow(
TokenRedisEntity tokenRedisEntity = repository.findById(certNum).orElseThrow(
() -> new CustomRuntimeException(ErrorCode.NOT_FOUND_TOKEN)
);
return Token.from(signUpTokenRedisEntity);
return Token.from(tokenRedisEntity);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package life.mosu.mosuserver.domain.auth.signup;
package life.mosu.mosuserver.domain.auth.token;

public interface TokenRepository {

Token findByCertNum(String certNum);

void save(Token signUpToken);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,10 @@ public boolean isMosuUser() {
return this.provider.equals(AuthProvider.MOSU);
}

public boolean isPendingUser() {
return this.userRole.equals(UserRole.ROLE_PENDING);
}

public void grantUserRole() {
this.userRole = UserRole.ROLE_USER;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

@Pattern(regexp = "^(?=.*[a-z])(?=.*[A-Z])(?=.*\\d)(?=.*[@$!%*?&])[A-Za-z\\d@$!%*?&]{8,20}$", message = "비밀번호 형식이 올바르지 않습니다.")
@Pattern(regexp = "^(?=.*[a-z])(?=.*[A-Z])(?=.*\\d)(?=.*[!@#$%^&*_/+=])[A-Za-z\\d!@#$%^&*_/+=]{8,20}$", message = "비밀번호 형식이 올바르지 않습니다.")
@NotBlank
@Target({ElementType.FIELD, ElementType.PARAMETER})
@Retention(RetentionPolicy.RUNTIME)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package life.mosu.mosuserver.global.exception;

import lombok.RequiredArgsConstructor;

@RequiredArgsConstructor
public enum CriticalLevel {
LOW("낮음"),
MEDIUM("중간"),
HIGH("높음"),
CRITICAL("심각함");

private final String description;

public String getDescription() {
return description;
}

public boolean isEmergency() {
return this == HIGH || this == CRITICAL;
}

@Override
public String toString() {
return this.description;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@
public class CustomRuntimeException extends RuntimeException {

private final HttpStatus status;
private final CriticalLevel criticalLevel;
private final String message;
private final String code;

public CustomRuntimeException(ErrorCode errorCode) {
this.status = errorCode.getStatus();
this.criticalLevel = errorCode.getCriticalLevel();
this.message = errorCode.getMessage();
this.code = errorCode.name();

Expand All @@ -22,6 +24,7 @@ public CustomRuntimeException(ErrorCode errorCode) {

public CustomRuntimeException(ErrorCode errorCode, Object... cause) {
this.status = errorCode.getStatus();
this.criticalLevel = errorCode.getCriticalLevel();
this.message = String.format(errorCode.getMessage(), cause);
this.code = errorCode.name();

Expand Down
Loading