인증 로직 수정 #355
인증 로직 수정 #355
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough클라이언트·서버용 통합 API 클라이언트들(clientApiClient, serverApiClient, backendApiClient)을 추가하고 인증 BFF 엔드포인트용 공개 함수(fetchUserInfo, logout) 및 Next.js 라우트(GET /api/auth/me, POST /api/auth/logout)를 도입했습니다. 기존 SafeFetch 호출을 backendApiClient로 대체하고 일부 사용자 관련 API/훅(userApi, useUsers)을 삭제·재구성했으며, 쿠키 기반 사용자 정보 흐름(useUserInfo, getUserInfoFromCookie)과 클라이언트 로그아웃 훅(useLogout)을 추가해 UI의 로그아웃 경로와 연동했습니다. Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/apis/report.ts (1)
4-7:⚠️ Potential issue | 🟠 MajorPR 목표와 상충: 하드코딩된
API_TOKEN이 여전히 남아 있습니다.PR 목표에 "Remove hardcoded API_TOKEN"이 명시되어 있으나, 이 파일은 여전히
NEXT_PUBLIC_API_TOKEN환경 변수를 사용하여Authorization헤더를 직접 구성하고 있습니다. 다른 API 모듈(chatApi.ts)처럼backendApiClient또는serverApiClient를 통해 인증을 처리하도록 마이그레이션이 필요합니다.src/components/layout/SideNavigation/components/Bottom/SideNavigationBottom.tsx (1)
26-36:⚠️ Potential issue | 🟡 Minor
close렌더 프롭이 사용되지 않아 로그아웃 클릭 시 Popover가 닫히지 않습니다.
PopoverContent의close콜백이handleLogout()호출 시 실행되지 않아, 로그아웃 진행 중에도 팝오버가 열려 있을 수 있습니다.제안
- onClick={() => handleLogout()} + onClick={() => { + close(); + handleLogout(); + }}
🤖 Fix all issues with AI agents
In `@src/apis/authApi.ts`:
- Around line 10-14: The interface UserInfoResponse declares data as
non-optional but the runtime checks (e.g., the usage of response.data with a
falsy check) indicate data can be absent; update the UserInfoResponse type (the
interface named UserInfoResponse in src/apis/authApi.ts) to reflect
optional/nullable data (e.g., data?: UserInfo | null) and then audit usages that
reference response.data (the falsy check at the call site) to ensure they handle
the undefined/null case safely (keep the existing !response.data logic or
replace with explicit null/undefined checks).
- Around line 3-8: 중복된 UserInfo 타입과 불일치한 응답 타입, 중복 에러 처리를 정리하세요: UserInfo 인터페이스를
src/types/auth.ts 같은 공유 타입 파일로 추출하고 src/apis/authApi.ts와
src/app/api/auth/me/route.ts에서 import하도록 변경하며, UserInfoResponse 타입의 data 필드를
data?: UserInfo | null 또는 data: UserInfo | null로 API 응답 스키마에 맞게 수정하세요(참조:
UserInfoResponse, UserInfo). 또한 fetchUserInfo 함수에서 현재 response.success 체크 후
throw하는 일반 Error를 제거하거나 clientApiClient가 던지는 ApiError를 일관되게 처리하도록 단일화하고, 필요하면
fetchUserInfo는 clientApiClient의 예외를 그대로 전달하거나 ApiError로 래핑하도록 정리하세요(참조:
fetchUserInfo, clientApiClient, ApiError).
In `@src/hooks/useLogout.ts`:
- Around line 10-19: The logout mutation should clear React Query cache to avoid
leaking previous user's cached data; update useLogout (the useMutation call
around mutationFn: logout and handlers) to obtain the QueryClient via
useQueryClient and on both onSuccess and onError call the appropriate
cache-clear method (e.g., queryClient.clear() or queryClient.removeQueries()
including ['userInfo']) before calling router.push('/'), ensuring all
user-related queries are reset prior to navigation.
In `@src/hooks/useUserInfo.ts`:
- Around line 28-39: The JSDoc claiming "SSR 지원" is incorrect because
getCookieUtil returns null when typeof window === 'undefined', so update either
the documentation or implementation: either change the JSDoc on
getUserInfoFromCookie to remove/clarify SSR support, or make
getUserInfoFromCookie SSR-capable by delegating to a server-side cookie reader
(e.g., accept a request/context or call a server-aware utility) and handle
COOKIES_KEYS.USER_INFO accordingly; locate the function getUserInfoFromCookie
and the call to getCookieUtil to apply the chosen fix.
In `@src/lib/client/apiClient.ts`:
- Around line 34-36: The success path currently returns response.json()
unguarded which can throw on empty or non-JSON bodies; replace that direct call
with a guarded parse: if response.status === 204 or
response.headers.get('content-length') === '0' return an empty object, otherwise
attempt response.json() but catch parsing errors and return a safe fallback
(e.g. {}) — i.e. wrap the response.json() call in a .catch(() => ({})) or
perform response.text() then try JSON.parse with a try/catch; update the code
around the existing response.json() return to use this guarded logic referencing
the same response variable and .json() call.
In `@src/lib/client/backendClient.ts`:
- Around line 49-51: The success path calls response.json() without handling
JSON parse failures; add robust parsing around response.json() (e.g., try/catch
or parsing via response.text() then JSON.parse with fallback) in the function
that currently returns response.json(), so that on invalid JSON it returns the
same safe fallback used in the error path (an empty object) — mirror the fix
applied to clientApiClient and update the return from response.json() to a
guarded parse that falls back to {} on parse errors.
In `@src/lib/server/apiClient.ts`:
- Around line 50-52: The success path currently returns response.json() directly
(seen in the function that ends with "return response.json();") which lacks a
safe fallback like the other clients; add a shared JSON-parsing utility (e.g.,
parseJsonResponse or safeParseJson) and replace direct response.json() calls in
clientApiClient and backendApiClient and this file's API client to use it; the
utility should attempt response.json() in a try/catch and return an empty object
or null (consistent with existing clients) on parse errors, and update the
callers to expect that fallback.
- Around line 25-31: The serverApiClient currently reads the token via cookies()
using COOKIES_KEYS.ACCESS_TOKEN; confirm the backend's HTTPOnly cookie name and
update the constant value (COOKIES_KEYS.ACCESS_TOKEN) in your cookie constants
so it exactly matches the backend name, then run a quick integration/manual test
to ensure serverApiClient(endpoint, options) finds the token and no longer
throws ApiError(401); optionally add a short fallback check for any alternate
cookie name(s) if the backend may vary.
🧹 Nitpick comments (11)
src/lib/client/apiClient.ts (1)
1-10:ApiError클래스가src/lib/server/apiClient.ts와 중복됩니다.클라이언트와 서버 모듈에 동일한 이름의
ApiError클래스가 각각 정의되어 있습니다. 소비하는 쪽에서 잘못된 모듈을 import 하면instanceof체크가 실패합니다. 공통 에러 모듈(예:src/lib/errors.ts)로 통합하는 것을 권장합니다.src/lib/client/backendClient.ts (1)
34-39: 401 처리에서window.location.href할당은 API 클라이언트의 관심사가 아닙니다.API 클라이언트 내에서 하드 네비게이션을 수행하면 호출자의 에러 핸들링 로직(예: cleanup, 상태 롤백)이 실행되지 않을 수 있습니다. 또한 테스트 환경에서 side-effect가 발생하여 테스트가 어려워집니다.
BackendApiError(401, ...)throw만 하고, 리다이렉트 로직은 호출 측이나 글로벌 에러 바운더리에서 처리하는 것을 권장합니다.제안
if (response.status === 401) { - if (typeof window !== 'undefined') { - window.location.href = '/?error=session_expired'; - } throw new BackendApiError(401, 'Unauthorized'); }그리고 호출 측 또는 글로벌 에러 핸들러에서:
catch (err) { if (err instanceof BackendApiError && err.status === 401) { window.location.href = '/?error=session_expired'; } }src/lib/server/apiClient.ts (1)
33-40:options.headersspread가Authorization헤더를 덮어쓸 수 있습니다.현재 spread 순서(
Authorization→...options.headers)로 인해 호출 측에서Authorization을 override 할 수 있습니다. 의도적인 설계라면 JSDoc에 명시하고, 아니라면 순서를 변경하세요.src/hooks/useCookie.ts (3)
62-69:deleteCookieUtil내typeof window !== 'undefined'체크가 중복됩니다.Line 63에서 이미
typeof window === 'undefined'이면 early return 하므로, Line 65의 동일한 체크는 항상true입니다.제안
export const deleteCookieUtil = (key: string, path: string = '/') => { if (typeof window === 'undefined') return; - const shouldBeSecure = typeof window !== 'undefined' && window.location.protocol === 'https:'; + const shouldBeSecure = window.location.protocol === 'https:'; const secureFlag = shouldBeSecure ? ';Secure' : ''; document.cookie = `${key}=;expires=Thu, 01 Jan 1970 00:00:00 GMT;path=${path};SameSite=Lax${secureFlag}`; };
13-15: JSDoc 코멘트가 오해의 소지가 있습니다."서버/클라이언트 모두 사용 가능"이라고 되어 있지만, 서버에서는 항상
null을 반환합니다. "클라이언트 전용, 서버에서는 안전하게 null 반환"으로 수정하는 것이 정확합니다.
30-57:setCookieUtil의 쿠키 속성 구분자에 공백이 없습니다.현재
;max-age=...,;path=...형태로 세미콜론 뒤에 공백 없이 작성되고 있습니다. RFC 6265에서는;뒤에 공백을 허용하며, 대부분의 구현체에서; max-age=...형태를 사용합니다. 동작에는 큰 문제가 없지만 관례적 형식과 맞추면 좋겠습니다.src/components/layout/SideNavigation/components/Bottom/SideNavigationBottom.tsx (1)
22-23:user데이터가 있는데 "User Name"이 아직 하드코딩되어 있습니다.
useUserInfo로 사용자 데이터를 가져오고 있으므로, 이제 하드코딩된 라벨을 실제 사용자 이름으로 교체할 수 있습니다.제안
- <NavItem label="User Name" icon="IC_Logo" ariaLabel="사용자 메뉴 버튼" /> + <NavItem label={user.name ?? 'User'} icon="IC_Logo" ariaLabel="사용자 메뉴 버튼" />Based on learnings: "the hardcoded 'User Name' label is intentional and temporary. It will be replaced with real user profile data after the login server stabilizes." — 이제 로그인 서버가 안정화되었으므로 교체 시점입니다.
src/app/api/auth/logout/route.ts (1)
8-31: 쿠키 삭제 로직이 try/catch에 중복되어 있습니다.finally블록으로 통합하세요.제안
export async function POST() { + const cookieStore = await cookies(); try { await serverApiClient('/v1/auth/logout', { method: 'POST', }); - const cookieStore = await cookies(); - cookieStore.delete(COOKIES_KEYS.ACCESS_TOKEN); - cookieStore.delete(COOKIES_KEYS.USER_INFO); - return NextResponse.json({ success: true }); } catch (error) { - const cookieStore = await cookies(); - cookieStore.delete(COOKIES_KEYS.ACCESS_TOKEN); - cookieStore.delete(COOKIES_KEYS.USER_INFO); - if (error instanceof ApiError) { return NextResponse.json({ error: error.message }, { status: error.status }); } return NextResponse.json({ error: 'Internal server error' }, { status: 500 }); + } finally { + cookieStore.delete(COOKIES_KEYS.ACCESS_TOKEN); + cookieStore.delete(COOKIES_KEYS.USER_INFO); } }src/app/api/auth/me/route.ts (1)
5-10:UserInfo인터페이스가src/apis/authApi.ts에도 동일하게 정의되어 있습니다.타입을 한 곳에서 정의하고 양쪽에서 import하여 DRY 원칙을 지켜주세요. 예를 들어
src/types/auth.ts같은 공유 위치로 이동하는 것을 권장합니다. 현재 route 파일은 서버 코드이고authApi.ts는 클라이언트 코드이므로, 별도의 공유 타입 파일이 적합합니다.src/app/LandingPage.tsx (1)
41-52: 에러 메시지 렌더링 로직을 맵 기반으로 리팩토링하면 유지보수가 편해집니다.새 에러 타입 추가 시 개별 조건문과 fallback의
includes배열 양쪽을 모두 수정해야 하므로 누락 위험이 있습니다.제안
+const ERROR_MESSAGES: Record<string, string> = { + auth_failed: '로그인에 실패했습니다.', + server_error: '서버 오류가 발생했습니다.', + unauthorized: '로그인이 필요합니다.', + session_expired: '세션이 만료되었습니다. 다시 로그인해주세요.', +}; {error && ( <div className="bg-red100 text-red700 mb-6 w-full rounded-lg p-4 text-sm"> - {error === 'auth_failed' && '로그인에 실패했습니다.'} - {error === 'server_error' && '서버 오류가 발생했습니다.'} - {error === 'unauthorized' && '로그인이 필요합니다.'} - {error === 'session_expired' && '세션이 만료되었습니다. 다시 로그인해주세요.'} - {!['auth_failed', 'server_error', 'unauthorized', 'session_expired'].includes(error) && - '오류가 발생했습니다.'} + {ERROR_MESSAGES[error] ?? '오류가 발생했습니다.'} <br /> 잠시 후 다시 시도해주세요. </div> )}src/apis/authApi.ts (1)
19-27: 이중 에러 처리 구조를 인지해 주세요.
clientApiClient가 non-2xx 응답에 대해ApiError를 throw하고, 여기서 추가로response.success검사 후 일반Error를 throw합니다. 호출하는 쪽에서 두 가지 에러 타입(ApiError,Error)을 모두 처리해야 합니다. 의도된 설계라면 괜찮지만, 호출부에서의 에러 핸들링이 복잡해질 수 있으니 확인해 주세요.
Bangdayeon
force-pushed
the
refactor/#353-modify-auth
branch
from
fa8d87d to
c2ea0a7
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@src/app/`(route)/home/_components/useCreateChatRoom.ts:
- Line 2: The catch block in the useCreateChatRoom hook must handle the new
BackendApiError thrown by chatApi.createChat via backendApiClient: update the
catch to test for BackendApiError (importing it) and branch on its status/code
(e.g., 401) instead of only FetchError/ParseError/TimeoutError so the 401 logic
in the hook runs; locate the catch inside the useCreateChatRoom function (the
code path that calls createChat) and replace or extend the existing error type
checks to handle BackendApiError appropriately and log/dispatch the same
responses the old branches did.
In `@src/app/LandingPage.tsx`:
- Around line 17-19: The Landing component uses useSearchParams(), which
requires a React Suspense boundary in Next.js App Router to avoid forcing full
client-side rendering; wrap the Landing component instantiation in the page
component (the exported default from page.tsx) with a Suspense boundary (import
Suspense from react) and provide a fallback UI (e.g., a simple loader) so
Landing remains a client component without converting the whole page to dynamic
rendering.
- Around line 48-54: The Tailwind class names in the error render inside the
LandingPage component are invalid (`bg-red100`, `text-red700`); update the error
JSX where `error && (...)` is returned to use valid Tailwind utilities (e.g.,
`bg-red-100 text-red-700`) or alternatively add those custom colors to
tailwind.config.ts; locate the conditional rendering block in LandingPage (the
`{error && ( ... )}` section) and replace the invalid classes with the corrected
utility names or configure the theme to expose `red100`/`red700`.
In `@src/hooks/useCookie.ts`:
- Around line 13-14: The JSDoc comment for the useCookie utility has a
duplicated closing parenthesis; update the comment string in the useCookie
declaration/comment block (the doc that currently reads "(클라이언트 전용, 서버에서는 항상
null 반환))") to remove the extra ")" so it becomes "(클라이언트 전용, 서버에서는 항상 null
반환)". Ensure you edit the comment above the useCookie function (or the
file-level comment in src/hooks/useCookie.ts) only—no code logic changes
required.
- Around line 30-57: The setCookieUtil function sets secure based on NODE_ENV,
which causes cookies with sameSite: 'None' to be rejected in non-production;
update setCookieUtil so that if options.sameSite === 'None' (or sameSite
variable equals 'None') it forces secure to true (override the computed default)
before building cookieString (or alternatively throw a clear error/warn if
secure is false when sameSite is 'None'); modify the default/assignment logic
around secure in setCookieUtil (and reference CookieOptions/sameSite/secure
variables) to ensure Secure is always present when SameSite=None.
In `@src/lib/errors/ApiError.ts`:
- Around line 1-11: The ApiError class duplicates BackendApiError (same status,
message, data) — remove the duplicate by replacing BackendApiError usage with
the shared ApiError: export and keep the existing ApiError class in
src/lib/errors/ApiError.ts, then update backendClient.ts to import ApiError and
remove or delete the BackendApiError definition and any references to that class
name, ensuring all thrown instances and type annotations use ApiError instead of
BackendApiError and adjust imports accordingly.
🧹 Nitpick comments (5)
src/app/LandingPage.tsx (1)
25-29:useEffect의console.error는 개발 디버깅 용도로만 유효에러가 이미 UI에 표시되고 있으므로 이
useEffect는 개발 시 디버깅 외 실질적 역할이 없습니다. 프로덕션에서는 외부 에러 트래킹 서비스(e.g. Sentry) 연동을 고려해 보세요.src/components/layout/SideNavigation/components/SideNavBottom/SideNavBottom.tsx (1)
7-16: 하드코딩된 사용자명을 실제 데이터로 교체한 점 좋습니다.이전 학습 내용에 따르면, 하드코딩된 "User Name"은 로그인 서버 안정화 후 교체 예정이었으며, 이번 변경으로 적절히 반영되었습니다.
user.name ?? 'User'폴백도 적절합니다.Line 29의 로그아웃
alert()TODO는 이 PR에서 추가된useLogout훅으로 교체가 필요해 보입니다.로그아웃 버튼에
useLogout훅을 연결하는 코드를 생성해 드릴까요?src/lib/client/backendClient.ts (1)
33-36: 401 응답 시 에러 본문을 파싱하지 않고 바로 throw합니다.백엔드가 401 응답에 유용한 정보(예: 토큰 만료 vs 권한 부족)를 포함할 수 있으므로, 다른 에러 경로와 동일하게 본문을 파싱한 후 throw하는 것이 디버깅에 유리합니다.
♻️ 수정 제안
if (res.status === 401) { - throw new BackendApiError(401, 'Unauthorized'); + const errorData = await res.json().catch(() => ({})); + throw new BackendApiError(401, errorData.message || 'Unauthorized', errorData); }src/lib/server/apiClient.ts (1)
6-10:NEXT_PUBLIC_접두사가 서버 전용 클라이언트에 불필요합니다.이 파일은 서버(API Routes)에서만 사용되므로
NEXT_PUBLIC_BASE_API_URL대신BASE_API_URL같은 서버 전용 환경변수를 사용하는 것이 좋습니다.NEXT_PUBLIC_접두사는 클라이언트 번들에 값을 노출하므로, 백엔드 URL이 클라이언트에 불필요하게 인라인될 수 있습니다.src/hooks/useCookie.ts (1)
62-69:Secure플래그 결정 로직이setCookieUtil과 일관적이지 않습니다.
setCookieUtil은process.env.NODE_ENV로,deleteCookieUtil은window.location.protocol로Secure여부를 판단합니다. HTTPS 프록시를 사용하는 개발 환경 등에서 set/delete 간Secure플래그 불일치로 쿠키 삭제가 실패할 수 있습니다. 동일한 기준을 사용하는 것이 안전합니다.
Bangdayeon
force-pushed
the
refactor/#353-modify-auth
branch
from
c2ea0a7 to
f558195
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@src/app/api/auth/me/route.ts`:
- Around line 7-33: The GET handler currently trusts USER_INFO cookie without
validating the ACCESS_TOKEN; update the GET function to call the backend auth
endpoint (e.g., POST/GET /v1/auth/me or whichever internal auth validation API)
using COOKIES_KEYS.ACCESS_TOKEN to verify the token and retrieve authoritative
user data instead of parsing COOKIES_KEYS.USER_INFO, handle token-invalid
responses by returning 401 via NextResponse.json, and only return the
backend-provided user object on success while keeping existing JSON error
handling for parse/network errors.
- Around line 21-23: The code parses userInfoStr with JSON.parse and blindly
casts to User (in the try block where const userInfo: User =
JSON.parse(decodeURIComponent(userInfoStr))); add runtime validation of the
parsed object before treating it as a User: after parsing, verify required
fields (e.g., id is a non-empty string/number and name is a string) or use a
lightweight validator/schema (e.g., isValidUser(obj) or a Zod/interface check)
and handle invalid shapes by throwing or returning a 400/unauthorized response;
update any downstream use of userInfo to assume it is validated.
In `@src/components/layout/SideNavigation/SideNavigation.tsx`:
- Line 3: getUserInfoFromCookie 호출 결과인 userInfo가 선언만 되고 사용되지 않으니 불필요한 dead code를
제거하세요: 삭제 대상은 import { getUserInfoFromCookie } from '@/hooks/useUserInfo'와 파일
내에서 호출/할당하는 userInfo 선언(및 중복 호출이 있다면 그 복사본, 예: 14번째 줄 근처)을 모두 제거하고,
SideNavigationBottom은 이미 내부적으로 useUserInfo()를 사용하므로 다른 변경은 하지 마세요.
In `@src/lib/client/backendClient.ts`:
- Around line 1-5: Remove the module-level throw that reads API_BASE_URL at
import time and instead read and validate the env var at runtime inside the
function that needs it (e.g., add a getApiBaseUrl() or move the check into
createBackendClient/fetchBackend functions); replace the top-level const
API_BASE_URL usage with a runtime getter that throws a descriptive Error when
NEXT_PUBLIC_BASE_API_URL is missing so imports no longer crash the build or
tests.
🧹 Nitpick comments (3)
src/components/layout/SideNavigation/components/Bottom/SideNavigationBottom.tsx (1)
25-36: Popover의close콜백이 호출되지 않습니다.Line 26의 render prop으로 전달받은
close가handleLogout()호출 시 사용되지 않습니다. 로그아웃 성공 후router.push('/')로 리다이렉트되므로 실질적 문제는 작지만, 로그아웃 실패 시에도 리다이렉트하는 현재useLogout구현과 맞물려 popover가 닫히지 않는 순간이 생길 수 있습니다.♻️ close 콜백 활용 제안
onClick={() => { + close(); handleLogout(); }}src/lib/client/backendClient.ts (1)
33-36: 401 응답의 에러 본문이 무시됩니다.401 응답에도 토큰 만료 사유 등 유용한 정보가 포함될 수 있습니다. 현재 에러 본문을 파싱하지 않고 바로 throw하므로, 호출자가 구체적인 원인을 알 수 없습니다.
제안: 일반 에러 처리와 통합
- if (res.status === 401) { - throw new BackendApiError(401, 'Unauthorized'); - } - if (!res.ok) { const errorData = await res.json().catch(() => ({})); + if (res.status === 401) { + throw new BackendApiError(401, errorData.message || 'Unauthorized', errorData); + } throw new BackendApiError(src/apis/chatApi.ts (1)
11-50: 에러 타입이 호출 계층과 불일치합니다.
backendApiClient는BackendApiError를 throw하지만, 비즈니스 로직 검증 실패 시 일반Error를 throw합니다. 호출자가 에러 타입으로 분기 처리할 때 혼란이 생길 수 있습니다.BackendApiError를 사용하거나, 별도의 비즈니스 에러 클래스 사용을 고려해 주세요.
Bangdayeon
force-pushed
the
refactor/#353-modify-auth
branch
from
f558195 to
672c017
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/app/LandingPage.tsx`:
- Around line 20-22: handleGoogleLogin currently builds a redirect URL using
process.env.NEXT_PUBLIC_BASE_API_URL which, if undefined, sends users to an
invalid "undefined/..." URL; update the logic in the LandingPage component and
the handleGoogleLogin function to guard against a missing
NEXT_PUBLIC_BASE_API_URL by (1) checking the env variable at runtime inside
handleGoogleLogin and returning early while showing a user-facing error/toast if
it's missing, and (2) disabling the Google login button (and showing a tooltip
or inline message) when process.env.NEXT_PUBLIC_BASE_API_URL is falsy so the UI
prevents clicks; reference the handleGoogleLogin function and the Google login
button render to implement these changes.
In `@src/hooks/useCookie.ts`:
- Around line 57-64: The deleteCookieUtil currently hardcodes SameSite=Lax which
prevents removal of cookies set with SameSite=None; update deleteCookieUtil to
accept an optional sameSite parameter (e.g., sameSite: 'Lax' | 'Strict' | 'None'
= 'Lax') and use that value in the cookie string, ensuring you also include the
Secure flag when sameSite === 'None' (or when window.location.protocol ===
'https:') so the constructed cookie (document.cookie =
`${key}=;expires=...;path=${path};SameSite=${sameSite}${secureFlag}`) matches
the original attributes and allows proper deletion.
In `@src/lib/client/backendClient.ts`:
- Around line 33-36: The 401 handling in backendClient.ts is inconsistent with
the PR description; either implement the centralized "redirect to login"
behavior there or ensure all callers handle 401 uniformly. Update
BackendApiError handling in backendClient.ts so that when res.status === 401 it
performs the login redirect (e.g., set window.location.href or call a shared
authRedirect helper) before throwing, or alternatively change BackendApiError to
include a specific code/flag (e.g., error.type = 'UNAUTHORIZED') and update
callers like useCreateChatRoom, ChatApiDemo.tsx, and createReport to catch that
error.type and perform the redirect to the login page; reference
BackendApiError, backendClient.ts, useCreateChatRoom.ts, ChatApiDemo.tsx, and
createReport when making the change.
🧹 Nitpick comments (5)
src/lib/constants/cookies.ts (1)
1-5: 쿠키 키 값의 네이밍 컨벤션이 일관되지 않습니다.
ACCESS_TOKEN과REFRESH_TOKEN은 camelCase('accessToken','refreshToken')를 사용하고,USER_INFO는 snake_case('user_info')를 사용하고 있습니다. 하나의 컨벤션으로 통일하면 유지보수 시 혼동을 줄일 수 있습니다.src/lib/server/apiClient.ts (1)
24-31:options.headers가Authorization헤더를 덮어쓸 수 있습니다.현재 스프레드 순서에서
...options.headers가Authorization앞에 위치해 있어, 호출자가 의도치 않게Authorization을 전달하면 서버가 설정한 토큰이 덮어쓰여질 수 있습니다. 반대로, 호출자가Content-Type을 커스텀하려 해도 고정된 기본값에 의해 무시됩니다.♻️ 헤더 스프레드 순서 조정 제안
const response = await fetch(`${API_BASE_URL}${endpoint}`, { ...options, headers: { 'Content-Type': 'application/json', - ...options.headers, - Authorization: `Bearer ${token}`, + Authorization: `Bearer ${token}`, + ...options.headers, }, });이렇게 하면 호출자가 필요 시
Content-Type을 오버라이드할 수 있으면서,Authorization은 기본값으로 제공되되 호출자가 의도적으로 변경할 수도 있습니다. 만약Authorization을 절대 오버라이드하지 못하게 하려면 현재 순서가 맞지만,Content-Type커스텀이 불가능한 점은 주의하세요.src/apis/authApi.ts (1)
14-16:ApiError생성 시 status200은 오해의 소지가 있습니다.HTTP 응답 자체는 200이지만 비즈니스 로직 실패를 나타내는 상황입니다. 에러 핸들러에서
status로 분기하는 경우(예: 401 → 로그인 페이지 리다이렉트) 200이면 의도와 다르게 동작할 수 있습니다. 비즈니스 로직 에러임을 명확히 할 수 있는 별도 status 코드나 커스텀 에러 클래스를 고려해 보세요.예시
- throw new ApiError(200, response.message || 'Failed to fetch user info', response); + throw new ApiError(422, response.message || 'Failed to fetch user info', response);또는 비즈니스 로직 에러 전용 클래스를 만드는 방법도 있습니다.
src/lib/client/backendClient.ts (2)
3-12:BackendApiError와ApiError가 구조적으로 중복됩니다.
src/lib/errors/ApiError.ts에 이미 동일한 구조(status,message,data)의ApiError클래스가 존재합니다. 두 에러 클래스를 유지하면 catch 블록에서 양쪽 모두 처리해야 하는 번거로움이 생깁니다. 하나로 통합하거나,BackendApiError를ApiError의 서브클래스로 만드는 것을 고려해 보세요.서브클래스 방식 예시
-export class BackendApiError extends Error { - constructor( - public status: number, - message: string, - public data?: unknown - ) { - super(message); - this.name = 'BackendApiError'; - } -} +import { ApiError } from '@/lib/errors/ApiError'; + +export class BackendApiError extends ApiError { + constructor(status: number, message: string, data?: unknown) { + super(status, message, data); + this.name = 'BackendApiError'; + } +}
19-30:credentials: 'include'가 항상 강제 적용됩니다.
...options뒤에credentials: 'include'가 위치하여, 호출부에서 다른 credentials 모드를 전달해도 무시됩니다. 인증 클라이언트의 의도된 동작이라면 문제없지만, 이 점을 JSDoc에 명시해 두면 사용자 혼란을 줄일 수 있습니다.
Bangdayeon
force-pushed
the
refactor/#353-modify-auth
branch
from
672c017 to
20be13d
Compare
Bangdayeon
force-pushed
the
refactor/#353-modify-auth
branch
from
20be13d to
aa5c15f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hooks/useCookie.ts (1)
73-101:⚠️ Potential issue | 🟡 Minor
useCookie훅의deleteCookie에서sameSite파라미터가 전달되지 않습니다.
deleteCookieUtil이sameSite파라미터를 지원하도록 개선되었지만,useCookie훅의deleteCookie콜백에서는path만 전달하고sameSite를 전달하지 않아 항상 기본값'Lax'가 사용됩니다.sameSite: 'None'으로 설정된 쿠키를 이 훅을 통해 삭제하면 실패할 수 있습니다.수정 제안
const deleteCookie = useCallback( - (path: string = '/') => { - deleteCookieUtil(key, path); + (path: string = '/', sameSite: 'Strict' | 'Lax' | 'None' = 'Lax') => { + deleteCookieUtil(key, path, sameSite); setCookieState(null); }, [key] );
🤖 Fix all issues with AI agents
In
`@src/components/layout/SideNavigation/components/Bottom/SideNavigationBottom.tsx`:
- Around line 11-17: The component SideNavigationBottom returns a bare <div>
when isLoading and null when !user, causing the surrounding layout classes
(bg-gray50 mt-auto shrink-0) to be missing and the side nav to shift; update the
render paths inside SideNavigationBottom so both the loading and empty-user
states still return the same outer container element that uses the existing
classes (bg-gray50 mt-auto shrink-0) and only change the inner content (e.g., a
spinner/skeleton or an empty placeholder) while keeping the container structure
consistent; locate the conditional checks for isLoading and user inside
SideNavigationBottom and wrap their returned content in the same container
element used for the normal state.
🧹 Nitpick comments (5)
src/lib/server/apiClient.ts (1)
6-10:NEXT_PUBLIC_접두사는 서버 전용 모듈에 불필요합니다.이 파일은 서버 사이드(Next.js API Routes)에서만 사용되므로
NEXT_PUBLIC_접두사 없이BASE_API_URL같은 이름을 사용하는 것이 적절합니다. 현재 접두사가 붙어 있으면 해당 값이 클라이언트 번들에도 노출됩니다. TODO가 이미 있으니 머지 전에 처리해 주세요.♻️ 환경변수명 변경 제안
-const API_BASE_URL = process.env.NEXT_PUBLIC_BASE_API_URL; // TODO: 환경변수 논의 후 BASE_API_URL로 변경 +const API_BASE_URL = process.env.BASE_API_URL; if (!API_BASE_URL) { - throw new Error('Missing environment variable: NEXT_PUBLIC_BASE_API_URL'); + throw new Error('Missing environment variable: BASE_API_URL'); }src/apis/authApi.ts (1)
14-16:ApiError생성 시 status 200은 의미적으로 부정확합니다.HTTP 요청 자체는 성공(200)했지만 비즈니스 로직이 실패한 경우입니다.
clientApiClient가 이미 HTTP 에러를ApiError로 throw하므로, 여기서도 200이 아닌 의미 있는 상태 코드를 사용하거나, 일반Error로 throw하는 것이 에러 핸들링 시 혼동을 줄입니다.제안
if (!response.success || !response.data) { - throw new ApiError(200, response.message || 'Failed to fetch user info', response); + throw new Error(response.message || 'Failed to fetch user info'); }또는 커스텀 비즈니스 에러 코드를 사용하세요 (예:
0또는 별도 에러 클래스).src/components/layout/SideNavigation/components/Bottom/SideNavigationBottom.tsx (1)
35-38:close()가 즉시 호출되어 로그아웃 진행 상태를 볼 수 없습니다.
handleLogout()은 비동기 mutation이므로 즉시 반환되고,close()가 바로 팝오버를 닫습니다. Line 30의'로그아웃 중...'라벨이 사용자에게 보이지 않게 됩니다. 의도된 동작이라면 무시하셔도 되지만, 진행 상태를 보여주려면onSuccess/onError콜백에서 닫는 것이 좋습니다.src/hooks/useUserInfo.ts (2)
14-21: React QuerystaleTime(5분)과 쿠키maxAge(24시간)의 불일치.
staleTime은 5분으로 설정되어 있어 데이터를 비교적 자주 갱신하지만, 쿠키의maxAge는 86400초(24시간)로 설정되어 있습니다. React Query 캐시가 만료되어 리패치되기 전에 컴포넌트가 언마운트되거나 페이지를 새로고침하면,getUserInfoFromCookie는 최대 24시간 동안 오래된 데이터를 반환할 수 있습니다.의도된 설계라면 괜찮지만, 두 값의 일관성을 맞추거나 쿠키 만료 시간을 상수로 분리하여 관리하는 것을 권장합니다.
31-39:JSON.parse결과에 대한 타입 검증 부재.
JSON.parse(userInfoStr)의 반환값을 검증 없이User로 반환하고 있습니다. 쿠키 데이터가 변조되거나 스키마가 변경된 경우,User타입과 맞지 않는 객체가 반환될 수 있습니다.간단한 타입 가드를 추가하면 런타임 안정성을 높일 수 있습니다.
♻️ 타입 가드 추가 제안
+function isUser(value: unknown): value is User { + return ( + typeof value === 'object' && + value !== null && + typeof (value as User).id === 'string' && + typeof (value as User).name === 'string' + ); +} + export function getUserInfoFromCookie(): User | null { const userInfoStr = getCookieUtil(COOKIES_KEYS.USER_INFO); if (!userInfoStr) return null; try { - return JSON.parse(userInfoStr); + const parsed: unknown = JSON.parse(userInfoStr); + return isUser(parsed) ? parsed : null; } catch { return null; } }
src/components/layout/SideNavigation/components/Bottom/SideNavigationBottom.tsx
Show resolved
Hide resolved
Bangdayeon
force-pushed
the
refactor/#353-modify-auth
branch
from
aa5c15f to
912224a
Compare
Bangdayeon
force-pushed
the
refactor/#353-modify-auth
branch
from
912224a to
adc2d10
Compare
Bangdayeon
added
bug
Bangdayeon
force-pushed
the
refactor/#353-modify-auth
branch
from
adc2d10 to
3188c70
Compare
Bangdayeon
force-pushed
the
refactor/#353-modify-auth
branch
from
3188c70 to
2a09ec9
Compare
1 participant
관련 이슈
PR 설명
인증 API
src/apis/authApi.tsfetchUserInfo(): 사용자 정보 조회logout(): 로그아웃BFF API Routes
src/app/api/auth/logout/route.ts/v1/auth/logout엔드포인트를 호출하여 BFF 로그아웃 처리src/app/api/auth/me/route.tsAPI 클라이언트 계층
src/lib/client/apiClient.ts/api/*엔드포인트 호출src/lib/client/backendClient.tscredentials: 'include'로 쿠키 포함src/lib/server/apiClient.tsAuthorization헤더에 포함기타
chatApi.ts,userApi.ts,LandingPage.tsxsrc/hooks/useCookie.tssrc/app/middleware.ts/) 접근 시 토큰이 있으면/home으로 리다이렉트참고