Add Email/Password Login to Mobile App#860
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a robust session-based authentication system to the mobile application, allowing users to log in with email and password, complete with multi-factor authentication options like TOTP and email OTP. The changes ensure a smooth transition for existing users while maintaining API key compatibility. Additionally, the server's authentication middleware has been streamlined, and several API-related types have been updated for better alignment and future development. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive session-based authentication system to the mobile application, replacing the previous API key-only approach. Key changes include the addition of LoginModal and ServerConfigModal components to App.tsx for handling user sign-in (email/password, multi-factor authentication including TOTP and email OTP) and server configuration. The useAuth hook is added to manage authentication state and modal visibility, triggering the login modal if no active server configuration is found or if a session expires. The authService and apiClient services are updated to support session tokens, handle MFA flows, and notify the app of session expiration or missing configurations. Corresponding test files (LoginModal.test.tsx, useAuth.test.ts, apiClient.test.ts, authService.test.ts, storage.test.ts) have been added or updated to cover the new authentication logic, including secure storage of session tokens. The OnboardingModal component has been removed, as its functionality is now integrated into the new login flow. Minor adjustments were made to API endpoint paths in profileService.ts and color variables in global.css. Additionally, the server-side authMiddleware.js is updated to correctly interpret Bearer tokens as either API keys or session tokens, and the /api/auth/settings endpoint now provides a trusted_origin for MFA. The authorize middleware was removed from authMiddleware.js and from several chatRoutes.js endpoints, simplifying permission checks. The chatRepository.js was updated to use getClient instead of getSystemClient for clearing chat history. Finally, several data models (foodEntries.ts, foodInfo.ts, foods.ts, goals.ts, mealTypes.ts, meals.ts, preferences.ts) were updated to include additional nutritional fields and preferences.
| if (req.headers.authorization && req.headers.authorization.startsWith("Bearer ")) { | ||
| const token = req.headers.authorization.split(' ')[1]; | ||
| if (token) { | ||
| if (token && token.length >= 64 && !token.includes('.')) { | ||
| req.headers['x-api-key'] = token; | ||
| log("debug", "Authentication: Mapped Bearer token to x-api-key header for Better Auth."); | ||
| delete req.headers.authorization; | ||
| log("debug", "Authentication: Mapped Bearer token to x-api-key (API key detected)."); | ||
| } else if (token) { | ||
| // Session token: sign it and inject as a session cookie so getSession() resolves it. | ||
| // We do this here instead of relying on the bearer plugin due to a compatibility | ||
| // issue with Buffer secrets in @better-auth/utils/hmac. | ||
| const prefix = auth.options.advanced?.cookiePrefix || "better-auth"; | ||
| const secureCookiePrefix = auth.options.advanced?.useSecureCookies ? "__Secure-" : ""; | ||
| const cookieName = `${secureCookiePrefix}${prefix}.session_token`; | ||
| const signed = await serializeSignedCookie("", token, auth.options.secret); | ||
| const signedValue = signed.replace("=", ""); // Strip leading = from empty cookie name | ||
| const cookieHeader = `${cookieName}=${signedValue}`; | ||
| req.headers.cookie = req.headers.cookie | ||
| ? `${req.headers.cookie}; ${cookieHeader}` | ||
| : cookieHeader; | ||
| delete req.headers.authorization; | ||
| log("debug", "Authentication: Converted Bearer session token to session cookie."); | ||
| } |
There was a problem hiding this comment.
The middleware manually injects a session cookie into req.headers.cookie when a Bearer token is provided. However, it simply appends the new cookie to the existing string. If an attacker provides a Cookie header that already contains a session token, many cookie parsers will prioritize the first occurrence. This could allow an attacker to bypass the Bearer token validation logic by providing a valid session token in the Cookie header and a dummy token in the Authorization header.
| }); | ||
| } else { | ||
| await saveServerConfig({ | ||
| id: Date.now().toString(), |
There was a problem hiding this comment.
Using Date.now().toString() is not guaranteed to generate a unique ID. While a collision is unlikely for this use case, a more robust method like generating a UUID would be preferable to prevent potential issues. If you have a crypto library available (like expo-crypto), using Crypto.randomUUID() would be ideal. Otherwise, you could improve uniqueness by adding a random component, for example: `${Date.now()}-${Math.random().toString(36).slice(2)}`.
This same pattern is also used in App.tsx for creating API key configurations and should be updated there as well.
2 participants
Tip
Help us review and merge your PR faster!
Please ensure you have completed the Checklist below.
For Frontend changes, please run
pnpm run validateto check for any errors.PRs that include tests and clear screenshots are highly preferred!
Description
Main feature: Adds support for session based (email/pw) authentication to the mobile app.
Aligned mobile types with server API in preparation for shared zod schemas
Fixed a typo a front end URL that was preventing users from setting up TOTP and Email MFA
Removed unused auth middleware leftover from chat bot routes. This let me get rid of a big scary comment that started with "In a real application, you would fetch user permissions from the DB..." thankfully the comment no longer applies to us since we are using RLS
Related Issue
PR type [ ] Issue [ ] New Feature [ ] Documentation
Linked Issue: #
Checklist
Please check all that apply:
pnpm run validate(especially for Frontend).en) translation file (if applicable).rls_policies.sqlfor any new user-specific tables.Screenshots (if applicable)
Before
[Insert screenshot/GIF here]
After
[Insert screenshot/GIF here]