Improve mobile UX and harden auth flows#81
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR targets two areas of the FitBuddyAI codebase: (1) a broad mobile UX refresh across key screens/components, and (2) an auth/data-sync hardening effort that removes client-side refresh token persistence and moves refresh handling server-side.
Changes:
- Adds a new intro/landing experience (plus a settings toggle) and improves mobile layout/styling across multiple pages.
- Introduces server-side refresh token storage/refresh endpoints and updates client auth flows accordingly.
- Updates shop purchase flow and several data-sync behaviors (cloud backup payload, loading progress telemetry, chat rendering, calendar empty-state behavior).
Reviewed changes
Copilot reviewed 47 out of 48 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| src/services/localStorage.ts | Adds home-intro preference storage + removes persisted auth token fallback/session token persistence. |
| src/services/cloudBackupService.ts | Adjusts what local keys get included in backup payload. |
| src/services/authService.ts | Updates sign-in/up flows, password policy helper, purchase endpoint call, server-side refresh storage calls. |
| src/services/aiService.ts | Updates Gemini model and increases chat context caps / improves fallback messages. |
| src/server/authServer.js | Adds rate limits, increases JSON body size, moves /api/user/buy to Supabase-backed logic, improves AI endpoint behavior. |
| src/server/authServer.cjs | Increases JSON body size limit. |
| src/server/adminRoutes.js | Increases JSON body size limit for admin routes. |
| src/index.css | Global mobile layout hardening (width/min-width, overflow wrapping, image max-width, mobile padding). |
| src/components/WorkoutsPage.tsx | Simplifies saved-workout mutations to direct list updates + persistence. |
| src/components/WorkoutsPage.css | Mobile responsiveness improvements (filters, hero layout, buttons). |
| src/components/WorkoutModal.tsx | Renders modal via portal to document.body. |
| src/components/WorkoutModal.css | Raises modal overlay z-index. |
| src/components/WorkoutCalendar.tsx | Introduces blank calendar plan when no plan exists; improves day header labels for mobile. |
| src/components/WorkoutCalendar.css | Mobile layout updates; short day headers; improved dark hero-card backdrop filter. |
| src/components/WelcomePage.tsx | Wires new intro animation component behind a settings toggle. |
| src/components/WelcomePage.css | Mobile hero/logo layout tweaks; overflow adjustments. |
| src/components/SignUpPage.tsx | Adds client-side password policy validation when using Supabase. |
| src/components/SignInPage.tsx | Adds page-level body class for mobile styling; removes username sessionStorage write. |
| src/components/SignInPage.css | Mobile-first sign-in layout refinements; hides footer on small screens. |
| src/components/ShopPage.tsx | Removes client-side streak-saver purchase logic; adds local override guard to polling. |
| src/components/SettingsPage.tsx | Adds toggle UI for enabling/disabling the home intro animation. |
| src/components/Questionnaire.tsx | Emits structured loading progress events/state to drive the global loading page. |
| src/components/NewIntroPage.tsx | Adds a new intro/landing page component. |
| src/components/NewIntroPage.css | Adds styling for new intro/landing page. |
| src/components/LoadingPage.tsx | Reads loading progress state from storage/events; displays progress bar + stage text. |
| src/components/LoadingPage.css | Adds progress bar styling + mobile-friendly layout. |
| src/components/IntroBubbles.tsx | Moves per-bubble motion variables from inline styles to CSS classes. |
| src/components/IntroBubbles.css | Adds per-bubble CSS variables for motion. |
| src/components/HomeIntroStorm.tsx | Adds new “storm” intro animation component. |
| src/components/HomeIntroStorm.css | Adds styling/animation for the new storm intro sequence (with reduced-motion support). |
| src/components/HelpCenter.css | Full help center CSS redesign with new responsive/theme-aware layout. |
| src/components/Header.tsx | Refactors explore drawer + adds a mobile bottom nav dock. |
| src/components/Header.css | Styles new drawer layout + mobile nav dock; hides desktop header elements on mobile. |
| src/components/GeminiChatPage.tsx | Adds link parsing/rendering for chat output and normalizes demo-mode messaging. |
| src/components/GeminiChatPage.css | Styles chat links and improves dark-mode scrollbar presentation. |
| src/components/Footer.css | Adds bottom padding on mobile to accommodate bottom nav dock. |
| src/components/BlogPage.css | Adds styles used by new blog list layout. |
| src/components/BlogListPage.tsx | Adds a blog list/landing page. |
| src/components/AgreementGuard.tsx | Attempts to fetch server acceptance state and persist local acceptance flags. |
| src/App.tsx | Adds home-intro preference wiring, server-side refresh call, user polling throttling/guards, blog routing changes, and local purchase handling changes. |
| api/userdata/index.ts | Expands allowed fields for persistence (avatar_url/energy/inventory/streak) and normalizes avatar mapping. |
| api/user/[id].js | Adds authenticated action=buy purchase flow on user route. |
| api/auth/index.ts | Adds encrypted refresh-token storage + refresh/revoke endpoints using HttpOnly cookie session id. |
| api/ai/generate.ts | Updates Gemini model string. |
| .vscode/settings.json | Adds workspace terminal auto-approve settings for chat tools. |
| .vscode/mcp.json | Adds Supabase MCP server configuration. |
| .gitignore | Ignores a local test credentials JSON file. |
| @@ -329,9 +406,44 @@ function App() { | |||
| // Deduct energy and add item to user inventory (simplified) | |||
| setUserData((prev: any) => { | |||
| if (!prev) return prev; | |||
| const newEnergy = (prev.energy || 0) - item.price; | |||
| const inventory = Array.isArray(prev.inventory) ? prev.inventory : []; | |||
| return { ...prev, energy: newEnergy, inventory: [...inventory, item] }; | |||
| const itemPrice = Number(item?.price ?? 0); | |||
| const newEnergy = Math.max(0, (prev.energy ?? 0) - itemPrice); | |||
| const inventory = Array.isArray(prev.inventory) ? [...prev.inventory] : []; | |||
| const itemId = String(item?.id || ''); | |||
| let nextInventory = [...inventory]; | |||
| if (itemId.startsWith('streak-saver')) { | |||
| const quantity = Number(item?.quantity ?? item?.count ?? 1); | |||
| const normalizedQuantity = Number.isFinite(quantity) && quantity > 0 ? quantity : 1; | |||
| const existingIndex = nextInventory.findIndex((entry: any) => String(entry?.id || '').startsWith('streak-saver')); | |||
| const purchasedItem = { | |||
| ...item, | |||
| quantity: normalizedQuantity, | |||
| purchased_at: new Date().toISOString() | |||
| }; | |||
| if (existingIndex >= 0) { | |||
| const existing = nextInventory[existingIndex]; | |||
| const existingQty = Number(existing.quantity ?? existing.count ?? 1); | |||
| nextInventory[existingIndex] = { | |||
| ...existing, | |||
| ...purchasedItem, | |||
| quantity: (Number.isFinite(existingQty) ? existingQty : 1) + normalizedQuantity | |||
| }; | |||
| } else { | |||
| nextInventory.push(purchasedItem); | |||
| } | |||
| } else { | |||
| nextInventory.push(item); | |||
| } | |||
| const nextUser = { ...prev, energy: newEnergy, inventory: nextInventory }; | |||
| try { | |||
| saveUserData({ data: nextUser }, { skipBackup: true }); | |||
| sessionStorage.setItem('fitbuddyai_local_user_override', '1'); | |||
| localStorage.setItem('fitbuddyai_local_user_override', '1'); | |||
| window.dispatchEvent(new Event('storage')); | |||
| } catch (error) { | |||
| console.warn('[App] Failed to persist local shop purchase:', error); | |||
| } | |||
| return nextUser; | |||
There was a problem hiding this comment.
handleShopPurchase locally deducts energy/adds items even though purchases are now server-backed, which can double-apply changes (ShopPage already persists updated). It also sets fitbuddyai_local_user_override to disable server polling, but there is no corresponding clear path, so user sync may be permanently disabled. Consider updating state from the server purchase response instead, and if an override is still needed, clear it after the next successful server sync / on navigation / after a timeout.
| if (fitbuddyai_questionnaire_progress != null) payload.fitbuddyai_questionnaire_progress = fitbuddyai_questionnaire_progress; | ||
| if (fitbuddyai_workout_plan != null) payload.fitbuddyai_workout_plan = fitbuddyai_workout_plan; | ||
| if (fitbuddyai_assessment_data != null) payload.fitbuddyai_assessment_data = fitbuddyai_assessment_data; | ||
| // include chat history and user data if present so server can persist chat_history into payload | ||
| if (fitbuddyai_chat != null) payload.chat_history = fitbuddyai_chat; | ||
| if (fitbuddyai_user_data != null) payload.fitbuddyai_user_data = fitbuddyai_user_data; | ||
|
|
||
| // Attach local fitbuddyai_user_data so server can cross-check client identity when needed | ||
| try { |
There was a problem hiding this comment.
The comment says to “include chat history and user data”, but fitbuddyai_user_data is no longer read or added to the backup payload in this function. Update the comment (or restore the behavior) so the implementation and documentation stay consistent.
| { | ||
| "servers": { | ||
| "supabase": { | ||
| "type": "http", | ||
| "url": "https://mcp.supabase.com/mcp?project_ref=hkvkomtaffybchjdjfws" | ||
| } | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
.vscode/mcp.json hardcodes a Supabase MCP server URL with a specific project_ref. This can unintentionally expose internal project identifiers and causes contributors' editors to connect to that project by default. Prefer documenting the setup and keeping per-developer MCP configuration out of the repository (or provide an example file).
| if (action === 'store_refresh') { | ||
| const { userId, refresh_token } = req.body as { userId?: string; refresh_token?: string }; | ||
| if (!userId || !refresh_token) return res.status(400).json({ message: 'userId and refresh_token required.' }); | ||
|
|
||
| try { | ||
| await supabase.from('fitbuddyai_refresh_tokens').update({ revoked: true }).eq('user_id', userId).neq('revoked', true); | ||
| } catch (e) { |
There was a problem hiding this comment.
store_refresh accepts an arbitrary userId + refresh_token without authenticating the caller, and revokes all existing sessions for that user. This enables trivial account-wide session revocation (DoS) and potentially session fixation. Require proof of identity (e.g., Bearer access token verified via supabase.auth.getUser() and matching userId) before writing/revoking refresh sessions, and avoid revoking all sessions unless explicitly intended/admin-only.
| const { data: user, error: fetchError } = await supabase | ||
| .from('fitbuddyai_userdata') | ||
| .select('*') | ||
| .eq('user_id', id) | ||
| .maybeSingle(); | ||
|
|
||
| if (fetchError) { | ||
| console.error('[authServer] /api/user/buy fetch error', fetchError); | ||
| return res.status(500).json({ message: 'Failed to load user.' }); | ||
| } | ||
| if (!user) return res.status(404).json({ message: 'User not found.' }); | ||
| if (typeof user.energy !== 'number' || user.energy < item.price) { | ||
|
|
||
| const currentEnergy = Number.isFinite(Number(user.energy)) ? Number(user.energy) : 0; | ||
| if (currentEnergy < itemPrice) { | ||
| return res.status(400).json({ message: 'Not enough energy.' }); | ||
| } | ||
| user.energy -= item.price; | ||
| if (!Array.isArray(user.inventory)) user.inventory = []; | ||
| user.inventory.push(item); | ||
| writeUsers(users); | ||
| const { password: _password, ...userSafe } = user; | ||
| res.json({ user: userSafe }); | ||
|
|
||
| const nextInventory = Array.isArray(user.inventory) ? [...user.inventory] : []; | ||
| const itemId = String(item.id || ''); | ||
| const purchasedItem = { | ||
| ...item, | ||
| price: itemPrice, | ||
| purchased_at: new Date().toISOString() | ||
| }; | ||
|
|
||
| if (itemId.startsWith('streak-saver')) { | ||
| const quantity = Number(item.quantity ?? item.count ?? 1); | ||
| const normalizedQuantity = Number.isFinite(quantity) && quantity > 0 ? quantity : 1; | ||
| const existingIndex = nextInventory.findIndex((entry) => String(entry?.id || '').startsWith('streak-saver')); | ||
| if (existingIndex >= 0) { | ||
| const existing = nextInventory[existingIndex]; | ||
| const existingQty = Number(existing?.quantity ?? existing?.count ?? 1); | ||
| nextInventory[existingIndex] = { | ||
| ...existing, | ||
| ...purchasedItem, | ||
| quantity: (Number.isFinite(existingQty) ? existingQty : 1) + normalizedQuantity | ||
| }; | ||
| } else { | ||
| nextInventory.push({ ...purchasedItem, quantity: normalizedQuantity }); | ||
| } | ||
| } else { | ||
| nextInventory.push(purchasedItem); | ||
| } | ||
|
|
||
| const updates = { | ||
| energy: currentEnergy - itemPrice, | ||
| inventory: nextInventory, | ||
| updated_at: new Date().toISOString() | ||
| }; | ||
|
|
||
| const { data: updatedUser, error: updateError } = await supabase | ||
| .from('fitbuddyai_userdata') | ||
| .update(updates) | ||
| .eq('user_id', id) | ||
| .select('*') | ||
| .maybeSingle(); |
There was a problem hiding this comment.
Purchase handling is not atomic: you read energy, check it, then write energy - price. Two concurrent requests can both pass the check and overspend. Prefer a single conditional update (e.g., update ... where user_id = ? and energy >= ? and verify 1 row updated) or a transaction/RPC so energy can't go negative under concurrency.
| const removeFromPlan = (item: Workout & { title: string }) => { | ||
| setMySavedNames(prev => { | ||
| if (!prev.includes(item.title)) { | ||
| showFitBuddyNotification({ message: 'That workout is not in your saved list.', variant: 'error' }); | ||
| return prev; | ||
| } | ||
| const next = removeSavedName(item.title); | ||
| showFitBuddyNotification({ message: item.title + ' removed from your workouts.' }); | ||
| return next; | ||
| }); | ||
| if (!mySavedNames.includes(item.title)) { | ||
| showFitBuddyNotification({ message: 'That workout is not in your saved list.', variant: 'error' }); | ||
| return; | ||
| } | ||
| const next = mySavedNames.filter(title => title !== item.title); | ||
| setMySavedNames(next); | ||
| persistSavedNames(next); | ||
| showFitBuddyNotification({ message: item.title + ' removed from your workouts.' }); |
There was a problem hiding this comment.
addToPlan/removeFromPlan compute the next saved list from the current mySavedNames closure and then call setMySavedNames(next). This can drop updates under rapid interactions because state updates are async and mySavedNames may be stale. Prefer functional setMySavedNames(prev => ...) and persist based on that computed next to keep UI and storage consistent.
| const app = express(); | ||
| app.use(cors()); | ||
| app.use(bodyParser.json()); | ||
| // Large AI-generated workout plans can exceed the default JSON body limit. | ||
| app.use(bodyParser.json({ limit: '20mb' })); |
There was a problem hiding this comment.
Raising the global JSON body limit to 20mb for all routes increases DoS risk and memory pressure. If only a subset of endpoints needs larger payloads (e.g., AI generation or plan uploads), apply a larger bodyParser.json({ limit }) only to those routes instead of globally.
| const app = express(); | ||
| app.use(cors()); | ||
| app.use(bodyParser.json()); | ||
| // Large AI-generated workout plans can exceed the default JSON body limit. | ||
| app.use(bodyParser.json({ limit: '20mb' })); |
There was a problem hiding this comment.
Same as authServer.js: increasing the global JSON body limit to 20mb affects all endpoints. If possible, keep a smaller default and only raise limits for the specific routes that require it.
| app.post('/api/user/buy', userBuyLimiter, async (req, res) => { | ||
| try { | ||
| const { id, item } = req.body; | ||
| const { id, item } = req.body || {}; | ||
| if (!id || !item) return res.status(400).json({ message: 'User ID and item required.' }); | ||
| const users = readUsers(); | ||
| const user = users.find(u => u.id === id); | ||
| if (!supabase) return res.status(500).json({ message: 'Supabase not configured.' }); | ||
|
|
There was a problem hiding this comment.
/api/user/buy updates Supabase user rows based only on the id in the request body; there is no authentication/authorization check that the caller owns that user id. This allows anyone to spend another user's energy / mutate inventory. Validate the Bearer token (or cookie session) and ensure it matches id before performing the purchase.
| @@ -92,17 +91,15 @@ const WorkoutsPage: React.FC = () => { | |||
| const picks = pickWorkoutsFromAssessment(assessment); | |||
| let added = 0; | |||
| // Persist only names using the new saved-names approach | |||
| setMySavedNames(prev => { | |||
| const next = [...prev]; | |||
| picks.forEach(p => { | |||
| if (!next.includes(p.title)) { | |||
| next.push(p.title); | |||
| added += 1; | |||
| } | |||
| }); | |||
| try { persistSavedNames(next); } catch (_e) {} | |||
| return next; | |||
| const next = [...mySavedNames]; | |||
| picks.forEach(p => { | |||
| if (!next.includes(p.title)) { | |||
| next.push(p.title); | |||
| added += 1; | |||
| } | |||
| }); | |||
| setMySavedNames(next); | |||
| try { persistSavedNames(next); } catch (_e) {} | |||
| showFitBuddyNotification( | |||
There was a problem hiding this comment.
In saveFromAssessment, next is derived from mySavedNames captured before the setTimeout, so it can overwrite newer saves/removals that happen during the delay. Use a functional state update inside the timeout (setMySavedNames(prev => { ... })) to merge picks with the latest list before persisting.
Co-authored-by: Copilot <copilot@github.com>
…ponent Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Summary:
Notes: