Merged
Conversation
…o OT-144-fix/OAuth2-kakao
Walkthrough관리자·사용자 인증에서 쿠키 처리를 Changes
Sequence Diagram(s)sequenceDiagram
participant User as 사용자
participant OAuth2 as OAuth2 핸들러
participant KakaoSvc as KakaoAuthService
participant Member as Member (도메인)
participant CookieUtil as CookieUtil
participant Browser as 브라우저
User->>OAuth2: Kakao 로그인 완료 콜백
OAuth2->>KakaoSvc: isNewMember(member)
KakaoSvc->>Member: member.isOnboardingCompleted()
Member-->>KakaoSvc: onboardingCompleted (boolean)
KakaoSvc-->>OAuth2: 신규회원 여부 반환
OAuth2->>CookieUtil: addCookie(accessToken)
OAuth2->>CookieUtil: addCookie(refreshToken)
CookieUtil->>Browser: Set-Cookie 헤더 전달
alt 신규 회원 (온보딩 미완료)
OAuth2->>Browser: 리다이렉트 /auth/userinfo
else 기존 회원 (온보딩 완료)
OAuth2->>Browser: 리다이렉트 /
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
apps/api-admin/src/main/java/com/ott/api_admin/config/SecurityConfig.java (1)
74-79: admin CORS에서localhost+ credentials 허용은 환경 분리가 필요합니다.Line 74-76과 Line 79 조합은 운영 반영 시 과도한 허용이 될 수 있습니다.
localhost는 dev 전용으로 분리하고, 운영은 신뢰 도메인만 허용하는 구성이 더 안전합니다.🔧 제안 diff
- config.setAllowedOriginPatterns(List.of( - "http://localhost:*", - "https://www.openthetaste.cloud")); + // 운영: 신뢰 도메인만 허용, localhost는 dev profile에서만 허용 + config.setAllowedOriginPatterns(List.of( + "https://www.openthetaste.cloud"));🤖 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/config/SecurityConfig.java` around lines 74 - 79, The current SecurityConfig uses config.setAllowedOriginPatterns(List.of("http://localhost:*", "https://www.openthetaste.cloud")) together with config.setAllowCredentials(true), which dangerously allows credentials for a wildcard localhost pattern in production; update SecurityConfig so localhost origins are only added in non-prod (e.g., using an active Spring profile or an env property) and production uses a fixed trusted origins list (no wildcard patterns) before calling setAllowCredentials(true); specifically modify the code paths around SecurityConfig (the config variable and the calls to setAllowedOriginPatterns / setAllowedOrigins and setAllowCredentials) to conditionally include "http://localhost:*" only when profile == "dev" (or a flag), and ensure production uses explicit trusted domains only.apps/api-admin/src/main/java/com/ott/api_admin/auth/dto/response/AdminTokenResponse.java (1)
14-18: @Schema 어노테이션 추가는 좋습니다. 예시 토큰 형식에 대한 선택적 개선 사항이 있습니다.OpenAPI 문서화를 위한
@Schema어노테이션 추가는 적절합니다. 다만, 예시 토큰이 실제 JWT 형식(header.payload.signature)과 다르게 보입니다. 문서 가독성을 위해 실제 JWT와 유사한 형식으로 변경을 고려해 볼 수 있습니다.♻️ 선택적 개선안: JWT 형식 예시
- `@Schema`(type = "String", description = "accessToken", example = "122Wjxf@djx1jcmxsizkds2fj-dsm2.dzj2") + `@Schema`(description = "JWT Access Token", example = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIn0.dozjgNryP4J3jVmNHl0w5N_XgL0n3I9PlFUP0THsR8U") private String accessToken; - `@Schema`(type = "String", description = "refreshToken", example = "eym122Wjxf@djx1jcmxsizkds2fj-dsm2.dzj2") + `@Schema`(description = "JWT Refresh Token", example = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIn0.Rq8IjqbeYzziJsRBcLfM4FlJlp7D0aq5mIDx2WwY_kE") private String refreshToken;참고:
type = "String"은 필드 타입에서 이미 추론되므로 생략 가능합니다.🤖 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/dto/response/AdminTokenResponse.java` around lines 14 - 18, Update the `@Schema` annotations on AdminTokenResponse.accessToken and AdminTokenResponse.refreshToken to use JWT-like example strings (e.g. "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjMifQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c") and remove the redundant type = "String" attribute so the annotation relies on the field type; adjust the description text if needed to indicate these are JWTs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/api-admin/src/main/java/com/ott/api_admin/auth/dto/request/AdminLoginRequest.java`:
- Around line 16-20: The `@Schema` annotations in AdminLoginRequest use invalid
OpenAPI type values ("String"); update the `@Schema` on the email and password
fields (the annotations above the email field and the password field in class
AdminLoginRequest) to either remove the type attribute so the generator can
infer it, or change type="String" to the correct OpenAPI primitive type
type="string", while keeping existing description and example values intact.
In
`@apps/api-admin/src/main/java/com/ott/api_admin/auth/dto/response/AdminLoginResponse.java`:
- Around line 19-23: Update the OpenAPI `@Schema` annotations in
AdminLoginResponse: change the memberId field's `@Schema` to use type="integer"
and format="int64" instead of type="Long", and change the role field's `@Schema`
to use type="string" (or omit type and rely on Java String which maps to
"string"); ensure you keep the existing description and example values while
updating only the type/format attributes on memberId and role.
In `@apps/api-admin/src/main/java/com/ott/api_admin/common/CookieUtil.java`:
- Around line 12-19: The ResponseCookie creation in CookieUtil currently
hard-codes .secure(false); change it to default to secure=true and make the
value configurable per environment (e.g., read a boolean flag from application
properties or an Environment variable used by CookieUtil). Update the code paths
that build cookies (the ResponseCookie.from(...) block in CookieUtil and the
similar block at lines 24-31) to use the injected/config-driven boolean (e.g.,
cookieSecure flag) instead of the literal false, ensure tests and local dev
override via config/profile (or a DEV flag) and keep comments about local
testing removed or clarified.
In `@apps/api-user/src/main/java/com/ott/api_user/common/CookieUtil.java`:
- Around line 11-18: The cookie builder hardcodes .secure(false) which is unsafe
for non-local deployments; update CookieUtil to determine the secure flag from
configuration (e.g., an environment property or Spring property) and pass that
boolean into ResponseCookie.from(...).secure(isSecure). Ensure the property is
read once (e.g., constructor or static config) and reused for all cookie
constructions in CookieUtil (including the other occurrence at lines 23-30) so
staging/production enforce Secure=true while local dev can use false.
In `@apps/api-user/src/main/java/com/ott/api_user/config/SecurityConfig.java`:
- Around line 59-75: The logout endpoint "/auth/logout" is currently listed in
the .requestMatchers permitAll block which weakens authorization; remove
"/auth/logout" from that permitAll list in SecurityConfig and ensure logout
requires an authenticated/member user (so it falls under
.anyRequest().hasRole("MEMBER") or change to .anyRequest().authenticated() if
preferred); update the requestMatchers array (inside SecurityConfig) to omit
"/auth/logout" and verify the .anyRequest().hasRole("MEMBER") rule covers the
logout flow.
- Around line 46-49: SecurityConfig currently disables CSRF while
JwtAuthenticationFilter stores JWT in an HttpOnly cookie and
corsConfigurationSource() enables allowCredentials, which makes the app
vulnerable; change the .csrf(AbstractHttpConfigurer::disable) line in
SecurityConfig to enable CSRF and configure a CookieCsrfTokenRepository (use
CookieCsrfTokenRepository.withHttpOnlyFalse()) or alternatively ensure the JWT
cookie is set with SameSite=Lax/Strict and keep CSRF enabled, and verify
corsConfigurationSource() still calls setAllowCredentials(true) only if CSRF
protection is in place.
---
Nitpick comments:
In
`@apps/api-admin/src/main/java/com/ott/api_admin/auth/dto/response/AdminTokenResponse.java`:
- Around line 14-18: Update the `@Schema` annotations on
AdminTokenResponse.accessToken and AdminTokenResponse.refreshToken to use
JWT-like example strings (e.g.
"eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjMifQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c")
and remove the redundant type = "String" attribute so the annotation relies on
the field type; adjust the description text if needed to indicate these are
JWTs.
In `@apps/api-admin/src/main/java/com/ott/api_admin/config/SecurityConfig.java`:
- Around line 74-79: The current SecurityConfig uses
config.setAllowedOriginPatterns(List.of("http://localhost:*",
"https://www.openthetaste.cloud")) together with
config.setAllowCredentials(true), which dangerously allows credentials for a
wildcard localhost pattern in production; update SecurityConfig so localhost
origins are only added in non-prod (e.g., using an active Spring profile or an
env property) and production uses a fixed trusted origins list (no wildcard
patterns) before calling setAllowCredentials(true); specifically modify the code
paths around SecurityConfig (the config variable and the calls to
setAllowedOriginPatterns / setAllowedOrigins and setAllowCredentials) to
conditionally include "http://localhost:*" only when profile == "dev" (or a
flag), and ensure production uses explicit trusted domains only.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
apps/api-admin/src/main/java/com/ott/api_admin/auth/controller/AdminAuthApi.javaapps/api-admin/src/main/java/com/ott/api_admin/auth/controller/AdminAuthController.javaapps/api-admin/src/main/java/com/ott/api_admin/auth/dto/request/AdminLoginRequest.javaapps/api-admin/src/main/java/com/ott/api_admin/auth/dto/response/AdminLoginResponse.javaapps/api-admin/src/main/java/com/ott/api_admin/auth/dto/response/AdminTokenResponse.javaapps/api-admin/src/main/java/com/ott/api_admin/common/CookieUtil.javaapps/api-admin/src/main/java/com/ott/api_admin/config/SecurityConfig.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/CustomOAuth2UserService.javaapps/api-user/src/main/java/com/ott/api_user/auth/oauth2/handler/OAuth2SuccessHandler.javaapps/api-user/src/main/java/com/ott/api_user/auth/service/KakaoAuthService.javaapps/api-user/src/main/java/com/ott/api_user/common/CookieUtil.javaapps/api-user/src/main/java/com/ott/api_user/config/SecurityConfig.javaapps/api-user/src/main/java/com/ott/api_user/member/service/MemberService.javamodules/domain/src/main/java/com/ott/domain/member/domain/Member.javamodules/infra-db/src/main/resources/db/migration/V3__member_onboarding_completed.sql
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/api-user/src/main/java/com/ott/api_user/config/SecurityConfig.java (1)
46-51:⚠️ Potential issue | 🔴 CriticalCSRF 전역 비활성화는 현재 쿠키 인증 흐름에서 보안 취약점을 만듭니다.
Line [46]에서 CSRF를 끈 상태로 Line [105]에서 credential 포함 CORS를 허용하면, 브라우저 자동 쿠키 전송으로 상태 변경 요청이 CSRF에 노출됩니다.
🔧 제안 수정안
+import org.springframework.security.web.csrf.CookieCsrfTokenRepository; ... - .csrf(AbstractHttpConfigurer::disable) // csrf 비활성화, Authorization 헤더로 보냄 + .csrf(csrf -> csrf + .csrfTokenRepository(CookieCsrfTokenRepository.withHttpOnlyFalse())) .formLogin(AbstractHttpConfigurer::disable) // 카카오 OAtuh2 + JWT기반이라 기본 로그인 폼 안씀 .httpBasic(AbstractHttpConfigurer::disable) // 카카오 OAtuh2 + JWT기반이라 Basic 인증 안씀#!/bin/bash set -e echo "[1] SecurityConfig의 CSRF/CORS 설정 확인" rg -n "csrf\\(|setAllowCredentials|setAllowedOriginPatterns|sessionCreationPolicy" apps/api-user/src/main/java/com/ott/api_user/config/SecurityConfig.java echo "[2] JwtAuthenticationFilter의 쿠키 기반 토큰 추출 여부 확인" fd -t f "JwtAuthenticationFilter.java" | xargs -I{} sh -c 'echo "== {} =="; rg -n "Cookie|getCookies|access|token|Authorization" "{}"'Also applies to: 105-105
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-user/src/main/java/com/ott/api_user/config/SecurityConfig.java` around lines 46 - 51, SecurityConfig currently disables CSRF while allowing CORS credentials which exposes cookie-based flows; update SecurityConfig (csrf(...), cors.configurationSource(...), sessionManagement(...)) to one of two safe options: (A) re-enable CSRF and use CookieCsrfTokenRepository.withHttpOnlyFalse() (or default CookieCsrfTokenRepository) so state-changing requests require a CSRF token, and keep CORS credentials=true only for trusted origins via setAllowedOriginPatterns(...) restricted to your frontend origin; or (B) if you truly only use Authorization headers/JWT (no cookies), keep CSRF disabled but change the CORS config to setAllowCredentials(false) and ensure JwtAuthenticationFilter does not rely on cookie tokens; also audit JwtAuthenticationFilter for cookie-based extraction (getCookies / Cookie usage) and adjust accordingly. Ensure the chosen fix updates csrf(...) and cors.configurationSource() in SecurityConfig and any cookie logic in JwtAuthenticationFilter.
🧹 Nitpick comments (1)
apps/api-user/src/main/java/com/ott/api_user/config/SecurityConfig.java (1)
39-40: CORS 허용 오리진을 코드 하드코딩 대신 환경설정으로 외부화하세요.Line [100]-[102]의 하드코딩된 CORS 오리진 패턴이 배포/로컬 전환 시 drift를 유발합니다. Line [39]-[40]에 정의된
frontedUrl프로퍼티는 현재 미사용 상태이며, 이를 CORS 설정과 연결하여 환경별로 관리해야 합니다.♻️ 제안 리팩터
- `@Value`("${app.frontend-url:http://localhost:8080}") - private String frontedUrl; + `@Value`("#{'${app.cors.allowed-origin-patterns:http://localhost:*,https://www.openthetaste.cloud}'.split(',')}") + private List<String> allowedOriginPatterns; ... - config.setAllowedOriginPatterns(List.of( - "http://localhost:*", - "https://www.openthetaste.cloud")); + config.setAllowedOriginPatterns(allowedOriginPatterns);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-user/src/main/java/com/ott/api_user/config/SecurityConfig.java` around lines 39 - 40, Replace the hardcoded CORS origin pattern with the environment-backed property currently declared as frontedUrl: update the CORS configuration method (the method that registers allowed origins at lines ~100-102) to read the frontedUrl field instead of the literal string, support comma-separated values (split frontedUrl on commas and trim) so multiple origins can be configured, and ensure the `@Value` field name (frontedUrl) is used there (or rename to frontendUrl consistently across the class if you prefer) so deployments can control allowed origins via configuration.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/api-user/src/main/java/com/ott/api_user/config/SecurityConfig.java`:
- Around line 46-51: SecurityConfig currently disables CSRF while allowing CORS
credentials which exposes cookie-based flows; update SecurityConfig (csrf(...),
cors.configurationSource(...), sessionManagement(...)) to one of two safe
options: (A) re-enable CSRF and use
CookieCsrfTokenRepository.withHttpOnlyFalse() (or default
CookieCsrfTokenRepository) so state-changing requests require a CSRF token, and
keep CORS credentials=true only for trusted origins via
setAllowedOriginPatterns(...) restricted to your frontend origin; or (B) if you
truly only use Authorization headers/JWT (no cookies), keep CSRF disabled but
change the CORS config to setAllowCredentials(false) and ensure
JwtAuthenticationFilter does not rely on cookie tokens; also audit
JwtAuthenticationFilter for cookie-based extraction (getCookies / Cookie usage)
and adjust accordingly. Ensure the chosen fix updates csrf(...) and
cors.configurationSource() in SecurityConfig and any cookie logic in
JwtAuthenticationFilter.
---
Nitpick comments:
In `@apps/api-user/src/main/java/com/ott/api_user/config/SecurityConfig.java`:
- Around line 39-40: Replace the hardcoded CORS origin pattern with the
environment-backed property currently declared as frontedUrl: update the CORS
configuration method (the method that registers allowed origins at lines
~100-102) to read the frontedUrl field instead of the literal string, support
comma-separated values (split frontedUrl on commas and trim) so multiple origins
can be configured, and ensure the `@Value` field name (frontedUrl) is used there
(or rename to frontendUrl consistently across the class if you prefer) so
deployments can control allowed origins via configuration.
phonil
reviewed
Feb 28, 2026
|
|
||
| public void addCookie(HttpServletResponse response, String name, String value, int maxAge) { | ||
| ResponseCookie cookie = ResponseCookie.from(name, value) | ||
| // .domain("openthetaste.cloud") // 로컬 테스트 시 주석처리!!! |
Contributor
There was a problem hiding this comment.
도메인 주소는 yml로 빼서 주입 받아 사용합시다!!
Collaborator
Author
There was a problem hiding this comment.
밑에 주신 피드백 반영해서 공통모듈로 변경했고
그로 인해서 yml을 통한 주입이 불가능해졌습니다
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
modules/common-security/src/main/java/com/ott/common/security/util/CookieUtil.java (1)
15-15:⚠️ Potential issue | 🔴 Critical운영 환경에서도
secure(false)가 고정되어 인증 쿠키 탈취 위험이 큽니다.
accessToken/refreshToken쿠키가 HTTP로도 전송될 수 있어 보안 리스크가 큽니다.Secure와SameSite를 환경값으로 분리해 주세요.🔧 제안 수정안
import jakarta.servlet.http.HttpServletResponse; +import org.springframework.beans.factory.annotation.Value; import org.springframework.http.HttpHeaders; import org.springframework.http.ResponseCookie; import org.springframework.stereotype.Component; `@Component` public class CookieUtil { + `@Value`("${app.cookie.secure:true}") + private boolean secureCookie; + + `@Value`("${app.cookie.same-site:Lax}") + private String sameSite; + public void addCookie(HttpServletResponse response, String name, String value, int maxAge) { ResponseCookie cookie = ResponseCookie.from(name, value) .httpOnly(true) - .secure(false) + .secure(secureCookie) .path("/") .maxAge(maxAge) - .sameSite("Lax") + .sameSite(sameSite) .build(); response.addHeader(HttpHeaders.SET_COOKIE, cookie.toString()); } public void deleteCookie(HttpServletResponse response, String name) { ResponseCookie cookie = ResponseCookie.from(name, "") .httpOnly(true) - .secure(false) + .secure(secureCookie) .path("/") .maxAge(0) - .sameSite("Lax") + .sameSite(sameSite) .build(); response.addHeader(HttpHeaders.SET_COOKIE, cookie.toString()); } }Also applies to: 27-27
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/common-security/src/main/java/com/ott/common/security/util/CookieUtil.java` at line 15, CookieUtil currently hardcodes .secure(false) (and SameSite) which allows auth cookies to be sent over HTTP; change CookieUtil to read Secure and SameSite values from configuration/environment and apply them when creating cookies (e.g., in methods that build accessToken and refreshToken cookies such as setAccessTokenCookie/setRefreshTokenCookie or wherever .secure(...) and SameSite are set). Implement a boolean flag (e.g., property cookie.secure or environment variable) used to set .secure(true/false) and a string property for SameSite (Lax/Strict/None) and ensure the code enforces Secure=true for production profiles; update both occurrences (the .secure(false) at the shown location and the other at the second occurrence) to use these config values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/api-admin/src/main/java/com/ott/api_admin/auth/controller/AdminAuthController.java`:
- Around line 68-69: SecurityConfig currently allows anonymous access to
"/back-office/reissue" via .permitAll(), so remove that permitAll and require
authentication for that path; update the HttpSecurity configuration (the method
in SecurityConfig that defines antMatchers) to change the rule for
"/back-office/reissue" to require authentication/roles (e.g.,
.antMatchers("/back-office/reissue").hasAnyRole("ADMIN","EDITOR") or at minimum
.authenticated()) so only authenticated users can call the AdminAuthController
reissue endpoint; ensure the controller method signature (reissue) continues to
accept Authentication if role checks rely on it.
---
Duplicate comments:
In
`@modules/common-security/src/main/java/com/ott/common/security/util/CookieUtil.java`:
- Line 15: CookieUtil currently hardcodes .secure(false) (and SameSite) which
allows auth cookies to be sent over HTTP; change CookieUtil to read Secure and
SameSite values from configuration/environment and apply them when creating
cookies (e.g., in methods that build accessToken and refreshToken cookies such
as setAccessTokenCookie/setRefreshTokenCookie or wherever .secure(...) and
SameSite are set). Implement a boolean flag (e.g., property cookie.secure or
environment variable) used to set .secure(true/false) and a string property for
SameSite (Lax/Strict/None) and ensure the code enforces Secure=true for
production profiles; update both occurrences (the .secure(false) at the shown
location and the other at the second occurrence) to use these config values.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
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.javamodules/common-security/src/main/java/com/ott/common/security/util/CookieUtil.java
yubin012
reviewed
Mar 1, 2026
| // .domain("openthetaste.cloud") // 로컬 테스트 시 주석처리!!! | ||
| .httpOnly(true) // JS 접근 차단 -> 크로스 사이트 스크립트 공격 대비 | ||
| .secure(false) // HTTPS 요청만 허용 | ||
| .path("/") // 모든 경로로 전송 |
Contributor
There was a problem hiding this comment.
현재는 secure(false)로 HTTP에서도 쿠키 전송이 가능한거 같은데 주석을 이렇게 단 이유가 궁금합니다!
Collaborator
Author
There was a problem hiding this comment.
pr 내용에 써놨듯이 배포환경으로 변경하려다가 이슈발생으로 다시 false처리 해놨습니다.
추후 인프라&프론트 협의 후 true로 변경 예정입니다.
…o OT-144-fix/OAuth2-kakao
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/api-user/src/main/java/com/ott/api_user/bookmark/controller/BookmarkAPI.java (1)
66-71:⚠️ Potential issue | 🔴 Critical
@AuthenticationPrincipal어노테이션 누락
getBookmarkShortFormList메서드의memberId파라미터에@AuthenticationPrincipal어노테이션이 누락되어 있습니다. 다른 메서드들(Line 38, 54)과 달리 이 어노테이션이 없으면 Spring이 인증된 사용자 ID를 주입하지 않아null이 전달됩니다.🐛 수정 제안
`@GetMapping`("/me/short-form") ResponseEntity<SuccessResponse<PageResponse<BookmarkShortFormResponse>>> getBookmarkShortFormList( `@Parameter`(description = "페이지 번호 (0부터 시작)") `@RequestParam`(defaultValue = "0") `@Min`(0) Integer page, `@Parameter`(description = "페이지 크기") `@RequestParam`(defaultValue = "10") `@Min`(0) `@Max`(100) Integer size, - `@Parameter`(hidden = true) Long memberId + `@Parameter`(hidden = true) `@AuthenticationPrincipal` Long memberId );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-user/src/main/java/com/ott/api_user/bookmark/controller/BookmarkAPI.java` around lines 66 - 71, The getBookmarkShortFormList API method is missing the `@AuthenticationPrincipal` annotation on the memberId parameter, so Spring won't inject the authenticated user ID; add `@AuthenticationPrincipal` to the Long memberId parameter in the getBookmarkShortFormList method declaration (matching other methods such as those on lines 38 and 54) so the authenticated principal is injected correctly.
🧹 Nitpick comments (1)
apps/api-user/src/main/java/com/ott/api_user/auth/controller/AuthController.java (1)
75-86:extractCookie메서드도 CookieUtil로 이동 고려
addCookie와deleteCookie는CookieUtil로 위임했지만,extractCookie는 여전히 로컬 메서드로 남아있습니다. 일관성을 위해 이 메서드도CookieUtil로 이동하는 것을 권장합니다. 다른 컨트롤러에서도 쿠키 추출이 필요할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api-user/src/main/java/com/ott/api_user/auth/controller/AuthController.java` around lines 75 - 86, Move the cookie-extraction logic from AuthController into CookieUtil to keep cookie handling consistent: create a public static method (e.g., CookieUtil.extractCookie(HttpServletRequest request, String name)) that implements the current logic from AuthController.extractCookie, update AuthController to call CookieUtil.extractCookie instead, and remove the now-unused private extractCookie method; ensure imports and any other controllers use the new CookieUtil method where appropriate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@apps/api-user/src/main/java/com/ott/api_user/bookmark/controller/BookmarkAPI.java`:
- Around line 66-71: The getBookmarkShortFormList API method is missing the
`@AuthenticationPrincipal` annotation on the memberId parameter, so Spring won't
inject the authenticated user ID; add `@AuthenticationPrincipal` to the Long
memberId parameter in the getBookmarkShortFormList method declaration (matching
other methods such as those on lines 38 and 54) so the authenticated principal
is injected correctly.
---
Nitpick comments:
In
`@apps/api-user/src/main/java/com/ott/api_user/auth/controller/AuthController.java`:
- Around line 75-86: Move the cookie-extraction logic from AuthController into
CookieUtil to keep cookie handling consistent: create a public static method
(e.g., CookieUtil.extractCookie(HttpServletRequest request, String name)) that
implements the current logic from AuthController.extractCookie, update
AuthController to call CookieUtil.extractCookie instead, and remove the
now-unused private extractCookie method; ensure imports and any other
controllers use the new CookieUtil method where appropriate.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/api-user/src/main/java/com/ott/api_user/auth/controller/AuthController.javaapps/api-user/src/main/java/com/ott/api_user/auth/service/KakaoAuthService.javaapps/api-user/src/main/java/com/ott/api_user/bookmark/controller/BookmarkAPI.java
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/api-user/src/main/java/com/ott/api_user/auth/service/KakaoAuthService.java
phonil
approved these changes
Mar 1, 2026
7 tasks
This was referenced Mar 4, 2026
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.
📝 작업 내용
📷 스크린샷
☑️ 체크 리스트
#️⃣ 연관된 이슈
close #85
💬 리뷰 요구사항
필독 사항
기존 토큰 생성 후 백엔드 -> 백엔드 리다이렉션 시켰던 것을 백엔드 -> 프론트 리다이렉션으로 변경했습니다.
백엔드 분들은 신경 안쓰셔도되는데 프론트쪽에서 API 연동 시 필요해 보여서 변경했습니다.
변경된 env는 노션 참고 해주시기 바랍니다.
Summary by CodeRabbit
새로운 기능
버그 수정
문서화
기타