[FEAT] auth-api 연동 — Passport 인증 필터 추가 및 자체 OAuth2 서버 제거 - #348
Open
kmando01 wants to merge 14 commits into
Open
Conversation
…istration to auth-api
EEOS-BE가 자체 OAuth2 Authorization Server(ClientController, OAuth2Controller,
TokenExchangeService 등)를 운영하던 구조를 제거한다. 클라이언트 등록은 auth-api의
POST /api/v1/admin/clients로, 토큰 발급은 auth-api SAS로 위임한다.
- 삭제: ClientController, ClientService, ClientEntity, ClientType
OAuth2Controller, OAuth2LoginService, TokenExchangeService
ClientRegistrationRequest/Response, ClientRedirectUriEntity
AuthorizationCodeRepository, AuthorizationCodeData, PkceValidator
관련 테스트 파일 7개
- 추가: V1.00.0.8 — oauth_client, oauth_client_redirect_uri 테이블 DROP
- 복원: TokenResponseConverter (잘못 삭제된 파일 복원)
- InternalApiKeyFilter: shouldNotFilter(/api/internal/ 외 경로 스킵)
- UnknownEndpointFilter: shouldNotFilter(Security 체인 처리 완료 요청 스킵)
- SecurityFilterChainConfig: /api/v2/auth/** 경로 제거
- MemberArgumentResolver: SecurityContext 우선 읽기 (Passport 지원 준비)
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
… injection Gateway(BearerToPassportFilter)가 JWT를 검증하고 X-User-Passport 헤더를 Base64(JSON) 형식으로 주입하면, EEOS-BE의 PassportAuthenticationFilter가 이를 파싱해 JwtAuthentication을 SecurityContext에 설정한다. - PassportAuthenticationFilter: Security 체인 내에만 등록(@component 없음) → Servlet 필터로 자동 등록되면 SecurityContextHolderFilter 리셋으로 무효화되는 문제를 방지하기 위해 new 인스턴스로 addFilterBefore(LogoutFilter) 등록 - AccessTokenFilter: JwtAuthentication이 이미 있으면 스킵 (Gateway 경유 요청은 EEOS 자체 HMAC 토큰 검증 불필요) - MemberArgumentResolver: SecurityContext 우선 읽기 (JwtAuthentication → memberId 직접 반환, fallback: 기존 토큰 파싱) - SecurityFilterChainConfig: PassportFilter를 authenticated 체인에 추가 인증 공존: - Gateway 경유: auth-api RS256 토큰 → X-User-Passport → PassportFilter ✅ - EEOS 직접: EEOS HMAC 토큰 → AccessTokenFilter ✅ Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> chore: apply Spotless formatting to PassportAuthenticationFilter
|
Warning Review limit reached
More reviews will be available in 50 minutes and 20 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
WalkthroughOAuth2 클라이언트 등록·교환 관련 코드와 엔티티를 제거하고, X-User-Passport 헤더 기반 PassportAuthenticationFilter를 도입해 보안 필터 체인을 재구성하며 관련 예외 처리·리졸버·테스트·DB 마이그레이션·문서를 갱신합니다. ChangesOAuth2 클라이언트 관리 제거 및 Passport 인증 도입
Sequence DiagramsequenceDiagram
participant Client
participant PassportFilter as PassportAuthenticationFilter
participant ObjectMapper
participant SecurityContext as SecurityContextHolder
Client->>PassportFilter: X-User-Passport 헤더 전달
PassportFilter->>PassportFilter: Base64 디코딩
PassportFilter->>ObjectMapper: JSON 파싱 (claims)
ObjectMapper-->>PassportFilter: memberId, roles
alt memberId 존재
PassportFilter->>SecurityContext: JwtAuthentication 설정
else 실패
PassportFilter->>PassportFilter: warn 로그 기록
end
PassportFilter->>Client: chain.doFilter 진행
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
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 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 |
Test Results202 tests - 24 200 ✅ - 24 10s ⏱️ -1s Results for commit 0327284. ± Comparison against base commit 7dc7faa. This pull request removes 36 and adds 12 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.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
eeos/docs/auth_README.md (1)
128-207:⚠️ Potential issue | 🟠 Major | ⚡ Quick win현재 아키텍처와 상충하는 OAuth 클라이언트 등록 문서입니다.
Line 128-207 섹션은 PR에서 제거된
/api/v2/auth/clients흐름과ClientController/ClientService/ClientEntity/ClientType를 여전히 활성 기능처럼 안내하고 있습니다. 지금 상태로 머지되면 운영 가이드가 실제 구현과 불일치합니다. 해당 섹션은 제거하거나, “삭제됨/비활성”으로 명확히 표기해 주세요.🤖 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/docs/auth_README.md` around lines 128 - 207, The docs section describing POST /api/v2/auth/clients and related classes (ClientController, ClientService, ClientEntity, ClientType, InvalidRedirectUriException) is stale because that flow was removed in the PR; update eeos/docs/auth_README.md by either deleting the entire "OAuth 클라이언트 등록 (ADMIN 전용)" block or clearly marking it as removed/disabled (e.g., add a short header "Removed — not implemented" and note the removed symbols: /api/v2/auth/clients, ClientController, ClientService, ClientEntity, ClientType) so the operational guide matches the current implementation.
🧹 Nitpick comments (1)
eeos/src/main/java/com/blackcompany/eeos/config/security/AccessTokenFilter.java (1)
38-56: ⚡ Quick win
"eeos.securityChainProcessed"리터럴을 공유 상수로 추출.이 문자열은 본 파일에서 3회(40, 51, 56) 사용되고
UnknownEndpointFilter에서 읽힙니다(getter). 한쪽에 오타가 생기면 스킵 계약이 조용히 깨져 인증된 요청에 404가 반환되는 식의 버그로 이어집니다. 두 필터가 참조하는 공유 상수로 추출하길 권장합니다.♻️ 공유 상수 추출 예시
+ static final String SECURITY_CHAIN_PROCESSED = "eeos.securityChainProcessed"; + `@Override` protected void doFilterInternal( HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) throws ServletException, IOException { // Passport 필터(Gateway 경유)가 이미 인증을 설정했으면 스킵 if (SecurityContextHolder.getContext().getAuthentication() instanceof JwtAuthentication) { - request.setAttribute("eeos.securityChainProcessed", Boolean.TRUE); + request.setAttribute(SECURITY_CHAIN_PROCESSED, Boolean.TRUE); filterChain.doFilter(request, response); return; }이후
UnknownEndpointFilter의shouldNotFilter도 동일 상수를 참조하도록 변경하면 됩니다.🤖 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/config/security/AccessTokenFilter.java` around lines 38 - 56, Extract the literal "eeos.securityChainProcessed" into a shared public constant (e.g., public static final String EEOS_SECURITY_CHAIN_PROCESSED = "eeos.securityChainProcessed") in a common class (e.g., SecurityConstants) and replace the three literal usages in AccessTokenFilter (the request.setAttribute calls and any checks) with SecurityConstants.EEOS_SECURITY_CHAIN_PROCESSED; then update UnknownEndpointFilter to read the same constant (instead of the literal) so both filters reference the single shared symbol. Ensure the constant is public and imported where needed.
🤖 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.
Inline comments:
In `@eeos/docs/auth_README.md`:
- Line 175: 문서에 노출된 민감한 값인 "clientSecret"의 실제 형태(예:
"xKz3Qp9mRvLs7wNt2YhJ4dUiOeAn0BfCgXvPqWmE5c")를 플레이스홀더로 교체하세요: 해당 예제의
"clientSecret" 값을 실제 키처럼 보이는 문자열에서 "<issued-once-secret>" 또는
"<your-client-secret>" 같은 안전한 플레이스홀더로 바꾸고, 동일한 파일 내 유사한 예제(또는 "clientSecret"을
사용하는 다른 코드 블록)가 있으면 동일하게 대체해 보안 스캐너 트리거를 방지하십시오.
In
`@eeos/src/main/java/com/blackcompany/eeos/common/exception/GlobalExceptionHandler.java`:
- Around line 90-96: The current GlobalExceptionHandler method
handleIllegalArgumentException maps all IllegalArgumentException to 400; change
this to only handle explicit input-validation exceptions (e.g., a custom
BadRequestException) by renaming/replacing the handler to target that custom
exception (e.g., `@ExceptionHandler`(BadRequestException.class) in
GlobalExceptionHandler) and update the method signature accordingly to still use
ApiResponseGenerator.fail; remove or stop handling raw IllegalArgumentException
so it falls through to the existing 500 error path (or let the generic exception
handler handle it) to avoid masking internal bugs.
In
`@eeos/src/main/java/com/blackcompany/eeos/config/security/PassportAuthenticationFilter.java`:
- Around line 52-55: PassportAuthenticationFilter converts passport claim
"roles" into SimpleGrantedAuthority as-is (see toStringList and
JwtAuthentication usage), which will fail authorization if passport sends
"ADMIN" while SecurityFilterChainConfig uses hasAnyRole(ADMIN) expecting
Spring's "ROLE_ADMIN" prefix; update PassportAuthenticationFilter to normalize
each role into the Spring authority format (ensure "ROLE_" prefix) before
mapping to SimpleGrantedAuthority or, alternatively, detect a custom role prefix
via GrantedAuthorityDefaults/rolePrefix and apply that; verify JwtAuthentication
receives authorities in the same format used by SecurityFilterChainConfig so
hasAnyRole checks succeed.
- Around line 43-58: PassportAuthenticationFilter currently decodes
X-User-Passport (PASSPORT_HEADER) and directly creates JwtAuthentication without
any integrity/signature binding, allowing header injection to impersonate users;
update PassportAuthenticationFilter to validate the passport before trusting it
— either verify a signature/HMAC on the passport payload or bind it to an
existing, validated token (e.g., require and verify a tunnel JWT/HMAC produced
by the gateway) and only then extract memberId/roles to construct
JwtAuthentication; additionally ensure AccessTokenFilter’s skip logic
(eeos.securityChainProcessed) still enforces that only validated passports are
accepted and add logging/audit for rejected/invalid passports so unauthorized
header injection is detectable.
In
`@eeos/src/main/resources/db/migration/V1.00.0.8__drop_oauth_client_tables.sql`:
- Around line 4-8: Remove the redundant ALTER TABLE ... DROP FOREIGN KEY
statement: delete the lines that execute "ALTER TABLE oauth_client_redirect_uri
DROP FOREIGN KEY fk_redirect_uri_client;" and leave only the DROP TABLE IF
EXISTS statements for oauth_client_redirect_uri and oauth_client; ensure no
other references to fk_redirect_uri_client remain in this migration so the
migration runs safely when the child table is already absent.
---
Outside diff comments:
In `@eeos/docs/auth_README.md`:
- Around line 128-207: The docs section describing POST /api/v2/auth/clients and
related classes (ClientController, ClientService, ClientEntity, ClientType,
InvalidRedirectUriException) is stale because that flow was removed in the PR;
update eeos/docs/auth_README.md by either deleting the entire "OAuth 클라이언트 등록
(ADMIN 전용)" block or clearly marking it as removed/disabled (e.g., add a short
header "Removed — not implemented" and note the removed symbols:
/api/v2/auth/clients, ClientController, ClientService, ClientEntity, ClientType)
so the operational guide matches the current implementation.
---
Nitpick comments:
In
`@eeos/src/main/java/com/blackcompany/eeos/config/security/AccessTokenFilter.java`:
- Around line 38-56: Extract the literal "eeos.securityChainProcessed" into a
shared public constant (e.g., public static final String
EEOS_SECURITY_CHAIN_PROCESSED = "eeos.securityChainProcessed") in a common class
(e.g., SecurityConstants) and replace the three literal usages in
AccessTokenFilter (the request.setAttribute calls and any checks) with
SecurityConstants.EEOS_SECURITY_CHAIN_PROCESSED; then update
UnknownEndpointFilter to read the same constant (instead of the literal) so both
filters reference the single shared symbol. Ensure the constant is public and
imported where needed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 05a45041-e337-4b87-8d42-1d255e8caae4
📒 Files selected for processing (33)
eeos/docs/auth_README.mdeeos/src/main/java/com/blackcompany/eeos/auth/application/domain/AuthorizationCodeData.javaeeos/src/main/java/com/blackcompany/eeos/auth/application/domain/ClientType.javaeeos/src/main/java/com/blackcompany/eeos/auth/application/domain/PkceValidator.javaeeos/src/main/java/com/blackcompany/eeos/auth/application/dto/converter/TokenResponseConverter.javaeeos/src/main/java/com/blackcompany/eeos/auth/application/dto/request/ClientRegistrationRequest.javaeeos/src/main/java/com/blackcompany/eeos/auth/application/dto/response/ClientRegistrationResponse.javaeeos/src/main/java/com/blackcompany/eeos/auth/application/service/ClientService.javaeeos/src/main/java/com/blackcompany/eeos/auth/application/service/OAuth2LoginService.javaeeos/src/main/java/com/blackcompany/eeos/auth/application/service/TokenExchangeService.javaeeos/src/main/java/com/blackcompany/eeos/auth/persistence/AuthorizationCodeRepository.javaeeos/src/main/java/com/blackcompany/eeos/auth/persistence/client/ClientEntity.javaeeos/src/main/java/com/blackcompany/eeos/auth/persistence/client/ClientRedirectUriEntity.javaeeos/src/main/java/com/blackcompany/eeos/auth/persistence/client/ClientRepository.javaeeos/src/main/java/com/blackcompany/eeos/auth/presentation/controller/ClientController.javaeeos/src/main/java/com/blackcompany/eeos/auth/presentation/controller/OAuth2Controller.javaeeos/src/main/java/com/blackcompany/eeos/auth/presentation/docs/ClientApi.javaeeos/src/main/java/com/blackcompany/eeos/auth/presentation/docs/OAuth2Api.javaeeos/src/main/java/com/blackcompany/eeos/auth/presentation/support/MemberArgumentResolver.javaeeos/src/main/java/com/blackcompany/eeos/common/exception/GlobalExceptionHandler.javaeeos/src/main/java/com/blackcompany/eeos/config/security/AccessTokenFilter.javaeeos/src/main/java/com/blackcompany/eeos/config/security/InternalApiKeyFilter.javaeeos/src/main/java/com/blackcompany/eeos/config/security/PassportAuthenticationFilter.javaeeos/src/main/java/com/blackcompany/eeos/config/security/SecurityFilterChainConfig.javaeeos/src/main/java/com/blackcompany/eeos/config/security/UnknownEndpointFilter.javaeeos/src/main/resources/db/migration/V1.00.0.8__drop_oauth_client_tables.sqleeos/src/test/java/com/blackcompany/eeos/auth/application/domain/ClientTypeTest.javaeeos/src/test/java/com/blackcompany/eeos/auth/application/domain/PkceValidatorTest.javaeeos/src/test/java/com/blackcompany/eeos/auth/application/domain/token/TokenProviderTest.javaeeos/src/test/java/com/blackcompany/eeos/auth/application/service/ClientServiceTest.javaeeos/src/test/java/com/blackcompany/eeos/auth/application/service/OAuth2LoginServiceTest.javaeeos/src/test/java/com/blackcompany/eeos/auth/application/service/TokenExchangeServiceTest.javaeeos/src/test/java/com/blackcompany/eeos/auth/presentation/controller/OAuth2ControllerTest.java
💤 Files with no reviewable changes (23)
- eeos/src/main/java/com/blackcompany/eeos/auth/application/domain/ClientType.java
- eeos/src/main/java/com/blackcompany/eeos/auth/presentation/docs/ClientApi.java
- eeos/src/main/java/com/blackcompany/eeos/auth/application/service/TokenExchangeService.java
- eeos/src/test/java/com/blackcompany/eeos/auth/application/domain/token/TokenProviderTest.java
- eeos/src/main/java/com/blackcompany/eeos/auth/presentation/controller/OAuth2Controller.java
- eeos/src/main/java/com/blackcompany/eeos/auth/application/service/OAuth2LoginService.java
- eeos/src/main/java/com/blackcompany/eeos/auth/application/domain/AuthorizationCodeData.java
- eeos/src/main/java/com/blackcompany/eeos/auth/presentation/controller/ClientController.java
- eeos/src/main/java/com/blackcompany/eeos/auth/persistence/client/ClientRepository.java
- eeos/src/main/java/com/blackcompany/eeos/auth/application/service/ClientService.java
- eeos/src/main/java/com/blackcompany/eeos/auth/persistence/client/ClientEntity.java
- eeos/src/test/java/com/blackcompany/eeos/auth/application/service/TokenExchangeServiceTest.java
- eeos/src/main/java/com/blackcompany/eeos/auth/persistence/client/ClientRedirectUriEntity.java
- eeos/src/main/java/com/blackcompany/eeos/auth/persistence/AuthorizationCodeRepository.java
- eeos/src/main/java/com/blackcompany/eeos/auth/application/dto/request/ClientRegistrationRequest.java
- eeos/src/main/java/com/blackcompany/eeos/auth/presentation/docs/OAuth2Api.java
- eeos/src/test/java/com/blackcompany/eeos/auth/application/service/OAuth2LoginServiceTest.java
- eeos/src/test/java/com/blackcompany/eeos/auth/application/domain/PkceValidatorTest.java
- eeos/src/main/java/com/blackcompany/eeos/auth/application/domain/PkceValidator.java
- eeos/src/main/java/com/blackcompany/eeos/auth/application/dto/response/ClientRegistrationResponse.java
- eeos/src/test/java/com/blackcompany/eeos/auth/application/service/ClientServiceTest.java
- eeos/src/test/java/com/blackcompany/eeos/auth/presentation/controller/OAuth2ControllerTest.java
- eeos/src/test/java/com/blackcompany/eeos/auth/application/domain/ClientTypeTest.java
| "code": "CREATE", | ||
| "data": { | ||
| "clientId": "a3f7c2d1-85b4-4e9a-bf32-1c0e7d9fa821", | ||
| "clientSecret": "xKz3Qp9mRvLs7wNt2YhJ4dUiOeAn0BfCgXvPqWmE5c" |
There was a problem hiding this comment.
문서 예시에 시크릿 형태 값 노출을 피해주세요.
Line 175의 clientSecret 값은 실제 키 여부와 무관하게 비밀정보 탐지 규칙을 트리거할 수 있습니다. 플레이스홀더(예: <issued-once-secret>)로 바꾸는 편이 안전합니다.
🔧 제안 변경
- "clientSecret": "xKz3Qp9mRvLs7wNt2YhJ4dUiOeAn0BfCgXvPqWmE5c"
+ "clientSecret": "<issued-once-secret>"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "clientSecret": "xKz3Qp9mRvLs7wNt2YhJ4dUiOeAn0BfCgXvPqWmE5c" | |
| "clientSecret": "<issued-once-secret>" |
🧰 Tools
🪛 Betterleaks (1.3.1)
[high] 175-175: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🤖 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/docs/auth_README.md` at line 175, 문서에 노출된 민감한 값인 "clientSecret"의 실제
형태(예: "xKz3Qp9mRvLs7wNt2YhJ4dUiOeAn0BfCgXvPqWmE5c")를 플레이스홀더로 교체하세요: 해당 예제의
"clientSecret" 값을 실제 키처럼 보이는 문자열에서 "<issued-once-secret>" 또는
"<your-client-secret>" 같은 안전한 플레이스홀더로 바꾸고, 동일한 파일 내 유사한 예제(또는 "clientSecret"을
사용하는 다른 코드 블록)가 있으면 동일하게 대체해 보안 스캐너 트리거를 방지하십시오.
Comment on lines
+90
to
+96
| /** 잘못된 인자값 예외 — enum 변환 실패 등 */ | ||
| @ExceptionHandler(IllegalArgumentException.class) | ||
| protected ApiResponse<FailureBody> handleIllegalArgumentException(IllegalArgumentException e) { | ||
| log.warn("IllegalArgumentException", e); | ||
| String code = String.valueOf(HttpStatus.BAD_REQUEST.value()); | ||
| return ApiResponseGenerator.fail(e.getMessage(), code, HttpStatus.BAD_REQUEST); | ||
| } |
There was a problem hiding this comment.
IllegalArgumentException 전역 400 매핑은 오류 분류를 왜곡할 수 있습니다.
Line 91-95처럼 모든 IllegalArgumentException을 400으로 처리하면, 내부 로직 버그까지 클라이언트 잘못으로 내려가 장애 신호가 가려집니다. 입력 검증 계열 예외(커스텀 BadRequest 예외 등)로 범위를 좁히고, 그 외 IllegalArgumentException은 기존 500 경로로 두는 편이 안전합니다.
🤖 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/common/exception/GlobalExceptionHandler.java`
around lines 90 - 96, The current GlobalExceptionHandler method
handleIllegalArgumentException maps all IllegalArgumentException to 400; change
this to only handle explicit input-validation exceptions (e.g., a custom
BadRequestException) by renaming/replacing the handler to target that custom
exception (e.g., `@ExceptionHandler`(BadRequestException.class) in
GlobalExceptionHandler) and update the method signature accordingly to still use
ApiResponseGenerator.fail; remove or stop handling raw IllegalArgumentException
so it falls through to the existing 500 error path (or let the generic exception
handler handle it) to avoid masking internal bugs.
Comment on lines
+43
to
+58
| String passportHeader = request.getHeader(PASSPORT_HEADER); | ||
| if (passportHeader != null && !passportHeader.isBlank()) { | ||
| try { | ||
| byte[] decoded = Base64.getDecoder().decode(passportHeader); | ||
| @SuppressWarnings("unchecked") | ||
| Map<String, Object> claims = mapper.readValue(decoded, Map.class); | ||
|
|
||
| Long memberId = toLong(claims.get("memberId")); | ||
| if (memberId != null) { | ||
| List<String> roles = toStringList(claims.get("roles")); | ||
| JwtAuthentication auth = | ||
| new JwtAuthentication( | ||
| memberId, roles.stream().map(SimpleGrantedAuthority::new).toList()); | ||
| SecurityContextHolder.getContext().setAuthentication(auth); | ||
| log.debug("Passport 인증 설정: memberId={}", memberId); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# X-User-Passport 및 처리 마킹 참조 전부 확인
rg -nP 'X-User-Passport|securityChainProcessed'
# 신뢰 프록시/포워딩/Gateway 출처 검증 로직 존재 여부 확인
rg -nPi 'trusted|forwarded-for|x-forwarded|remoteaddr|gateway|mtls' -g '*.java' -g '*.yml' -g '*.yaml' -g '*.properties'Repository: JNU-econovation/EEOS-BE
Length of output: 2169
X-User-Passport를 서명/무결성 검증 없이 신뢰하는 구조라 인증 우회·권한상승 위험이 큼
PassportAuthenticationFilter가 X-User-Passport(base64(JSON))의 memberId/roles를 그대로 디코딩해 SecurityContextHolder에 JwtAuthentication을 세팅하지만, 코드에서 서명/원본 검증 로직이 보이지 않습니다. 또한 AccessTokenFilter는 “Passport 필터가 이미 인증을 설정했으면 스킵”하도록 되어 있어(그리고 eeos.securityChainProcessed 마킹 사용), 공격자가 인바운드로 임의 X-User-Passport를 주입할 수 있는 경로가 존재하면 HMAC/JWT 검증을 완전히 우회할 수 있습니다.
X-User-Passport의 인바운드(strip/overwrite) 보장(게이트웨이 외부에서 주입 불가)이 코드/인프라 수준에서 확실한지 확인 필요- 또는
X-User-Passport에 대해 **서명 검증(또는 기존 HMAC JWT와의 바인딩)**을 추가해 신뢰 모델을 강화 필요 - 최소한 “Gateway → BearerToPassportFilter → X-User-Passport 헤더 주입” 전제가 실제로 강제되는지(게이트웨이 우회 불가/네트워크 격리, 신뢰 경로 mTLS 등) 확인 필요
🤖 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/config/security/PassportAuthenticationFilter.java`
around lines 43 - 58, PassportAuthenticationFilter currently decodes
X-User-Passport (PASSPORT_HEADER) and directly creates JwtAuthentication without
any integrity/signature binding, allowing header injection to impersonate users;
update PassportAuthenticationFilter to validate the passport before trusting it
— either verify a signature/HMAC on the passport payload or bind it to an
existing, validated token (e.g., require and verify a tunnel JWT/HMAC produced
by the gateway) and only then extract memberId/roles to construct
JwtAuthentication; additionally ensure AccessTokenFilter’s skip logic
(eeos.securityChainProcessed) still enforces that only validated passports are
accepted and add logging/audit for rejected/invalid passports so unauthorized
header injection is detectable.
Comment on lines
+4
to
+8
| ALTER TABLE oauth_client_redirect_uri | ||
| DROP FOREIGN KEY fk_redirect_uri_client; | ||
|
|
||
| DROP TABLE IF EXISTS oauth_client_redirect_uri; | ||
| DROP TABLE IF EXISTS oauth_client; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does MySQL automatically drop foreign key constraints when dropping the child table, and does MySQL support DROP FOREIGN KEY IF EXISTS?
💡 Result:
Yes, when you drop a table in MySQL, all foreign key constraints defined on that table are automatically dropped [1][2]. Because the table itself is being removed, any constraints that were tied to its structure are also removed [1][2]. However, MySQL does not support the syntax DROP FOREIGN KEY IF EXISTS [1][2][3]. If you attempt to use this syntax, MySQL will return an error [4]. To drop a foreign key safely, you must either know the specific constraint name or verify its existence beforehand. You can find the constraint name by using the SHOW CREATE TABLE statement [1][5]. If you need to drop a foreign key conditionally to avoid errors in scripts, you can use a stored procedure or an IF statement within a script to check for the constraint's existence in the information_schema.table_constraints table before executing the ALTER TABLE DROP FOREIGN KEY command [4].
Citations:
- 1: https://dev.mysql.com/doc/refman/8.4/en/create-table-foreign-keys.html
- 2: http://dev.mysql.com/doc/refman/9.2/en/create-table-foreign-keys.html
- 3: https://dev.mysql.com/doc/refman/5.7/en/create-table-foreign-keys.html
- 4: https://stackoverflow.com/questions/17161496/drop-foreign-key-only-if-it-exists
- 5: https://dev.mysql.com/doc/refman/9.7/en/create-table-foreign-keys.html
중복 ALTER TABLE ... DROP FOREIGN KEY 제거 권장
MySQL은 테이블을 DROP하면 그 테이블에 정의된 FK 제약도 자동으로 함께 제거하므로(자식 테이블을 DROP TABLE IF EXISTS oauth_client_redirect_uri;로 이미 제거), 라인 4-5의 FK 제거 구문은 중복입니다. 또한 MySQL은 DROP FOREIGN KEY IF EXISTS 문법을 지원하지 않아, 자식 테이블이 없는 환경에서는 이 ALTER가 마이그레이션을 실패시킬 수 있어 제거로 방어성과 일관성을 맞추는 게 좋습니다.
♻️ 제안: 중복 ALTER 구문 제거
-ALTER TABLE oauth_client_redirect_uri
- DROP FOREIGN KEY fk_redirect_uri_client;
-
DROP TABLE IF EXISTS oauth_client_redirect_uri;
DROP TABLE IF EXISTS oauth_client;🤖 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/resources/db/migration/V1.00.0.8__drop_oauth_client_tables.sql`
around lines 4 - 8, Remove the redundant ALTER TABLE ... DROP FOREIGN KEY
statement: delete the lines that execute "ALTER TABLE oauth_client_redirect_uri
DROP FOREIGN KEY fk_redirect_uri_client;" and leave only the DROP TABLE IF
EXISTS statements for oauth_client_redirect_uri and oauth_client; ensure no
other references to fk_redirect_uri_client remain in this migration so the
migration runs safely when the child table is already absent.
PassportAuthenticationFilterTest (신규, 13개): - 유효한 X-User-Passport 헤더 → JwtAuthentication 설정 (memberId, roles, String memberId 파싱, 다중 roles) - 헤더 없음 / 빈 문자열 → SecurityContext 미설정, 체인 계속 - Base64 디코딩 실패 / JSON 형식 오류 / memberId 없음 → 예외 미발생, 체인 계속 - PassportFilter → AccessTokenFilter 연동 검증 AccessTokenFilterTest (기존 3개 + 1개 추가): - PassportFilter가 JwtAuthentication 설정 시 AccessTokenFilter 스킵 검증 (headerExtractor.extract() 호출 없이 기존 인증 유지) Co-Authored-By: Claude Sonnet 4.6
There was a problem hiding this comment.
🧹 Nitpick comments (2)
eeos/src/test/java/com/blackcompany/eeos/config/security/AccessTokenFilterTest.java (1)
95-99: ⚡ Quick winimport 문 추가를 고려하세요.
Line 99에서
SimpleGrantedAuthority의 전체 경로(fully qualified name)를 사용하고 있습니다. 파일 상단에 import를 추가하면 가독성이 향상됩니다.♻️ import 추가 제안
파일 상단에 import 추가:
import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.security.core.authority.SimpleGrantedAuthority;그런 다음:
JwtAuthentication passportAuth = new JwtAuthentication( 42L, - List.of( - new org.springframework.security.core.authority.SimpleGrantedAuthority("USER"))); + List.of(new SimpleGrantedAuthority("USER")));🤖 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/config/security/AccessTokenFilterTest.java` around lines 95 - 99, The test uses the fully-qualified class name org.springframework.security.core.authority.SimpleGrantedAuthority in the JwtAuthentication construction which reduces readability; add an import for SimpleGrantedAuthority at the top of AccessTokenFilterTest and then replace the fully-qualified usage in the JwtAuthentication instantiation with SimpleGrantedAuthority to simplify the code and improve clarity (refer to the JwtAuthentication creation block).eeos/src/test/java/com/blackcompany/eeos/config/security/PassportAuthenticationFilterTest.java (1)
185-207: ⚡ Quick win통합 테스트가 실제로 AccessTokenFilter를 호출하지 않습니다.
이 테스트는 이름과 주석에서 PassportFilter와 AccessTokenFilter의 연동을 검증한다고 명시하지만, 실제로는
PassportAuthenticationFilter만 호출하고AccessTokenFilter는 전혀 실행하지 않습니다.
- Line 195: PassportFilter만 호출
- Lines 202-206: PassportFilter가 설정한 인증을 다시 확인할 뿐 AccessTokenFilter의 스킵 동작은 검증하지 않음
- Line 203 주석: "AccessTokenFilter의 instanceof JwtAuthentication 분기가 동작함을 간접 검증"이라고 하지만 AccessTokenFilter가 실행되지 않아 부정확함
실제 통합 동작은
AccessTokenFilterTest.java의skip_token_extraction_when_passport_already_authenticated()테스트에서 이미 검증되고 있습니다.제안:
- 이 테스트를
ValidPassportHeader섹션으로 이동 (실제로는 유효한 passport 파싱만 테스트하므로), 또는- 실제로
AccessTokenFilter를 인스턴스화하고 호출하여 진정한 통합 테스트로 변경♻️ 제안 1: ValidPassportHeader로 이동
이 테스트는 실제로 유효한 passport 처리만 검증하므로
ValidPassportHeader중첩 클래스로 이동하고 이름을 변경:`@Nested` `@DisplayName`("유효한 X-User-Passport 헤더") class ValidPassportHeader { // ... 기존 테스트들 ... + + `@Test` + `@DisplayName`("PassportFilter가 JwtAuthentication을 SecurityContext에 설정") + void passport_filter_sets_authentication_in_context() throws Exception { + String passport = passport(10L, "[\"USER\"]"); + when(request.getHeader(PassportAuthenticationFilter.PASSPORT_HEADER)).thenReturn(passport); + filter.doFilterInternal(request, response, filterChain); + + Authentication auth = SecurityContextHolder.getContext().getAuthentication(); + assertThat(auth).isInstanceOf(JwtAuthentication.class); + assertThat(auth.getPrincipal()).isEqualTo(10L); + } } - - `@Nested` - `@DisplayName`("PassportFilter → AccessTokenFilter 연동") - class PassportAccessTokenIntegration { - // 이 섹션 제거 - }🤖 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/config/security/PassportAuthenticationFilterTest.java` around lines 185 - 207, The test access_token_filter_skips_when_passport_already_authenticated in PassportAuthenticationFilterTest only invokes PassportAuthenticationFilter and never executes AccessTokenFilter, so either move/rename this test into the ValidPassportHeader nested class (e.g., rename to valid_passport_parsing_confirms_authentication) to reflect it only verifies passport parsing, or convert it into a real integration test by instantiating and invoking AccessTokenFilter after calling filter.doFilterInternal (or by chaining the filters in the same mock filter chain) and then asserting AccessTokenFilter's skip behavior; update the test method name accordingly and reference PassportAuthenticationFilterTest, access_token_filter_skips_when_passport_already_authenticated, AccessTokenFilter, and the existing AccessTokenFilterTest.skip_token_extraction_when_passport_already_authenticated for guidance.
🤖 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.
Nitpick comments:
In
`@eeos/src/test/java/com/blackcompany/eeos/config/security/AccessTokenFilterTest.java`:
- Around line 95-99: The test uses the fully-qualified class name
org.springframework.security.core.authority.SimpleGrantedAuthority in the
JwtAuthentication construction which reduces readability; add an import for
SimpleGrantedAuthority at the top of AccessTokenFilterTest and then replace the
fully-qualified usage in the JwtAuthentication instantiation with
SimpleGrantedAuthority to simplify the code and improve clarity (refer to the
JwtAuthentication creation block).
In
`@eeos/src/test/java/com/blackcompany/eeos/config/security/PassportAuthenticationFilterTest.java`:
- Around line 185-207: The test
access_token_filter_skips_when_passport_already_authenticated in
PassportAuthenticationFilterTest only invokes PassportAuthenticationFilter and
never executes AccessTokenFilter, so either move/rename this test into the
ValidPassportHeader nested class (e.g., rename to
valid_passport_parsing_confirms_authentication) to reflect it only verifies
passport parsing, or convert it into a real integration test by instantiating
and invoking AccessTokenFilter after calling filter.doFilterInternal (or by
chaining the filters in the same mock filter chain) and then asserting
AccessTokenFilter's skip behavior; update the test method name accordingly and
reference PassportAuthenticationFilterTest,
access_token_filter_skips_when_passport_already_authenticated,
AccessTokenFilter, and the existing
AccessTokenFilterTest.skip_token_extraction_when_passport_already_authenticated
for guidance.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: de23f23a-249d-45d5-b3a0-ba1917fd0220
📒 Files selected for processing (2)
eeos/src/test/java/com/blackcompany/eeos/config/security/AccessTokenFilterTest.javaeeos/src/test/java/com/blackcompany/eeos/config/security/PassportAuthenticationFilterTest.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.
개요
auth-api + api-gateway 인증 인프라 도입에 맞춰 EEOS-BE를 변경합니다.
EEOS 자체 OAuth2 서버를 제거하고, Gateway가 주입하는 X-User-Passport 헤더로만 인증을 처리합니다.
인증 흐름 다이어그램: docs/SEQUENCE-DIAGRAMS.md
관련 PR: auth-common #4
영향 범위
DB 변경 사항
V1.00.0.8__drop_oauth_client_tables.sql—oauth_client,oauth_client_redirect_uri테이블 DROP.이 테이블들은 EEOS가 자체 OAuth2 서버를 운영할 때 클라이언트 앱 등록 정보를 저장하던 테이블입니다.
OAuth2 서버를 제거하고 auth-api로 이관했으므로 더 이상 사용하지 않습니다.
배포 전 필수 확인
scripts/migrate-eeos-members.py로 EEOS MySQL → auth-api PostgreSQL 이관/api/v2/auth/**엔드포인트 제거됨. 해당 경로 사용 여부 점검 필요