Skip to content

MOSU refactor: enhance OAuth2 login failure handling with redirect #305

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wlgns12370 merged 1 commit into from
Aug 10, 2025
Merged

MOSU refactor: enhance OAuth2 login failure handling with redirect #305

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
40 changes: 13 additions & 27 deletions src/main/java/life/mosu/mosuserver/global/handler/OAuth2LoginFailureHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,15 @@
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import java.io.IOException;
import life.mosu.mosuserver.global.exception.ErrorResponse;
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.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.security.core.AuthenticationException;
import org.springframework.security.oauth2.core.OAuth2AuthenticationException;
import org.springframework.security.oauth2.core.OAuth2Error;
import org.springframework.security.web.authentication.AuthenticationFailureHandler;
import org.springframework.stereotype.Component;
import org.springframework.web.util.UriComponentsBuilder;
import org.springframework.web.util.UriUtils;

@Slf4j
@Component
Expand All @@ -28,29 +27,16 @@ public class OAuth2LoginFailureHandler implements
public void onAuthenticationFailure(HttpServletRequest request, HttpServletResponse response,
AuthenticationException exception) throws IOException, ServletException {

String errorCode = "UNKNOWN_ERROR";
String errorMessage = "로그인에 실패했습니다. 관리자에게 문의하세요.";
LoginResponse loginResponse = LoginResponse.from();
String jsonResponse = UriUtils.encode(objectMapper.writeValueAsString(loginResponse),
StandardCharsets.UTF_8);

if (exception instanceof OAuth2AuthenticationException oAuth2Ex) {
OAuth2Error error = oAuth2Ex.getError();
errorCode = error.getErrorCode();
errorMessage = error.getDescription() != null ? error.getDescription() : errorCode;
final String redirectWithAccessToken = UriComponentsBuilder.fromUriString(
"https://www.mosuedu.com/auth/kakao/redirect")
.queryParam("data", jsonResponse)
.build()
.toUriString();

log.info("OAuth2 Error: {}, Description: {}", error.getErrorCode(),
error.getDescription());
}

ErrorResponse errorResponse = ErrorResponse.builder()
.status(HttpStatus.UNAUTHORIZED.value())
.message("인증에 실패했습니다")
.errors(errorMessage)
.code(errorCode)
.build();

response.setStatus(HttpServletResponse.SC_UNAUTHORIZED);
response.setContentType("application/json;charset=UTF-8");
response.setContentType(MediaType.APPLICATION_JSON_VALUE);

objectMapper.writeValue(response.getWriter(), errorResponse);
response.sendRedirect(redirectWithAccessToken);
Comment on lines +30 to +40
Copy link

@gemini-code-assist gemini-code-assist bot Aug 10, 2025

Choose a reason for hiding this comment

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

high

This method can be improved in several ways:

  1. Missing Failure Logging: The AuthenticationException is ignored. It's crucial to log authentication failures for security monitoring and debugging.
  2. Hardcoded Redirect URL: The redirect URL is hardcoded, which is inflexible for different environments or clients. It should be derived from the state parameter in the request, similar to OAuth2LoginSuccessHandler.

I've included a suggested implementation that addresses these points. This suggestion also anticipates renaming LoginResponse.from() to LoginResponse.createForFailure() for better clarity (see my other comment on LoginResponse.java).

To apply this suggestion, you will also need to:

  • Copy the parseState private method from OAuth2LoginSuccessHandler into this class.
  • Add the necessary imports for Map, HashMap, and REDIRECT_PARAM_KEY.
        log.warn("OAuth2 Authentication failed: {}", exception.getMessage());

        LoginResponse loginResponse = LoginResponse.createForFailure();
        String jsonResponse = UriUtils.encode(objectMapper.writeValueAsString(loginResponse),
                StandardCharsets.UTF_8);

        final String state = request.getParameter("state");
        final Map<String, String> stateParams = parseState(state);
        final String redirectUri = stateParams.getOrDefault(REDIRECT_PARAM_KEY, "/");

        final String redirectUrl = UriComponentsBuilder.fromUriString(redirectUri)
                .queryParam("data", jsonResponse)
                .build()
                .toUriString();

        response.sendRedirect(redirectUrl);

}
}
4 changes: 4 additions & 0 deletions src/main/java/life/mosu/mosuserver/presentation/auth/dto/request/LoginResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,8 @@ public static LoginResponse from(Boolean isProfileRegistered, final UserJpaEntit
}
return new LoginResponse(false, LoginUserResponse.from(user));
}

public static LoginResponse from() {
return new LoginResponse(null, null);
}
Comment on lines +25 to +27
Copy link

@gemini-code-assist gemini-code-assist bot Aug 10, 2025

Choose a reason for hiding this comment

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

medium

The method name from() is ambiguous for creating a failure response, especially since other from methods handle success cases. To improve code clarity and make the method's purpose self-documenting, consider renaming it to something more descriptive, like createForFailure().

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

}