Conversation
toothlessdev
left a comment
There was a problem hiding this comment.
수고하셨습니다
코멘트 남긴거 한번 확인부탁드려요!
그리고 클라이언트에서 사용되는것과 서버단에서 사용되는것 구별을 위해서
*.client.ts, *.client.tsx 또는
*.server.ts, *.server.tsx
처럼 suffix 로 구분되는 파일 네이밍 컨벤션을 사용하는건 어떤가요 ? @eunwoo-levi @kimgho
예전에 잠깐 공부헸는데 Remix 에서는 클라이언트 컴포넌트와 서버 컴포넌트를 저런식으로 분리한다고 하더라구요
app/api/auth/login/route.ts
Outdated
| interface LoginSuccessResponse { | ||
| status: string; | ||
| message: string; | ||
| data: { | ||
| accessToken: string; | ||
| refreshToken: string; | ||
| }; | ||
| } |
There was a problem hiding this comment.
Swagger 를 보니 백엔드의 응답구조가 비슷비슷하게 생긴것 같아요
declare type SuccessResponse<T> = {
status: "SUCCESS";
message: string;
data: T;
};
declare type ErrorResponse = {
status: "ERROR";
message: string;
data: null;
};
declare type BaseResponse<T> = SuccessResponse<T> | ErrorResponse;를 src/@types/api.d.ts 에 정의해두고 tsconfig 에 types, typeRoot 에 경로를 지정해두고
해당 제네릭을 사용하는건 어떤가요 ?
export type LoginSuccessResponse = BaseResponse<{
accessToken: string;
refreshToken: string;
}>;There was a problem hiding this comment.
오오 공감합니다! 응답 구조가 status/message/data로 반복되고 있어서 저도 해당 부분을 공통화하면 좋겠다고 생각했네요.
말씀하신대로 BaseResponse<T>로 통일하면 중복도 줄고, status를 "SUCCESS" | "ERROR"로 좁혀서 분기 처리도 더 안전해질 것 같습니다. src/shared/types/api.ts 같은 위치에 공통 타입을 두고 각 API 응답 타입은 BaseResponse<{...}>로 정의하는 방식으로 리팩토링 하였습니다! d262942
다만 BaseResponse 같은 건 전역 타입이 아니라 우리 프로젝트 내부에서 import해서 쓰는 공통 타입이기에 src/shared/types/api.ts에서 관리하는게 더 나을 것 같은데 어떻게 생각하시나요!
.d.ts로 전역 선언해버리면 추후에 어디서 타입이 오는지 숨겨져서 규모 커질수록 디버깅 및 탐색이 불편해지고, 타입 이름 충돌 위험도 생길 것 같습니다!
app/api/auth/refresh/route.ts
Outdated
| res.cookies.set('accessToken', newAccessToken, { | ||
| httpOnly: true, | ||
| secure: isProd, | ||
| sameSite: 'lax', | ||
| path: '/', | ||
| maxAge: 60 * 15, // TODO: 백엔드 만료와 맞추기 | ||
| }); | ||
|
|
||
| if (newRefreshToken) { | ||
| res.cookies.set('refreshToken', newRefreshToken, { | ||
| httpOnly: true, | ||
| secure: isProd, | ||
| sameSite: 'lax', | ||
| path: '/', | ||
| maxAge: 60 * 60 * 24 * 14, // TODO: 백엔드 만료와 맞추기 | ||
| }); | ||
| } |
There was a problem hiding this comment.
maxAge 를 외부 env 에서 받아오는쪽으로 하는건 어떻게 생각하시나용?
There was a problem hiding this comment.
중앙 관리 필요성에는 동의합니다!
다만 maxAge는 백엔드 토큰 만료가 실제 기준이라 프론트 env로 별도로 관리하면 백엔드와 값이 어긋날 때 일치하지 않는 상황이 생길 수 있는 것 같긴하네요.
공통 유틸/상수로 묶어 한 곳에서 관리하거나 필요 시 백엔드에서 expiresIn 내려주는 방식 (혹은 server env)에 대해서 같이 얘기해보면 좋을 것 같습니다.
현재는 한 곳에서 관리하기에 편리하도록 부분에서 개선되도록 수정하였습니다! 377ef1a
There was a problem hiding this comment.
한번도 생각못해본 방식인데 server env 방식 좋은것같아요 👍
There was a problem hiding this comment.
Nextjs 공식문서를 최근에 공부중인데 server-only 라는 패키지를 사용하면 해당 번들이 서버에서만 사용할 수 있도록 빌드타임에 오류를 띄워준다고 해요
해당 패키지 사용하는거 어떻게 생각하시나요?
@eunwoo-levi @kimgho
There was a problem hiding this comment.
오.. 좋은 정보 감사합니다! 해당 내용은 discussion에서 같이 의논해도 좋을 것 같아요!
There was a problem hiding this comment.
추후 로그인이 필요한 페이지로 접근했는데, 인증된 사용자가 아닌경우 로그인 페이지로 리다이렉트 되고,
로그인을 완료하면 이전에 접속했던 페이지로 이동하는 기능을 도입하면 좋을것 같아요
보통 리액트에서는 저는 ?redirect=/mypage 와 같은 방식으로 구현하는데,
NextJS 에서는 어떻게 처리하는게 좋을까요?
discussion 에서 한번 얘기해봐도 좋겠네요 @eunwoo-levi @kimgho
There was a problem hiding this comment.
앗 middleware.ts 코드에 대해서 설명이 부족했네요!
추후 로그인이 필요한 페이지로 접근했는데, 인증된 사용자가 아닌경우 로그인 페이지로 리다이렉트 되고,
에 대한 로직은 아래 코드 입니다!
const PUBLIC_ONLY = ['/login', '/signup']; 에 해당되는 페이지 경로들은
const accessToken = request.cookies.get('accessToken')?.value;
const isAuthed = Boolean(accessToken); 즉, 권한이 없을 경우 (인증된 사용자X) 로그인으로 리다이렉트 됩니다.
if (PUBLIC_ONLY.includes(pathname) && isAuthed) {
return NextResponse.redirect(new URL('/', request.url));
}로그인을 완료하면 이전에 접속했던 페이지로 이동하는 기능을 도입하면 좋을것 같아요
에 대한 로직이 아래 코드 입니다!!
const isProtected = PROTECTED_PREFIXES.some((prefix) => pathname.startsWith(prefix));
if (isProtected && !isAuthed) {
const url = new URL('/login', request.url);
url.searchParams.set('next', pathname);
return NextResponse.redirect(url);
}위 코드에서 로그인을 성공했을 때, url.searchParams.set('next', pathname); 코드에 의해 이전에 사용자가 위치했던 경로로 리다이렉트 됩니다!
kimgho
left a comment
There was a problem hiding this comment.
얼마없는 API route에서만 쓴다면 이런 방식도 나쁘지 않다고 생각해요
만약 api route가 늘어나거나, 에러 처리까지 포함시키는 것을 생각했을 때 다른 방식을 고려해봐야할 것 같아요
app/api/auth/refresh/route.ts
Outdated
| secure: isProd, | ||
| sameSite: 'lax', | ||
| path: '/', | ||
| maxAge: 60 * 15, // TODO: 백엔드 만료와 맞추기 |
There was a problem hiding this comment.
이 부분은 나중에 제가 매직 넘버 관련 eslint를 설정할 예정이라 미리 상수로 대체해주시면 좋을 것 같아요
There was a problem hiding this comment.
의견 감사합니다! 상수로 한 곳에서 관리하면 위험성도 줄고 관리하기에 더 편리할 것 같네요. 해당 내용 반영해서 수정하였습니다! 377ef1a
app/api/auth/refresh/route.ts
Outdated
| const errorData = await error.response.json().catch(() => ({} as Record<string, unknown>)); | ||
|
|
||
| return NextResponse.json( | ||
| { message: errorData.message || '토큰 재발급 실패' }, |
There was a problem hiding this comment.
이 메시지 부분도 미리 정해진 응답과 관련해서 메시지를 상수로 관리하는게 좋아보여요
There was a problem hiding this comment.
해당 정해진 응답은 각각의 비즈니스 로직 내부에 강하게 결합되어 있어서 여러 곳에서 안쓰일 것 같긴한데 한 파일에서 상수로 하는게 나을까요? 어떻게 생각하시나요!
There was a problem hiding this comment.
우선 공통적인 응답들(500 - 서버 문제가 발생했습니다. 나중에 다시 시도해주세요)은 api client쪽에 두고, 특정 응답은 해당 api쪽에 만들어두는게 제 생각이긴 합니다
| export const config = { | ||
| matcher: [ | ||
| '/login', | ||
| '/signup', | ||
| '/mypage/:path*', | ||
| ], | ||
| }; |
There was a problem hiding this comment.
이 url config같은 경우 따로 config 상수에서 관리하는 것은 어떻게 생각하시나욥?
There was a problem hiding this comment.
Next.js가 export const config를 빌드할 때 정적으로 읽어야 해서
export const config = {
matcher: [
"/login",
"/signup",
"/mypage/:path*",
],
};위와 같이 인라인 리터럴로 적어야 해요!
There was a problem hiding this comment.
아하 제가 Next를 잘 몰라서 공부해보기 좋은 자료네용
1️⃣ PR 타입 (PR Type)
2️⃣ 변경 사항 (Changes)
3️⃣ 관련 이슈 (Related Issues)
4️⃣ 코드 설명 (Description)
Route Handler 추가 (
/api/auth/*)POST /api/auth/login: 백엔드auth/login호출 → 성공 시accessToken,refreshToken을 httpOnly 쿠키로 저장 후 200 응답.POST /api/auth/logout:accessToken,refreshToken(및 임시 토큰 사용 시signupToken) 쿠키 삭제 후 200 응답.POST /api/auth/refresh: 쿠키의refreshToken을 사용해 백엔드auth/refresh호출 → 성공 시accessToken(및 회전 시refreshToken) 쿠키 갱신.POST /api/auth/signup/phase1: 아이디/비밀번호 검증 단계 완료 후 임시 토큰(signupToken)을 httpOnly 쿠키로 저장.POST /api/auth/signup/phase2: 쿠키의signupToken으로 백엔드에 2단계 정보 전달 → 성공 시accessToken,refreshToken쿠키 저장 +signupToken삭제.공통으로
ky의HTTPError를 처리해 백엔드 에러 메시지를 그대로 전달하고, 토큰 누락 등 비정상 응답은 502로 처리했습니다.Middleware 인증 가드
commit: 440da10
/login,/signup)는 이미 로그인 상태(accessToken존재)면/로 리다이렉트./mypage/*)는 미인증 시/login?next=<원래경로>로 리다이렉트하여 로그인 후 원래 페이지로 복귀 가능하도록 처리.matcher를 필요한 경로로만 제한해 불필요한 미들웨어 실행을 줄였습니다.API 레이어 분리 (
/shared/lib) - ky 라이브러리commit: 86d39ac
apiClient: 브라우저에서 사용하는 클라이언트로,prefixUrl: '/api'기반으로 Next Route Handler만 호출하도록 구성(쿠키 기반 인증 흐름 유지).apiServer: 서버(Route Handler)에서 백엔드로 직접 호출하는 클라이언트로,API_BASE_URL을prefixUrl로 사용해 백엔드 요청을 표준화했습니다.부가 내용
Middleware 역할
middleware에서도 서버에서 실행되긴 하지만 기본이 Edge runtime이라 Node 환경처럼 못 쓰는 기능/라이브러리 제약이 많습니다.그래서 middleware는 Routing 가드(리다이렉트, 간단한 쿠키 체크) 같은 가벼운 제어 로직에 쓰고, 로그인/토큰 재발급/쿠키 세팅처럼 백엔드 호출·인증 플로우가 들어가는 서버 로직은 제약이 적은 **Route Handler(Node 런타임)**에서 처리하는 게 안정적이라고 생각합니다.
Error 처리
기타
TODO:로 표기해두었습니다. 추후에 변경이 필요할 것 같습니다.package.json의 script에"format": "prettier --write .","format:check": "prettier --check ."추가하였습니다.참고 docs