-
Notifications
You must be signed in to change notification settings - Fork 117
fix: bug fixes in backend and frontend integration #155
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,18 @@ | |
| import axios, { AxiosInstance } from 'axios'; | ||
| import { supabase } from './supabaseClient'; | ||
|
|
||
| // Prevents multiple simultaneous 401 handlers from racing and triggering | ||
| // sign-out/redirect more than once. | ||
| let isHandlingUnauthorized = false; | ||
|
Comment on lines
+7
to
+9
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Race condition guard never resets — breaks subsequent 401 handling.
Proposed fixSince if (!isHandlingUnauthorized) {
isHandlingUnauthorized = true;
try {
await supabase.auth.signOut();
} catch (e) {
// Log but continue to redirect; signing out isn't critical here
console.error(
'Error signing out on 401 handler',
e
);
}
const returnUrl = encodeURIComponent(
pathname + window.location.search
);
window.location.href = `/login?returnUrl=${returnUrl}`;
+ // Reset after a delay in case navigation is blocked (e.g., beforeunload handler)
+ setTimeout(() => {
+ isHandlingUnauthorized = false;
+ }, 2000);
}Also applies to: 109-125 🤖 Prompt for AI Agents |
||
|
|
||
| // Public routes where we should NOT force a redirect to login | ||
| const PUBLIC_PATHS = [ | ||
| '/login', | ||
| '/signup', | ||
| '/forgot-password', | ||
| '/reset-password', | ||
| ]; | ||
|
|
||
| // Backend API base URL | ||
| const API_BASE_URL = | ||
| import.meta.env.VITE_BACKEND_URL || 'http://localhost:8000'; | ||
|
|
@@ -83,10 +95,34 @@ class ApiClient { | |
| // Add response interceptor for error handling | ||
| this.client.interceptors.response.use( | ||
| (response) => response, | ||
| (error) => { | ||
| async (error) => { | ||
| if (error.response?.status === 401) { | ||
| // Handle unauthorized - could redirect to login | ||
| console.error('Unauthorized request'); | ||
| const pathname = window.location.pathname || '/'; | ||
|
|
||
| // If we're already on a public/auth page (login, signup, reset, etc.) | ||
| // don't yank the user away mid-flow. | ||
| if (PUBLIC_PATHS.some((p) => pathname.startsWith(p))) { | ||
| return Promise.reject(error); | ||
| } | ||
|
|
||
| // Deduplicate concurrent 401 handlers to avoid multiple sign-outs / redirects | ||
| if (!isHandlingUnauthorized) { | ||
| isHandlingUnauthorized = true; | ||
| try { | ||
| await supabase.auth.signOut(); | ||
| } catch (e) { | ||
| // Log but continue to redirect; signing out isn't critical here | ||
| console.error( | ||
| 'Error signing out on 401 handler', | ||
| e | ||
| ); | ||
| } | ||
|
|
||
| const returnUrl = encodeURIComponent( | ||
| pathname + window.location.search | ||
| ); | ||
| window.location.href = `/login?returnUrl=${returnUrl}`; | ||
| } | ||
| } | ||
| return Promise.reject(error); | ||
| } | ||
|
|
@@ -166,7 +202,10 @@ class ApiClient { | |
| const response = await this.client.get('/v1/health'); | ||
| return response.status === 200; | ||
| } catch (error) { | ||
| console.error('Backend health check failed:', error); | ||
| // Log in development for debugging | ||
| if (import.meta.env.DEV) { | ||
| console.error('Health check failed:', error); | ||
| } | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Open redirect vulnerability:
startsWith('/')is insufficient.The check
decoded.startsWith('/')can be bypassed with protocol-relative URLs like//evil.comor backslash variants like/\evil.com, which browsers may interpret as external redirects.Proposed fix
const rawReturn = searchParams.get('returnUrl'); try { const decoded = rawReturn ? decodeURIComponent(rawReturn) : null; // Protect against open redirects by only allowing internal paths - if (decoded && decoded.startsWith('/')) { + if (decoded && decoded.startsWith('/') && !decoded.startsWith('//') && !decoded.startsWith('/\\')) { navigate(decoded, { replace: true }); } else { navigate('/', { replace: true }); }📝 Committable suggestion
🤖 Prompt for AI Agents