[FEAT] 애플 로그인 구현#606
Open
Chaejy wants to merge 3 commits into
Open
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthrough이 PR은 Apple OAuth 로그인 기능을 완전히 구현합니다. 클라이언트의 AppleLoginButton 컴포넌트에서 인증을 시작하고, 서버의 Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 상세 코드 리뷰 피드백1. 안정성 & 예외 처리🔴 exchangeAppleLogin.ts - 네트워크 타임아웃 미처리// 현재 코드 (위험)
const upstream = await fetch(url, { ... });문제점:
개선안: const controller = new AbortController();
const timeoutId = setTimeout(() => controller.abort(), 10000); // 10초
try {
const upstream = await fetch(url, {
signal: controller.signal,
// ...
});
} finally {
clearTimeout(timeoutId);
}참고: MDN Fetch API - Signal & AbortController 🔴 applyProxyAuthResponse.ts - 쿠키 파싱 에러 핸들링 부족// 현재 코드
function parseCookie(cookieString: string): CookieInfo | null {
// 파싱 로직...
}문제점:
개선안: function parseCookie(cookieString: string): CookieInfo | null {
try {
const [cookiePair, ...attributes] = cookieString.split(';');
const [name, value] = cookiePair.split('=');
if (!name || !value) return null;
const maxAgeStr = attributes.find(a => a.trim().toLowerCase().startsWith('max-age='))?.split('=')[1];
const maxAge = maxAgeStr ? parseInt(maxAgeStr, 10) : undefined;
if (maxAgeStr && isNaN(maxAge)) {
console.warn(`Invalid Max-Age value: ${maxAgeStr}`);
return null;
}
// ... 나머지 파싱
} catch (error) {
console.error('Cookie parsing error:', error);
return null;
}
}🟡 route.ts - FormData 추출 방어 코드 강화 필요const getFormString = (form: FormData, key: string): string => {
const value = form.get(key);
return typeof value === 'string' ? value : '';
};개선안: const getFormString = (form: FormData, key: string): string => {
const value = form.get(key);
if (typeof value !== 'string') {
console.warn(`Expected string for key '${key}', got ${typeof value}`);
}
return typeof value === 'string' ? value.trim() : '';
};2. 가독성 & 유지보수성🟡 AppleCallbackPage.tsx - 매직 넘버 상수화// 현재 코드
delay={index * 100} // 왜 100?
style={{ animationDelay: `${index * 100}ms` }}개선안: const ANIMATION_DELAY_MS = 100;
const CHARACTER_COUNT = 5;
{characters.map((Character, index) => (
<Character
key={index}
delay={index * ANIMATION_DELAY_MS}
className="animate-bounce"
/>
))}🟡 exchangeAppleLogin.ts - 함수 이름 명확화// 현재: extractLoginData - 무엇에서 추출하는지 불명확
export function extractLoginData(parsed: unknown): OAuthLoginData | null
// 개선안:
export function extractOAuthLoginDataFromUpstream(
parsed: unknown
): OAuthLoginData | null3. 타입 안정성🔴 exchangeAppleLogin.ts - isRecord 타입 가드 강화// 현재 코드
function isRecord(value: unknown): value is Record<string, unknown> {
return typeof value === 'object' && value !== null;
}문제점:
개선안: function isRecord(value: unknown): value is Record<string, unknown> {
return (
typeof value === 'object' &&
value !== null &&
!Array.isArray(value) &&
Object.getPrototypeOf(value) === Object.prototype
);
}참고: TypeScript Handbook - Type Guards 🟡 types.ts - OAuthLoginData 필드 선택성 검토export type OAuthLoginData = {
nickname: string;
email: string;
profileImageUrl: string;
};고려사항:
개선안: export type OAuthLoginData = {
email: string; // 필수 (검증됨)
nickname?: string; // 선택
profileImageUrl?: string; // 선택
};4. 보안🔴 route.ts - CSRF 토큰 검증 누락// 현재 코드: state 값을 받기만 함
const state = getFormString(form, 'state');
// 개선: 세션 저장 state와 비교 필요
if (state !== storedState) {
return NextResponse.redirect(`${baseUrl}${PAGE_ROUTES.LOGIN}?msg=invalid_state`);
}참고: OAuth 2.0 Security Best Practices - State Parameter 🟡 applyProxyAuthResponse.ts - SameSite=None 처리// 현재: SameSite=none을 lax로 강제 변환
if (sameSite?.toLowerCase() === 'none') {
sameSite = 'lax';
}검토:
// SameSite=None은 HTTPS 필수이므로,
// 프록시 환경에서는 Lax로 다운그레이드
if (sameSite?.toLowerCase() === 'none') {
sameSite = 'lax';
}5. 접근성 & UX🟡 AppleLoginButton.tsx - 접근성 개선// 현재 코드
<button type="button" onClick={appleLogin}>
<AppleIcon />
애플로 로그인하기
</button>개선안: <button
type="button"
onClick={appleLogin}
aria-label="Apple 계정으로 로그인하기"
title="Apple 계정으로 로그인"
disabled={isLoading} // 중복 클릭 방지
>
<AppleIcon aria-hidden="true" />
<span>애플로 로그인하기</span>
</button>🟡 AppleCallbackPage.tsx - 로딩 상태 접근성// 개선안: aria-live로 상태 변경 알림
<div aria-live="polite" aria-label="로그인 처리 중">
<LoadingCharacter />
<div className="animate-pulse">잠시만 기다려 주세요</div>
</div>6. 테스트 전략🔴 테스트 케이스 계획 필요// exchangeAppleLogin.test.ts
describe('exchangeAppleLogin', () => {
it('should return error when upstream responds with 400', async () => {
// 예상: msg = "Apple 로그인 인증에 실패했습니다."
});
it('should return error when response lacks email', async () => {
// 예상: msg = "로그인 응답이 비어있어요."
});
it('should extract email but provide default nickname', async () => {
// nickname 누락 시 기본값 처리 검증
});
it('should handle network timeout', async () => {
// AbortController 타임아웃 검증
});
});7. Clean Architecture 검토✅ 레이어 분리 - 양호
🟡 의존성 흐름 개선 제안개선안:
// 제안: middleware/authProxyMiddleware.ts
export async function withProxyAuth(
handler: (req: NextRequest) => Promise<NextResponse>,
options?: ProxyAuthOptions
) {
return async (req: NextRequest) => {
const res = await handler(req);
// ...
};
}8. 브라우저 호환성🟡 AppleLoginButton - 브라우저 지원 범위 확인// Apple 로그인 가능 환경 검사 필요
const isAppleSignInSupported = () => {
return (
/^((?!chrome|android).)*safari/i.test(navigator.userAgent) ||
navigator.platform === 'MacIntel' ||
navigator.platform === 'iPhone'
);
};9. 에러 메시지 일관성🟡 한국어 에러 메시지 표준화// getAppleLoginErrorMessage의 메시지들이 다양함
// 일관된 톤으로 통일 필요:
// - "로그인 응답이 비어있어요." (친근한 톤)
// - "Apple 로그인 인증에 실패했습니다." (격식적 톤)
const APPLE_ERROR_MESSAGES = {
INVALID_CREDENTIALS: '인증 정보가 올바르지 않습니다.',
DUPLICATE_ACCOUNT: '이미 가입된 계정입니다.',
SERVER_ERROR: '로그인 처리 중 오류가 발생했습니다. 다시 시도해주세요.',
} as const;10. 성능 최적화🟡 KakaoCallBackPage의 의존성 배열// 현재: 많은 의존성
useEffect(() => { ... }, [provider, nickname, email, profileImageUrl, showToast, router])개선안: const params = useMemo(() => ({
provider,
nickname,
email,
profileImageUrl,
}), [provider, nickname, email, profileImageUrl]);
useEffect(() => {
// params 사용
}, [params, showToast, router]);종합 평가
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
apps/web/src/app-pages/login/ui/AppleCallbackPage.tsx (1)
23-46: 🏗️ Heavy lift페이지 레이어에 콜백 비즈니스 로직이 과도하게 포함되어 있습니다.
문제: Line 23-46에서 쿼리 파싱/검증/스토어 반영/리다이렉트를 모두 처리해
app-pages가 조립 레이어 역할을 넘었습니다.
원인: 콜백 처리 규칙이 UI 컴포넌트에 결합됨.
대안:features/auth로useOAuthCallbackHandler(또는 service) 추출하고, 이 컴포넌트는 결과 상태 렌더링만 담당하세요.
장점: Apple/Kakao 분기 로직 재사용·일관성 확보, 변경 시 파급 범위 축소.
단점: 초기 분리 작업으로 파일/테스트 보강이 필요합니다.As per coding guidelines, "
apps/web/**: 페이지/라우트는 “조립”만: 복잡한 로직은 features/entities로 내려보낸다."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app-pages/login/ui/AppleCallbackPage.tsx` around lines 23 - 46, The AppleCallbackPage contains too much callback business logic (query parsing, validation, store updates and redirects); extract this into a reusable handler in features/auth (e.g., create useOAuthCallbackHandler or OAuthCallbackService) that accepts the URL/searchParams and returns a result/status and any onboarding data, then update AppleCallbackPage to call that hook/service for side effects and only render based on its result; specifically move logic from AppleCallbackPage (the useEffect block that reads searchParams, validates email, calls setOnboarding, shows toast, and router.push) into the new useOAuthCallbackHandler and have AppleCallbackPage just call/use that hook and render loading/error/success UI accordingly so parsing/validation/store mutation and routing live in features/auth instead of the page layer.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/app-pages/login/ui/AppleCallbackPage.tsx`:
- Around line 49-65: The loading UI in AppleCallbackPage.tsx lacks accessibility
announcements; add role="status" and aria-live="polite" to the loading text
container (the div with className "text-body-body8 text-foreground-tertiary
...") so screen readers receive updates, and mark decorative character SVGs as
aria-hidden="true" when rendering them in the characters.map (the Character
components) — ensure any purely decorative markup inside Character is flagged
aria-hidden to avoid redundant announcements.
In `@apps/web/src/app-pages/login/ui/KakaoCallBackPage.tsx`:
- Around line 53-67: The Apple callback branch in KakaoCallBackPage.tsx is
rejecting logins when appleNickname or appleProfileImageUrl are missing; change
the validation to require only appleEmail and treat missing appleNickname and
appleProfileImageUrl as defaults (e.g., empty string or null) before calling
setOnboarding and router.push; update the branch that checks provider ===
'apple' to only show the error and redirect if appleEmail is absent, and
populate setOnboarding({ nickname: appleNickname ?? '', email: appleEmail,
profileImageUrl: appleProfileImageUrl ?? '' }) so behavior matches
AppleCallbackPage and allows logins when nickname/profileImageUrl are omitted.
In `@apps/web/src/app/api/auth/apple/callback/route.ts`:
- Around line 15-18: The getBaseUrl function currently trusts request headers
('x-forwarded-proto' and 'host') which allows header-based redirect tampering;
change getBaseUrl to use a fixed trusted origin (from configuration or an
environment variable) or validate the combined origin against a server-side
allowlist, and if the origin is not recognized throw/log and refuse to use it;
specifically stop using req.headers.get('x-forwarded-proto') and
req.headers.get('host') directly in getBaseUrl and instead return the
configured/trusted origin or a validated/allowlisted origin.
- Around line 50-55: The code is exposing PII by placing
nickname/email/profileImageUrl into the redirectUrl query; instead generate a
server-side temporary session key or signed short-lived token (e.g., via a new
helper like createTempAuthSession or signShortLivedToken) that stores the
result.data (nickname, email, profileImageUrl) server-side and map it to that
key, then set only provider='apple' and the temp session key/token on
redirectUrl (keep LOGIN_CALLBACK and baseUrl usage), avoid adding raw PII to
query params, and ensure whatever storage/mechanism you add includes an
expiration and lookup by the callback handler to retrieve the PII.
- Around line 38-41: The code calls exchangeAppleLogin with a hardcoded origin
'http://localhost:3000' which breaks in production; replace that literal by
deriving the origin dynamically (e.g., compute const origin = new
URL(req.url).origin or read a configured env var like NEXT_PUBLIC_APP_URL /
NEXTAUTH_URL with a sensible fallback) and pass that origin variable into
exchangeAppleLogin instead of the hardcoded string so the upstream call uses the
actual deployment domain; update the call site in route.ts where
exchangeAppleLogin({ code, state, user }, ...) is invoked.
In `@apps/web/src/features/auth/api/exchangeAppleLogin.ts`:
- Around line 38-42: The fetch call that assigns to upstream in
exchangeAppleLogin.ts currently has no timeout/abort and can hang; add an
AbortSignal with a sensible timeout (e.g., using AbortSignal.timeout or an
AbortController + setTimeout) and pass the signal option into the fetch call,
then handle the abort/timeout case in the surrounding try/catch to log the
timeout (including details) and throw a user-facing error or retry as
appropriate; update the upstream fetch invocation and its error handling paths
in the exchangeAppleLogin function to use the signal and detect
DOMException/AbortError to differentiate timeout vs other failures.
In `@apps/web/src/features/auth/lib/applyProxyAuthResponse.ts`:
- Around line 68-71: The toProxyCookiePath function currently returns '/' for
path === '/' which lets the cookie propagate app-wide; update to map the root
and undefined cases to the proxy boundary instead. Specifically, in
toProxyCookiePath(path?: string) return '/api/proxy' when path is undefined or
path === '/', keep returning path if it already startsWith('/api/proxy'), and
otherwise prepend '/api/proxy' to the supplied path; adjust only the logic
inside toProxyCookiePath so cookie scope is constrained to the proxy boundary.
---
Nitpick comments:
In `@apps/web/src/app-pages/login/ui/AppleCallbackPage.tsx`:
- Around line 23-46: The AppleCallbackPage contains too much callback business
logic (query parsing, validation, store updates and redirects); extract this
into a reusable handler in features/auth (e.g., create useOAuthCallbackHandler
or OAuthCallbackService) that accepts the URL/searchParams and returns a
result/status and any onboarding data, then update AppleCallbackPage to call
that hook/service for side effects and only render based on its result;
specifically move logic from AppleCallbackPage (the useEffect block that reads
searchParams, validates email, calls setOnboarding, shows toast, and
router.push) into the new useOAuthCallbackHandler and have AppleCallbackPage
just call/use that hook and render loading/error/success UI accordingly so
parsing/validation/store mutation and routing live in features/auth instead of
the page layer.
🪄 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: b0fb3006-7e11-4111-aa5a-3d72b0f622e8
⛔ Files ignored due to path filters (1)
apps/web/public/images/apple.svgis excluded by!**/*.svg
📒 Files selected for processing (11)
apps/web/src/app-pages/login/ui/AppleCallbackPage.tsxapps/web/src/app-pages/login/ui/KakaoCallBackPage.tsxapps/web/src/app-pages/login/ui/LoginPage.tsxapps/web/src/app/(public)/login/apple/callback/page.tsxapps/web/src/app/api/auth/apple/callback/route.tsapps/web/src/features/auth/api/exchangeAppleLogin.tsapps/web/src/features/auth/api/types.tsapps/web/src/features/auth/lib/appleLogin.tsapps/web/src/features/auth/lib/applyProxyAuthResponse.tsapps/web/src/features/auth/model/types.tsapps/web/src/features/auth/ui/AppleLoginButton.tsx
Comment on lines
+49
to
+65
| <div className="flex h-full w-full flex-col items-center justify-center"> | ||
| <div className="flex flex-col items-center gap-10"> | ||
| <div className="flex flex-row items-center"> | ||
| {characters.map((Character, i) => ( | ||
| <Character | ||
| key={i} | ||
| className="animate-float" | ||
| style={{ animationDelay: `${i * 0.1}s` }} | ||
| /> | ||
| ))} | ||
| </div> | ||
| <div className="text-body-body8 text-foreground-tertiary flex items-center gap-2"> | ||
| <span>잠시만 기다려 주세요</span> | ||
| <span className="animate-dot-appear-1 inline-block">.</span> | ||
| <span className="animate-dot-appear-2 inline-block">.</span> | ||
| <span className="animate-dot-appear-3 inline-block">.</span> | ||
| </div> |
There was a problem hiding this comment.
로딩 화면 접근성 상태 알림이 누락되어 있습니다.
Line 49-65 로딩 UI는 시각적 애니메이션만 있고, 스크린리더에 진행 상태를 전달하지 못합니다. role="status" + aria-live="polite"를 추가하고 장식용 SVG는 aria-hidden="true" 처리하는 게 좋습니다.
접근성 보완 예시
- <div className="flex h-full w-full flex-col items-center justify-center">
+ <div
+ className="flex h-full w-full flex-col items-center justify-center"
+ role="status"
+ aria-live="polite"
+ aria-label="로그인 처리 중입니다. 잠시만 기다려 주세요."
+ >
...
- <div className="flex flex-row items-center">
+ <div className="flex flex-row items-center" aria-hidden="true">As per coding guidelines, "**: 접근성: aria-label, role, 키보드 내비게이션, 포커스 트랩, 대비/색상 의존 금지."
📝 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
| <div className="flex h-full w-full flex-col items-center justify-center"> | |
| <div className="flex flex-col items-center gap-10"> | |
| <div className="flex flex-row items-center"> | |
| {characters.map((Character, i) => ( | |
| <Character | |
| key={i} | |
| className="animate-float" | |
| style={{ animationDelay: `${i * 0.1}s` }} | |
| /> | |
| ))} | |
| </div> | |
| <div className="text-body-body8 text-foreground-tertiary flex items-center gap-2"> | |
| <span>잠시만 기다려 주세요</span> | |
| <span className="animate-dot-appear-1 inline-block">.</span> | |
| <span className="animate-dot-appear-2 inline-block">.</span> | |
| <span className="animate-dot-appear-3 inline-block">.</span> | |
| </div> | |
| <div | |
| className="flex h-full w-full flex-col items-center justify-center" | |
| role="status" | |
| aria-live="polite" | |
| aria-label="로그인 처리 중입니다. 잠시만 기다려 주세요." | |
| > | |
| <div className="flex flex-col items-center gap-10"> | |
| <div className="flex flex-row items-center" aria-hidden="true"> | |
| {characters.map((Character, i) => ( | |
| <Character | |
| key={i} | |
| className="animate-float" | |
| style={{ animationDelay: `${i * 0.1}s` }} | |
| /> | |
| ))} | |
| </div> | |
| <div className="text-body-body8 text-foreground-tertiary flex items-center gap-2"> | |
| <span>잠시만 기다려 주세요</span> | |
| <span className="animate-dot-appear-1 inline-block">.</span> | |
| <span className="animate-dot-appear-2 inline-block">.</span> | |
| <span className="animate-dot-appear-3 inline-block">.</span> | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/app-pages/login/ui/AppleCallbackPage.tsx` around lines 49 - 65,
The loading UI in AppleCallbackPage.tsx lacks accessibility announcements; add
role="status" and aria-live="polite" to the loading text container (the div with
className "text-body-body8 text-foreground-tertiary ...") so screen readers
receive updates, and mark decorative character SVGs as aria-hidden="true" when
rendering them in the characters.map (the Character components) — ensure any
purely decorative markup inside Character is flagged aria-hidden to avoid
redundant announcements.
Comment on lines
+15
to
+18
| function getBaseUrl(req: NextRequest): string { | ||
| const proto = req.headers.get('x-forwarded-proto') ?? 'http'; | ||
| const host = req.headers.get('host') ?? 'localhost:3000'; | ||
| return `${proto}://${host}`; |
There was a problem hiding this comment.
Host/Proto 헤더를 그대로 신뢰하면 리다이렉트 대상 변조 위험이 있습니다.
외부 입력 헤더 기반으로 base URL을 조합하면, 악의적 Host 헤더로 의도치 않은 도메인 리다이렉트가 발생할 수 있습니다. 서버 설정의 고정 origin(또는 allowlist 검증된 origin)만 사용하세요.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/app/api/auth/apple/callback/route.ts` around lines 15 - 18, The
getBaseUrl function currently trusts request headers ('x-forwarded-proto' and
'host') which allows header-based redirect tampering; change getBaseUrl to use a
fixed trusted origin (from configuration or an environment variable) or validate
the combined origin against a server-side allowlist, and if the origin is not
recognized throw/log and refuse to use it; specifically stop using
req.headers.get('x-forwarded-proto') and req.headers.get('host') directly in
getBaseUrl and instead return the configured/trusted origin or a
validated/allowlisted origin.
Comment on lines
+38
to
+41
| const result = await exchangeAppleLogin( | ||
| { code, state, user }, | ||
| 'http://localhost:3000', | ||
| req.headers.get('cookie') ?? undefined, |
There was a problem hiding this comment.
토큰 교환 origin이 localhost로 고정되어 운영 환경에서 실패합니다.
현재는 어떤 환경에서도 http://localhost:3000으로 업스트림 호출합니다. 실제 배포 도메인 기준으로 호출하도록 바꿔야 합니다.
🔧 제안 코드
const result = await exchangeAppleLogin(
{ code, state, user },
- 'http://localhost:3000',
+ baseUrl,
req.headers.get('cookie') ?? undefined,
);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/app/api/auth/apple/callback/route.ts` around lines 38 - 41, The
code calls exchangeAppleLogin with a hardcoded origin 'http://localhost:3000'
which breaks in production; replace that literal by deriving the origin
dynamically (e.g., compute const origin = new URL(req.url).origin or read a
configured env var like NEXT_PUBLIC_APP_URL / NEXTAUTH_URL with a sensible
fallback) and pass that origin variable into exchangeAppleLogin instead of the
hardcoded string so the upstream call uses the actual deployment domain; update
the call site in route.ts where exchangeAppleLogin({ code, state, user }, ...)
is invoked.
Comment on lines
+50
to
+55
| const { nickname, email, profileImageUrl } = result.data; | ||
| const redirectUrl = new URL(LOGIN_CALLBACK, baseUrl); | ||
| redirectUrl.searchParams.set('provider', 'apple'); | ||
| redirectUrl.searchParams.set('nickname', nickname); | ||
| redirectUrl.searchParams.set('email', email); | ||
| redirectUrl.searchParams.set('profileImageUrl', profileImageUrl); |
There was a problem hiding this comment.
이메일/닉네임을 URL 쿼리로 전달해 PII 노출 위험이 큽니다.
email, nickname을 쿼리로 넘기면 브라우저 히스토리/로그/리퍼러로 유출될 수 있습니다. 서버 측 임시 세션 키(또는 서명된 단기 토큰)만 전달하고, 실제 PII는 서버에서 조회하는 방식으로 전환하는 게 안전합니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/app/api/auth/apple/callback/route.ts` around lines 50 - 55, The
code is exposing PII by placing nickname/email/profileImageUrl into the
redirectUrl query; instead generate a server-side temporary session key or
signed short-lived token (e.g., via a new helper like createTempAuthSession or
signShortLivedToken) that stores the result.data (nickname, email,
profileImageUrl) server-side and map it to that key, then set only
provider='apple' and the temp session key/token on redirectUrl (keep
LOGIN_CALLBACK and baseUrl usage), avoid adding raw PII to query params, and
ensure whatever storage/mechanism you add includes an expiration and lookup by
the callback handler to retrieve the PII.
Comment on lines
+38
to
+42
| const upstream = await fetch(url.toString(), { | ||
| method: 'POST', | ||
| headers, | ||
| cache: 'no-store', | ||
| }); |
There was a problem hiding this comment.
외부 인증 호출에 timeout/abort가 없어 요청 스레드가 장시간 점유될 수 있습니다.
Line 38의 fetch가 무제한 대기라서 업스트림 지연 시 로그인 라우트가 묶일 수 있습니다. AbortSignal.timeout(...) 같은 명시적 제한을 넣어주세요.
🔧 제안 코드
- const upstream = await fetch(url.toString(), {
+ const upstream = await fetch(url.toString(), {
method: 'POST',
headers,
cache: 'no-store',
+ signal: AbortSignal.timeout(8000),
});As per coding guidelines, 5) 에러/예외 처리 기준: 사용자-facing 에러 메시지, 로깅, 재시도 전략, timeout, abort 등을 일관되게.
📝 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
| const upstream = await fetch(url.toString(), { | |
| method: 'POST', | |
| headers, | |
| cache: 'no-store', | |
| }); | |
| const upstream = await fetch(url.toString(), { | |
| method: 'POST', | |
| headers, | |
| cache: 'no-store', | |
| signal: AbortSignal.timeout(8000), | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/features/auth/api/exchangeAppleLogin.ts` around lines 38 - 42,
The fetch call that assigns to upstream in exchangeAppleLogin.ts currently has
no timeout/abort and can hang; add an AbortSignal with a sensible timeout (e.g.,
using AbortSignal.timeout or an AbortController + setTimeout) and pass the
signal option into the fetch call, then handle the abort/timeout case in the
surrounding try/catch to log the timeout (including details) and throw a
user-facing error or retry as appropriate; update the upstream fetch invocation
and its error handling paths in the exchangeAppleLogin function to use the
signal and detect DOMException/AbortError to differentiate timeout vs other
failures.
Comment on lines
+68
to
+71
| function toProxyCookiePath(path?: string) { | ||
| if (!path || path === '/') return path ?? '/'; | ||
| if (path.startsWith('/api/proxy')) return path; | ||
| return `/api/proxy${path}`; |
There was a problem hiding this comment.
루트 경로 쿠키가 프록시 경계를 벗어나도록 설정됩니다.
path === '/'일 때 그대로 /를 반환해서 쿠키가 앱 전역으로 전파됩니다. 프록시 전용 경계 의도라면 루트도 /api/proxy로 매핑해야 합니다.
🔧 제안 코드
function toProxyCookiePath(path?: string) {
- if (!path || path === '/') return path ?? '/';
+ if (!path || path === '/') return '/api/proxy';
if (path.startsWith('/api/proxy')) return path;
return `/api/proxy${path}`;
}As per coding guidelines, 3) workspace 경계를 넘는 import는 반드시 목적(재사용/표준화)을 설명하고, 사이드이펙트/전역 상태 공유를 최소화.
📝 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
| function toProxyCookiePath(path?: string) { | |
| if (!path || path === '/') return path ?? '/'; | |
| if (path.startsWith('/api/proxy')) return path; | |
| return `/api/proxy${path}`; | |
| function toProxyCookiePath(path?: string) { | |
| if (!path || path === '/') return '/api/proxy'; | |
| if (path.startsWith('/api/proxy')) return path; | |
| return `/api/proxy${path}`; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/features/auth/lib/applyProxyAuthResponse.ts` around lines 68 -
71, The toProxyCookiePath function currently returns '/' for path === '/' which
lets the cookie propagate app-wide; update to map the root and undefined cases
to the proxy boundary instead. Specifically, in toProxyCookiePath(path?: string)
return '/api/proxy' when path is undefined or path === '/', keep returning path
if it already startsWith('/api/proxy'), and otherwise prepend '/api/proxy' to
the supplied path; adjust only the logic inside toProxyCookiePath so cookie
scope is constrained to the proxy boundary.
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
1 participant
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.
✅ 체크리스트
🔗 관련 이슈
📌 작업 목적
Apple OAuth를 통한 소셜 로그인을 구현합니다.
🔨 주요 작업 내용
타입
OAuthLoginData공통 타입 추가 (nickname,email,profileImageUrl)KakaoLoginData가OAuthLoginData를 extends하도록 리팩토링CLICK_LOGIN_APPLE추가버튼 / 트리거
AppleLoginButton컴포넌트 추가appleLogin()—/api/proxy/login/apple로 리다이렉트하는 트리거 함수 추가LoginPage)에AppleLoginButton노출콜백 플로우
POST /api/auth/apple/callback— Apple이 보내는form_post수신, 백엔드 프록시에 code/state 전달exchangeAppleLogin()— 백엔드 프록시 응답에서 로그인 데이터 파싱 및 에러 메시지 처리applyProxyAuthToResponse()— 프록시 응답의Set-Cookie를 Next.js response에 전파,accessToken쿠키 세팅/login/apple/callback라우트 및AppleCallbackPage— 쿼리스트링으로 받은 유저 정보를 온보딩 스토어에 주입 후 홈으로 이동☑️ 해결 방법 (선택)
🧪 테스트 방법
code/state누락) 시 로그인 페이지로 리다이렉트되는지 확인✔️ 설치 라이브러리
📷 실행 영상 또는 스크린샷
💬 논의할 점
🙋🏻 참고 자료
Summary by CodeRabbit
릴리스 노트