Fix: isLoggedIn memory leak (#2952)

* Fix memory leak from isLoggedIn

* Fix tests with mocking for clearAuthCache

---------

Co-authored-by: Johannes Klein <johannes.t.klein@gmail.com>
This commit is contained in:
Amanda Bullington
2025-06-11 10:04:54 -07:00
committed by GitHub
parent a049d5022d
commit 3c4ec368f5
5 changed files with 99 additions and 21 deletions

View File

@@ -1,3 +1,20 @@
let clearAuthCache = () => {}; // Default no-op function
// Try to get clearAuthCache function safely
try {
const authModule = require( "components/LoginSignUp/AuthenticationService.ts" );
if ( authModule && typeof authModule.clearAuthCache === "function" ) {
// eslint-disable-next-line prefer-destructuring
clearAuthCache = authModule.clearAuthCache;
} else if ( authModule.default && typeof authModule.default.clearAuthCache === "function" ) {
// eslint-disable-next-line prefer-destructuring
clearAuthCache = authModule.default.clearAuthCache;
}
} catch ( error ) {
console.warn( "Could not import clearAuthCache, using no-op function", error );
}
class RNSInfo {
static stores = new Map();
@@ -11,6 +28,15 @@ class RNSInfo {
if ( typeof s !== "string" ) { throw new Error( "Invalid string:", s ); }
}
static clearAllStores = jest.fn( () => {
RNSInfo.stores.clear();
clearAuthCache();
} );
static clearAuthCache = jest.fn( () => {
clearAuthCache();
} );
static getItem = jest.fn( async ( k, o ) => {
RNSInfo.validateString( k );
@@ -27,11 +53,8 @@ class RNSInfo {
let mappedValues = [];
if ( service?.size ) {
// for ( const [k, v] of service.entries() ) {
// mappedValues.push( { key: k, value: v, service: serviceName } );
// }
mappedValues = service.entries( ).map(
( key, value ) => ( { key, value, service: serviceName } )
mappedValues = Array.from( service.entries() ).map(
( [key, value] ) => ( { key, value, service: serviceName } )
);
}
@@ -52,6 +75,8 @@ class RNSInfo {
service.set( k, v );
clearAuthCache( );
return null;
} );
@@ -63,6 +88,8 @@ class RNSInfo {
if ( service ) { service.delete( k ); }
clearAuthCache( );
return null;
} );

View File

@@ -44,6 +44,25 @@ const API_HOST: string = Config.OAUTH_API_URL || process.env.OAUTH_API_URL || "h
// expire every 30 mins, so might as well be futureproof.
const JWT_EXPIRATION_MINS = 25;
/**
* Cache for isLoggedIn, to avoid making too many calls to RNSInfo.getItem
*/
const authCache = {
isLoggedIn: null,
lastChecked: null,
cacheTimeout: 5000
};
/**
* Clear cache for isLoggedIn.
*
* @returns {void}
*/
const clearAuthCache = ( ) => {
authCache.isLoggedIn = null;
authCache.lastChecked = null;
};
async function getSensitiveItem( key: string, options = {} ) {
try {
return await RNSInfo.getItem( key, options );
@@ -65,7 +84,9 @@ async function getSensitiveItem( key: string, options = {} ) {
async function setSensitiveItem( key: string, value: string, options = {} ) {
try {
return await RNSInfo.setItem( key, value, options );
const result = await RNSInfo.setItem( key, value, options );
clearAuthCache( );
return result;
} catch ( e ) {
if ( isDebugMode( ) ) {
localLogger.info( `RNSInfo.setItem not available for ${key}, sleeping` );
@@ -84,7 +105,9 @@ async function setSensitiveItem( key: string, value: string, options = {} ) {
async function deleteSensitiveItem( key: string, options = {} ) {
try {
return await RNSInfo.deleteItem( key, options );
const result = await RNSInfo.deleteItem( key, options );
clearAuthCache( );
return result;
} catch ( e ) {
if ( isDebugMode( ) ) {
localLogger.info( `RNSInfo.deleteItem not available for ${key}, sleeping` );
@@ -120,8 +143,29 @@ const createAPI = ( additionalHeaders?: { [header: string]: string } ) => create
* @returns {Promise<boolean>}
*/
const isLoggedIn = async (): Promise<boolean> => {
const accessToken = await getSensitiveItem( "accessToken" );
return typeof accessToken === "string";
const now = Date.now();
// if cached value is fresh, return it before checking storage
if (
authCache.isLoggedIn !== null
&& authCache.lastChecked
&& ( now - authCache.lastChecked ) < authCache.cacheTimeout
) {
return authCache.isLoggedIn;
}
try {
const accessToken = await getSensitiveItem( "accessToken" );
const result = typeof accessToken === "string";
authCache.isLoggedIn = result;
authCache.lastChecked = now;
return result;
} catch ( error ) {
console.warn( "Auth check failed:", error );
return false;
}
};
/**
@@ -189,6 +233,7 @@ const signOut = async (
await removeAllFilesFromDirectory( photoUploadPath );
await removeAllFilesFromDirectory( rotatedOriginalPhotosPath );
await removeAllFilesFromDirectory( soundUploadPath );
// delete all keys from mmkv
storage.clearAll( );
RNRestart.restart( );
@@ -622,6 +667,7 @@ export {
API_HOST,
authenticateUser,
authenticateUserByAssertion,
clearAuthCache,
emailAvailable,
getAnonymousJWT,
getJWT,

View File

@@ -1,5 +1,6 @@
import { RouteProp, useNavigation, useRoute } from "@react-navigation/native";
import classnames from "classnames";
import { authenticateUser } from "components/LoginSignUp/AuthenticationService.ts";
import {
Body1, Body2, Button, Heading4, INatIcon, INatIconButton, List2
} from "components/SharedComponents";
@@ -19,7 +20,6 @@ import { useLayoutPrefs } from "sharedHooks";
import useKeyboardInfo from "sharedHooks/useKeyboardInfo";
import colors from "styles/tailwindColors";
import { authenticateUser } from "./AuthenticationService";
import Error from "./Error";
import { signInWithApple, signInWithGoogle } from "./loginFormHelpers";
import LoginSignUpInputField from "./LoginSignUpInputField";
@@ -88,6 +88,7 @@ const LoginForm = ( {
setLoading( false );
return;
}
setLoading( false );
// Set a state to zustand that we just logged in while in default mode

View File

@@ -16,17 +16,19 @@ const useAuthenticatedQuery = (
const [userLoggedIn, setUserLoggedIn] = useState( LOGGED_IN_UNKNOWN );
const route = useSafeRoute( );
// Whether we perform this query and whether we need to re-perform it
// depends on whether the user is signed in. The possible vulnerability
// here is that this effect might not run frequently enough to change when
// a user signs in or out. The reason we're not using useCurrentUser is it
// doesn't tell us whether we know the user's auth state yet, it only
// returns null when we don't know OR the user is signed out.
useEffect( ( ) => {
isLoggedIn()
.then( result => setUserLoggedIn( result ) )
.catch( ( ) => setUserLoggedIn( LOGGED_IN_UNKNOWN ) );
}, [queryKey, queryOptions] );
const checkAuth = async ( ) => {
try {
const result = await isLoggedIn( );
setUserLoggedIn( result );
} catch ( error ) {
console.warn( "Auth check failed:", error );
setUserLoggedIn( false );
}
};
checkAuth( );
}, [] );
// The results will probably be different depending on whether the user is
// signed in or we wouldn't be using useAuthenticatedQuery in the first

View File

@@ -5,7 +5,7 @@
// tests finish
// eslint-disable-next-line testing-library/no-manual-cleanup
import { cleanup } from "@testing-library/react-native";
import { API_HOST } from "components/LoginSignUp/AuthenticationService.ts";
import { API_HOST, clearAuthCache } from "components/LoginSignUp/AuthenticationService.ts";
import { getInatLocaleFromSystemLocale } from "i18n/initI18next";
import i18next from "i18next";
import inatjs from "inaturalistjs";
@@ -36,6 +36,7 @@ async function signOut( options = {} ) {
await RNSInfo.deleteItem( "jwtToken" );
await RNSInfo.deleteItem( "jwtGeneratedAt" );
await RNSInfo.deleteItem( "accessToken" );
clearAuthCache( );
inatjs.users.me.mockClear( );
}
@@ -45,6 +46,7 @@ async function signIn( user, options = {} ) {
await RNSInfo.setItem( "jwtToken", TEST_JWT );
await RNSInfo.setItem( "jwtGeneratedAt", Date.now( ).toString( ), {} );
await RNSInfo.setItem( "accessToken", TEST_ACCESS_TOKEN );
clearAuthCache( );
inatjs.users.me.mockResolvedValue( makeResponse( [user] ) );
user.signedIn = true;
safeRealmWrite( realm, ( ) => {