feat: Implement smooth OAuth device connect flow #94
Conversation
- Add DeviceConnect component for handling CLI device authorization - Update LoginForm to handle device code flow for new and existing users - Fix race conditions in ProtectedRoute and useAuth for post-onboarding redirection - Implement smooth state-based navigation to avoid page flashes - Update SignupForm to persist device code - Format code with Prettier
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
bot
added
type:refactor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
📝 WalkthroughWalkthroughThis PR introduces a device connection authorization feature by adding a new DeviceConnect component and integrating device code handling throughout the authentication flow. Device codes are extracted from URL parameters, preserved through login/signup/OAuth flows, and users are directed to a confirmation page post-authentication where they authorize CLI device connections. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant LoginFlow as Login/Signup
participant DeviceConnect
participant Backend API
CLI->>User: Initiates device auth<br/>(generates code)
User->>LoginFlow: Navigates to login?code=xyz
LoginFlow->>LoginFlow: Extracts deviceCode from URL<br/>Stores in sessionStorage
LoginFlow->>Backend API: OAuth login flow
Backend API->>LoginFlow: Returns auth token
LoginFlow->>DeviceConnect: Redirects to /device/connect<br/>with deviceCode
DeviceConnect->>DeviceConnect: Loads deviceCode from<br/>navigation state/localStorage
DeviceConnect->>User: Displays confirmation UI<br/>with device code & user info
User->>DeviceConnect: Clicks "Connect CLI"
DeviceConnect->>Backend API: POST /api/oauth/device/confirm<br/>{user_code, token}
Backend API->>Backend API: Authorizes device
Backend API->>DeviceConnect: Success response
DeviceConnect->>CLI: Device authorization complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
|
👋 Thanks for opening this pull request! A maintainer will review it soon. Please make sure all CI checks pass. |
![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
🤖 Fix all issues with AI agents
In `@src/components/DeviceConnect.tsx`:
- Around line 51-56: The redirect in the useEffect (symbols: useEffect,
navigate, userCode, loading, isAuthenticated) currently always appends
?code=${userCode} which yields "/login?code=null" when userCode is
null/undefined; change the redirect to include the code query param only when
userCode is a defined/non-empty value (build the target path conditionally or
use URLSearchParams to add the code param only if truthy) and keep the existing
guard on loading and isAuthenticated; call navigate with the resulting URL and {
replace: true } as before.
In `@src/components/OAuthCallback.tsx`:
- Around line 37-42: The device code is removed from sessionStorage immediately
after reading, which prevents retries if auth fails; change the logic in the
OAuthCallback component so you only call
sessionStorage.removeItem('oauth_device_code') after the token exchange/success
path completes (e.g., after the function that exchanges deviceCode for tokens or
the success branch of handleAuthResponse), and leave the stored deviceCode
intact on error paths so the UI can retry or show recovery options.
- Around line 77-83: The redirect currently interpolates deviceCode directly
into the query string (in the OAuthCallback component where deviceCode is
checked before calling navigate), which can break URLs; fix it by encoding the
value with encodeURIComponent when constructing the path passed to navigate
(i.e., replace usage of deviceCode in the `/device/connect?code=${deviceCode}`
string with the encoded form) so reserved characters are safely escaped before
calling navigate(..., { replace: true }).
In `@src/components/SignupForm.tsx`:
- Around line 60-65: The route builder uses raw deviceCode when calling
navigate, which can break the query string; update the calls that build the URL
(the block using deviceCode in SignupForm around the
navigate(`/device/connect?code=${deviceCode}`, ...) call and the other
occurrences at the indicated spots) to URL-encode the parameter by calling
encodeURIComponent(deviceCode) when interpolating the query value (e.g.,
`/device/connect?code=${encodeURIComponent(deviceCode)}`), preserving the
existing navigate options like { replace: true }.
In `@src/components/VerifyEmail.tsx`:
- Line 115: The login link builds a query string using deviceCode in
VerifyEmail.tsx which can break if deviceCode contains reserved characters;
update every place that sets the JSX "to" prop with a query using deviceCode
(the occurrences around where deviceCode is used for login links) to URL-encode
the value by applying encodeURIComponent to deviceCode when constructing the URL
so the query parameter is safely encoded.
- Around line 17-23: Remove the debug console logs that output sensitive device
codes in the VerifyEmail component: delete the console.log calls referencing
deviceCode, location.state?.deviceCode, and
localStorage.getItem('pending_device_code'); ensure no other places in the
component (or helper functions used by VerifyEmail) log these values and keep
any non-sensitive diagnostic logging only.
In `@src/hooks/useAuth.tsx`:
- Around line 172-193: In completeOnboarding (in useAuth.tsx) remove any console
logs that print the actual device code retrieved from localStorage
('pending_device_code'); instead log only a boolean/presence check or remove the
logs entirely, e.g., replace console.log('[completeOnboarding] Device code from
localStorage:', deviceCode) with a presence-only message and avoid outputting
deviceCode, and ensure any other logging around navigate('/device/connect', {
state: { deviceCode } }) does not expose the code (DeviceConnect will handle
cleanup).
🧹 Nitpick comments (5)
src/components/ProtectedRoute.tsx (1)
14-55: Consider gating debug logs behind a dev flag.
These logs will run on every route render; wrapping them in a development-only guard avoids noisy production consoles.🧹 Example guard
- console.log('[ProtectedRoute]', { - pathname: location.pathname, - isAuthenticated, - loading, - onboardingCompleted: user?.onboardingCompleted, - }); + if (process.env.NODE_ENV === 'development') { + console.debug('[ProtectedRoute]', { + pathname: location.pathname, + isAuthenticated, + loading, + onboardingCompleted: user?.onboardingCompleted, + }); + }src/components/DeviceConnect.tsx (2)
34-47: Remove debug console.log statements before merging.These logging statements are useful for development but should be removed or replaced with a proper logging utility that can be disabled in production.
🧹 Suggested cleanup
- console.log( - '[DeviceConnect] Loaded with code:', - userCode, - 'from', - (location.state as { deviceCode?: string })?.deviceCode ? 'state' : 'URL' - ); - // Clean up pending_device_code after successful mount useEffect(() => { if (userCode) { localStorage.removeItem('pending_device_code'); - console.log( - '[DeviceConnect] Cleaned up pending_device_code from localStorage' - ); } }, [userCode]);
80-118: Consider handling non-JSON error responses gracefully.If the server returns a non-JSON response (e.g., 500 with HTML error page),
response.json()will throw an exception. The current catch block handles this, but the error message might be unclear.♻️ More robust error handling
try { const apiBaseUrl = process.env.REACT_APP_API_BASE_URL || 'http://localhost:3001'; const token = localStorage.getItem('accessToken'); const response = await fetch(`${apiBaseUrl}/api/oauth/device/confirm`, { method: 'POST', headers: { 'Content-Type': 'application/json', ...(token && { Authorization: `Bearer ${token}` }), }, credentials: 'include', body: JSON.stringify({ user_code: userCode }), }); - const data = await response.json(); - - if (!response.ok) { - throw new Error(data.error_description || 'Failed to authorize device'); + if (!response.ok) { + const data = await response.json().catch(() => ({})); + throw new Error(data.error_description || 'Failed to authorize device'); } setStatus('success');src/components/LoginForm.tsx (2)
35-38: Consider usinguseSearchParamshook for consistency.The current approach works but react-router-dom provides
useSearchParams()hook which is more idiomatic and would be consistent with howuseLocationanduseNavigateare already being used.♻️ Use useSearchParams hook
+import { Link, useNavigate, useLocation, useSearchParams } from 'react-router-dom'; ... - const [searchParams] = useState( - () => new URLSearchParams(window.location.search) - ); - const deviceCode = searchParams.get('code'); + const [searchParams] = useSearchParams(); + const deviceCode = searchParams.get('code');
51-100: Remove debug console.log statements and simplify logging.Multiple debug statements throughout this effect should be removed before production. If logging is needed for observability, consider using a configurable logger that can be disabled in production builds.
🧹 Suggested cleanup
useEffect(() => { // Only run this effect on the login page to prevent interference with other flows if (location.pathname !== '/login') { return; } // Prevent multiple navigations as auth state updates if (hasNavigated.current) { - console.log('[LoginForm] Already navigated, skipping'); return; } - console.log('[LoginForm] Auth state:', { - loading, - isAuthenticated, - user, - deviceCode, - }); - console.log( - '[LoginForm] user.onboardingCompleted:', - user?.onboardingCompleted - ); - if (!loading && isAuthenticated && user) { hasNavigated.current = true; // Mark as navigated to prevent re-runs if (deviceCode) { // Check onboarding status before redirecting if (user.onboardingCompleted) { // Existing user with device code → device connect - console.log( - '[LoginForm] Existing user, redirecting to device connect' - ); navigate(`/device/connect?code=${deviceCode}`, { replace: true }); } else { // New user with device code → store in localStorage for post-onboarding redirect // ProtectedRoute will redirect to /onboarding - console.log( - '[LoginForm] New user, storing device code and redirecting to dashboard (will be intercepted by ProtectedRoute)' - ); localStorage.setItem('pending_device_code', deviceCode); navigate('/dashboard', { replace: true }); } } else { // No device code → normal dashboard redirect - console.log('[LoginForm] No device code, redirecting to dashboard'); navigate('/dashboard', { replace: true }); } } }, [isAuthenticated, loading, navigate, deviceCode, user, location.pathname]);
| // Redirect if not authenticated | ||
| useEffect(() => { | ||
| if (!loading && !isAuthenticated) { | ||
| navigate(`/login?code=${userCode}`, { replace: true }); | ||
| } | ||
| }, [loading, isAuthenticated, navigate, userCode]); |
There was a problem hiding this comment.
Guard against null/undefined userCode in redirect URL.
If userCode is null or undefined, the redirect URL becomes /login?code=null, which will cause issues downstream. Only include the code param when it has a valid value.
🐛 Proposed fix
// Redirect if not authenticated
useEffect(() => {
if (!loading && !isAuthenticated) {
- navigate(`/login?code=${userCode}`, { replace: true });
+ const redirectUrl = userCode ? `/login?code=${userCode}` : '/login';
+ navigate(redirectUrl, { replace: true });
}
}, [loading, isAuthenticated, navigate, userCode]);📝 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.
| // Redirect if not authenticated | |
| useEffect(() => { | |
| if (!loading && !isAuthenticated) { | |
| navigate(`/login?code=${userCode}`, { replace: true }); | |
| } | |
| }, [loading, isAuthenticated, navigate, userCode]); | |
| // Redirect if not authenticated | |
| useEffect(() => { | |
| if (!loading && !isAuthenticated) { | |
| const redirectUrl = userCode ? `/login?code=${userCode}` : '/login'; | |
| navigate(redirectUrl, { replace: true }); | |
| } | |
| }, [loading, isAuthenticated, navigate, userCode]); |
🤖 Prompt for AI Agents
In `@src/components/DeviceConnect.tsx` around lines 51 - 56, The redirect in the
useEffect (symbols: useEffect, navigate, userCode, loading, isAuthenticated)
currently always appends ?code=${userCode} which yields "/login?code=null" when
userCode is null/undefined; change the redirect to include the code query param
only when userCode is a defined/non-empty value (build the target path
conditionally or use URLSearchParams to add the code param only if truthy) and
keep the existing guard on loading and isAuthenticated; call navigate with the
resulting URL and { replace: true } as before.
| // Retrieve device code from sessionStorage (stored before OAuth redirect) | ||
| const deviceCode = sessionStorage.getItem('oauth_device_code'); | ||
| // Clean up after reading | ||
| if (deviceCode) { | ||
| sessionStorage.removeItem('oauth_device_code'); | ||
| } |
There was a problem hiding this comment.
Defer device code cleanup until after successful auth.
If the OAuth flow fails, the device code is lost and the user can’t retry seamlessly.
✅ Proposed adjustment
- // Retrieve device code from sessionStorage (stored before OAuth redirect)
- const deviceCode = sessionStorage.getItem('oauth_device_code');
- // Clean up after reading
- if (deviceCode) {
- sessionStorage.removeItem('oauth_device_code');
- }
+ // Retrieve device code from sessionStorage (stored before OAuth redirect)
+ const deviceCode = sessionStorage.getItem('oauth_device_code');- setStatus('success');
+ setStatus('success');
+
+ // Clean up only after successful authentication
+ if (deviceCode) {
+ sessionStorage.removeItem('oauth_device_code');
+ }🤖 Prompt for AI Agents
In `@src/components/OAuthCallback.tsx` around lines 37 - 42, The device code is
removed from sessionStorage immediately after reading, which prevents retries if
auth fails; change the logic in the OAuthCallback component so you only call
sessionStorage.removeItem('oauth_device_code') after the token exchange/success
path completes (e.g., after the function that exchanges deviceCode for tokens or
the success branch of handleAuthResponse), and leave the stored deviceCode
intact on error paths so the UI can retry or show recovery options.
| // If there's a device code, redirect to device connect | ||
| if (deviceCode) { | ||
| navigate(`/device/connect?code=${deviceCode}`, { | ||
| replace: true, | ||
| }); | ||
| } else { | ||
| navigate(result.data?.redirectTo || '/dashboard', { |
There was a problem hiding this comment.
URL-encode the device code when building the redirect URL.
Unencoded values can break query strings if they include reserved characters.
🛠️ Proposed fix
- if (deviceCode) {
- navigate(`/device/connect?code=${deviceCode}`, {
+ if (deviceCode) {
+ const encodedDeviceCode = encodeURIComponent(deviceCode);
+ navigate(`/device/connect?code=${encodedDeviceCode}`, {
replace: true,
});
} else {📝 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.
| // If there's a device code, redirect to device connect | |
| if (deviceCode) { | |
| navigate(`/device/connect?code=${deviceCode}`, { | |
| replace: true, | |
| }); | |
| } else { | |
| navigate(result.data?.redirectTo || '/dashboard', { | |
| // If there's a device code, redirect to device connect | |
| if (deviceCode) { | |
| const encodedDeviceCode = encodeURIComponent(deviceCode); | |
| navigate(`/device/connect?code=${encodedDeviceCode}`, { | |
| replace: true, | |
| }); | |
| } else { | |
| navigate(result.data?.redirectTo || '/dashboard', { |
🤖 Prompt for AI Agents
In `@src/components/OAuthCallback.tsx` around lines 77 - 83, The redirect
currently interpolates deviceCode directly into the query string (in the
OAuthCallback component where deviceCode is checked before calling navigate),
which can break URLs; fix it by encoding the value with encodeURIComponent when
constructing the path passed to navigate (i.e., replace usage of deviceCode in
the `/device/connect?code=${deviceCode}` string with the encoded form) so
reserved characters are safely escaped before calling navigate(..., { replace:
true }).
| // If there's a device code, go to device connect page | ||
| if (deviceCode) { | ||
| navigate(`/device/connect?code=${deviceCode}`, { replace: true }); | ||
| } else { | ||
| navigate('/dashboard', { replace: true }); | ||
| } |
There was a problem hiding this comment.
URL-encode deviceCode when building routes.
Unencoded values can break the query string or be interpreted incorrectly.
🛠️ Proposed fix
- const deviceCode = searchParams.get('code');
+ const deviceCode = searchParams.get('code');
+ const encodedDeviceCode = deviceCode ? encodeURIComponent(deviceCode) : null;- if (deviceCode) {
- navigate(`/device/connect?code=${deviceCode}`, { replace: true });
+ if (encodedDeviceCode) {
+ navigate(`/device/connect?code=${encodedDeviceCode}`, { replace: true });
} else {
navigate('/dashboard', { replace: true });
}- state: { email: formData.email, deviceCode },
+ state: { email: formData.email, deviceCode },
});- to={deviceCode ? `/login?code=${deviceCode}` : '/login'}
+ to={encodedDeviceCode ? `/login?code=${encodedDeviceCode}` : '/login'}Also applies to: 201-203, 446-447
🤖 Prompt for AI Agents
In `@src/components/SignupForm.tsx` around lines 60 - 65, The route builder uses
raw deviceCode when calling navigate, which can break the query string; update
the calls that build the URL (the block using deviceCode in SignupForm around
the navigate(`/device/connect?code=${deviceCode}`, ...) call and the other
occurrences at the indicated spots) to URL-encode the parameter by calling
encodeURIComponent(deviceCode) when interpolating the query value (e.g.,
`/device/connect?code=${encodeURIComponent(deviceCode)}`), preserving the
existing navigate options like { replace: true }.
| // Debug: log device code source | ||
| console.log('[VerifyEmail] Device code:', deviceCode); | ||
| console.log('[VerifyEmail] From state:', location.state?.deviceCode); | ||
| console.log( | ||
| '[VerifyEmail] From localStorage:', | ||
| localStorage.getItem('pending_device_code') | ||
| ); |
There was a problem hiding this comment.
Remove device code logs from the console.
Device codes are sensitive; logging them increases exposure risk.
🔒 Suggested fix
- // Debug: log device code source
- console.log('[VerifyEmail] Device code:', deviceCode);
- console.log('[VerifyEmail] From state:', location.state?.deviceCode);
- console.log(
- '[VerifyEmail] From localStorage:',
- localStorage.getItem('pending_device_code')
- );
+ // Avoid logging device codes in console (sensitive)📝 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.
| // Debug: log device code source | |
| console.log('[VerifyEmail] Device code:', deviceCode); | |
| console.log('[VerifyEmail] From state:', location.state?.deviceCode); | |
| console.log( | |
| '[VerifyEmail] From localStorage:', | |
| localStorage.getItem('pending_device_code') | |
| ); | |
| // Avoid logging device codes in console (sensitive) |
🤖 Prompt for AI Agents
In `@src/components/VerifyEmail.tsx` around lines 17 - 23, Remove the debug
console logs that output sensitive device codes in the VerifyEmail component:
delete the console.log calls referencing deviceCode, location.state?.deviceCode,
and localStorage.getItem('pending_device_code'); ensure no other places in the
component (or helper functions used by VerifyEmail) log these values and keep
any non-sensitive diagnostic logging only.
| </p> | ||
| <Link | ||
| to="/login" | ||
| to={deviceCode ? `/login?code=${deviceCode}` : '/login'} |
There was a problem hiding this comment.
URL-encode deviceCode in login links.
If the code contains reserved characters, the query string can break.
🛠️ Proposed fix
- const deviceCode =
- location.state?.deviceCode || localStorage.getItem('pending_device_code');
+ const deviceCode =
+ location.state?.deviceCode || localStorage.getItem('pending_device_code');
+ const encodedDeviceCode = deviceCode ? encodeURIComponent(deviceCode) : null;- to={deviceCode ? `/login?code=${deviceCode}` : '/login'}
+ to={encodedDeviceCode ? `/login?code=${encodedDeviceCode}` : '/login'}Also applies to: 157-157, 177-177
🤖 Prompt for AI Agents
In `@src/components/VerifyEmail.tsx` at line 115, The login link builds a query
string using deviceCode in VerifyEmail.tsx which can break if deviceCode
contains reserved characters; update every place that sets the JSX "to" prop
with a query using deviceCode (the occurrences around where deviceCode is used
for login links) to URL-encode the value by applying encodeURIComponent to
deviceCode when constructing the URL so the query parameter is safely encoded.
| // Check if there's a pending device code | ||
| const deviceCode = localStorage.getItem('pending_device_code'); | ||
| console.log( | ||
| '[completeOnboarding] Device code from localStorage:', | ||
| deviceCode | ||
| ); | ||
|
|
||
| if (deviceCode) { | ||
| // Navigate with state (cleanup will happen in DeviceConnect after mount) | ||
| console.log( | ||
| '[completeOnboarding] Navigating to device connect with code via state' | ||
| ); // Don't remove yet - ProtectedRoute needs to detect this! | ||
| // localStorage.removeItem('pending_device_code'); | ||
| navigate('/device/connect', { | ||
| state: { deviceCode }, | ||
| replace: true, | ||
| }); | ||
| } else { | ||
| console.log( | ||
| '[completeOnboarding] No device code found, redirecting to dashboard' | ||
| ); | ||
| navigate('/dashboard'); |
There was a problem hiding this comment.
Avoid logging device codes in console.
Device codes are sensitive; console logs can leak them in shared/devtools sessions. Prefer removing or logging only presence (boolean).
🔒 Suggested fix (redact or remove)
- console.log(
- '[completeOnboarding] Device code from localStorage:',
- deviceCode
- );
+ if (process.env.NODE_ENV === 'development') {
+ console.debug(
+ '[completeOnboarding] Has device code in localStorage:',
+ Boolean(deviceCode)
+ );
+ }📝 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.
| // Check if there's a pending device code | |
| const deviceCode = localStorage.getItem('pending_device_code'); | |
| console.log( | |
| '[completeOnboarding] Device code from localStorage:', | |
| deviceCode | |
| ); | |
| if (deviceCode) { | |
| // Navigate with state (cleanup will happen in DeviceConnect after mount) | |
| console.log( | |
| '[completeOnboarding] Navigating to device connect with code via state' | |
| ); // Don't remove yet - ProtectedRoute needs to detect this! | |
| // localStorage.removeItem('pending_device_code'); | |
| navigate('/device/connect', { | |
| state: { deviceCode }, | |
| replace: true, | |
| }); | |
| } else { | |
| console.log( | |
| '[completeOnboarding] No device code found, redirecting to dashboard' | |
| ); | |
| navigate('/dashboard'); | |
| // Check if there's a pending device code | |
| const deviceCode = localStorage.getItem('pending_device_code'); | |
| if (process.env.NODE_ENV === 'development') { | |
| console.debug( | |
| '[completeOnboarding] Has device code in localStorage:', | |
| Boolean(deviceCode) | |
| ); | |
| } | |
| if (deviceCode) { | |
| // Navigate with state (cleanup will happen in DeviceConnect after mount) | |
| console.log( | |
| '[completeOnboarding] Navigating to device connect with code via state' | |
| ); // Don't remove yet - ProtectedRoute needs to detect this! | |
| // localStorage.removeItem('pending_device_code'); | |
| navigate('/device/connect', { | |
| state: { deviceCode }, | |
| replace: true, | |
| }); | |
| } else { | |
| console.log( | |
| '[completeOnboarding] No device code found, redirecting to dashboard' | |
| ); | |
| navigate('/dashboard'); |
🤖 Prompt for AI Agents
In `@src/hooks/useAuth.tsx` around lines 172 - 193, In completeOnboarding (in
useAuth.tsx) remove any console logs that print the actual device code retrieved
from localStorage ('pending_device_code'); instead log only a boolean/presence
check or remove the logs entirely, e.g., replace
console.log('[completeOnboarding] Device code from localStorage:', deviceCode)
with a presence-only message and avoid outputting deviceCode, and ensure any
other logging around navigate('/device/connect', { state: { deviceCode } }) does
not expose the code (DeviceConnect will handle cleanup).
There was a problem hiding this comment.
Pull request overview
This PR implements an OAuth device code flow for CLI authorization, allowing users to authenticate their CLI tools through the web application. The implementation handles both new and existing users, managing the device code through signup, login, email verification, and onboarding flows.
Changes:
- Added DeviceConnect component to handle CLI device authorization confirmation
- Updated LoginForm and SignupForm to extract, persist, and propagate device codes through authentication flows
- Modified useAuth's completeOnboarding to redirect to device connect when a pending device code exists
- Enhanced ProtectedRoute to handle device code scenarios during onboarding redirection
- Updated OAuthCallback to preserve device codes through OAuth flows
- Modified VerifyEmail to maintain device codes through email verification
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 29 comments.
Show a summary per file
| File | Description |
|---|---|
| src/components/DeviceConnect.tsx | New component for confirming CLI device authorization with user code display and confirmation UI |
| src/components/LoginForm.tsx | Added device code extraction from URL, storage in localStorage/sessionStorage, and conditional navigation based on onboarding status |
| src/components/SignupForm.tsx | Added device code persistence through localStorage/sessionStorage for email verification and OAuth flows |
| src/hooks/useAuth.tsx | Modified completeOnboarding to check for pending device codes and redirect to device connect page |
| src/components/ProtectedRoute.tsx | Added logic to handle pending device codes during onboarding redirect checks |
| src/components/OAuthCallback.tsx | Added device code retrieval from sessionStorage and navigation to device connect page |
| src/components/VerifyEmail.tsx | Added device code propagation through verification flow and login redirects |
| src/components/AuthApp.tsx | Registered new /device/connect protected route |
Comments suppressed due to low confidence (1)
src/components/LoginForm.tsx:250
- There is duplicated navigation logic in this component. The same device code handling logic appears in the useEffect (lines 77-93) and in the handleSubmit function (lines 235-244). This duplication increases maintenance burden and the risk of inconsistencies. Consider extracting this logic into a shared function or removing one of the implementations. The setTimeout navigation after login (lines 234-250) may also conflict with the useEffect navigation if the component re-renders after the login call updates the auth state.
setTimeout(() => {
if (deviceCode) {
// Check onboarding status before redirecting
if (data.user.onboardingCompleted) {
// Existing user with device code → device connect
navigate(`/device/connect?code=${deviceCode}`);
} else {
// New user with device code → store for post-onboarding redirect
localStorage.setItem('pending_device_code', deviceCode);
navigate('/onboarding');
}
} else if (data.user.onboardingCompleted) {
navigate('/dashboard');
} else {
navigate('/onboarding');
}
}, 1000);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // BUT: Don't redirect if there's a pending device code (user just completed onboarding and navigating to device connect) | ||
| if (user && user.onboardingCompleted && location.pathname === '/onboarding') { | ||
| const hasPendingDeviceCode = localStorage.getItem('pending_device_code'); | ||
| if (hasPendingDeviceCode) { | ||
| console.log( | ||
| '[ProtectedRoute] Has pending device code, allowing navigation to complete' | ||
| ); | ||
| return <>{children}</>; |
There was a problem hiding this comment.
The logic for checking pending device code allows rendering children when on the onboarding page with a pending device code, but this doesn't make sense. If the user is on the onboarding page with onboarding already completed AND has a pending device code, they should be redirected to the device connect page, not allowed to view the onboarding page. The current implementation would render the onboarding page content when it should redirect away from it.
| // BUT: Don't redirect if there's a pending device code (user just completed onboarding and navigating to device connect) | |
| if (user && user.onboardingCompleted && location.pathname === '/onboarding') { | |
| const hasPendingDeviceCode = localStorage.getItem('pending_device_code'); | |
| if (hasPendingDeviceCode) { | |
| console.log( | |
| '[ProtectedRoute] Has pending device code, allowing navigation to complete' | |
| ); | |
| return <>{children}</>; | |
| // BUT: If there's a pending device code, redirect to the device connect flow instead | |
| if (user && user.onboardingCompleted && location.pathname === '/onboarding') { | |
| const hasPendingDeviceCode = localStorage.getItem('pending_device_code'); | |
| if (hasPendingDeviceCode) { | |
| console.log( | |
| '[ProtectedRoute] Has pending device code, redirecting to device connect' | |
| ); | |
| return <Navigate to="/device-connect" replace />; |
| return <Navigate to="/dashboard" replace />; | ||
| } | ||
|
|
||
| console.log('[ProtectedRoute] Rendering children'); |
There was a problem hiding this comment.
Remove debug console.log statement before merging to production.
| console.log('[ProtectedRoute] Rendering children'); |
| console.log( | ||
| '[LoginForm] Existing user, redirecting to device connect' | ||
| ); | ||
| navigate(`/device/connect?code=${deviceCode}`, { replace: true }); | ||
| } else { | ||
| // New user with device code → store in localStorage for post-onboarding redirect | ||
| // ProtectedRoute will redirect to /onboarding | ||
| console.log( | ||
| '[LoginForm] New user, storing device code and redirecting to dashboard (will be intercepted by ProtectedRoute)' | ||
| ); | ||
| localStorage.setItem('pending_device_code', deviceCode); | ||
| navigate('/dashboard', { replace: true }); |
There was a problem hiding this comment.
Remove debug console.log statements before merging to production.
| if (!loading && !userCode) { | ||
| return ( | ||
| <div className="min-h-screen bg-black flex items-center justify-center px-4 font-space"> | ||
| <div className="text-center"> | ||
| <h2 className="text-2xl font-normal text-white mb-4"> | ||
| No Device Code | ||
| </h2> | ||
| <p className="text-neutral-400 mb-6"> | ||
| No device code was provided in the URL. | ||
| </p> | ||
| <button | ||
| onClick={() => navigate('/dashboard')} | ||
| className="bg-white text-black font-medium px-6 py-3 rounded-lg hover:opacity-90" | ||
| > | ||
| Go to Dashboard | ||
| </button> | ||
| </div> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
The condition checks if userCode is falsy, but userCode could be null when loading. This means the error UI could flash briefly before the authentication check completes. Consider checking the loading state here as well to avoid showing the error UI prematurely.
| console.log( | ||
| '[DeviceConnect] Loaded with code:', | ||
| userCode, | ||
| 'from', | ||
| (location.state as { deviceCode?: string })?.deviceCode ? 'state' : 'URL' | ||
| ); |
There was a problem hiding this comment.
Sensitive information (device authorization code) is being logged to the console. Even though these are debug logs meant to be removed, the pattern of logging potentially sensitive data should be avoided entirely. Device codes should be treated as secrets and never logged, as they could be exposed in production if these logs are accidentally left in.
| console.log( | |
| '[DeviceConnect] Loaded with code:', | |
| userCode, | |
| 'from', | |
| (location.state as { deviceCode?: string })?.deviceCode ? 'state' : 'URL' | |
| ); | |
| // Note: Do not log userCode or device authorization codes, as they are sensitive. |
| setStatus('success'); | ||
| } catch (error) { | ||
| setStatus('error'); | ||
| setErrorMessage( | ||
| error instanceof Error | ||
| ? error.message | ||
| : 'Failed to authorize device. Please try again.' | ||
| ); | ||
| } finally { | ||
| setConfirming(false); | ||
| } |
There was a problem hiding this comment.
After successfully confirming the device authorization, the cleanup of localStorage for 'pending_device_code' is missing. The code removes it on mount, but if a user lands on this page via URL parameter (not via state), the localStorage will already have been cleaned up. However, if they successfully confirm, there's no additional cleanup. Also consider cleaning up sessionStorage 'oauth_device_code' if it exists to ensure no stale data remains.
| // Debug: log device code source | ||
| console.log('[VerifyEmail] Device code:', deviceCode); | ||
| console.log('[VerifyEmail] From state:', location.state?.deviceCode); | ||
| console.log( | ||
| '[VerifyEmail] From localStorage:', | ||
| localStorage.getItem('pending_device_code') | ||
| ); |
There was a problem hiding this comment.
Sensitive information (device code) is being logged to the console. Device codes should be treated as secrets and never logged to the console, as they could be exposed if these logs are accidentally left in production or viewed by malicious actors through browser DevTools.
| // Debug: log device code source | |
| console.log('[VerifyEmail] Device code:', deviceCode); | |
| console.log('[VerifyEmail] From state:', location.state?.deviceCode); | |
| console.log( | |
| '[VerifyEmail] From localStorage:', | |
| localStorage.getItem('pending_device_code') | |
| ); | |
| // Optional debug (non-sensitive): indicate whether a device code is present and its source | |
| if (process.env.NODE_ENV === 'development') { | |
| const hasStateCode = Boolean(location.state?.deviceCode); | |
| const hasStoredCode = Boolean(localStorage.getItem('pending_device_code')); | |
| // Note: do NOT log the actual device code value | |
| console.log('[VerifyEmail] Device code presence:', { | |
| fromState: hasStateCode, | |
| fromLocalStorage: hasStoredCode, | |
| }); | |
| } |
| // Debug: log device code source | ||
| console.log('[VerifyEmail] Device code:', deviceCode); | ||
| console.log('[VerifyEmail] From state:', location.state?.deviceCode); | ||
| console.log( | ||
| '[VerifyEmail] From localStorage:', | ||
| localStorage.getItem('pending_device_code') | ||
| ); | ||
|
|
There was a problem hiding this comment.
Remove debug console.log statements before merging to production. These logging statements should be removed or replaced with a proper logging mechanism.
| // Debug: log device code source | |
| console.log('[VerifyEmail] Device code:', deviceCode); | |
| console.log('[VerifyEmail] From state:', location.state?.deviceCode); | |
| console.log( | |
| '[VerifyEmail] From localStorage:', | |
| localStorage.getItem('pending_device_code') | |
| ); |
| // Redirect if not authenticated | ||
| useEffect(() => { | ||
| if (!loading && !isAuthenticated) { | ||
| navigate(`/login?code=${userCode}`, { replace: true }); |
There was a problem hiding this comment.
The redirect uses the userCode from state, which might be undefined if there's no device code. This would result in a redirect to '/login?code=undefined'. Add a check to ensure userCode exists before including it in the navigation URL, or provide a fallback redirect to '/login' without the code parameter.
| navigate(`/login?code=${userCode}`, { replace: true }); | |
| if (userCode) { | |
| navigate(`/login?code=${userCode}`, { replace: true }); | |
| } else { | |
| navigate('/login', { replace: true }); | |
| } |
| Device Code | ||
| </p> | ||
| <p className="text-2xl font-mono text-white tracking-widest"> | ||
| {userCode} |
There was a problem hiding this comment.
The device code from the URL query parameter is displayed directly in the UI without explicit sanitization. While React generally escapes values automatically, it's important to validate that the device code matches the expected format (e.g., alphanumeric, specific length) before displaying it to prevent potential XSS or UI injection attacks. Add validation to ensure the code matches the expected format from your backend.
| {userCode} | |
| {typeof userCode === 'string' && /^[A-Z0-9-]{6,32}$/.test(userCode) | |
| ? userCode | |
| : 'Invalid device code'} |
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.