Skip to content

fix: Login sometimes fails to redirect to index, requiring reload #80

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.

Sign up for GitHub

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

Open
nhatt-dev wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: Login sometimes fails to redirect to index, requiring reload #80

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 4 additions & 16 deletions backend/src/server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,22 +27,10 @@ app.get(
// Google callback route
app.get(
"/auth/google/callback",
passport.authenticate("google", { failureRedirect: "http://localhost:8081/NotAuthorized" }),
(req, res, next) => {
if (!req.user) {
console.error("User is undefined during login.");
return res.status(401).json({ error: "User not authenticated" });
}
req.login(req.user, (err) => {
if (err) {
console.error("Error during req.login:", err);
return next(err); // Pass the error to the error handler
}
console.log("User logged in:", req.user); // Debug log
console.log("Session data:", req.session); // Debug log
res.redirect("http://localhost:8081/"); // Redirect after successful login
});
},
passport.authenticate("google", {
successRedirect: "http://localhost:8081/",
failureRedirect: "http://localhost:8081/NotAuthorized",
}),
);
app.get("/auth/me", (req, res) => {
if (!req.isAuthenticated()) return res.status(401).json({ error: "Not Authenticated" });
Expand Down
26 changes: 24 additions & 2 deletions frontend/app/(tabs)/_layout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,42 @@
import React from "react";
import { createBottomTabNavigator } from "@react-navigation/bottom-tabs";
import { Ionicons } from "@expo/vector-icons";
import Home from ".";
import History from "./History"; // Import the History component
import { useEffect } from "react";
// import LoginPage from "../Login";
import Account from "./Account";
import Goals from "./Goals";
import { useAuth } from "@/context/authContext";
import { Redirect } from "expo-router";
import Checking from "@/components/Checking";
import { BACKEND_PORT } from "@env";
const Tab = createBottomTabNavigator();

export default function TabLayout() {
const { user } = useAuth();
const { user, isLoading, login } = useAuth();

useEffect(() => {
fetch(`http://localhost:${BACKEND_PORT}/auth/me`, {
credentials: "include",
})
.then((response) => {
if (response.ok) {
response.json().then(login);
}
})
.catch((error) => {
console.error("Error checking auth:", error);
});
}, []);
Comment on lines +18 to +30
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This useEffect creates a race condition with the session validation already happening in useStorageState. When the TabLayout mounts, it fetches /auth/me and calls login(), which triggers another session storage update. This is redundant since useStorageState already validates the session on mount. Consider removing this effect and relying on the centralized session management in useStorageState.

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The useEffect dependency array is missing login, which is used inside the effect. This violates React's rules of hooks and could lead to stale closures. Either add login to the dependency array or acknowledge the intentional omission with an eslint-disable comment if this is by design.

Suggested change
}, []);
}, [login]);

Copilot uses AI. Check for mistakes.

if (isLoading) {
return <Checking />;
}

if (!user) {
return <Redirect href="/Login" />;
}

