fix: OAuth2 웹 로그인 CORS 에러 해결 (303 리다이렉트 → 200 + redirectUrl) - #353
Merged
Conversation
브라우저 XHR/fetch가 크로스 오리진 3xx 리다이렉트를 따라갈 때 Origin 헤더가
null로 교체되는 Fetch 스펙 때문에 auth.econovation.kr에 CORS 헤더를 추가해도
브라우저가 허용하지 않는 문제가 있었다.
POST /api/v2/auth/login에서 303 SEE_OTHER 대신 200 OK와 함께
{ "redirectUrl": "..." }를 반환하도록 변경한다.
FE는 응답 받은 후 window.location.href = redirectUrl로 브라우저 레벨
네비게이션을 수행해야 한다. 브라우저 레벨 이동은 CORS 적용 대상이 아니다.
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
WalkthroughOAuth2 로그인 엔드포인트( ChangesOAuth2 로그인 응답 방식 리다이렉트 → JSON 전환
Sequence Diagram(s)sequenceDiagram
participant Client as 클라이언트
participant OAuth2Controller as OAuth2Controller
participant CookieService as CookieService
rect rgba(255, 100, 100, 0.5)
note over Client,CookieService: 이전 방식 (303 SEE_OTHER)
Client->>OAuth2Controller: GET /login (client_id, redirect_uri, state)
OAuth2Controller->>CookieService: 쿠키 설정 (WEB)
OAuth2Controller-->>Client: 303 SEE_OTHER<br/>Location: redirect_uri?state=...
end
rect rgba(100, 180, 100, 0.5)
note over Client,CookieService: 변경 후 방식 (200 OK + JSON)
Client->>OAuth2Controller: GET /login (client_id, redirect_uri, state)
alt WEB 클라이언트
OAuth2Controller->>CookieService: 쿠키 설정
OAuth2Controller-->>Client: 200 OK {"redirectUrl": "redirect_uri?state=..."}
else APP 클라이언트
OAuth2Controller-->>Client: 200 OK {"redirectUrl": "redirect_uri?code=...&state=..."}
else 인증 실패
OAuth2Controller-->>Client: 200 OK {"redirectUrl": "loginPageUrl?error=invalid_credentials", "error": "invalid_credentials"}
end
Client->>Client: redirectUrl로 직접 이동
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Collaborator
Author
FE 팀 필수 변경사항
변경 전// 서버가 303 리다이렉트를 내려주면 브라우저가 자동으로 따라감
await fetch('/api/v2/auth/login', {
method: 'POST',
credentials: 'include',
body: new URLSearchParams({ client_id, redirect_uri, state, email, password }),
})
// 이후 처리 없음 - 브라우저가 알아서 이동변경 후const res = await fetch('/api/v2/auth/login', {
method: 'POST',
credentials: 'include',
body: new URLSearchParams({ client_id, redirect_uri, state, email, password }),
})
const { redirectUrl, error } = await res.json()
if (error === 'invalid_credentials') {
// 로그인 실패 - 에러 메시지 표시
// redirectUrl에는 loginPage?...&error=invalid_credentials 가 담겨 있음
showError('이메일 또는 비밀번호를 확인해주세요.')
return
}
// 성공 - 브라우저 레벨로 직접 이동 (CORS 적용 안 됨)
window.location.href = redirectUrl응답 스펙
핵심 포인트
|
Test Results226 tests ±0 224 ✅ ±0 9s ⏱️ -1s Results for commit 95f94df. ± Comparison against base commit 3ff1b37. This pull request removes 2 and adds 2 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
eeos/src/main/java/com/blackcompany/eeos/auth/presentation/controller/OAuth2Controller.java (1)
104-107:⚠️ Potential issue | 🟠 Major | ⚡ Quick win광범위한 예외 처리로 인해 서버 오류가 숨겨질 수 있음
catch(Exception e)는NullPointerException,IllegalStateException등 서버 측 버그까지 모두invalid_credentials로 처리합니다. 실제 인증 실패와 서버 오류를 구분하지 못해 디버깅이 어려워집니다.🛠️ 제안: 인증 관련 예외만 명시적으로 처리
- } catch (Exception e) { + } catch (AuthenticationFailedException e) { return redirectToLoginPageWithError( clientId, redirectUri, state, codeChallenge, codeChallengeMethod); }또는 최소한 예외를 로깅하여 서버 오류 추적이 가능하도록 하세요:
} catch (Exception e) { + log.warn("Login failed for client {}: {}", clientId, e.getMessage()); return redirectToLoginPageWithError( clientId, redirectUri, state, codeChallenge, codeChallengeMethod); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@eeos/src/main/java/com/blackcompany/eeos/auth/presentation/controller/OAuth2Controller.java` around lines 104 - 107, The catch block in OAuth2Controller is catching all exceptions broadly and treating them uniformly as authentication failures, which hides actual server-side errors like NullPointerException or IllegalStateException. Replace the generic catch(Exception e) with either: (1) specific exception handling for authentication-related exceptions only, or (2) at minimum, add logging of the actual exception before calling redirectToLoginPageWithError method so that server errors can be traced and debugged. This will allow distinguishing between legitimate authentication failures and unexpected server errors.eeos/src/test/java/com/blackcompany/eeos/auth/presentation/controller/OAuth2ControllerTest.java (2)
101-113:⚠️ Potential issue | 🟡 Minor | ⚡ Quick wininvalid client 응답 body의
error필드 검증 누락컨트롤러가
Map.of("error", "invalid_client")를 반환하지만 테스트에서 body 내용을 검증하지 않습니다.✅ 제안: error 필드 검증 추가
assertEquals(HttpStatus.BAD_REQUEST, result.getStatusCode()); + assertNotNull(result.getBody()); + assertEquals("invalid_client", result.getBody().get("error"));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@eeos/src/test/java/com/blackcompany/eeos/auth/presentation/controller/OAuth2ControllerTest.java` around lines 101 - 113, The test for the controller.login() method verifies only the HTTP status code (BAD_REQUEST) but does not validate the response body. Add an assertion to verify that the response body returned by result.getBody() contains the "error" field with the value "invalid_client". This ensures the controller is returning the expected error details in the response body when an invalid client is provided.
116-195:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win인증 실패 시나리오 테스트 누락
redirectToLoginPageWithError가 호출되는 케이스(잘못된 credentials)에 대한 테스트가 없습니다. 새로운 JSON 응답 형식({ "redirectUrl": "...", "error": "invalid_credentials" })이 올바르게 반환되는지 검증이 필요합니다.인증 실패 케이스 테스트를 생성해 드릴까요?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@eeos/src/test/java/com/blackcompany/eeos/auth/presentation/controller/OAuth2ControllerTest.java` around lines 116 - 195, The test class is missing test cases for authentication failure scenarios where redirectToLoginPageWithError is called with invalid credentials. Add new test methods (such as web_login_with_invalid_credentials and app_login_with_invalid_credentials) that mock the oAuth2LoginService to throw an exception or return failure for both WEB and APP client types. In these tests, verify that the controller returns the new JSON response format containing both a redirectUrl with error information and an error field with value like "invalid_credentials", ensuring the HTTP status code appropriately reflects the authentication failure.eeos/src/main/java/com/blackcompany/eeos/auth/presentation/docs/OAuth2Api.java (1)
42-67:⚠️ Potential issue | 🟠 Major테스트 스크립트 갱신 필수: API 응답 코드 변경으로 인한 호환성 깨짐
API 응답 코드가 303에서 200으로 변경되었으나, 테스트 스크립트가 여전히 303 리다이렉트를 검증하고 있습니다. 다음 파일들을 갱신하지 않으면 CI/CD 테스트가 실패합니다:
scripts/api-test/test-auth-api.sh: 215, 227, 283, 331 라인 (assert_status 및 assert_redirect로 303 검증)scripts/api-test/test-login-flow.sh: 108, 109, 127 라인 (LOGIN_STATUS = 303 비교)테스트 스크립트를 새로운 200 응답 코드와 JSON 응답 본문(redirectUrl)에 맞게 갱신하세요.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@eeos/src/main/java/com/blackcompany/eeos/auth/presentation/docs/OAuth2Api.java` around lines 42 - 67, The API response code for the OAuth2 login endpoint has changed from HTTP 303 (redirect) to HTTP 200 (with JSON body containing redirectUrl), but the test scripts are still validating the old 303 response format. Update the test scripts test-auth-api.sh and test-login-flow.sh to replace all assertions and status code comparisons that check for 303 redirect responses with validations for HTTP 200 status codes. Additionally, modify these test scripts to parse and validate the JSON response body (specifically the redirectUrl field) instead of checking HTTP redirect headers or Location headers, since the endpoint now returns the redirect URL in the response body rather than using HTTP redirects.
🧹 Nitpick comments (2)
eeos/src/test/java/com/blackcompany/eeos/auth/presentation/controller/OAuth2ControllerTest.java (2)
192-194: ⚡ Quick win동일하게
result.getBody()null 체크 누락✅ 제안
assertEquals(HttpStatus.OK, result.getStatusCode()); + assertNotNull(result.getBody()); assertTrue(result.getBody().get("redirectUrl").contains("code=auth-code-123"));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@eeos/src/test/java/com/blackcompany/eeos/auth/presentation/controller/OAuth2ControllerTest.java` around lines 192 - 194, The test assertions in OAuth2ControllerTest are missing a null check for result.getBody() before accessing its contents with the get() method. Add a null assertion or check to verify that result.getBody() is not null before calling get("redirectUrl") on it in the subsequent assertions. This ensures the test properly validates that the response body exists before attempting to retrieve values from it.
154-157: ⚡ Quick win
result.getBody()null 체크 없이 접근
getBody()가 null을 반환할 경우 테스트가 NPE로 실패하여 실제 원인 파악이 어려워집니다.✅ 제안: assertNotNull 추가
assertEquals(HttpStatus.OK, result.getStatusCode()); + assertNotNull(result.getBody()); assertTrue(result.getBody().get("redirectUrl").contains("http://web/callback"));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@eeos/src/test/java/com/blackcompany/eeos/auth/presentation/controller/OAuth2ControllerTest.java` around lines 154 - 157, The test in OAuth2ControllerTest is accessing result.getBody() multiple times without verifying it is not null first. If getBody() returns null, the subsequent calls to get("redirectUrl") will throw a NullPointerException instead of providing a clear test failure. Add an assertNotNull check on result.getBody() before the lines that call get("redirectUrl") to ensure the response body exists, providing better error messages if the assertion fails.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@eeos/src/main/java/com/blackcompany/eeos/auth/presentation/controller/OAuth2Controller.java`:
- Around line 104-107: The catch block in OAuth2Controller is catching all
exceptions broadly and treating them uniformly as authentication failures, which
hides actual server-side errors like NullPointerException or
IllegalStateException. Replace the generic catch(Exception e) with either: (1)
specific exception handling for authentication-related exceptions only, or (2)
at minimum, add logging of the actual exception before calling
redirectToLoginPageWithError method so that server errors can be traced and
debugged. This will allow distinguishing between legitimate authentication
failures and unexpected server errors.
In
`@eeos/src/main/java/com/blackcompany/eeos/auth/presentation/docs/OAuth2Api.java`:
- Around line 42-67: The API response code for the OAuth2 login endpoint has
changed from HTTP 303 (redirect) to HTTP 200 (with JSON body containing
redirectUrl), but the test scripts are still validating the old 303 response
format. Update the test scripts test-auth-api.sh and test-login-flow.sh to
replace all assertions and status code comparisons that check for 303 redirect
responses with validations for HTTP 200 status codes. Additionally, modify these
test scripts to parse and validate the JSON response body (specifically the
redirectUrl field) instead of checking HTTP redirect headers or Location
headers, since the endpoint now returns the redirect URL in the response body
rather than using HTTP redirects.
In
`@eeos/src/test/java/com/blackcompany/eeos/auth/presentation/controller/OAuth2ControllerTest.java`:
- Around line 101-113: The test for the controller.login() method verifies only
the HTTP status code (BAD_REQUEST) but does not validate the response body. Add
an assertion to verify that the response body returned by result.getBody()
contains the "error" field with the value "invalid_client". This ensures the
controller is returning the expected error details in the response body when an
invalid client is provided.
- Around line 116-195: The test class is missing test cases for authentication
failure scenarios where redirectToLoginPageWithError is called with invalid
credentials. Add new test methods (such as web_login_with_invalid_credentials
and app_login_with_invalid_credentials) that mock the oAuth2LoginService to
throw an exception or return failure for both WEB and APP client types. In these
tests, verify that the controller returns the new JSON response format
containing both a redirectUrl with error information and an error field with
value like "invalid_credentials", ensuring the HTTP status code appropriately
reflects the authentication failure.
---
Nitpick comments:
In
`@eeos/src/test/java/com/blackcompany/eeos/auth/presentation/controller/OAuth2ControllerTest.java`:
- Around line 192-194: The test assertions in OAuth2ControllerTest are missing a
null check for result.getBody() before accessing its contents with the get()
method. Add a null assertion or check to verify that result.getBody() is not
null before calling get("redirectUrl") on it in the subsequent assertions. This
ensures the test properly validates that the response body exists before
attempting to retrieve values from it.
- Around line 154-157: The test in OAuth2ControllerTest is accessing
result.getBody() multiple times without verifying it is not null first. If
getBody() returns null, the subsequent calls to get("redirectUrl") will throw a
NullPointerException instead of providing a clear test failure. Add an
assertNotNull check on result.getBody() before the lines that call
get("redirectUrl") to ensure the response body exists, providing better error
messages if the assertion fails.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 18aa293d-9878-4106-89a2-3dcd7c37683a
📒 Files selected for processing (3)
eeos/src/main/java/com/blackcompany/eeos/auth/presentation/controller/OAuth2Controller.javaeeos/src/main/java/com/blackcompany/eeos/auth/presentation/docs/OAuth2Api.javaeeos/src/test/java/com/blackcompany/eeos/auth/presentation/controller/OAuth2ControllerTest.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
개요
브라우저에서
POST /api/v2/auth/login을 XHR/fetch로 호출했을 때 CORS 에러가 발생했다.근본 원인은 Fetch 스펙의 크로스 오리진 리다이렉트 시
Origin: null교체 동작이다.auth.econovation.kr에 CORS 헤더를 추가해도 오리진이null로 바뀌어 영원히 실패하는 구조였다.303 리다이렉트대신200 + { "redirectUrl": "..." }을 반환하고, FE가window.location.href로 브라우저 레벨 이동을 수행하도록 변경한다.왜 에러가 났는가
왜 이 방법으로 해결되는가
영향 범위
프론트엔드:
POST /api/v2/auth/login응답 처리 로직 변경 필수.redirectUrl을 읽어window.location.href = redirectUrl수행"error": "invalid_credentials"키가 함께 포함됨WEB/APP 공통 적용: WEB(쿠키 방식)과 APP(PKCE authorization code) 모두 동일하게
200 + redirectUrl반환으로 변경됨.배포 전 필수 확인
POST /api/v2/auth/login응답 처리를window.location.href = redirectUrl방식으로 변경했는지 확인COOKIE_TOKEN_DOMAIN=.econovation.kr설정 여부 확인 (WEB 타입 쿠키가auth.econovation.kr에서 읽혀야 하므로)Summary by CodeRabbit
릴리스 노트
Refactor
redirectUrl을 받아 직접 처리하면 됩니다.Bug Fixes
Documentation