Merged
Conversation
WalkthroughAPI 관리·사용 서비스의 CloudFront 서명 쿠키 및 인증 쿠키 처리에 도메인 인자 지원을 추가했습니다. 공통 Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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 Tip CodeRabbit can suggest fixes for GitHub Check annotations.Configure the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/api-admin/src/main/java/com/ott/api_admin/auth/cdn/CloudFrontSignedCookieService.java (1)
71-77: P2: CloudFront 쿠키 도메인은 코드 주석 토글 대신 설정으로 분리하는 편이 안전합니다.근거:
apps/api-admin/src/main/java/com/ott/api_admin/auth/cdn/CloudFrontSignedCookieService.javaLine 71-77, Line 86-92에서 환경 고정 도메인 리터럴을 하드코딩하고 local용 호출을 주석으로 남겨 두었습니다. 이 방식은 local/dev/prod 전환 때 소스 수정이 필요해 잘못된 도메인이 커밋되기 쉽고,apps/api-user/src/main/java/com/ott/api_user/auth/cdn/CloudFrontSignedCookieService.javaLine 71-92에도 같은 패턴이 반복됩니다.cloudfront.signed-cookie.cookie-domain같은 설정을 주입하고, 값이 비어 있으면 기본 오버로드를 사용하도록 바꾸면 환경별 분기를 코드 변경 없이 처리할 수 있습니다.♻️ 리팩터링 예시
+ `@Value`("${cloudfront.signed-cookie.cookie-domain:}") + private String cookieDomain; + public void addSignedCookies(HttpServletResponse response) { ... - cookieUtil.addCookie(response, POLICY_COOKIE, cloudFrontBase64(policy.getBytes(StandardCharsets.UTF_8)), ttlMillis, "openthetaste.cloud"); - cookieUtil.addCookie(response, SIGNATURE_COOKIE, cloudFrontBase64(signatureBytes), ttlMillis, "openthetaste.cloud"); - cookieUtil.addCookie(response, KEY_PAIR_ID_COOKIE, keyPairId, ttlMillis, "openthetaste.cloud"); + if (cookieDomain == null || cookieDomain.isBlank()) { + cookieUtil.addCookie(response, POLICY_COOKIE, cloudFrontBase64(policy.getBytes(StandardCharsets.UTF_8)), ttlMillis); + cookieUtil.addCookie(response, SIGNATURE_COOKIE, cloudFrontBase64(signatureBytes), ttlMillis); + cookieUtil.addCookie(response, KEY_PAIR_ID_COOKIE, keyPairId, ttlMillis); + } else { + cookieUtil.addCookie(response, POLICY_COOKIE, cloudFrontBase64(policy.getBytes(StandardCharsets.UTF_8)), ttlMillis, cookieDomain); + cookieUtil.addCookie(response, SIGNATURE_COOKIE, cloudFrontBase64(signatureBytes), ttlMillis, cookieDomain); + cookieUtil.addCookie(response, KEY_PAIR_ID_COOKIE, keyPairId, ttlMillis, cookieDomain); + } }// clearSignedCookies(...)도 같은 기준으로 분기Also applies to: 86-92
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/main/java/com/ott/api_admin/auth/cdn/CloudFrontSignedCookieService.java` around lines 71 - 77, Replace the hardcoded domain and commented local calls in CloudFrontSignedCookieService by injecting a configurable property (e.g. cloudfront.signed-cookie.cookie-domain) into the service and use it when calling cookieUtil.addCookie(POLICY_COOKIE, SIGNATURE_COOKIE, KEY_PAIR_ID_COOKIE); if the injected value is null/empty, fall back to the existing overloads that don't specify a domain so behavior remains unchanged for local tests; apply the exact same change to clearSignedCookies(...) and mirror the refactor in the corresponding CloudFrontSignedCookieService in the api-user module so both services read the shared configuration instead of using literal "openthetaste.cloud" or commented calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@modules/common-security/src/main/java/com/ott/common/security/util/CookieUtil.java`:
- Around line 18-25: The migration missed expiring legacy shared-domain cookies
alongside the new host-only cookies; update places that call addCookie/addCookie
overloads (e.g., usages in OAuth2SuccessHandler and AuthController) so that
before issuing new accessToken/refreshToken you call deleteCookie(response,
"accessToken", "openthetaste.cloud") and deleteCookie(response, "refreshToken",
"openthetaste.cloud"), then call addCookie(response, ...) as normal, and
likewise update logout paths to call both deleteCookie(response, "accessToken",
"openthetaste.cloud") and deleteCookie(response, "refreshToken",
"openthetaste.cloud") in addition to the existing deleteCookie(response,
"accessToken") and deleteCookie(response, "refreshToken"); use the existing
CookieUtil methods (addCookie, deleteCookie, buildCookie) so both legacy domain
and new host-only cookies are expired during roll-out.
---
Nitpick comments:
In
`@apps/api-admin/src/main/java/com/ott/api_admin/auth/cdn/CloudFrontSignedCookieService.java`:
- Around line 71-77: Replace the hardcoded domain and commented local calls in
CloudFrontSignedCookieService by injecting a configurable property (e.g.
cloudfront.signed-cookie.cookie-domain) into the service and use it when calling
cookieUtil.addCookie(POLICY_COOKIE, SIGNATURE_COOKIE, KEY_PAIR_ID_COOKIE); if
the injected value is null/empty, fall back to the existing overloads that don't
specify a domain so behavior remains unchanged for local tests; apply the exact
same change to clearSignedCookies(...) and mirror the refactor in the
corresponding CloudFrontSignedCookieService in the api-user module so both
services read the shared configuration instead of using literal
"openthetaste.cloud" or commented calls.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 37f221a7-de8a-4c04-b588-a8a1102bfd38
📒 Files selected for processing (3)
apps/api-admin/src/main/java/com/ott/api_admin/auth/cdn/CloudFrontSignedCookieService.javaapps/api-user/src/main/java/com/ott/api_user/auth/cdn/CloudFrontSignedCookieService.javamodules/common-security/src/main/java/com/ott/common/security/util/CookieUtil.java
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/api-admin/src/main/java/com/ott/api_admin/auth/controller/AdminAuthController.java (1)
49-51: 레거시 쿠키 삭제 로직을 상수/헬퍼로 묶어 중복을 줄이는 것을 권장합니다.현재처럼 하드코딩이 3곳에 반복되면, 마이그레이션 종료 시점 정리나 도메인 변경 시 누락 가능성이 커집니다.
리팩터링 예시
public class AdminAuthController implements AdminAuthApi { + private static final String LEGACY_SHARED_COOKIE_DOMAIN = "openthetaste.cloud"; + private static final String ACCESS_TOKEN_COOKIE = "accessToken"; + private static final String REFRESH_TOKEN_COOKIE = "refreshToken"; + + private void deleteLegacySharedDomainAuthCookies(HttpServletResponse response) { + cookie.deleteCookie(response, ACCESS_TOKEN_COOKIE, LEGACY_SHARED_COOKIE_DOMAIN); + cookie.deleteCookie(response, REFRESH_TOKEN_COOKIE, LEGACY_SHARED_COOKIE_DOMAIN); + } @@ - cookie.deleteCookie(response, "accessToken", "openthetaste.cloud"); - cookie.deleteCookie(response, "refreshToken", "openthetaste.cloud"); + deleteLegacySharedDomainAuthCookies(response); @@ - cookie.deleteCookie(response, "accessToken", "openthetaste.cloud"); - cookie.deleteCookie(response, "refreshToken", "openthetaste.cloud"); + deleteLegacySharedDomainAuthCookies(response); @@ - cookie.deleteCookie(response, "accessToken", "openthetaste.cloud"); - cookie.deleteCookie(response, "refreshToken", "openthetaste.cloud"); + deleteLegacySharedDomainAuthCookies(response);Also applies to: 75-77, 95-97
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-admin/src/main/java/com/ott/api_admin/auth/controller/AdminAuthController.java` around lines 49 - 51, The three duplicated legacy-cookie delete calls in AdminAuthController (the cookie.deleteCookie(response, "accessToken", "openthetaste.cloud") and cookie.deleteCookie(response, "refreshToken", "openthetaste.cloud") occurrences at lines around 49-51, 75-77, 95-97) should be consolidated: introduce a constant for the legacy domain and constants (or an enum) for legacy cookie names, and add a small helper method (e.g., deleteLegacyCookies(HttpServletResponse response) or CookieUtils.deleteLegacyCookies(response)) that performs both deletes; then replace the repeated inline calls with a single call to that helper to reduce duplication and centralize future changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/api-admin/src/main/java/com/ott/api_admin/auth/controller/AdminAuthController.java`:
- Around line 49-51: The three duplicated legacy-cookie delete calls in
AdminAuthController (the cookie.deleteCookie(response, "accessToken",
"openthetaste.cloud") and cookie.deleteCookie(response, "refreshToken",
"openthetaste.cloud") occurrences at lines around 49-51, 75-77, 95-97) should be
consolidated: introduce a constant for the legacy domain and constants (or an
enum) for legacy cookie names, and add a small helper method (e.g.,
deleteLegacyCookies(HttpServletResponse response) or
CookieUtils.deleteLegacyCookies(response)) that performs both deletes; then
replace the repeated inline calls with a single call to that helper to reduce
duplication and centralize future changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1214052d-bdb4-450f-8333-77b9e0b51568
📒 Files selected for processing (3)
apps/api-admin/src/main/java/com/ott/api_admin/auth/controller/AdminAuthController.javaapps/api-user/src/main/java/com/ott/api_user/auth/controller/AuthController.javaapps/api-user/src/main/java/com/ott/api_user/auth/oauth2/handler/OAuth2SuccessHandler.java
This was referenced Mar 23, 2026
Merged
Merged
Merged
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
2 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.
📝 작업 내용
📷 스크린샷
☑️ 체크 리스트
#️⃣ 연관된 이슈
close #218
💬 리뷰 요구사항
현 pr 이후 기존 cookie 유틸 함수에서 주석처리하던게 -> cdn쪽 주석처리하는걸로 변경되었습니다.
Summary by CodeRabbit
릴리스 노트