Implemented Google Authentication using firebase #68
Conversation
… project (#52) * refactor(nixpacks): streamline installation commands and enhance build phase logging * docs: Add AI coding guide for Splitwiser project
--- updated-dependencies: - dependency-name: fastapi dependency-version: 0.116.1 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis update introduces Google/Firebase authentication to the Splitwiser app. It adds Firebase configuration and initialization on both backend and frontend, integrates Google OAuth login in the frontend with new UI elements, updates authentication state management, and enhances Axios interceptors. Several configuration and documentation files are also updated. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant LoginScreen
participant AuthContext
participant Firebase
participant Backend
User->>LoginScreen: Tap "Sign in with Google"
LoginScreen->>AuthContext: googleLogin()
AuthContext->>Firebase: Start Google OAuth flow
Firebase-->>AuthContext: Return Google credential
AuthContext->>Firebase: Sign in with credential
Firebase-->>AuthContext: Return Firebase ID token
AuthContext->>Backend: POST /auth/login/google (with ID token)
Backend-->>AuthContext: Return app tokens & user info
AuthContext->>LoginScreen: Update auth state/UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~35 minutes Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (5)
backend/app/firebase_config.py (1)
2-2: Remove unused import.The
authimport fromfirebase_adminis not used in this module.-from firebase_admin import credentials, auth +from firebase_admin import credentialsfrontend/package.json (1)
32-32: Consider moving expo-build-properties to devDependencies
expo-build-propertiesis a build-time configuration package that's typically not needed at runtime. Consider moving it todevDependenciesfor better dependency organization."react-native-screens": "~3.22.0", - "react-native-web": "~0.19.6", - "expo-build-properties": "~0.8.3" + "react-native-web": "~0.19.6" }, "devDependencies": { "@babel/core": "^7.20.0", "@expo/ngrok": "^4.1.0", "@types/react": "~18.2.14", - "typescript": "^5.1.3" + "typescript": "^5.1.3", + "expo-build-properties": "~0.8.3" },frontend/screens/LoginScreen.tsx (1)
167-167: Add space after comma for consistency- }, button: { + }, + button: {nixpacks.toml (2)
13-15:cp -r backend/* .is brittle and can silently overwrite filesFlattening the backend into the repo root:
- Overwrites identically-named files without warning (except for the fallback
echo, which executes after the destructive copy).- Ignores dot-files (e.g.,
.env.example,.firebase.json), whichcpdoes not copy by default.- Creates two copies of the codebase inside the image, increasing final size.
Safer alternatives:
- Run the app from
backend/directly:
cmd = "uvicorn backend.main:app …"and drop the copy step entirely.- If copying is mandatory, prefer
rsync -a --ignore-existing backend/ ./backendto preserve structure and avoid overwrites.
25-26: Redundant empty[nixpacks]sectionThe comment correctly states that you’re letting Nixpacks manage the venv, but the empty table can be removed; it has no effect and adds cognitive noise.
-# Let Nixpacks handle virtual environment creation automatically -[nixpacks] + +# Nixpacks manages the virtual env automatically (no extra config needed)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (10)
frontend/assets/adaptive-icon.pngis excluded by!**/*.pngfrontend/assets/adaptive-icon/background.pngis excluded by!**/*.pngfrontend/assets/adaptive-icon/foreground.pngis excluded by!**/*.pngfrontend/assets/favicon.pngis excluded by!**/*.pngfrontend/assets/icon.pngis excluded by!**/*.pngfrontend/assets/pwa/chrome-icon/chrome-icon-144.pngis excluded by!**/*.pngfrontend/assets/pwa/chrome-icon/chrome-icon-192.pngis excluded by!**/*.pngfrontend/assets/pwa/chrome-icon/chrome-icon-512.pngis excluded by!**/*.pngfrontend/assets/splash.pngis excluded by!**/*.pngfrontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
.github/copilot-instructions.md(1 hunks).gitignore(1 hunks)backend/app/firebase_config.py(1 hunks)backend/requirements.txt(1 hunks)frontend/app.json(1 hunks)frontend/contexts/AuthContext.tsx(3 hunks)frontend/firebase/firebaseConfig.ts(1 hunks)frontend/package.json(1 hunks)frontend/screens/LoginScreen.tsx(7 hunks)nixpacks.toml(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
backend/app/firebase_config.py (1)
frontend/firebase/firebaseConfig.ts (1)
auth(21-21)
frontend/screens/LoginScreen.tsx (1)
frontend/contexts/AuthContext.tsx (1)
useAuth(27-31)
frontend/contexts/AuthContext.tsx (1)
frontend/firebase/firebaseConfig.ts (1)
auth(21-21)
🪛 Ruff (0.12.2)
backend/app/firebase_config.py
2-2: firebase_admin.auth imported but unused
Remove unused import: firebase_admin.auth
(F401)
🔇 Additional comments (7)
backend/requirements.txt (1)
1-1: FastAPI 0.116.1 upgrade – compatibility and security confirmed
- Breaking Change – FastAPI now requires Starlette >=0.40.0,<0.48.0; no explicit Starlette pins were found in backend/requirements.txt or elsewhere in the repo.
- Security – No new vulnerabilities reported between 0.115.12 and 0.116.1.
- No further action required.
.gitignore (1)
27-27: LGTM! Appropriate addition for Expo development.Adding
eas.jsonto the gitignore is correct as this file typically contains build configuration and credentials that shouldn't be committed to version control.frontend/app.json (2)
16-18: LGTM! Consistent bundle identifier across platforms.The addition of the iOS
bundleIdentifierand alignment with the Android package name follows best practices and supports the Google OAuth integration.
25-25: LGTM! Consistent package naming.The Android package name now matches the iOS bundle identifier format, ensuring consistency across platforms.
frontend/firebase/firebaseConfig.ts (1)
18-21: LGTM! Proper Firebase v9+ initialization.The Firebase app initialization and auth export follow the correct patterns for the modern Firebase SDK.
frontend/screens/LoginScreen.tsx (1)
31-47: Clean validation logic and comprehensive error handlingThe early return pattern for validation is clean and readable. The error message extraction handles multiple possible error response formats effectively.
frontend/contexts/AuthContext.tsx (1)
171-177: Good implementation of provider-specific logoutThe code properly checks if the user signed in with Google and handles Firebase sign-out accordingly, with appropriate error handling.
.github/copilot-instructions.md
Outdated
| Splitwiser is an expense tracking and splitting application with: | ||
| - Backend: FastAPI + MongoDB | ||
| - Frontend POC: Streamlit | ||
| - Planned Frontend: Expo/React Native (in development) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update documentation to reflect current implementation state
The documentation states that Expo/React Native frontend is "planned" and "in development", but the PR shows it's actively implemented with authentication features. Consider updating this to reflect the current state.
-- Planned Frontend: Expo/React Native (in development)
+- Frontend: Expo/React Native (implemented with Google/Firebase authentication)📝 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.
| - Planned Frontend: Expo/React Native (in development) | |
| - Frontend: Expo/React Native (implemented with Google/Firebase authentication) |
🤖 Prompt for AI Agents
In .github/copilot-instructions.md at line 10, update the description of the
frontend from "Planned Frontend: Expo/React Native (in development)" to indicate
that the Expo/React Native frontend is actively implemented with authentication
features, reflecting the current state rather than a planned or in-development
status.
backend/app/firebase_config.py
Outdated
| cred = credentials.Certificate("../firebase_service_account.json") | ||
| firebase_admin.initialize_app(cred) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling and make credentials path configurable.
The current implementation has several issues:
- No error handling if the credentials file is missing
- Relative path may fail depending on execution context
- Credentials path should be configurable via environment variables
+import os
+from pathlib import Path
+
# Path to your Firebase Admin SDK JSON
-cred = credentials.Certificate("../firebase_service_account.json")
-firebase_admin.initialize_app(cred)
+def initialize_firebase():
+ """Initialize Firebase Admin SDK with proper error handling."""
+ try:
+ # Use environment variable or fallback to relative path
+ credentials_path = os.getenv(
+ 'FIREBASE_CREDENTIALS_PATH',
+ '../firebase_service_account.json'
+ )
+
+ if not Path(credentials_path).exists():
+ raise FileNotFoundError(f"Firebase credentials file not found: {credentials_path}")
+
+ cred = credentials.Certificate(credentials_path)
+ firebase_admin.initialize_app(cred)
+ print("Firebase Admin SDK initialized successfully")
+ except Exception as e:
+ print(f"Failed to initialize Firebase Admin SDK: {e}")
+ raise
+
+# Initialize Firebase
+initialize_firebase()📝 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.
| cred = credentials.Certificate("../firebase_service_account.json") | |
| firebase_admin.initialize_app(cred) | |
| import os | |
| from pathlib import Path | |
| # Path to your Firebase Admin SDK JSON | |
| def initialize_firebase(): | |
| """Initialize Firebase Admin SDK with proper error handling.""" | |
| try: | |
| # Use environment variable or fallback to relative path | |
| credentials_path = os.getenv( | |
| 'FIREBASE_CREDENTIALS_PATH', | |
| '../firebase_service_account.json' | |
| ) | |
| if not Path(credentials_path).exists(): | |
| raise FileNotFoundError(f"Firebase credentials file not found: {credentials_path}") | |
| cred = credentials.Certificate(credentials_path) | |
| firebase_admin.initialize_app(cred) | |
| print("Firebase Admin SDK initialized successfully") | |
| except Exception as e: | |
| print(f"Failed to initialize Firebase Admin SDK: {e}") | |
| raise | |
| # Initialize Firebase | |
| initialize_firebase() |
🤖 Prompt for AI Agents
In backend/app/firebase_config.py around lines 5 to 6, improve the code by
adding error handling for missing or invalid credentials file. Replace the
hardcoded relative path with a configurable path obtained from an environment
variable. Wrap the initialization in a try-except block to catch and log errors
related to loading the credentials or initializing Firebase, ensuring the
application fails gracefully with informative error messages.
frontend/contexts/AuthContext.tsx
Outdated
| const API_URL = 'https://splitwiser-production.up.railway.app'; // Replace with your actual backend URL | ||
| WebBrowser.maybeCompleteAuthSession(); | ||
|
|
||
| const API_URL = "Your API URL here"; // Replace with your actual API URL |
There was a problem hiding this comment.
Replace placeholder values with environment variables
The placeholder values for API URL and Google client IDs will cause runtime errors. These should be replaced with environment variables.
-const API_URL = "Your API URL here"; // Replace with your actual API URL
+const API_URL = process.env.EXPO_PUBLIC_API_URL || "http://10.0.2.2:8000";
const [request, response, promptAsync] = Google.useAuthRequest({
- webClientId: "your-web-client-id.apps.googleusercontent.com",
- androidClientId: "your-android-client-id.apps.googleusercontent.com",
+ webClientId: process.env.EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID,
+ androidClientId: process.env.EXPO_PUBLIC_GOOGLE_ANDROID_CLIENT_ID,
});Then create a .env file with the actual values and ensure it's in .gitignore.
Also applies to: 42-43
🤖 Prompt for AI Agents
In frontend/contexts/AuthContext.tsx at lines 11 and 42-43, replace the
hardcoded placeholder strings for API_URL and Google client IDs with references
to environment variables using process.env. Then create a .env file at the
project root containing the actual values for these variables, and add .env to
.gitignore to prevent committing sensitive information.
frontend/contexts/AuthContext.tsx
Outdated
| } catch (err) { | ||
| console.log("Google Sign-In Error:", err); | ||
| alert("Login failed. Check network connection."); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling with specific user messages
The generic error message doesn't help users understand what went wrong. Consider providing more specific error messages.
} catch (err: any) {
console.log("Google Sign-In Error:", err);
- alert("Login failed. Check network connection.");
+ const errorMessage = err.response?.data?.detail ||
+ err.message ||
+ "Google sign-in failed. Please try again.";
+ alert(errorMessage);
}📝 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.
| } catch (err) { | |
| console.log("Google Sign-In Error:", err); | |
| alert("Login failed. Check network connection."); | |
| } | |
| } catch (err: any) { | |
| console.log("Google Sign-In Error:", err); | |
| const errorMessage = err.response?.data?.detail || | |
| err.message || | |
| "Google sign-in failed. Please try again."; | |
| alert(errorMessage); | |
| } |
🤖 Prompt for AI Agents
In frontend/contexts/AuthContext.tsx around lines 69 to 72, the catch block uses
a generic alert message that does not inform users about the specific error.
Improve error handling by inspecting the error object to determine the cause and
display more specific, user-friendly messages accordingly. Update the alert to
reflect these detailed messages instead of the generic "Login failed" text.
| useEffect(() => { | ||
| axios.defaults.baseURL = API_URL; | ||
| axios.interceptors.request.use((config) => { | ||
| if (accessToken) config.headers.Authorization = `Bearer ${accessToken}`; | ||
| return config; | ||
| }); | ||
|
|
||
| // Set up response interceptor for token refresh | ||
| axios.interceptors.response.use( | ||
| (response) => response, | ||
| (res) => res, | ||
| async (error) => { | ||
| const originalRequest = error.config; | ||
|
|
||
| // If 401 response and not already retrying | ||
| if (error.response?.status === 401 && !originalRequest._retry && refreshToken) { | ||
| originalRequest._retry = true; | ||
|
|
||
| try { | ||
| // Call refresh token endpoint | ||
| const response = await axios.post('/auth/refresh', { refresh_token: refreshToken }); | ||
|
|
||
| // Update tokens | ||
| const response = await axios.post("/auth/refresh", { refresh_token: refreshToken }); | ||
| setAccessToken(response.data.access_token); | ||
| if (response.data.refresh_token) { | ||
| setRefreshToken(response.data.refresh_token); | ||
| await SecureStore.setItemAsync('refreshToken', response.data.refresh_token); | ||
| await SecureStore.setItemAsync("refreshToken", response.data.refresh_token); | ||
| } | ||
|
|
||
| // Retry the original request with new token | ||
| originalRequest.headers.Authorization = `Bearer ${response.data.access_token}`; | ||
| return axios(originalRequest); | ||
| } catch (refreshError) { | ||
| // If refresh fails, log out the user | ||
| } catch (err) { | ||
| logout(); | ||
| return Promise.reject(refreshError); | ||
| return Promise.reject(err); | ||
| } | ||
| } | ||
|
|
||
| return Promise.reject(error); | ||
| } | ||
| ); | ||
|
|
||
| // Check for existing session on app load | ||
| const loadTokens = async () => { | ||
| try { | ||
| // Get refresh token from secure storage | ||
| const storedRefreshToken = await SecureStore.getItemAsync('refreshToken'); | ||
|
|
||
| const storedRefreshToken = await SecureStore.getItemAsync("refreshToken"); | ||
| if (storedRefreshToken) { | ||
| // Try to get a new access token | ||
| const response = await axios.post('/auth/refresh', { refresh_token: storedRefreshToken }); | ||
|
|
||
| // Set authentication state | ||
| const response = await axios.post("/auth/refresh", { refresh_token: storedRefreshToken }); | ||
| setAccessToken(response.data.access_token); | ||
| setRefreshToken(response.data.refresh_token || storedRefreshToken); | ||
| setUser(response.data.user); | ||
| setIsAuthenticated(true); | ||
|
|
||
| // Update storage if we got a new refresh token | ||
| if (response.data.refresh_token) { | ||
| await SecureStore.setItemAsync('refreshToken', response.data.refresh_token); | ||
| await SecureStore.setItemAsync("refreshToken", response.data.refresh_token); | ||
| } | ||
| } | ||
| } catch (error) { | ||
| // Clear any invalid tokens | ||
| await SecureStore.deleteItemAsync('refreshToken'); | ||
| } catch { | ||
| await SecureStore.deleteItemAsync("refreshToken"); | ||
| } finally { | ||
| setLoading(false); | ||
| } | ||
| }; | ||
|
|
||
| loadTokens(); | ||
| }, []); // Email/Password login | ||
| }, []); |
There was a problem hiding this comment.
Add cleanup for axios interceptors to prevent memory leaks
The axios interceptors are added in useEffect but never removed, which could cause memory leaks if the component remounts.
useEffect(() => {
axios.defaults.baseURL = API_URL;
- axios.interceptors.request.use((config) => {
+ const requestInterceptor = axios.interceptors.request.use((config) => {
if (accessToken) config.headers.Authorization = `Bearer ${accessToken}`;
return config;
});
- axios.interceptors.response.use(
+ const responseInterceptor = axios.interceptors.response.use(
(res) => res,
async (error) => {
// ... existing code ...
}
);
const loadTokens = async () => {
// ... existing code ...
};
loadTokens();
+
+ return () => {
+ axios.interceptors.request.eject(requestInterceptor);
+ axios.interceptors.response.eject(responseInterceptor);
+ };
}, []);📝 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.
| useEffect(() => { | |
| axios.defaults.baseURL = API_URL; | |
| axios.interceptors.request.use((config) => { | |
| if (accessToken) config.headers.Authorization = `Bearer ${accessToken}`; | |
| return config; | |
| }); | |
| // Set up response interceptor for token refresh | |
| axios.interceptors.response.use( | |
| (response) => response, | |
| (res) => res, | |
| async (error) => { | |
| const originalRequest = error.config; | |
| // If 401 response and not already retrying | |
| if (error.response?.status === 401 && !originalRequest._retry && refreshToken) { | |
| originalRequest._retry = true; | |
| try { | |
| // Call refresh token endpoint | |
| const response = await axios.post('/auth/refresh', { refresh_token: refreshToken }); | |
| // Update tokens | |
| const response = await axios.post("/auth/refresh", { refresh_token: refreshToken }); | |
| setAccessToken(response.data.access_token); | |
| if (response.data.refresh_token) { | |
| setRefreshToken(response.data.refresh_token); | |
| await SecureStore.setItemAsync('refreshToken', response.data.refresh_token); | |
| await SecureStore.setItemAsync("refreshToken", response.data.refresh_token); | |
| } | |
| // Retry the original request with new token | |
| originalRequest.headers.Authorization = `Bearer ${response.data.access_token}`; | |
| return axios(originalRequest); | |
| } catch (refreshError) { | |
| // If refresh fails, log out the user | |
| } catch (err) { | |
| logout(); | |
| return Promise.reject(refreshError); | |
| return Promise.reject(err); | |
| } | |
| } | |
| return Promise.reject(error); | |
| } | |
| ); | |
| // Check for existing session on app load | |
| const loadTokens = async () => { | |
| try { | |
| // Get refresh token from secure storage | |
| const storedRefreshToken = await SecureStore.getItemAsync('refreshToken'); | |
| const storedRefreshToken = await SecureStore.getItemAsync("refreshToken"); | |
| if (storedRefreshToken) { | |
| // Try to get a new access token | |
| const response = await axios.post('/auth/refresh', { refresh_token: storedRefreshToken }); | |
| // Set authentication state | |
| const response = await axios.post("/auth/refresh", { refresh_token: storedRefreshToken }); | |
| setAccessToken(response.data.access_token); | |
| setRefreshToken(response.data.refresh_token || storedRefreshToken); | |
| setUser(response.data.user); | |
| setIsAuthenticated(true); | |
| // Update storage if we got a new refresh token | |
| if (response.data.refresh_token) { | |
| await SecureStore.setItemAsync('refreshToken', response.data.refresh_token); | |
| await SecureStore.setItemAsync("refreshToken", response.data.refresh_token); | |
| } | |
| } | |
| } catch (error) { | |
| // Clear any invalid tokens | |
| await SecureStore.deleteItemAsync('refreshToken'); | |
| } catch { | |
| await SecureStore.deleteItemAsync("refreshToken"); | |
| } finally { | |
| setLoading(false); | |
| } | |
| }; | |
| loadTokens(); | |
| }, []); // Email/Password login | |
| }, []); | |
| useEffect(() => { | |
| axios.defaults.baseURL = API_URL; | |
| const requestInterceptor = axios.interceptors.request.use((config) => { | |
| if (accessToken) config.headers.Authorization = `Bearer ${accessToken}`; | |
| return config; | |
| }); | |
| const responseInterceptor = axios.interceptors.response.use( | |
| (res) => res, | |
| async (error) => { | |
| const originalRequest = error.config; | |
| if (error.response?.status === 401 && !originalRequest._retry && refreshToken) { | |
| originalRequest._retry = true; | |
| try { | |
| const response = await axios.post("/auth/refresh", { refresh_token: refreshToken }); | |
| setAccessToken(response.data.access_token); | |
| if (response.data.refresh_token) { | |
| setRefreshToken(response.data.refresh_token); | |
| await SecureStore.setItemAsync("refreshToken", response.data.refresh_token); | |
| } | |
| originalRequest.headers.Authorization = `Bearer ${response.data.access_token}`; | |
| return axios(originalRequest); | |
| } catch (err) { | |
| logout(); | |
| return Promise.reject(err); | |
| } | |
| } | |
| return Promise.reject(error); | |
| } | |
| ); | |
| const loadTokens = async () => { | |
| try { | |
| const storedRefreshToken = await SecureStore.getItemAsync("refreshToken"); | |
| if (storedRefreshToken) { | |
| const response = await axios.post("/auth/refresh", { refresh_token: storedRefreshToken }); | |
| setAccessToken(response.data.access_token); | |
| setRefreshToken(response.data.refresh_token || storedRefreshToken); | |
| setUser(response.data.user); | |
| setIsAuthenticated(true); | |
| if (response.data.refresh_token) { | |
| await SecureStore.setItemAsync("refreshToken", response.data.refresh_token); | |
| } | |
| } | |
| } catch { | |
| await SecureStore.deleteItemAsync("refreshToken"); | |
| } finally { | |
| setLoading(false); | |
| } | |
| }; | |
| loadTokens(); | |
| return () => { | |
| axios.interceptors.request.eject(requestInterceptor); | |
| axios.interceptors.response.eject(responseInterceptor); | |
| }; | |
| }, []); |
🤖 Prompt for AI Agents
In frontend/contexts/AuthContext.tsx around lines 80 to 131, the axios
interceptors are added inside useEffect but not removed, risking memory leaks on
component remounts. Fix this by storing the references to the request and
response interceptors when adding them, then return a cleanup function from
useEffect that ejects these interceptors using axios.interceptors.request.eject
and axios.interceptors.response.eject to properly remove them.
💡 Verification agent
🧩 Analysis chain
Verify potential stale closure issue with tokens in interceptors
The axios interceptors are set up once with an empty dependency array, but they reference accessToken and refreshToken from state. This could lead to stale closures where the interceptors use outdated token values.
🏁 Script executed:
#!/bin/bash
# Search for similar patterns in the codebase where interceptors might be properly handled
ast-grep --pattern 'axios.interceptors.$_.use($$$)'
# Look for examples of proper token handling in interceptors
rg -A 10 "interceptors.*Authorization.*Bearer" --type tsx --type tsLength of output: 2254
Fix stale closure in axios interceptors in AuthContext
The useEffect at frontend/contexts/AuthContext.tsx (lines ~80–131) has an empty dependency array yet references accessToken and refreshToken, causing the interceptor callbacks to hold on to initial (stale) values and never pick up updates.
• File: frontend/contexts/AuthContext.tsx
• Location: the useEffect(() => { … }, []); block starting around line 80
Recommendations:
- Include
accessTokenandrefreshTokenin the dependency array so the interceptors are recreated when tokens change. - Store the interceptor IDs and eject them in a cleanup function to avoid accumulating handlers on re-render.
Example diff:
--- a/frontend/contexts/AuthContext.tsx
+++ b/frontend/contexts/AuthContext.tsx
useEffect(() => {
axios.defaults.baseURL = API_URL;
- axios.interceptors.request.use((config) => {
- if (accessToken) config.headers.Authorization = `Bearer ${accessToken}`;
- return config;
- });
-
- axios.interceptors.response.use(
- (res) => res,
- async (error) => { /* refresh logic */ }
- );
+ const reqInterceptor = axios.interceptors.request.use((config) => {
+ if (accessToken) config.headers.Authorization = `Bearer ${accessToken}`;
+ return config;
+ });
+
+ const resInterceptor = axios.interceptors.response.use(
+ (res) => res,
+ async (error) => {
+ // existing 401/refresh logic
+ }
+ );
+
+ return () => {
+ axios.interceptors.request.eject(reqInterceptor);
+ axios.interceptors.response.eject(resInterceptor);
+ };
-}, []);
+}, [accessToken, refreshToken]);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In frontend/contexts/AuthContext.tsx around lines 80 to 131, the useEffect hook
sets up axios interceptors but has an empty dependency array while referencing
accessToken and refreshToken, causing stale closures. To fix this, add
accessToken and refreshToken to the dependency array so interceptors update when
tokens change. Also, store the interceptor IDs returned by
axios.interceptors.request.use and axios.interceptors.response.use, and eject
these interceptors in a cleanup function returned by useEffect to prevent
accumulating multiple handlers on re-renders.
| const firebaseConfig = { | ||
| apiKey: "<YOUR_API_KEY>", | ||
| authDomain: "<YOUR_AUTH_DOMAIN>", | ||
| projectId: "<YOUR_PROJECT_ID>", | ||
| storageBucket: "<YOUR_STORAGE_BUCKET>", | ||
| messagingSenderId: "<YOUR_MESSAGING_SENDER_ID>", | ||
| appId: "<YOUR_APP_ID>", | ||
| // measurementId: "<YOUR_MEASUREMENT_ID>" // Only if you enabled Google Analytics | ||
| }; |
There was a problem hiding this comment.
Replace placeholder values and use environment variables.
The Firebase configuration contains placeholder values that must be replaced with actual values. For security and maintainability, consider using environment variables.
const firebaseConfig = {
- apiKey: "<YOUR_API_KEY>",
- authDomain: "<YOUR_AUTH_DOMAIN>",
- projectId: "<YOUR_PROJECT_ID>",
- storageBucket: "<YOUR_STORAGE_BUCKET>",
- messagingSenderId: "<YOUR_MESSAGING_SENDER_ID>",
- appId: "<YOUR_APP_ID>",
+ apiKey: process.env.EXPO_PUBLIC_FIREBASE_API_KEY,
+ authDomain: process.env.EXPO_PUBLIC_FIREBASE_AUTH_DOMAIN,
+ projectId: process.env.EXPO_PUBLIC_FIREBASE_PROJECT_ID,
+ storageBucket: process.env.EXPO_PUBLIC_FIREBASE_STORAGE_BUCKET,
+ messagingSenderId: process.env.EXPO_PUBLIC_FIREBASE_MESSAGING_SENDER_ID,
+ appId: process.env.EXPO_PUBLIC_FIREBASE_APP_ID,
// measurementId: "<YOUR_MEASUREMENT_ID>" // Only if you enabled Google Analytics
};Add validation to ensure all required values are present:
// Validate required configuration
const requiredKeys = ['apiKey', 'authDomain', 'projectId', 'storageBucket', 'messagingSenderId', 'appId'];
const missingKeys = requiredKeys.filter(key => !firebaseConfig[key]);
if (missingKeys.length > 0) {
throw new Error(`Missing Firebase configuration keys: ${missingKeys.join(', ')}`);
}🤖 Prompt for AI Agents
In frontend/firebase/firebaseConfig.ts around lines 7 to 15, replace all
placeholder Firebase config values with references to environment variables to
avoid hardcoding sensitive data. Then, add validation code after the config
object to check that all required keys (apiKey, authDomain, projectId,
storageBucket, messagingSenderId, appId) are present, throwing an error listing
any missing keys to ensure configuration completeness.
frontend/package.json
Outdated
| "expo-secure-store": "~12.3.1", | ||
| "expo-status-bar": "~1.6.0", | ||
| "expo-web-browser": "~12.3.2", | ||
| "firebase": "^9.22.2", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify Firebase version for security vulnerabilities
The Firebase version 9.22.2 appears to be outdated. Please ensure you're using a secure and maintained version.
🌐 Web query:
What is the latest stable version of Firebase JavaScript SDK and are there any known security vulnerabilities in version 9.22.2?
💡 Result:
The latest stable version of the Firebase JavaScript SDK is 12.0.0 as of July 23, 2025[1].
Regarding security vulnerabilities in version 9.22.2:
- There is a known vulnerability described by CVE-2024-11023, which affects Firebase JavaScript SDK versions below 10.9.0. This vulnerability allows attackers to manipulate the
_authTokenSyncURLand potentially redirect authentication tokens to their own servers[5]. - The first patched version that addresses this vulnerability is 10.9.0[5].
- There are also historical reports of low severity vulnerabilities in the dependencies used by Firebase SDK v9.x.x and v10.x.x[2][3]. However, CVE-2024-11023 is the most significant, with a CVSS score of 5.3[5].
Key points:
- Version 9.22.2 is vulnerable to CVE-2024-11023 and should not be used in production.
- Upgrade to at least version 10.9.0 or higher (ideally to the latest release, 12.0.0 as of July 2025) to address known vulnerabilities and other improvements[1][5].
If you need to check the status for other vulnerabilities or dependencies, consider checking official release notes or security advisories accompanying each SDK release.
Citations:
- 1: https://firebase.google.com/support/releases
- 2: Firebase 10.8.0 produces terminal error recomending downgrade to 10.6.0 firebase/firebase-tools#6806
- 3: https://security.snyk.io/package/npm/@firebase%2Fauth
- 4: https://developers.google.com/android/guides/releases
- 5: https://www.miggo.io/vulnerability-database/cve/CVE-2024-11023
- 6: https://firebaseopensource.com/projects/firebase/firebase-js-sdk/
- 7: https://firebase.google.com/support/release-notes/js
- 8: https://github.com/firebase/firebase-js-sdk/blob/master/packages/firebase/CHANGELOG.md
- 9: https://docs.expo.dev/guides/using-firebase/
- 10: https://socket.dev/npm/package/firebase
Upgrade Firebase SDK to Address CVE-2024-11023
The Firebase JavaScript SDK v9.22.2 is vulnerable to CVE-2024-11023 (affecting all versions below 10.9.0). Upgrade to at least v10.9.0—or preferably the latest v12.0.0—to mitigate this and other security issues.
• File: frontend/package.json (Line 25)
• CVE-2024-11023 – allows manipulation of _authTokenSyncURL and token redirection; patched in v10.9.0
• Latest stable release: v12.0.0 (July 23, 2025)
Suggested change:
--- frontend/package.json
@@ lines 25
- "firebase": "^9.22.2",
+ "firebase": "^12.0.0",📝 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.
| "firebase": "^9.22.2", | |
| "firebase": "^12.0.0", |
🤖 Prompt for AI Agents
In frontend/package.json at line 25, the Firebase SDK version is set to 9.22.2,
which is vulnerable to CVE-2024-11023. Update the Firebase dependency version to
at least 10.9.0, preferably the latest stable version 12.0.0, to fix the
security vulnerability. Modify the version string in the dependencies section
accordingly and run a package install to apply the update.
frontend/screens/LoginScreen.tsx
Outdated
| const redirectUri = AuthSession.makeRedirectUri({}); | ||
| console.log("Redirect URI:", redirectUri); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove console.log from production code
Debug logging of the redirect URI should be removed or wrapped in a development-only condition.
const redirectUri = AuthSession.makeRedirectUri({});
-console.log("Redirect URI:", redirectUri);
+if (__DEV__) {
+ console.log("Redirect URI:", redirectUri);
+}📝 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.
| const redirectUri = AuthSession.makeRedirectUri({}); | |
| console.log("Redirect URI:", redirectUri); | |
| const redirectUri = AuthSession.makeRedirectUri({}); | |
| if (__DEV__) { | |
| console.log("Redirect URI:", redirectUri); | |
| } |
🤖 Prompt for AI Agents
In frontend/screens/LoginScreen.tsx around lines 20 to 21, the console.log
statement printing the redirect URI should be removed or wrapped in a condition
that only logs in development mode. To fix this, either delete the console.log
line or wrap it in a check like if (__DEV__) { console.log(...) } to ensure it
does not execute in production.
| <TouchableOpacity style={styles.googleButton} onPress={googleLogin}> | ||
| <Image source={{ uri: "https://img.icons8.com/color/48/google-logo.png" }} style={styles.googleIcon} /> | ||
| <Text style={styles.googleButtonText}>Continue with Google</Text> | ||
| </TouchableOpacity> |
There was a problem hiding this comment.
Replace external image URL with local asset
Using an external URL for the Google logo poses security, reliability, and privacy risks. The external service could be compromised, unavailable, or track user requests.
Download the Google logo and include it as a local asset:
- Download the official Google logo
- Add it to your assets folder
- Update the code:
-<Image source={{ uri: "https://img.icons8.com/color/48/google-logo.png" }} style={styles.googleIcon} />
+<Image source={require("../assets/google-logo.png")} style={styles.googleIcon} />📝 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.
| <TouchableOpacity style={styles.googleButton} onPress={googleLogin}> | |
| <Image source={{ uri: "https://img.icons8.com/color/48/google-logo.png" }} style={styles.googleIcon} /> | |
| <Text style={styles.googleButtonText}>Continue with Google</Text> | |
| </TouchableOpacity> | |
| <TouchableOpacity style={styles.googleButton} onPress={googleLogin}> | |
| <Image source={require("../assets/google-logo.png")} style={styles.googleIcon} /> | |
| <Text style={styles.googleButtonText}>Continue with Google</Text> | |
| </TouchableOpacity> |
🤖 Prompt for AI Agents
In frontend/screens/LoginScreen.tsx around lines 107 to 110, replace the
external Google logo URL with a local image asset to improve security and
reliability. Download the official Google logo, add it to the project's assets
folder, then update the Image component's source prop to reference the local
file using require or import instead of the external URI.
nixpacks.toml
Outdated
| # Use the default Python install behavior but ensure we use the right requirements file | ||
| [phases.install] | ||
| cmds = [ | ||
| "python -m pip install --upgrade pip", | ||
| "pip install --upgrade pip", | ||
| "pip install -r requirements.txt" | ||
| ] |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Install phase is executed before backend sources are copied – requirements.txt may be missing, leading to runtime crashes
pip install -r requirements.txt assumes the file is already at the project root, yet you only copy backend/* in the build phase.
If backend/requirements.txt (which now includes Firebase, FastAPI-0.116.1, etc.) is the authoritative lock-file, those deps will never be installed and the container will start without them.
Consider one of the following fixes:
[phases.install]
cmds = [
- "pip install --upgrade pip",
- "pip install -r requirements.txt"
+# Upgrade pip (optional – see note below)
+ "python -m pip install --upgrade pip",
+# Install backend requirements first; fall back to root file for monorepo builds
+ "pip install -r backend/requirements.txt || pip install -r requirements.txt"
]or move the copy step into the setup/install phase so the file is present beforehand.
Side note: upgrading pip inside a Nix build often defeats hermetic builds; unless you rely on a feature from a newer pip, you can drop that line.
Otherwise, call it via python -m pip to be explicit.
🏁 Script executed:
#!/bin/bash
set -euxo pipefail
# Locate nixpacks.toml
NIX_FILE=$(fd -t f nixpacks.toml | head -n 1)
echo "Using file: $NIX_FILE"
# Show phase definitions around install and any copy/setup steps
grep -nE '^\[phases\.' -n "$NIX_FILE"
echo "------"
sed -n '1,200p' "$NIX_FILE"Length of output: 932
🏁 Script executed:
#!/bin/bash
set -euxo pipefail
# List all requirements.txt files in the repository
fd -t f -g "requirements.txt"Length of output: 92
Ensure backend dependencies are installed before copying sources
The current install phase runs before any backend files are copied, so it only ever installs the root requirements.txt—never backend/requirements.txt. You should update nixpacks.toml so that your authoritative lock-file is installed (and fall back to the root file if needed), for example:
• File: nixpacks.toml
• Section: [phases.install]
[phases.install]
cmds = [
- "pip install --upgrade pip",
- "pip install -r requirements.txt"
+ # (Optional) upgrade pip via the Python executable to avoid PATH ambiguity
+ "python -m pip install --upgrade pip",
+ # Install backend requirements first; fall back to root requirements.txt
+ "pip install -r backend/requirements.txt || pip install -r requirements.txt"
]Alternatively, you can move the cp -r backend/* . into the setup or install phase so that backend/requirements.txt exists before you call pip install.
Side note: Dropping the pip-upgrade step helps maintain hermetic builds unless you depend on a newer pip feature.
🤖 Prompt for AI Agents
In nixpacks.toml around lines 4 to 9, the install phase runs before backend
files are copied, so it installs only the root requirements.txt and misses
backend/requirements.txt. To fix this, update the install phase to first copy
backend files into the build context before running pip install, or modify the
install commands to install dependencies from backend/requirements.txt if it
exists, falling back to the root file otherwise. Also, consider removing the pip
upgrade command to keep builds hermetic unless a newer pip feature is required.
|
@meghan9908 can you resolve the conflicts? |
|
Yeah 👍 |
|
@meghan9908, what you did was resolve review comments. Could you please solve merge conflicts that happen because of unclear head during branch creation?
|
2 participants
This PR updates the development setup to support Google OAuth with redirect URIs and ensure the backend is accessible from the Android emulator.
Key Changes:
Switched to Expo Development Build
Google restricts redirect URIs for expo-go.
Using custom development build allows deep linking (splitwiser://) to work properly with Google OAuth.
Backend Server Configuration
Changed backend start command to:
uvicorn main:app --host 0.0.0.0 --port 8000Ensures backend is reachable from the Android emulator (10.0.2.2:8000) .
Frontend Auth Changes
Removed reliance on expo-go restricted redirect URIs.
Impact
Enables working Google Sign-In flow in development.
Fixes network connectivity issues when calling backend from emulator.
Test Plan
Run expo run:android with development build.
Perform Google login → verify user authenticated and backend receives /auth/login/google POST request.
Fixes #8
Summary by CodeRabbit
New Features
Improvements
Configuration
Documentation
Chores