return (
<Tab.Navigator
initialRouteName="Home"
Expand Down
51 changes: 17 additions & 34 deletions frontend/app/Login.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,49 +1,32 @@
// src/LoginPage.tsx
import { useAuth } from "@/context/authContext";
import React, { useEffect, useState } from "react";
import { View, Text, Button, StyleSheet } from "react-native";
import { BACKEND_PORT } from "@env";
// import { useRouter } from "expo-router";
import { Redirect, useRouter } from "expo-router";
import Checking from "@/components/Checking";

const LoginPage = () => {
// const router = useRouter();
const [isRedirected, setIsRedirected] = useState(false);
const { login } = useAuth();
const handleGoogleLogin = () => {
// Redirect to backend Google OAuth URL
window.location.href = `http://localhost:${BACKEND_PORT}/auth/google`;
};
useEffect(() => {
const checkAuth = async () => {
try {
const response = await fetch(
`http://localhost:${BACKEND_PORT}/auth/me`,
{
credentials: "include",
},
);
const router = useRouter();
const { isLoading, user } = useAuth();

if (response.ok) {
const userData = await response.json();
await login(userData);
}
} catch (error) {
console.error("Error checking auth:", error);
}
};
if (isLoading) {
return <Checking />;
}

if (user) {
return <Redirect href="/" />;
}

// Check if we've been redirected back from OAuth
// You can also check for specific query parameters from your OAuth callback
if (window.location.pathname === "/Login" && !isRedirected) {
setIsRedirected(true);
checkAuth();
}
}, [isRedirected]);
return (
<View style={styles.container}>
<Text style={styles.title}>Login Page</Text>
<Text style={styles.message}>Sign in to access your account.</Text>
<Button title="Sign in with Google" onPress={handleGoogleLogin} />
<Button
title="Sign in with Google"
onPress={() => {
router.replace(`http://localhost:${BACKEND_PORT}/auth/google`);
}}
Comment on lines +26 to +28
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

router.replace() is not designed to navigate to external URLs. For OAuth redirects, use window.location.href in web or Linking.openURL() in React Native. The current implementation will likely fail to redirect to the backend OAuth endpoint.

Copilot uses AI. Check for mistakes.
/>
</View>
);
};
Expand Down
39 changes: 1 addition & 38 deletions frontend/app/_layout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,56 +6,19 @@ import {
import { useFonts } from "expo-font";
import { Stack } from "expo-router";
import * as SplashScreen from "expo-splash-screen";
import { useEffect, useState } from "react";
import { useEffect } from "react";
import { useColorScheme } from "@/hooks/useColorScheme";
import Header from "@/components/Header/Header";
import Toast from "react-native-toast-message";
import { AuthProvider } from "@/context/authContext";
import { useAuth } from "@/context/authContext";
import { useRouter } from "expo-router";
import { ActivityIndicator, View } from "react-native";
import { BACKEND_PORT } from "@env";

SplashScreen.preventAutoHideAsync();

function AuthCheck() {
const { user } = useAuth();
const router = useRouter();
const colorScheme = useColorScheme();
const [checking, setChecking] = useState(true);

useEffect(() => {
const checkAuth = async () => {
try {
const response = await fetch(
`http://localhost:${BACKEND_PORT}/auth/me`,
{
credentials: "include",
},
);

if (!response.ok) {
setChecking(false);
return;
}

setChecking(false);
} catch (error) {
console.error("Auth check failed:", error);
} finally {
setChecking(false); // Even if failed, stop loading
}
};
checkAuth();
}, [router]);

if (checking) {
return (
<View style={{ flex: 1, justifyContent: "center", alignItems: "center" }}>
<ActivityIndicator size="large" color="#0000ff" />
</View>
);
}
return (
<>
<ThemeProvider value={colorScheme === "dark" ? DarkTheme : DefaultTheme}>
Expand Down
9 changes: 9 additions & 0 deletions frontend/components/Checking.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { ActivityIndicator, View } from "react-native";

export default function Checking() {
return (
<View style={{ flex: 1, justifyContent: "center", alignItems: "center" }}>
<ActivityIndicator size="large" color="#0000ff" />
</View>
);
}
73 changes: 17 additions & 56 deletions frontend/context/authContext.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,24 +1,17 @@
import {
createContext,
useContext,
useState,
useEffect,
ReactNode,
} from "react";
import AsyncStorage from "@react-native-async-storage/async-storage";
import { createContext, useContext, ReactNode } from "react";
import { useRouter } from "expo-router";
import { BACKEND_PORT } from "@env";
import useStorageState, { AuthUserSession } from "@/hooks/useStorageState";

interface AuthContextType {
user: any | null;
userId: any | null;
interface AuthContextType extends AuthUserSession {
isLoading: boolean;
login: (userData: any) => Promise<void>;
logout: () => Promise<void>;
}

const AuthContext = createContext<AuthContextType>({
user: null,
userId: null,
isLoading: false,
login: async () => {},
logout: async () => {},
});
Expand All @@ -28,61 +21,29 @@ interface AuthProviderProps {
}

export const AuthProvider: React.FC<AuthProviderProps> = ({ children }) => {
const [user, setUser] = useState<any | null>(null);
const [userId, setUserId] = useState<any | null>(null);
const [[isLoading, session], setSession] = useStorageState();
const router = useRouter();
const [loading, setLoading] = useState(true);

useEffect(() => {
const loadUser = async () => {
const storedUser = await AsyncStorage.getItem("user");
if (storedUser) {
try {
const userData = JSON.parse(storedUser);

// Validate session with backend
const res = await fetch(`http://localhost:${BACKEND_PORT}/auth/me`, {
credentials: "include",
});

if (!res.ok) throw new Error("Session invalid");
setUser(userData);
setUserId(userData.id);
} catch (err) {
await AsyncStorage.clear();
setUser(null);
setUserId(null);
router.replace("/Login");
}
}
setLoading(false);
};

loadUser();
}, []);

const login = async (userData: any) => {
await Promise.all([
AsyncStorage.setItem("user", JSON.stringify(userData)),
AsyncStorage.setItem("userId", userData.id),
]);
setUser(userData);
setUserId(userData.id);
setSession({ user: userData, userId: userData.id });
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no validation that userData.id exists before accessing it. If the OAuth response doesn't include an id field, this will result in userId being undefined. Consider adding validation to ensure the user data has the expected structure before storing it.

Suggested change
setSession({ user: userData, userId: userData.id });
const userId =
userData && typeof userData === "object" && "id" in userData
? (userData as { id: string | number }).id
: null;
setSession({ user: userData, userId });

Copilot uses AI. Check for mistakes.
Comment on lines 27 to +28
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using any type for the userData parameter defeats TypeScript's type safety. Consider defining a proper User interface that describes the expected structure of the user data being passed from the OAuth callback.

Copilot uses AI. Check for mistakes.
router.replace("/");
};

const logout = async () => {
await AsyncStorage.removeItem("user");
await AsyncStorage.removeItem("userId");
setUser(null);
setUserId(null);
setSession({ user: null, userId: null });
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logout function sets the session to { user: null, userId: null } which is inconsistent with how setStorageItem handles null values. Looking at setStorageItem, passing null removes the items from storage, but passing an object with null properties stores those null values. Consider passing null directly to setSession instead of an object with null properties for consistency with the intended logout behavior.

Suggested change
setSession({ user: null, userId: null });
setSession(null);

Copilot uses AI. Check for mistakes.
router.replace("/Login");
};

if (loading) return null;

return (
<AuthContext.Provider value={{ user, userId, login, logout }}>
<AuthContext.Provider
value={{
user: session.user,
userId: session.userId,
isLoading,
login,
logout,
}}
>
{children}
</AuthContext.Provider>
);
Expand Down
Loading
Loading