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 @@ -4,6 +4,7 @@
import life.mosu.mosuserver.domain.user.entity.UserJpaEntity;
import life.mosu.mosuserver.domain.user.entity.UserRole;
import life.mosu.mosuserver.domain.user.repository.UserJpaRepository;
import life.mosu.mosuserver.global.exception.OAuthException;
import life.mosu.mosuserver.global.processor.StepProcessor;
import life.mosu.mosuserver.global.util.PhoneNumberUtil;
import lombok.RequiredArgsConstructor;
Expand All @@ -28,9 +29,9 @@ public UserJpaEntity process(final OAuthUserInfo info) {
return userRepository.findByPhoneNumber(
PhoneNumberUtil.formatPhoneNumber(info.phoneNumber()))
.map(existingUser -> {
// if (existingUser.isMosuUser()) {
// throw new IllegalArgumentException("이미 모수 회원입니다.");
// }
if (existingUser.isMosuUser()) {
throw new OAuthException("DUPLICATE");

Choose a reason for hiding this comment

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

medium

에러 메시지로 하드코딩된 문자열 "DUPLICATE"을 사용하는 대신, OAuthErrorType enum을 직접 사용하여 타입 안정성을 높이고 유지보수를 용이하게 하는 것이 좋습니다. OAuthErrorType.DUPLICATE.getMessage()를 사용하면, 향후 에러 코드 문자열이 변경되더라도 한 곳(OAuthErrorType enum)에서만 수정하면 되므로 코드 관리가 더 수월해집니다. 이 변경을 적용하려면 life.mosu.mosuserver.global.handler.OAuthErrorType를 import해야 합니다.

Suggested change
throw new OAuthException("DUPLICATE");
throw new OAuthException(OAuthErrorType.DUPLICATE.getMessage());

}
existingUser.updateOAuthUser(
info.gender(),
info.name(),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package life.mosu.mosuserver.global.exception;

import org.springframework.security.core.AuthenticationException;

public class OAuthException extends AuthenticationException {

public OAuthException(String msg) {
super(msg);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import jakarta.servlet.http.HttpServletResponse;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import life.mosu.mosuserver.presentation.auth.dto.request.LoginResponse;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.security.core.AuthenticationException;
Expand All @@ -27,8 +26,11 @@ public class OAuth2LoginFailureHandler implements
public void onAuthenticationFailure(HttpServletRequest request, HttpServletResponse response,
AuthenticationException exception) throws IOException, ServletException {

LoginResponse loginResponse = LoginResponse.from();
String jsonResponse = UriUtils.encode(objectMapper.writeValueAsString(loginResponse),
OAuthErrorType errorType = OAuthErrorType.from(exception.getMessage());
OAuthFailureResponse oAuthFailureResponse = OAuthFailureResponse.from(
errorType.getMessage());
Comment on lines +29 to +31

Choose a reason for hiding this comment

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

high

exception.getMessage()를 직접 사용하여 에러 유형을 확인하는 것은 OAuth 제공자의 에러 메시지 변경에 취약할 수 있습니다.

AuthenticationExceptionOAuth2AuthenticationException의 인스턴스인 경우, getError().getErrorCode()를 통해 보다 안정적으로 에러 코드를 추출할 수 있습니다. 이렇게 하면 "access_denied"와 같은 표준 에러 코드를 사용하여 OAuthErrorType에서 분기 처리를 할 수 있어 코드가 더 견고해집니다.

이 변경을 적용하려면 org.springframework.security.oauth2.core.OAuth2AuthenticationException를 import해야 합니다.

OAuthErrorType.java의 관련 수정 제안도 함께 확인해주세요.

        String errorCode = exception.getMessage();
        if (exception instanceof org.springframework.security.oauth2.core.OAuth2AuthenticationException e) {
            errorCode = e.getError().getErrorCode();
        }

        OAuthErrorType errorType = OAuthErrorType.from(errorCode);
        OAuthFailureResponse oAuthFailureResponse = OAuthFailureResponse.from(
                errorType.getMessage());


String jsonResponse = UriUtils.encode(objectMapper.writeValueAsString(oAuthFailureResponse),
StandardCharsets.UTF_8);

final String redirectWithAccessToken = UriComponentsBuilder.fromUriString(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package life.mosu.mosuserver.global.handler;

import lombok.Getter;

@Getter
public enum OAuthErrorType {
CANCELED("CANCELED"),
DUPLICATE("DUPLICATE"),
UNKNOWN("UNKNOWN");

private final String message;

OAuthErrorType(String message) {
this.message = message;
}

public static OAuthErrorType from(String text) {
if (text == null) {
return UNKNOWN;
}
return switch (text) {
case "DUPLICATE" -> DUPLICATE;
case "[access_denied] User denied access" -> CANCELED;
default -> UNKNOWN;
};
Comment on lines +21 to +25

Choose a reason for hiding this comment

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

high

에러 메시지 문자열 "[access_denied] User denied access"에 직접 의존하는 것은 좋지 않은 방법입니다. OAuth 제공자가 에러 메시지 형식을 변경하면 코드가 오작동할 수 있습니다.

OAuth2LoginFailureHandler에서 exception 객체가 OAuth2AuthenticationException의 인스턴스인지 확인하고, 그렇다면 getError().getErrorCode()를 사용하여 "access_denied"와 같은 표준 오류 코드를 가져오는 것이 더 안정적입니다.

관련하여 OAuth2LoginFailureHandler.java 파일에도 수정 제안을 남겼습니다. 이 제안과 함께 적용하는 것을 권장합니다.

아래와 같이 수정할 수 있습니다.

Suggested change
return switch (text) {
case "DUPLICATE" -> DUPLICATE;
case "[access_denied] User denied access" -> CANCELED;
default -> UNKNOWN;
};
return switch (text) {
case "DUPLICATE" -> DUPLICATE;
case "access_denied" -> CANCELED;
default -> UNKNOWN;
};

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package life.mosu.mosuserver.global.handler;

public record OAuthFailureResponse(
Boolean isProfileRegistered,
String errorCode
) {

public static OAuthFailureResponse from(String errorCode) {
return new OAuthFailureResponse(null, errorCode);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,4 @@ public static LoginResponse from(Boolean isProfileRegistered, final UserJpaEntit
}
return new LoginResponse(false, LoginUserResponse.from(user));
}

public static LoginResponse from() {
return new LoginResponse(null, null);
}
}