[FE-Feat] 로그인 토큰 쿠키가 아닌 헤더로 보내도록 변경 #181
Conversation
WalkthroughThis pull request implements significant modifications to the authentication and routing logic, updates the login flow to use a JWT-based approach, refines the environment configuration, and adjusts certain UI layout styles. The middleware for access token verification is removed, while new API endpoints, hooks, and route definitions are introduced. In addition, model interfaces are updated, and JSX formatting is tweaked, ensuring that the overall control and navigation flow are altered consistently across authentication, routing, and UI components. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant LP as LoginPage
participant OAuth as OAuth Provider
participant RD as Redirect Component
participant API as Login API Server
U->>LP: Clicks Google login button
LP->>OAuth: Redirects via Link using OAuth URL
OAuth->>U: Returns with code parameter in URL
U->>RD: Browser loads /oauth/redirect/?code=XYZ
RD->>API: POST /api/v1/login with code payload
API-->>RD: Returns JWTResponse (accessToken)
RD->>LocalStorage: Stores accessToken
RD->>U: Navigates to last path or /home
sequenceDiagram
participant U as User
participant R as Route Loader
participant A as isLogin()
U->>R: Requests a protected route
R->>A: Check if user is authenticated
A-->>R: Returns true/false
alt Authenticated
R->>U: Loads requested page (/home)
else Not Authenticated
R->>U: Redirects to /login (or /landing)
end
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
hamo-o
changed the title
![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
🔭 Outside diff range comments (1)
frontend/src/utils/fetch/index.ts (1)
23-23: 🛠️ Refactor suggestionRemove unnecessary credentials setting.
Since the authentication has been moved from cookies to JWT in headers, the
credentials: 'include'setting is no longer needed.const defaultOptions: RequestInit = { headers, mode: 'cors', - // accessToken을 쿠키로 관리하기 위한 설정 - credentials: 'include', };
🧹 Nitpick comments (7)
frontend/src/pages/LoginPage/index.tsx (2)
12-26: Streamline error handling for missing environment variable.Currently, there's no fallback or error handling if
serviceENV.VITE_GOOGLE_OAUTH_URLwere undefined or empty. This could leave users unable to initiate the login flow if the environment is misconfigured. Consider adding a sanity check or fallback URL to improve resilience.const LoginPage = () => ( <div className={backdropStyle}> <Modal description={`Google 계정으로 간편하게 가입...`} isOpen={true} subTitle='로그인/회원가입' title='언제만나 시작하기' > <Modal.Footer> - <GoogleLoginButton /> + {serviceENV.VITE_GOOGLE_OAUTH_URL ? ( + <GoogleLoginButton /> + ) : ( + <p>로그인 URL이 설정되지 않았습니다.</p> + )} </Modal.Footer> </Modal> </div> );
28-46: Confirm usage of direct link for login.Using a direct link instead of a mutation simplifies the login process, but also removes the ability to handle errors (e.g., network failures, invalid code) before navigation. If you need more advanced control or error messaging down the road, consider migrating the logic back into a client-side function call or a dedicated login route.
frontend/src/utils/auth/index.ts (1)
1-1: Beware of storing tokens in local storage.Placing tokens in local storage can expose them to XSS attacks, as JS code can read and modify local storage. For better security, consider using
HttpOnlycookies or other approaches (e.g., rotating tokens, storing them in secure states) if feasible.frontend/src/envconfig.ts (1)
5-8: Consider standardizing environment variable naming.The environment variable naming is inconsistent:
BASE_URLuses a different naming convention thanVITE_GOOGLE_OAUTH_URL- Both variables are accessed from
import.meta.env.VITE_*but only one includes theVITE_prefix in its property nameexport const serviceENV = Object.freeze({ - BASE_URL: import.meta.env.VITE_API_URL, + API_URL: import.meta.env.VITE_API_URL, VITE_GOOGLE_OAUTH_URL: import.meta.env.VITE_GOOGLE_OAUTH_URL, });frontend/src/features/login/api/index.ts (1)
1-12: Consider adding request timeout and retry logic.For critical authentication endpoints, it's recommended to:
- Add request timeout
- Implement retry logic for transient failures
- Add proper error types for different failure scenarios
Would you like me to provide an implementation example with these improvements?
frontend/src/routes/_main.tsx (1)
13-17: LGTM! Centralized authentication check implementation.The beforeLoad hook effectively implements client-side authentication verification for all routes under '_main', aligning with the new token management approach.
Consider enhancing error handling.
The current implementation might benefit from more robust error handling:
- Handle potential network errors during isLogin check
- Consider adding a loading state
beforeLoad: async () => { - if (!isLogin()) { - throw redirect({ to: '/login' }); - } + try { + if (!isLogin()) { + throw redirect({ to: '/login' }); + } + } catch (error) { + console.error('Authentication check failed:', error); + throw redirect({ to: '/login', search: { error: 'auth_check_failed' } }); + } },frontend/src/routes/oauth.redirect/index.tsx (1)
13-18: Prevent potential race conditions in useEffect.The current implementation might trigger multiple mutations if dependencies change rapidly.
useEffect(() => { + let isLatestRequest = true; const controller = new AbortController(); if (code && !isPending) { - loginMutate({ code, lastPath }, { signal: controller.signal }); + loginMutate( + { code, lastPath }, + { + signal: controller.signal, + onSuccess: () => { + if (isLatestRequest) { + // Handle success + } + }, + } + ); } - return () => controller.abort(); + return () => { + isLatestRequest = false; + controller.abort(); + }; }, [code, loginMutate, isPending, lastPath]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
frontend/server/index.js(0 hunks)frontend/src/envconfig.ts(1 hunks)frontend/src/features/login/api/index.ts(1 hunks)frontend/src/features/login/api/mutations.ts(1 hunks)frontend/src/features/login/model/index.ts(1 hunks)frontend/src/pages/DiscussionPage/DiscussionCreateFinishPage/index.css.ts(1 hunks)frontend/src/pages/DiscussionPage/DiscussionCreateFinishPage/index.tsx(1 hunks)frontend/src/pages/LoginPage/index.tsx(1 hunks)frontend/src/pages/MyCalendarPage/index.css.ts(1 hunks)frontend/src/routeTree.gen.ts(17 hunks)frontend/src/routes/_main.tsx(2 hunks)frontend/src/routes/_main/discussion/create/$id.tsx(1 hunks)frontend/src/routes/_main/my-calendar/index.lazy.tsx(1 hunks)frontend/src/routes/index.tsx(1 hunks)frontend/src/routes/oauth.redirect/index.lazy.tsx(0 hunks)frontend/src/routes/oauth.redirect/index.tsx(1 hunks)frontend/src/utils/auth/index.ts(1 hunks)frontend/src/utils/fetch/index.ts(3 hunks)
💤 Files with no reviewable changes (2)
- frontend/server/index.js
- frontend/src/routes/oauth.redirect/index.lazy.tsx
✅ Files skipped from review due to trivial changes (1)
- frontend/src/pages/DiscussionPage/DiscussionCreateFinishPage/index.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Send PR Notification to Reviewers
🔇 Additional comments (6)
frontend/src/features/login/model/index.ts (1)
1-7: Renamed and restructured interfaces look consistent.Renaming to
JWTRequestandJWTResponseclarifies their purpose. This is a clear and concise approach to describing the request and response data for JWT-based login.frontend/src/pages/DiscussionPage/DiscussionCreateFinishPage/index.css.ts (1)
7-9: Verify fixed positioning impact on page layout.Using
position: fixedwith full viewport dimensions could cause several issues:
- Content may overlap with other fixed/absolute positioned elements
- May break scrolling behavior on mobile devices
- Could interfere with responsive design patterns
Consider using
position: absoluteor a different layout approach if the intent is to maintain the container within its parent element's flow.frontend/src/routes/_main/my-calendar/index.lazy.tsx (1)
15-17: LGTM! Route path update aligns with authentication restructuring.The route path change to include the '_main' prefix correctly places this route under the protected main layout, which is consistent with the new authentication flow.
frontend/src/routes/_main/discussion/create/$id.tsx (1)
15-17: LGTM! Route path update is consistent with authentication changes.The route path modification to include '_main' prefix correctly places this route under the protected main layout.
frontend/src/pages/MyCalendarPage/index.css.ts (1)
9-11: Verify the layout implications of fixed positioning.Setting
position: fixedwithtop: 0andleft: 0might cause the calendar to overlay other content or create scrolling issues, especially with the existingpaddingTop: 56andoverflow: hiddenproperties.frontend/src/routeTree.gen.ts (1)
85-89: LGTM! Route changes align with the new authentication flow.The OAuth redirect route is correctly configured to handle the client-side token management.
Also applies to: 402-404
dioo1461
left a comment
dioo1461
left a comment
There was a problem hiding this comment.
고생하셨습니다~~ 확실히 localStorage로 하니까 관리하기 편한 느낌이네요!
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/src/pages/LoginPage/index.tsx (1)
28-46:Details
❓ Verification inconclusive
Verify OAuth configuration and redirect handling.
The direct navigation to Google OAuth looks good, but please ensure:
- The GOOGLE_OAUTH_URL is properly configured in
.local.envas mentioned in the PR description- The OAuth redirect URI is updated to handle the new header-based token approach
Let's verify the environment configuration:
🏁 Script executed:
#!/bin/bash # Check if GOOGLE_OAUTH_URL is configured in .local.env if [ -f "frontend/.local.env" ]; then echo "Checking .local.env configuration..." grep "GOOGLE_OAUTH_URL" frontend/.local.env else echo "Warning: .local.env file not found in frontend directory" fi # Check for any remaining cookie-related code that might need updating echo "Checking for remaining cookie-related code..." rg -i "cookie" frontend/srcLength of output: 284
OAuth configuration verification required.
The Google login flow implementation looks correct regarding the direct navigation. However, note the following:
- The expected environment file (
frontend/.local.env) wasn’t found, so please verify whetherGOOGLE_OAUTH_URLis defined elsewhere or if the file should be added to ensure proper configuration.- Confirm that the OAuth redirect URI is updated to support the new header-based token approach.
- Ensure that no residual cookie-related logic remains that might interfere with the authentication flow.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/src/envconfig.ts(1 hunks)frontend/src/pages/LoginPage/index.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/envconfig.ts
🔇 Additional comments (2)
frontend/src/pages/LoginPage/index.tsx (2)
1-8: LGTM! Import changes align with the new authentication approach.The changes correctly reflect the shift from API-based login to direct OAuth navigation.
12-26: LGTM! Component successfully simplified.The LoginPage component has been streamlined by removing unnecessary mutation logic while maintaining the core UI structure.
#️⃣ 연관된 이슈>
📝 작업 내용> 이번 PR에서 작업한 내용을 간략히 설명해주세요(이미지 첨부 가능)
beforeLoad에서 처리합니다.🙏 여기는 꼭 봐주세요! > 리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요
.local.env세팅 이후 테스트 하셔야 합니다!Summary by CodeRabbit
New Features
Style & Navigation