Feat/#105 외부 AD API 연동 util 클래스 및 뼈대 제작, aes 암호화 util 클래스 제작#106
Merged
kingmingyu merged 9 commits intodevelopfrom Mar 31, 2026
Merged
Feat/#105 외부 AD API 연동 util 클래스 및 뼈대 제작, aes 암호화 util 클래스 제작#106kingmingyu merged 9 commits intodevelopfrom
kingmingyu merged 9 commits intodevelopfrom
Conversation
3 tasks
Walkthrough외부 광고 플랫폼 인증 전략(Strategy)과 AES 기반 암/복호화 유틸을 도입하고, NAVER 광고용 클라이언트·서비스·컨트롤러 및 관련 예외·에러코드를 추가했습니다. 환경변수(AES_SECRET)와 애플리케이션 설정(aes.secret)도 함께 도입되었습니다. Changes
Sequence DiagramsequenceDiagram
actor Client
participant Controller as NaverAdApiController
participant Service as NaverAdApiService
participant Repo as PlatformConnectionRepository
participant AuthUtil as AdApiAuthUtil
participant Factory as AdAuthFactory
participant Strategy as NaverAdAuthStrategy
participant AES as AESUtil
participant NaverClient as NaverClient(Feign)
Client->>Controller: GET /api/naver/{orgId}/campaigns
Controller->>Service: getCampaigns(orgId)
Service->>Repo: findByPlatformAccount_Organization_IdAndPlatformAccount_Provider(orgId, NAVER)
Repo-->>Service: PlatformConnection (connectionId)
Service->>AuthUtil: generateAuthHeaders(connectionId, AdAuthRequest)
AuthUtil->>Repo: findById(connectionId)
Repo-->>AuthUtil: PlatformConnection
AuthUtil->>Factory: getStrategy(Provider.NAVER)
Factory-->>AuthUtil: NaverAdAuthStrategy
AuthUtil->>Strategy: generateHeaders(connection, request)
Strategy->>AES: decryptAES(encryptedApiKey)
AES-->>Strategy: decrypted bytes
Strategy->>AES: decryptAES(encryptedSecretKey)
AES-->>Strategy: decrypted bytes
Strategy->>Strategy: compute HmacSHA256 signature
Strategy-->>AuthUtil: headers map (X-Timestamp, X-API-KEY, X-Customer, X-Signature)
AuthUtil-->>Service: headers
Service->>NaverClient: getCampaigns(headers)
NaverClient-->>Service: List<Campaign>
Service-->>Controller: List<Campaign>
Controller-->>Client: 200 OK (DataResponse)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 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
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (8)
src/main/java/com/whereyouad/WhereYouAd/infrastructure/client/naver/client/NaverClient.java (1)
11-19: 기본 구현은 좋습니다. 다만, 외부 API 호출에 대한 타임아웃 및 에러 처리 설정을 고려해주세요.Feign 클라이언트가 잘 구성되어 있지만, 외부 API 호출 시 네트워크 지연이나 장애가 발생하면 요청 스레드가 블로킹될 수 있습니다. 타임아웃과 에러 처리를 설정하면 시스템 안정성이 향상됩니다.
♻️ Feign 설정 추가 예시 (application.yml)
feign: client: config: naverClient: connectTimeout: 5000 readTimeout: 10000 loggerLevel: BASIC♻️ 또는 별도 Configuration 클래스 예시
`@Configuration` public class NaverClientConfig { `@Bean` public Request.Options naverClientOptions() { return new Request.Options( 5, TimeUnit.SECONDS, // connectTimeout 10, TimeUnit.SECONDS, // readTimeout true // followRedirects ); } `@Bean` public Retryer naverClientRetryer() { return new Retryer.Default(100, 1000, 3); } }추가로, Naver API에서 오류 응답(4xx, 5xx)이 올 경우를 대비해
ErrorDecoder나FallbackFactory를 구현하면 더 견고한 에러 처리가 가능합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/whereyouad/WhereYouAd/infrastructure/client/naver/client/NaverClient.java` around lines 11 - 19, NaverClient currently lacks timeout and error handling configuration; add a Feign configuration (e.g., class NaverClientConfig) and bind it to the NaverClient to set Request.Options (connectTimeout/readTimeout), a Retryer (retry policy), and logging level, and register either an ErrorDecoder or a FallbackFactory bean to translate 4xx/5xx responses into domain exceptions or fallbacks; reference the interface NaverClient to attach the config and implement Request.Options and Retryer beans plus an ErrorDecoder/FallbackFactory to ensure timeouts and robust error handling for external calls.src/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/repository/PlatformConnectionRepository.java (1)
10-12: N+1 쿼리 발생 가능성 확인이 필요합니다.이 메서드로 조회한
PlatformConnection리스트를 순회하면서platformAccount필드에 접근할 경우, LAZY 로딩으로 인해 각 접근마다 별도 쿼리가 발생할 수 있습니다. 예를 들어, 10개의 Connection을 조회 후 각각getPlatformAccount()를 호출하면 추가로 10개의 SELECT가 실행됩니다.호출부에서
platformAccount접근이 필요하다면@EntityGraph또는JOIN FETCH를 사용하는 것이 좋습니다.♻️ EntityGraph를 활용한 개선 예시
+import org.springframework.data.jpa.repository.EntityGraph; + public interface PlatformConnectionRepository extends JpaRepository<PlatformConnection, Long> { // 조직 ID와 플랫폼으로 등록된 연동 정보(Account/Connection) 목록 조회 + `@EntityGraph`(attributePaths = {"platformAccount"}) List<PlatformConnection> findByPlatformAccount_Organization_IdAndPlatformAccount_Provider(Long orgId, Provider provider); }As per coding guidelines: "JPA 사용 시 N+1 문제나 불필요한 쿼리가 발생하지 않는지, FetchType.LAZY가 잘 적용되었는지 체크하라."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/repository/PlatformConnectionRepository.java` around lines 10 - 12, The repository method findByPlatformAccount_Organization_IdAndPlatformAccount_Provider on PlatformConnection can cause N+1s when callers access the platformAccount association; modify the repository to eagerly fetch platformAccount (and any other needed relations) by either adding an `@EntityGraph` on findByPlatformAccount_Organization_IdAndPlatformAccount_Provider to include "platformAccount" (and nested attributes as needed) or replace it with a `@Query` that uses JOIN FETCH to load platformAccount in the same SQL so iterating connections doesn't trigger additional selects..env.example (1)
43-45: 파일 끝에 빈 줄(newline)이 누락되었습니다.대부분의 텍스트 편집기와 도구에서 파일 끝 빈 줄을 권장합니다. POSIX 표준에서도 텍스트 파일은 newline으로 끝나야 한다고 정의합니다.
🔧 수정 제안
`#10` AES AES_SECRET=your_aes_secret_key +🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.env.example around lines 43 - 45, Add a trailing newline at the end of the .env.example file so the file ends with a newline character; specifically ensure the final line containing AES_SECRET=your_aes_secret_key is followed by one blank newline (POSIX-compliant EOF newline) to satisfy editors and tooling.src/main/java/com/whereyouad/WhereYouAd/global/adapi/AdAuthFactory.java (1)
14-23: 팩토리 패턴과 Spring DI를 잘 활용했습니다! 👍전략 패턴을 Spring의 의존성 주입과 깔끔하게 연결한 좋은 구현입니다. 다만 두 가지 개선 포인트가 있어요:
@Slf4j미사용: 로그 어노테이션이 선언되어 있지만 실제로 사용되지 않습니다. 불필요한 어노테이션은 제거하거나, 전략 조회 시 디버그 로그를 추가하면 좋겠습니다.중복 Provider 방어 로직 부재: 만약 두 개의 전략이 동일한
Provider를 반환하면Collectors.toMap()이IllegalStateException을 던집니다. 명시적인 병합 함수를 추가하거나, 빌드 시점에 검증 로직을 넣으면 더 안전합니다.♻️ 중복 Provider 처리 및 로그 추가 제안
`@Slf4j` `@Component` public class AdAuthFactory { private final Map<Provider, AdAuthStrategy> strategies; public AdAuthFactory(List<AdAuthStrategy> strategyList) { - this.strategies = strategyList.stream() - .collect(Collectors.toMap(AdAuthStrategy::getProvider, Function.identity())); + this.strategies = strategyList.stream() + .collect(Collectors.toMap( + AdAuthStrategy::getProvider, + Function.identity(), + (existing, duplicate) -> { + throw new IllegalStateException( + "중복된 Provider 전략 발견: " + existing.getProvider()); + } + )); + log.info("AdAuthFactory 초기화 완료 - 등록된 전략: {}", strategies.keySet()); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/whereyouad/WhereYouAd/global/adapi/AdAuthFactory.java` around lines 14 - 23, Remove or use the unused `@Slf4j`: either delete the annotation or add a debug log in AdAuthFactory (e.g., inside the constructor) that logs which Provider keys are registered; and harden the strategies map construction in AdAuthFactory(List<AdAuthStrategy> strategyList) to handle duplicate Provider values by providing an explicit merge strategy or by validating duplicates before collecting (e.g., detect duplicate AdAuthStrategy::getProvider and throw a clear IllegalArgumentException), ensuring the Map<Provider, AdAuthStrategy> strategies is built deterministically and with a clear error message when duplicates occur.src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/presentation/docs/NaverAdApiControllerDocs.java (1)
16-25: Swagger 문서화를 인터페이스로 분리한 좋은 패턴입니다! 👍컨트롤러 로직과 API 문서를 분리해서 코드 가독성이 좋아졌어요.
다만, 보안 관련 문서가 누락된 것 같습니다. 이 API가 인증이 필요하다면
@SecurityRequirement와 401/403 응답 코드를 추가하면 Swagger UI에서 더 명확한 가이드를 제공할 수 있어요.🔒 보안 문서화 추가 제안
+import io.swagger.v3.oas.annotations.security.SecurityRequirement; + public interface NaverAdApiControllerDocs { `@Operation`(summary = "네이버 광고 캠페인 목록 조회", description = "연동된 네이버 광고 계정의 캠페인 목록을 조회합니다.") + `@SecurityRequirement`(name = "bearerAuth") `@ApiResponses`({ `@ApiResponse`(responseCode = "200", description = "캠페인 목록 반환"), + `@ApiResponse`(responseCode = "401", description = "인증 실패"), + `@ApiResponse`(responseCode = "403", description = "해당 조직에 대한 접근 권한 없음"), `@ApiResponse`(responseCode = "404", description = "해당 조직의 네이버 연결 정보 없음"), `@ApiResponse`(responseCode = "500", description = "네이버 API 호출 실패") })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/presentation/docs/NaverAdApiControllerDocs.java` around lines 16 - 25, The API docs for NaverAdApiControllerDocs#getCampaigns are missing security metadata; add an appropriate `@SecurityRequirement` (or class-level security annotation) to the NaverAdApiControllerDocs interface and extend the `@ApiResponses` for getCampaigns to include 401 (Unauthorized) and 403 (Forbidden) responses so Swagger shows authentication requirements; update the method-level annotations for getCampaigns to reference the security scheme name used in your OpenAPI config and include the new response entries.src/main/java/com/whereyouad/WhereYouAd/global/utils/AdApiAuthUtil.java (1)
32-35:@NotNull어노테이션이 실제로 동작하려면 추가 설정이 필요해요.
jakarta.validation.constraints.NotNull은 Spring에서 자동 검증되지 않아요. 메서드 레벨 검증을 활성화하려면 클래스에@Validated를 추가하거나, 수동으로 null 체크를 해주셔야 합니다. 현재 상태에서는connectionId나request가 null이면NullPointerException이 발생해요.♻️ 검증 활성화 또는 명시적 null 체크
Option 1:
@Validated추가+import org.springframework.validation.annotation.Validated; `@Slf4j` `@Component` +@Validated `@RequiredArgsConstructor` public class AdApiAuthUtil {Option 2: 명시적 null 체크
public Map<String, String> generateAuthHeaders( - `@NotNull` Long connectionId, - `@NotNull` AdAuthRequest request + Long connectionId, + AdAuthRequest request ) throws GeneralSecurityException { + if (connectionId == null || request == null) { + throw new IllegalArgumentException("connectionId와 request는 null일 수 없습니다."); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/whereyouad/WhereYouAd/global/utils/AdApiAuthUtil.java` around lines 32 - 35, The method AdApiAuthUtil.generateAuthHeaders is annotated with `@NotNull` but method-level validation isn’t enabled; either add Spring’s `@Validated` to the AdApiAuthUtil class to enable automatic parameter validation, or add explicit null checks at the start of generateAuthHeaders (validate connectionId and request and throw a clear IllegalArgumentException/NullPointerException) so nulls are handled deterministically; update the class-level annotations or the beginning of generateAuthHeaders accordingly.src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/domain/service/NaverAdApiService.java (2)
39-42:RuntimeException대신 도메인 특화 예외를 사용하면 더 좋아요.현재 모든 예외를
RuntimeException으로 감싸고 있는데, 프로젝트에 이미AdApiHandler나PlatformHandler같은 커스텀 예외 핸들러가 있다면 이를 활용하는 게 GlobalExceptionHandler와의 일관성 유지에 도움이 됩니다.♻️ 도메인 예외 활용 예시
} catch (Exception e) { log.error("[NAVER] 캠페인 조회 실패 - orgId={}", orgId, e); - throw new RuntimeException("네이버 캠페인 조회 실패", e); + throw new AdApiHandler(AdApiErrorCode.EXTERNAL_API_CALL_FAILED); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/domain/service/NaverAdApiService.java` around lines 39 - 42, The catch block in NaverAdApiService currently wraps all errors in a generic RuntimeException; replace it with a domain-specific exception (e.g., AdApiException or PlatformHandler-specific exception used across the project) in the catch where Naver campaign fetching fails: keep the existing log.error call, construct and throw the project’s custom exception (such as AdApiException or PlatformHandlerException) with a descriptive message like "네이버 캠페인 조회 실패" and pass the caught exception e as the cause so the original stack trace is preserved; update any imports/usages accordingly so GlobalExceptionHandler handles it consistently.
48-55: 중복 연결 존재 시 첫 번째 항목만 조용히 선택하는 문제가 있어요.
findByPlatformAccount_Organization_IdAndPlatformAccount_Provider가List<PlatformConnection>을 반환하는데, 같은 조직에 여러 개의 NAVER 연결이 존재할 경우connections.get(0)으로 첫 번째만 선택합니다. 이런 상황이 비즈니스 로직상 허용되지 않는다면 명시적으로 예외를 던지거나, 의도된 동작이라면 로그를 남겨두는 게 좋겠어요.♻️ 중복 연결 처리 개선안
private PlatformConnection resolveNaverConnection(Long orgId) { List<PlatformConnection> connections = connectionRepository .findByPlatformAccount_Organization_IdAndPlatformAccount_Provider(orgId, Provider.NAVER); if (connections.isEmpty()) { throw new PlatformHandler(PlatformErrorCode.PLATFORM_CONNECTION_NOT_FOUND); } + if (connections.size() > 1) { + log.warn("[NAVER] 조직 ID={}에 여러 개의 연결이 존재합니다. 첫 번째 연결 사용.", orgId); + } return connections.get(0); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/domain/service/NaverAdApiService.java` around lines 48 - 55, The resolveNaverConnection method currently silently returns connections.get(0) when connectionRepository.findByPlatformAccount_Organization_IdAndPlatformAccount_Provider returns multiple PlatformConnection entries; change this to explicitly handle duplicates by detecting connections.size() > 1 and either throwing a clear PlatformHandler exception (e.g., PlatformHandler(PlatformErrorCode.PLATFORM_MULTIPLE_CONNECTIONS) — add this error code if missing) or, if returning the first is intended, add a processLogger.warn call documenting that multiple NAVER connections exist and why the first is chosen; update resolveNaverConnection to perform this check and act accordingly instead of quietly using get(0).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 59-60: The CI AES test key is too short and causes
InvalidKeyException when Cipher.init() is used by auth code paths like
NaverAdAuthStrategy.generateHeaders() and KakaoAdAuthStrategy.generateHeaders();
update the AES_SECRET value in the workflow (the
KAFKA_BOOTSTRAP_SERVERS/AES_SECRET env block) to a valid AES key length (16, 24,
or 32 bytes) such as "test_aes_key_1234" (16 bytes) so runtime encryption
initialization succeeds.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/presentation/NaverAdApiController.java`:
- Around line 20-26: The controller method NaverAdApiController.getCampaigns
currently accepts only `@PathVariable` orgId and lacks requester info—fix by
extracting the authenticated userId via `@AuthenticationPrincipal` (expression =
"userId") and add `@Positive` on orgId, then update the service API by changing
NaverAdApiService.getCampaigns(Long userId, Long orgId) and implement a
membership/permission check inside that service (or in resolveNaverConnection)
to verify the user belongs to the requested org before fetching campaigns;
ensure the controller forwards both userId and orgId to the service.
In
`@src/main/java/com/whereyouad/WhereYouAd/global/adapi/strategy/GoogleAdAuthStrategy.java`:
- Around line 27-39: The method generateHeaders in GoogleAdAuthStrategy
currently decrypts accessToken but returns an empty map; update it to populate
standard Google Ads auth headers: set "Authorization" to "Bearer {accessToken}"
using the decrypted value (from connection.getAccessTokenEnc()), and add
"developer-token" using connection.getDeveloperToken() (or
request.getDeveloperToken() if applicable) when present; ensure you only add
non-null/empty tokens and return the populated headers map from generateHeaders.
In
`@src/main/java/com/whereyouad/WhereYouAd/global/adapi/strategy/KakaoAdAuthStrategy.java`:
- Around line 27-39: KakaoAdAuthStrategy.generateHeaders currently decrypts
accessToken (accessToken via aesUtil.decryptAES(connection.getAccessTokenEnc()))
but returns an empty headers map; replace the dead-code by populating and
returning headers with the required authorization header (e.g., set
"Authorization" to "Bearer " + accessToken) so callers (AdApiAuthUtil) receive
valid credentials; if you prefer a temporary guard instead, throw
UnsupportedOperationException from generateHeaders to prevent accidental use
until full implementation is added.
In
`@src/main/java/com/whereyouad/WhereYouAd/global/adapi/strategy/NaverAdAuthStrategy.java`:
- Around line 44-47: In NaverAdAuthStrategy, prevent NPEs by null-checking
encrypted fields and the related entity before decrypting or accessing account
data: verify connection.getAccessTokenEnc() and connection.getRefreshTokenEnc()
are non-null before calling aesUtil.decryptAES(...) and handle nulls (throw a
clear IllegalStateException or return a controlled error), and verify
connection.getPlatformAccount() is non-null before calling
getExternalAccountId() (again fail-fast with a descriptive exception or
fallback); update the logic around apiKey, secretKey and customerId assignment
to perform these checks and include the field names in the error messages to aid
debugging.
In `@src/main/java/com/whereyouad/WhereYouAd/global/utils/AESUtil.java`:
- Around line 33-34: The code reuses key.getBytes() for both SecretKey
(SecretKey aesKey) and IvParameterSpec iv which is insecure; change encrypt path
to generate a new random 16-byte IV (use SecureRandom) each time, construct
IvParameterSpec from that IV, perform encryption with Cipher using
SecretKeySpec(key.getBytes(), ALGORITHM), then prepend (or otherwise store) the
IV bytes to the ciphertext output; change decrypt path to read/extract the first
16 bytes as the IV, construct IvParameterSpec from those bytes, and use the
remaining bytes as the ciphertext to decrypt with the same SecretKeySpec; update
any other usages (e.g., where IvParameterSpec iv is created) to follow this
pattern and ensure IV length is 16 bytes for AES/CBC.
- Around line 41-56: The Javadoc says the method returns a Base64-encoded STRING
but encryptAES currently returns byte[], so update encryptAES in AESUtil to
return a String: after cipher.doFinal(...), Base64-encode the bytes and return
the resulting UTF-8 String (instead of byte[]), keep the same exception
signature and update Javadoc to match the String return; ensure callers are
updated to expect a String result.
- Around line 20-21: Add a startup-time key length validation in AESUtil: create
a `@PostConstruct` method that checks the injected String key field has a byte
length of 16, 24, or 32 (e.g., key.getBytes(StandardCharsets.UTF_8).length) and
throw an IllegalStateException with a clear message if it doesn't; this ensures
AESUtil (and callers like decryptAES) will fail fast during bean initialization
instead of throwing InvalidKeyException at runtime when cipher.init(...) is
invoked.
---
Nitpick comments:
In @.env.example:
- Around line 43-45: Add a trailing newline at the end of the .env.example file
so the file ends with a newline character; specifically ensure the final line
containing AES_SECRET=your_aes_secret_key is followed by one blank newline
(POSIX-compliant EOF newline) to satisfy editors and tooling.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/domain/service/NaverAdApiService.java`:
- Around line 39-42: The catch block in NaverAdApiService currently wraps all
errors in a generic RuntimeException; replace it with a domain-specific
exception (e.g., AdApiException or PlatformHandler-specific exception used
across the project) in the catch where Naver campaign fetching fails: keep the
existing log.error call, construct and throw the project’s custom exception
(such as AdApiException or PlatformHandlerException) with a descriptive message
like "네이버 캠페인 조회 실패" and pass the caught exception e as the cause so the
original stack trace is preserved; update any imports/usages accordingly so
GlobalExceptionHandler handles it consistently.
- Around line 48-55: The resolveNaverConnection method currently silently
returns connections.get(0) when
connectionRepository.findByPlatformAccount_Organization_IdAndPlatformAccount_Provider
returns multiple PlatformConnection entries; change this to explicitly handle
duplicates by detecting connections.size() > 1 and either throwing a clear
PlatformHandler exception (e.g.,
PlatformHandler(PlatformErrorCode.PLATFORM_MULTIPLE_CONNECTIONS) — add this
error code if missing) or, if returning the first is intended, add a
processLogger.warn call documenting that multiple NAVER connections exist and
why the first is chosen; update resolveNaverConnection to perform this check and
act accordingly instead of quietly using get(0).
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/presentation/docs/NaverAdApiControllerDocs.java`:
- Around line 16-25: The API docs for NaverAdApiControllerDocs#getCampaigns are
missing security metadata; add an appropriate `@SecurityRequirement` (or
class-level security annotation) to the NaverAdApiControllerDocs interface and
extend the `@ApiResponses` for getCampaigns to include 401 (Unauthorized) and 403
(Forbidden) responses so Swagger shows authentication requirements; update the
method-level annotations for getCampaigns to reference the security scheme name
used in your OpenAPI config and include the new response entries.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/repository/PlatformConnectionRepository.java`:
- Around line 10-12: The repository method
findByPlatformAccount_Organization_IdAndPlatformAccount_Provider on
PlatformConnection can cause N+1s when callers access the platformAccount
association; modify the repository to eagerly fetch platformAccount (and any
other needed relations) by either adding an `@EntityGraph` on
findByPlatformAccount_Organization_IdAndPlatformAccount_Provider to include
"platformAccount" (and nested attributes as needed) or replace it with a `@Query`
that uses JOIN FETCH to load platformAccount in the same SQL so iterating
connections doesn't trigger additional selects.
In `@src/main/java/com/whereyouad/WhereYouAd/global/adapi/AdAuthFactory.java`:
- Around line 14-23: Remove or use the unused `@Slf4j`: either delete the
annotation or add a debug log in AdAuthFactory (e.g., inside the constructor)
that logs which Provider keys are registered; and harden the strategies map
construction in AdAuthFactory(List<AdAuthStrategy> strategyList) to handle
duplicate Provider values by providing an explicit merge strategy or by
validating duplicates before collecting (e.g., detect duplicate
AdAuthStrategy::getProvider and throw a clear IllegalArgumentException),
ensuring the Map<Provider, AdAuthStrategy> strategies is built deterministically
and with a clear error message when duplicates occur.
In `@src/main/java/com/whereyouad/WhereYouAd/global/utils/AdApiAuthUtil.java`:
- Around line 32-35: The method AdApiAuthUtil.generateAuthHeaders is annotated
with `@NotNull` but method-level validation isn’t enabled; either add Spring’s
`@Validated` to the AdApiAuthUtil class to enable automatic parameter validation,
or add explicit null checks at the start of generateAuthHeaders (validate
connectionId and request and throw a clear
IllegalArgumentException/NullPointerException) so nulls are handled
deterministically; update the class-level annotations or the beginning of
generateAuthHeaders accordingly.
In
`@src/main/java/com/whereyouad/WhereYouAd/infrastructure/client/naver/client/NaverClient.java`:
- Around line 11-19: NaverClient currently lacks timeout and error handling
configuration; add a Feign configuration (e.g., class NaverClientConfig) and
bind it to the NaverClient to set Request.Options (connectTimeout/readTimeout),
a Retryer (retry policy), and logging level, and register either an ErrorDecoder
or a FallbackFactory bean to translate 4xx/5xx responses into domain exceptions
or fallbacks; reference the interface NaverClient to attach the config and
implement Request.Options and Retryer beans plus an ErrorDecoder/FallbackFactory
to ensure timeouts and robust error handling for external calls.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cddac991-9976-4dc4-ba4f-9f8aa6d2f3ed
📒 Files selected for processing (23)
.env.example.github/workflows/ci.ymlsrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/domain/service/NaverAdApiService.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/presentation/NaverAdApiController.javasrc/main/java/com/whereyouad/WhereYouAd/domains/advertisement/presentation/docs/NaverAdApiControllerDocs.javasrc/main/java/com/whereyouad/WhereYouAd/domains/platform/exception/.gitkeepsrc/main/java/com/whereyouad/WhereYouAd/domains/platform/exception/PlatformHandler.javasrc/main/java/com/whereyouad/WhereYouAd/domains/platform/exception/code/.gitkeepsrc/main/java/com/whereyouad/WhereYouAd/domains/platform/exception/code/PlatformErrorCode.javasrc/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/repository/PlatformConnectionRepository.javasrc/main/java/com/whereyouad/WhereYouAd/global/adapi/AdAuthFactory.javasrc/main/java/com/whereyouad/WhereYouAd/global/adapi/AdAuthStrategy.javasrc/main/java/com/whereyouad/WhereYouAd/global/adapi/dto/AdAuthRequest.javasrc/main/java/com/whereyouad/WhereYouAd/global/adapi/exception/AdApiHandler.javasrc/main/java/com/whereyouad/WhereYouAd/global/adapi/exception/code/AdApiErrorCode.javasrc/main/java/com/whereyouad/WhereYouAd/global/adapi/strategy/GoogleAdAuthStrategy.javasrc/main/java/com/whereyouad/WhereYouAd/global/adapi/strategy/KakaoAdAuthStrategy.javasrc/main/java/com/whereyouad/WhereYouAd/global/adapi/strategy/NaverAdAuthStrategy.javasrc/main/java/com/whereyouad/WhereYouAd/global/utils/AESUtil.javasrc/main/java/com/whereyouad/WhereYouAd/global/utils/AdApiAuthUtil.javasrc/main/java/com/whereyouad/WhereYouAd/infrastructure/client/naver/client/NaverClient.javasrc/main/java/com/whereyouad/WhereYouAd/infrastructure/client/naver/dto/NaverDTO.javasrc/main/resources/application.yml
Comment on lines
+59
to
+60
| KAFKA_BOOTSTRAP_SERVERS: localhost:9092 | ||
| AES_SECRET: test_aes_key No newline at end of file |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# AESUtil 클래스에서 키 길이 검증 로직이 있는지 확인
ast-grep --pattern $'class AESUtil {
$$$
}'
# 또는 키 관련 로직 검색
rg -n "secret|key" --type java -g '*AESUtil*'Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 816
🏁 Script executed:
cat -n src/main/java/com/whereyouad/WhereYouAd/global/utils/AESUtil.javaRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 2350
🏁 Script executed:
# ci.yml에서 env 변수가 어떻게 사용되는지 확인
cat -n .github/workflows/ci.yml | head -80
# test 환경의 application properties 확인
find . -name "application-test.yml" -o -name "application-test.properties" -o -name "application-test.yaml" | xargs cat 2>/dev/nullRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 2187
🏁 Script executed:
# AESUtil을 사용하는 테스트 파일 찾기
rg -l "AESUtil" --type java -g "**/*Test.java"
# 또는 test 디렉토리에서 AES 관련 테스트 검색
find . -path "*/test/*" -name "*.java" | xargs rg -l "AES|aes" 2>/dev/nullRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 223
🏁 Script executed:
# 테스트 디렉토리 구조 확인
find . -type d -name "test" -o -type d -name "tests" | head -5
# 테스트 파일 찾기
find . -path "*/src/test/*" -name "*.java" | head -20
# AESUtil 사용 여부 확인
rg "AESUtil" --type java 2>/dev/null | grep -i testRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 157
🏁 Script executed:
# 테스트 파일 내용 확인
cat ./src/test/java/com/whereyouad/WhereYouAd/WhereYouAdApplicationTests.java
# 다른 test 파일이 있는지 확인
find ./src/test -name "*.java" -type f
# AESUtil이 어디서 사용되는지 확인
rg "AESUtil" --type javaRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 1250
🏁 Script executed:
# 전략 클래스들이 AESUtil을 어떻게 사용하는지 확인
cat src/main/java/com/whereyouad/WhereYouAd/global/adapi/strategy/NaverAdAuthStrategy.java
cat src/main/java/com/whereyouad/WhereYouAd/global/adapi/strategy/KakaoAdAuthStrategy.java
cat src/main/java/com/whereyouad/WhereYouAd/global/adapi/strategy/GoogleAdAuthStrategy.javaRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 5149
AES 테스트 키 길이를 반드시 수정하세요 — 런타임 오류가 발생합니다.
test_aes_key는 12바이트인데, AES는 키 길이가 16(AES-128), 24(AES-192), 또는 32(AES-256) 바이트여야 합니다. 현재 코드는 Cipher.init()할 때 InvalidKeyException을 발생시킵니다.
현재 CI 빌드는 성공하지만, 그 이유는 테스트가 단순히 Spring 컨텍스트만 로드하기 때문입니다. 실제로 NaverAdAuthStrategy.generateHeaders()나 KakaoAdAuthStrategy.generateHeaders() 같은 메서드가 호출되는 순간 런타임 오류가 발생합니다.
해결책: test_aes_key를 최소 16바이트 이상의 문자열로 변경하세요. 예: test_aes_key_1234 (16바이트)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ci.yml around lines 59 - 60, The CI AES test key is too
short and causes InvalidKeyException when Cipher.init() is used by auth code
paths like NaverAdAuthStrategy.generateHeaders() and
KakaoAdAuthStrategy.generateHeaders(); update the AES_SECRET value in the
workflow (the KAFKA_BOOTSTRAP_SERVERS/AES_SECRET env block) to a valid AES key
length (16, 24, or 32 bytes) such as "test_aes_key_1234" (16 bytes) so runtime
encryption initialization succeeds.
Comment on lines
+20
to
+26
| // 캠페인 목록 조회 | ||
| @GetMapping("/campaigns") | ||
| public ResponseEntity<DataResponse<List<NaverDTO.Campaign>>> getCampaigns( | ||
| @PathVariable Long orgId | ||
| ) { | ||
| return ResponseEntity.ok(DataResponse.from(naverAdApiService.getCampaigns(orgId))); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: NaverAdApiService에서 권한 검증 로직이 있는지 확인
# 1. NaverAdApiService의 getCampaigns 메서드 구현 확인
ast-grep --pattern $'class NaverAdApiService {
$$$
getCampaigns($_) {
$$$
}
$$$
}'
# 2. 조직 멤버십 검증 관련 코드 검색
rg -n "orgId|organization|member|access|permission" --type java -g '*Service.java' -C 3Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 20728
🏁 Script executed:
#!/bin/bash
# 1. NaverAdApiController 전체 구현 확인
echo "=== NaverAdApiController implementation ==="
find . -name "NaverAdApiController.java" -type f | head -1 | xargs cat
# 2. `@PreAuthorize` 또는 `@Secured` 어노테이션 사용 패턴 찾기
echo -e "\n=== Authorization patterns in controllers ==="
rg "@PreAuthorize|@Secured" --type java -B 2 -A 2 | head -50
# 3. SecurityConfig 또는 보안 설정 찾기
echo -e "\n=== Security configuration files ==="
find . -name "*SecurityConfig*" -o -name "*Security*Config*" | grep -v targetRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 1317
🏁 Script executed:
#!/bin/bash
# 1. SecurityConfig 전체 확인
echo "=== SecurityConfig.java ==="
cat src/main/java/com/whereyouad/WhereYouAd/global/security/SecurityConfig.java
# 2. CurrentUser, AuthenticationPrincipal 등의 패턴 찾기
echo -e "\n=== Authentication/User extraction patterns ==="
rg "@AuthenticationPrincipal|@CurrentUser|SecurityContextHolder" --type java -B 1 -A 1 | head -30
# 3. 조직 멤버십 검증 로직 찾기
echo -e "\n=== Organization membership validation patterns ==="
rg "isMember|hasAccess|authorized|permission" --type java -i -C 2 | head -40Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 13385
🔐 orgId에 대한 권한 검증이 필수입니다
현재 엔드포인트는 인증된 사용자가 경로의 orgId를 임의로 변경하면 자신이 속하지 않은 조직의 Naver 캠페인 데이터에 접근할 수 있습니다. 이는 IDOR(Insecure Direct Object Reference) 취약점입니다.
코드베이스의 다른 API들(ProjectController, DashboardService 등)은 모두 @AuthenticationPrincipal로 현재 사용자 정보를 추출한 후, userId와 orgId를 함께 검증합니다. 하지만 이 엔드포인트는 이러한 패턴을 따르지 않습니다:
- NaverAdApiController:
userId추출 없음 → 누가 요청했는지 알 수 없음 - NaverAdApiService.getCampaigns():
orgId파라미터만 있음 → 사용자-조직 관계 검증 불가능 - resolveNaverConnection(): 단순히 해당
orgId의 연결이 존재하는지만 확인 → 사용자 권한 검증 없음
필수 개선:
-
사용자 정보 추출 및 권한 검증:
`@GetMapping`("/campaigns") public ResponseEntity<DataResponse<List<NaverDTO.Campaign>>> getCampaigns( `@AuthenticationPrincipal`(expression = "userId") Long userId, `@PathVariable` `@Positive` Long orgId ) { return ResponseEntity.ok(DataResponse.from( naverAdApiService.getCampaigns(userId, orgId) )); }
-
서비스 레이어에서 권한 검증:
NaverAdApiService.getCampaigns()의 시그니처를getCampaigns(Long userId, Long orgId)로 변경하고, 서비스 내에서 사용자가 해당 조직의 멤버인지 확인합니다. -
입력 검증:
@Positive어노테이션으로 음수orgId방지
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/advertisement/presentation/NaverAdApiController.java`
around lines 20 - 26, The controller method NaverAdApiController.getCampaigns
currently accepts only `@PathVariable` orgId and lacks requester info—fix by
extracting the authenticated userId via `@AuthenticationPrincipal` (expression =
"userId") and add `@Positive` on orgId, then update the service API by changing
NaverAdApiService.getCampaigns(Long userId, Long orgId) and implement a
membership/permission check inside that service (or in resolveNaverConnection)
to verify the user belongs to the requested org before fetching campaigns;
ensure the controller forwards both userId and orgId to the service.
Comment on lines
+27
to
+39
| @Override | ||
| public Map<String, String> generateHeaders(PlatformConnection connection, AdAuthRequest request) throws GeneralSecurityException { | ||
| Map<String, String> headers = new HashMap<>(); | ||
|
|
||
| // 1. 암호화된 액세스 토큰 복호화 | ||
| String accessToken = new String(aesUtil.decryptAES(connection.getAccessTokenEnc()), StandardCharsets.UTF_8); | ||
|
|
||
| // 헤더 생성 로직 추가 | ||
|
|
||
|
|
||
|
|
||
| return headers; | ||
| } |
There was a problem hiding this comment.
KakaoAdAuthStrategy와 마찬가지로, 복호화된 accessToken이 사용되지 않고 빈 헤더 맵을 반환합니다.
Google Ads API는 일반적으로 OAuth 2.0 Bearer 토큰을 사용하므로, 최소한 다음과 같은 헤더가 필요합니다:
Authorization: Bearer {accessToken}developer-token: {developerToken}(Google Ads 특정)
현재 상태로는 Google 광고 API 호출 시 인증 실패가 발생합니다.
🛠️ 임시 방어 로직 또는 기본 구현 제안
`@Override`
public Map<String, String> generateHeaders(PlatformConnection connection, AdAuthRequest request) throws GeneralSecurityException {
- Map<String, String> headers = new HashMap<>();
-
- // 1. 암호화된 액세스 토큰 복호화
- String accessToken = new String(aesUtil.decryptAES(connection.getAccessTokenEnc()), StandardCharsets.UTF_8);
-
- // 헤더 생성 로직 추가
-
-
-
- return headers;
+ // TODO: Google Ads API 인증 헤더 완성 필요 (developer-token 등)
+ throw new UnsupportedOperationException("Google 인증 헤더 생성이 아직 구현되지 않았습니다.");
}또는 기본 Bearer 토큰만 우선 구현:
`@Override`
public Map<String, String> generateHeaders(PlatformConnection connection, AdAuthRequest request) throws GeneralSecurityException {
Map<String, String> headers = new HashMap<>();
// 1. 암호화된 액세스 토큰 복호화
String accessToken = new String(aesUtil.decryptAES(connection.getAccessTokenEnc()), StandardCharsets.UTF_8);
- // 헤더 생성 로직 추가
+ // 2. Authorization 헤더 설정
+ headers.put("Authorization", "Bearer " + accessToken);
return headers;
}📝 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
| @Override | |
| public Map<String, String> generateHeaders(PlatformConnection connection, AdAuthRequest request) throws GeneralSecurityException { | |
| Map<String, String> headers = new HashMap<>(); | |
| // 1. 암호화된 액세스 토큰 복호화 | |
| String accessToken = new String(aesUtil.decryptAES(connection.getAccessTokenEnc()), StandardCharsets.UTF_8); | |
| // 헤더 생성 로직 추가 | |
| return headers; | |
| } | |
| `@Override` | |
| public Map<String, String> generateHeaders(PlatformConnection connection, AdAuthRequest request) throws GeneralSecurityException { | |
| Map<String, String> headers = new HashMap<>(); | |
| // 1. 암호화된 액세스 토큰 복호화 | |
| String accessToken = new String(aesUtil.decryptAES(connection.getAccessTokenEnc()), StandardCharsets.UTF_8); | |
| // 2. Authorization 헤더 설정 | |
| headers.put("Authorization", "Bearer " + accessToken); | |
| return headers; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/whereyouad/WhereYouAd/global/adapi/strategy/GoogleAdAuthStrategy.java`
around lines 27 - 39, The method generateHeaders in GoogleAdAuthStrategy
currently decrypts accessToken but returns an empty map; update it to populate
standard Google Ads auth headers: set "Authorization" to "Bearer {accessToken}"
using the decrypted value (from connection.getAccessTokenEnc()), and add
"developer-token" using connection.getDeveloperToken() (or
request.getDeveloperToken() if applicable) when present; ensure you only add
non-null/empty tokens and return the populated headers map from generateHeaders.
Comment on lines
+27
to
+39
| @Override | ||
| public Map<String, String> generateHeaders(PlatformConnection connection, AdAuthRequest request) throws GeneralSecurityException { | ||
| Map<String, String> headers = new HashMap<>(); | ||
|
|
||
| // 1. 암호화된 액세스 토큰 복호화 | ||
| String accessToken = new String(aesUtil.decryptAES(connection.getAccessTokenEnc()), StandardCharsets.UTF_8); | ||
|
|
||
| // 헤더 생성 로직 추가 | ||
|
|
||
|
|
||
|
|
||
| return headers; | ||
| } |
There was a problem hiding this comment.
현재 accessToken을 복호화한 후 사용하지 않고, 빈 headers 맵을 반환합니다. 이 상태로 배포되면 Kakao API 호출 시 인증 실패가 발생할 거예요.
문제점:
accessToken변수가 선언 후 사용되지 않음 (dead code)- 헤더에 인증 정보가 추가되지 않음
- 호출자(
AdApiAuthUtil)는 빈 헤더를 받아 API 호출 시 401 에러 발생 가능
Naver 구현체(NaverAdAuthStrategy)를 참고하면, 최소한 Authorization 헤더가 필요합니다. 구현이 완료될 때까지 이 전략이 호출되지 않도록 방어 로직이나 UnsupportedOperationException을 던지는 것을 권장합니다.
🛠️ 임시 방어 로직 제안
`@Override`
public Map<String, String> generateHeaders(PlatformConnection connection, AdAuthRequest request) throws GeneralSecurityException {
- Map<String, String> headers = new HashMap<>();
-
- // 1. 암호화된 액세스 토큰 복호화
- String accessToken = new String(aesUtil.decryptAES(connection.getAccessTokenEnc()), StandardCharsets.UTF_8);
-
- // 헤더 생성 로직 추가
-
-
-
- return headers;
+ // TODO: Kakao 인증 헤더 구현 필요
+ throw new UnsupportedOperationException("Kakao 인증 헤더 생성이 아직 구현되지 않았습니다.");
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/whereyouad/WhereYouAd/global/adapi/strategy/KakaoAdAuthStrategy.java`
around lines 27 - 39, KakaoAdAuthStrategy.generateHeaders currently decrypts
accessToken (accessToken via aesUtil.decryptAES(connection.getAccessTokenEnc()))
but returns an empty headers map; replace the dead-code by populating and
returning headers with the required authorization header (e.g., set
"Authorization" to "Bearer " + accessToken) so callers (AdApiAuthUtil) receive
valid credentials; if you prefer a temporary guard instead, throw
UnsupportedOperationException from generateHeaders to prevent accidental use
until full implementation is added.
Comment on lines
+44
to
+47
| // DB에서 암호화된 키 복호화 | ||
| String apiKey = new String(aesUtil.decryptAES(connection.getAccessTokenEnc()), StandardCharsets.UTF_8); | ||
| String secretKey = new String(aesUtil.decryptAES(connection.getRefreshTokenEnc()), StandardCharsets.UTF_8); | ||
| String customerId = connection.getPlatformAccount().getExternalAccountId(); |
There was a problem hiding this comment.
NPE(NullPointerException) 발생 가능성이 있어요 - 암호화된 필드와 관계 필드 null 체크 필요
PlatformConnection 엔티티를 보면 accessTokenEnc, refreshTokenEnc가 nullable이고, platformAccount 관계도 @JoinColumn에 nullable = false가 없어요. 이 값들이 null인 경우:
aesUtil.decryptAES(null)호출 시 예외 발생connection.getPlatformAccount()null 시getExternalAccountId()호출에서 NPE 발생
🛡️ Null 체크 추가 제안
+ private static final String MISSING_CREDENTIAL_MSG = "NAVER 연결 정보에 필수 인증 정보가 누락되었습니다.";
+
`@Override`
public Map<String, String> generateHeaders(PlatformConnection connection, AdAuthRequest request) throws GeneralSecurityException {
if (request.method() == null || request.path() == null) {
log.error("Naver 요청에는 method와 path가 필수 입니다.");
throw new AppException(ErrorCode.BAD_REQUEST);
}
+
+ if (connection.getAccessTokenEnc() == null || connection.getRefreshTokenEnc() == null) {
+ log.error("NAVER 인증 정보 누락 - connectionId={}", connection.getId());
+ throw new AppException(ErrorCode.BAD_REQUEST);
+ }
+
+ if (connection.getPlatformAccount() == null) {
+ log.error("NAVER PlatformAccount 누락 - connectionId={}", connection.getId());
+ throw new AppException(ErrorCode.BAD_REQUEST);
+ }
Map<String, String> headers = new HashMap<>();📝 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
| // DB에서 암호화된 키 복호화 | |
| String apiKey = new String(aesUtil.decryptAES(connection.getAccessTokenEnc()), StandardCharsets.UTF_8); | |
| String secretKey = new String(aesUtil.decryptAES(connection.getRefreshTokenEnc()), StandardCharsets.UTF_8); | |
| String customerId = connection.getPlatformAccount().getExternalAccountId(); | |
| private static final String MISSING_CREDENTIAL_MSG = "NAVER 연결 정보에 필수 인증 정보가 누락되었습니다."; | |
| `@Override` | |
| public Map<String, String> generateHeaders(PlatformConnection connection, AdAuthRequest request) throws GeneralSecurityException { | |
| if (request.method() == null || request.path() == null) { | |
| log.error("Naver 요청에는 method와 path가 필수 입니다."); | |
| throw new AppException(ErrorCode.BAD_REQUEST); | |
| } | |
| if (connection.getAccessTokenEnc() == null || connection.getRefreshTokenEnc() == null) { | |
| log.error("NAVER 인증 정보 누락 - connectionId={}", connection.getId()); | |
| throw new AppException(ErrorCode.BAD_REQUEST); | |
| } | |
| if (connection.getPlatformAccount() == null) { | |
| log.error("NAVER PlatformAccount 누락 - connectionId={}", connection.getId()); | |
| throw new AppException(ErrorCode.BAD_REQUEST); | |
| } | |
| Map<String, String> headers = new HashMap<>(); | |
| // DB에서 암호화된 키 복호화 | |
| String apiKey = new String(aesUtil.decryptAES(connection.getAccessTokenEnc()), StandardCharsets.UTF_8); | |
| String secretKey = new String(aesUtil.decryptAES(connection.getRefreshTokenEnc()), StandardCharsets.UTF_8); | |
| String customerId = connection.getPlatformAccount().getExternalAccountId(); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/whereyouad/WhereYouAd/global/adapi/strategy/NaverAdAuthStrategy.java`
around lines 44 - 47, In NaverAdAuthStrategy, prevent NPEs by null-checking
encrypted fields and the related entity before decrypting or accessing account
data: verify connection.getAccessTokenEnc() and connection.getRefreshTokenEnc()
are non-null before calling aesUtil.decryptAES(...) and handle nulls (throw a
clear IllegalStateException or return a controlled error), and verify
connection.getPlatformAccount() is non-null before calling
getExternalAccountId() (again fail-fast with a descriptive exception or
fallback); update the logic around apiKey, secretKey and customerId assignment
to perform these checks and include the field names in the error messages to aid
debugging.
Comment on lines
+20
to
+21
| @Value("${aes.secret}") | ||
| private String key; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# CI에서 설정된 AES_SECRET 길이 확인
echo "CI에서 설정된 AES_SECRET 길이: $(echo -n 'test_aes_key' | wc -c) bytes"
echo "AES 허용 키 길이: 16, 24, 32 bytes"
# application.yml에서 aes.secret 설정 확인
fd -t f 'application*.yml' --exec grep -l 'aes' {} \; | xargs catRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 4414
🏁 Script executed:
find . -name "AESUtil.java" -type fRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 140
🏁 Script executed:
fd -type f 'AESUtil.java' --exec cat -n {} \;Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 243
🏁 Script executed:
rg "AESUtil" --type java -A 3 -B 1Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 4378
🏁 Script executed:
cat -n ./src/main/java/com/whereyouad/WhereYouAd/global/utils/AESUtil.javaRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 2350
🏁 Script executed:
rg "encryptAES|decryptAES" -A 2Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 2150
🏁 Script executed:
find . -path "*/test/*" -name "*Test.java" | head -20 | xargs grep -l "AESUtil\|AES_SECRET" 2>/dev/nullRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 57
🏁 Script executed:
find . -path "*/test/*" -name "application*.yml" -o -name "application*.properties" | xargs cat 2>/dev/nullRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 55
🏁 Script executed:
find . -path "*/test/*" -name "*.java" | wc -lRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 74
🏁 Script executed:
find . -path "*/test/*" -name "*Strategy*Test.java"Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 55
🏁 Script executed:
find . -path "*/test/*" -name "*.java" -exec grep -l "decryptAES\|encryptAES" {} \;Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 55
🏁 Script executed:
find . -path "*/test/*" -name "*.java" -exec cat {} \;Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 293
🏁 Script executed:
find . -path "*/test/*" -name "*.yml" -o -path "*/test/*" -name "*.properties"Repository: WhereYouAd/WhereYouAd-Backend
Length of output: 55
🏁 Script executed:
find . -path "*/test/resources/*" -type fRepository: WhereYouAd/WhereYouAd-Backend
Length of output: 55
AES 키 길이 검증 필요 - 12바이트 키는 암/복호화 시 실패
CI에서 설정한 AES_SECRET=test_aes_key (12바이트)는 AES가 요구하는 16, 24, 32바이트를 만족하지 않습니다. @SpringBootTest 로딩 시에는 예외가 발생하지 않지만, AdAuthStrategy에서 decryptAES() 메서드를 호출하는 순간 InvalidKeyException이 발생합니다.
현재 코드는 cipher.init()할 때 키 길이를 검증하지 않기 때문에, 런타임 에러로 응답합니다. @PostConstruct를 통해 빈 생성 시점에 키 길이를 검증하면 문제를 조기에 발견할 수 있습니다.
🛡️ 키 길이 검증 추가
+import jakarta.annotation.PostConstruct;
+import java.nio.charset.StandardCharsets;
+import java.util.Set;
`@Component`
`@RequiredArgsConstructor`
public class AESUtil {
+ private static final Set<Integer> VALID_KEY_LENGTHS = Set.of(16, 24, 32);
+
+ `@PostConstruct`
+ void validateKeyLength() {
+ int keyLength = key.getBytes(StandardCharsets.UTF_8).length;
+ if (!VALID_KEY_LENGTHS.contains(keyLength)) {
+ throw new IllegalStateException(
+ "AES 키 길이가 올바르지 않습니다. 현재: " + keyLength + "바이트, 허용: 16/24/32바이트");
+ }
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/whereyouad/WhereYouAd/global/utils/AESUtil.java` around
lines 20 - 21, Add a startup-time key length validation in AESUtil: create a
`@PostConstruct` method that checks the injected String key field has a byte
length of 16, 24, or 32 (e.g., key.getBytes(StandardCharsets.UTF_8).length) and
throw an IllegalStateException with a clear message if it doesn't; this ensures
AESUtil (and callers like decryptAES) will fail fast during bean initialization
instead of throwing InvalidKeyException at runtime when cipher.init(...) is
invoked.
Comment on lines
+33
to
+34
| SecretKey aesKey = new SecretKeySpec(key.getBytes(), ALGORITHM); | ||
| IvParameterSpec iv = new IvParameterSpec(key.getBytes()); |
There was a problem hiding this comment.
[보안] 동일한 키를 AES 키와 IV로 재사용하면 안 돼요.
현재 key.getBytes()를 AES 키와 IV 양쪽에 모두 사용하고 있어요. CBC 모드에서 IV는 예측 불가능해야 하며, 키와 동일하면 공격자가 패턴을 분석할 수 있는 취약점이 생깁니다.
권장사항:
- IV는 암호화 시마다 랜덤 생성 (16바이트)
- 암호문 앞에 IV를 붙여서 저장하고, 복호화 시 분리해서 사용
🔒 보안 강화 예시
+import java.security.SecureRandom;
+
`@Component`
`@RequiredArgsConstructor`
public class AESUtil {
private static final String ALGORITHM = "AES";
private static final String CIPHER_ALGORITHM = "AES/CBC/PKCS5Padding";
+ private static final int IV_LENGTH = 16;
`@Value`("${aes.secret}")
private String key;
- public byte[] encryptAES(String plainText) throws GeneralSecurityException {
+ public String encryptAES(String plainText) throws GeneralSecurityException {
+ // 랜덤 IV 생성
+ byte[] ivBytes = new byte[IV_LENGTH];
+ new SecureRandom().nextBytes(ivBytes);
+ IvParameterSpec iv = new IvParameterSpec(ivBytes);
+
Cipher cipher = Cipher.getInstance(CIPHER_ALGORITHM);
SecretKey aesKey = new SecretKeySpec(key.getBytes(), ALGORITHM);
- IvParameterSpec iv = new IvParameterSpec(key.getBytes());
cipher.init(Cipher.ENCRYPT_MODE, aesKey, iv);
- return Base64.getEncoder().encode(cipher.doFinal(plainText.getBytes()));
+
+ byte[] encrypted = cipher.doFinal(plainText.getBytes(StandardCharsets.UTF_8));
+ // IV + 암호문을 합쳐서 Base64 인코딩
+ byte[] combined = new byte[ivBytes.length + encrypted.length];
+ System.arraycopy(ivBytes, 0, combined, 0, ivBytes.length);
+ System.arraycopy(encrypted, 0, combined, ivBytes.length, encrypted.length);
+ return Base64.getEncoder().encodeToString(combined);
}Also applies to: 50-51
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/whereyouad/WhereYouAd/global/utils/AESUtil.java` around
lines 33 - 34, The code reuses key.getBytes() for both SecretKey (SecretKey
aesKey) and IvParameterSpec iv which is insecure; change encrypt path to
generate a new random 16-byte IV (use SecureRandom) each time, construct
IvParameterSpec from that IV, perform encryption with Cipher using
SecretKeySpec(key.getBytes(), ALGORITHM), then prepend (or otherwise store) the
IV bytes to the ciphertext output; change decrypt path to read/extract the first
16 bytes as the IV, construct IvParameterSpec from those bytes, and use the
remaining bytes as the ciphertext to decrypt with the same SecretKeySpec; update
any other usages (e.g., where IvParameterSpec iv is created) to follow this
pattern and ensure IV length is 16 bytes for AES/CBC.
Comment on lines
+41
to
+56
| /** | ||
| * 평문을 AES로 암호화합니다. | ||
| * @param plainText 암호화할 평문 | ||
| * @return AES로 암호화한 뒤 Base64로 인코딩한 문자열 | ||
| */ | ||
| public byte[] encryptAES( | ||
| String plainText | ||
| ) throws GeneralSecurityException { | ||
| Cipher cipher = Cipher.getInstance(CIPHER_ALGORITHM); | ||
| SecretKey aesKey = new SecretKeySpec(key.getBytes(), ALGORITHM); | ||
| IvParameterSpec iv = new IvParameterSpec(key.getBytes()); | ||
| cipher.init(Cipher.ENCRYPT_MODE, aesKey, iv); | ||
|
|
||
| // AES 암호화 -> Base64 인코딩 | ||
| return Base64.getEncoder().encode(cipher.doFinal(plainText.getBytes())); | ||
| } |
There was a problem hiding this comment.
Javadoc과 실제 반환 타입이 일치하지 않아요.
Javadoc에는 "AES로 암호화한 뒤 Base64로 인코딩한 문자열"이라고 되어 있는데, 실제로는 byte[]를 반환합니다. 호출하는 쪽에서 혼란이 생길 수 있어요.
📝 반환 타입 수정 제안
/**
* 평문을 AES로 암호화합니다.
* `@param` plainText 암호화할 평문
* `@return` AES로 암호화한 뒤 Base64로 인코딩한 문자열
*/
- public byte[] encryptAES(
+ public String encryptAES(
String plainText
) throws GeneralSecurityException {
Cipher cipher = Cipher.getInstance(CIPHER_ALGORITHM);
SecretKey aesKey = new SecretKeySpec(key.getBytes(), ALGORITHM);
IvParameterSpec iv = new IvParameterSpec(key.getBytes());
cipher.init(Cipher.ENCRYPT_MODE, aesKey, iv);
// AES 암호화 -> Base64 인코딩
- return Base64.getEncoder().encode(cipher.doFinal(plainText.getBytes()));
+ return Base64.getEncoder().encodeToString(cipher.doFinal(plainText.getBytes(StandardCharsets.UTF_8)));
}참고: 정적 분석 도구에서 DES/ECB 경고가 나왔는데, 실제로 코드는 AES/CBC/PKCS5Padding을 사용하고 있어서 **오탐(false positive)**입니다. 무시하셔도 됩니다.
🧰 Tools
🪛 ast-grep (0.41.1)
[warning] 48-48: Triple DES (3DES or DESede) is considered deprecated. AES is the recommended cipher. Upgrade to use AES.
Context: Cipher.getInstance(CIPHER_ALGORITHM)
Note: [CWE-326]: Inadequate Encryption Strength [OWASP A03:2017]: Sensitive Data Exposure [OWASP A02:2021]: Cryptographic Failures [REFERENCES]
- https://find-sec-bugs.github.io/bugs.htm#TDES_USAGE
- https://csrc.nist.gov/News/2017/Update-to-Current-Use-and-Deprecation-of-TDEA
(desede-is-deprecated-java)
[warning] 48-48: Use of AES with ECB mode detected. ECB doesn't provide message confidentiality and is not semantically secure so should not be used. Instead, use a strong, secure cipher: Cipher.getInstance("AES/CBC/PKCS7PADDING"). See https://owasp.org/www-community/Using_the_Java_Cryptographic_Extensions for more information.
Context: Cipher.getInstance(CIPHER_ALGORITHM)
Note: [CWE-327]: Use of a Broken or Risky Cryptographic Algorithm [OWASP A03:2017]: Sensitive Data Exposure [OWASP A02:2021]: Cryptographic Failures [REFERENCES]
- https://owasp.org/Top10/A02_2021-Cryptographic_Failures
- https://googleprojectzero.blogspot.com/2022/10/rc4-is-still-considered-harmful.html
(use-of-aes-ecb-java)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/whereyouad/WhereYouAd/global/utils/AESUtil.java` around
lines 41 - 56, The Javadoc says the method returns a Base64-encoded STRING but
encryptAES currently returns byte[], so update encryptAES in AESUtil to return a
String: after cipher.doFinal(...), Base64-encode the bytes and return the
resulting UTF-8 String (instead of byte[]), keep the same exception signature
and update Javadoc to match the String return; ensure callers are updated to
expect a String result.
ojy0903
reviewed
Mar 28, 2026
|
|
||
| @Component | ||
| @RequiredArgsConstructor | ||
| public class AESUtil { |
Collaborator
There was a problem hiding this comment.
P4: 저는 Bcrypt 만 알고 있었어서 잘 몰랐었는데 이렇게 암호화 Util 클래스를 따로 만드는 것 좋은 거 같아요!!
ojy0903
reviewed
Mar 28, 2026
|
|
||
| @Component | ||
| @RequiredArgsConstructor | ||
| public class KakaoAdAuthStrategy implements AdAuthStrategy { |
Collaborator
There was a problem hiding this comment.
P4: 저도 조사하면서 해보다가 이런 방식으로 코드 진행하고 있었는데 미리 해주셨네요..!! 감사합니다ㅠㅠ 적용해서 해보겠습니다!
ojy0903
approved these changes
Mar 28, 2026
Collaborator
ojy0903
left a comment
ojy0903
left a comment
There was a problem hiding this comment.
P4: 고생하셨습니다!! API 키 값 암호화 관련 진행하시면서 API 통신 기본 뼈대를 만들어주신거 같아서 좋은 것 같습니다 👍
-
제가 볼때는 지금 strategy + factory 구조 깔끔해서 좋은 것 같아요.
-
캠페인 조회 시에 API 통신으로 받아올지 DB 에서 끌어올지 같은 경우에는 제 생각에는 스케줄러로 매일 특정 시간마다 API 통신으로 조회해서 DB 에 UPSERT 같은 방식으로 반영하고 조회는 DB 에서 끌어오는 방식이 어떨까 싶은데, 스케줄러를 쓰는 부분이 어렵다거나 시간이 오래 걸릴까 싶긴 합니다... 스케줄러가 힘들다면 DB 에서 읽는 것을 기본으로 하고 플랫폼과 API 통신해서 DB 를 업데이트 하는 API 를 하나 더하는 건 어떨까요...(화면에서 사용자가 수동으로 "플랫폼 정보 업데이트 하기" 같은 버튼 누르면 API 통신해서 DB 에 반영되는 방식?) 제 생각이긴 합니다...
-
제 생각에는 accessTokenEnc, refreshTokenEnc 가 아닌 다른 범용적 용어로 필드명을 하는 것도 좋을 것 같긴 합니다!
kingmingyu
changed the title
jinnieusLab
approved these changes
Mar 30, 2026
Collaborator
There was a problem hiding this comment.
P4: 고생하셨습니다! 보고서 작성 기간이랑 겹쳐 리뷰가 늦어졌네요 ㅠㅠ
플랫폼별로 어떻게 통합해서 코드를 짤지 막막했었는데, Strategy와 Factory 패턴으로 깔끔하게 미리 작성해주셔서 작업이 훨씬 편해질 것 같아요 감사합니다!! 암호화도 util로 빼주셔서 다른 로직에도 재사용 가능할 것 같아 좋습니다!
캠페인 데이터를 db에 저장해서 사용하는 방법과 api 호출로 바로 가져오는 방법 중 저는 전자가 괜찮다고 생각합니다. 바로 호출해서 저장 없이 가져오면 저희가 데이터를 가공해서 새로운 자료를 보여줄 때 등 제약이 있을 거 같아, api를 하루 한 번 정도만 호출하여(스케줄링) db에 저장해두고, 기존처럼 db에 저장된 것을 조회하는 방향이 좋을 것 같아요!
Collaborator
Author
|
좋습니다! 그러면 일단 속성명만 |
Collaborator
Author
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/entity/PlatformConnection.java (1)
30-34: 필드명 리팩토링 잘 하셨습니다! 👍
accessTokenEnc→authIdentifier,refreshTokenEnc→authCredential로 이름을 변경하신 건 정말 좋은 결정입니다. 이제 OAuth 토큰뿐만 아니라 API Key, Client ID/Secret 등 다양한 인증 방식을 하나의 엔티티로 유연하게 처리할 수 있네요.JPA 설정도 잘 되어 있습니다:
@ManyToOne에FetchType.LAZY적용이 되어 있어서 N+1 쿼리 문제를 방지했습니다. 👌- Jakarta Persistence import로 Spring Boot 3.x 표준을 따르고 있습니다.
한 가지 제안:
현재 프로젝트는 Hibernate의
ddl-auto: update모드를 사용하고 있어서, 스키마 변경이 자동으로 처리됩니다. 따라서 즉시 문제는 없지만, 프로덕션 환경에서는 명시적인 데이터베이스 마이그레이션 도구(Flyway나 Liquibase)를 도입하는 것을 추천합니다.이유:
- 자동 DDL은 개발 환경에서는 편하지만, 프로덕션에선 예측 불가능한 스키마 변경이 발생할 수 있습니다.
- 버전 관리와 롤백이 체계적으로 이루어질 수 없습니다.
예를 들어, Flyway를 추가하려면:
implementation 'org.flywaydb:flyway-core' implementation 'org.flywaydb:flyway-mysql'그리고
src/main/resources/db/migration/V1__create_tables.sql같은 파일에서 스키마를 명시적으로 관리하면 더 안정적입니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/entity/PlatformConnection.java` around lines 30 - 34, Add a proper DB migration tool and stop relying on Hibernate auto-ddl: add Flyway (or Liquibase) as a dependency in build files, create an initial migration SQL (e.g., V1__create_platform_connection.sql) under src/main/resources/db/migration that defines the table/columns reflecting the PlatformConnection entity (including auth_identifier and auth_credential), disable or set spring.jpa.hibernate.ddl-auto to validate in production profiles, and configure Flyway properties (URL/user/password/locations) so schema changes are versioned and applied at startup; this aligns the PlatformConnection entity (authIdentifier/authCredential) with an explicit migration-managed schema.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/entity/PlatformConnection.java`:
- Around line 30-34: Add a proper DB migration tool and stop relying on
Hibernate auto-ddl: add Flyway (or Liquibase) as a dependency in build files,
create an initial migration SQL (e.g., V1__create_platform_connection.sql) under
src/main/resources/db/migration that defines the table/columns reflecting the
PlatformConnection entity (including auth_identifier and auth_credential),
disable or set spring.jpa.hibernate.ddl-auto to validate in production profiles,
and configure Flyway properties (URL/user/password/locations) so schema changes
are versioned and applied at startup; this aligns the PlatformConnection entity
(authIdentifier/authCredential) with an explicit migration-managed schema.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 562b8ac8-6b2f-4410-85c8-65b6f9bd4f5e
📒 Files selected for processing (4)
src/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/entity/PlatformConnection.javasrc/main/java/com/whereyouad/WhereYouAd/global/adapi/strategy/GoogleAdAuthStrategy.javasrc/main/java/com/whereyouad/WhereYouAd/global/adapi/strategy/KakaoAdAuthStrategy.javasrc/main/java/com/whereyouad/WhereYouAd/global/adapi/strategy/NaverAdAuthStrategy.java
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/java/com/whereyouad/WhereYouAd/global/adapi/strategy/GoogleAdAuthStrategy.java
- src/main/java/com/whereyouad/WhereYouAd/global/adapi/strategy/KakaoAdAuthStrategy.java
- src/main/java/com/whereyouad/WhereYouAd/global/adapi/strategy/NaverAdAuthStrategy.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
3 participants
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.
📌 관련 이슈
🚀 개요
📄 작업 내용
AdApiAuthUtil클래스를 이용해서 서비스 로직에서 필요한 헤더 제작하여 통신, 각 플랫폼 별 API 통신 인증에 필요한 헤더 제작은 globla/adapi/strategy안에서 제작 (GoogleAdAuthStrategy랑KakaoAdAuthStrategy만 수정해주시면 됩니다)📸 스크린샷 / 테스트 결과 (선택)
✅ 체크리스트
🔍 리뷰 포인트 (Review Points)
Summary by CodeRabbit
릴리스 노트
새로운 기능
문서
Chores