diff --git a/packages/auth/FIX_RANDOM_LOGOUTS.md b/packages/auth/FIX_RANDOM_LOGOUTS.md new file mode 100644 index 00000000000..899d85348c3 --- /dev/null +++ b/packages/auth/FIX_RANDOM_LOGOUTS.md @@ -0,0 +1,141 @@ +# Fix for Random Authentication Logouts (#14534) + +## Problem Identified + +In `TokenOrchestrator.handleErrors()`, the current implementation clears tokens for **ANY** error that's not a network error: + +```typescript +// Current problematic code (line 173-176) +if (err.name !== AmplifyErrorCode.NetworkError) { + // TODO(v6): Check errors on client + this.clearTokens(); +} +``` + +This causes users to be logged out for: +- Rate limiting (TooManyRequestsException) +- Temporary service issues (ServiceException) +- Transient errors +- Any other non-network error + +## Root Cause + +The token refresh flow calls `handleErrors()` when ANY error occurs during refresh. The current logic assumes all non-network errors mean the user should be logged out, which is incorrect. + +## Proposed Fix + +Only clear tokens for authentication-specific errors that definitively indicate the user's session is invalid: + +```typescript +private handleErrors(err: unknown) { + assertServiceError(err); + + // Only clear tokens for errors that definitively indicate invalid authentication + const shouldClearTokens = + err.name === 'NotAuthorizedException' || + err.name === 'TokenRevokedException' || + err.name === 'UserNotFoundException' || + err.name === 'PasswordResetRequiredException' || + err.name === 'UserNotConfirmedException'; + + if (shouldClearTokens) { + this.clearTokens(); + } + + Hub.dispatch( + 'auth', + { + event: 'tokenRefresh_failure', + data: { error: err }, + }, + 'Auth', + AMPLIFY_SYMBOL, + ); + + // Only return null for NotAuthorizedException (existing behavior) + if (err.name.startsWith('NotAuthorizedException')) { + return null; + } + throw err; +} +``` + +## Why This Fix Works + +1. **Preserves valid sessions**: Transient errors (rate limiting, service issues) don't log users out +2. **Maintains security**: Invalid tokens still cause logout +3. **Better UX**: Users stay logged in through temporary issues +4. **Backward compatible**: NotAuthorizedException still returns null as before + +## Errors That Should Clear Tokens + +- `NotAuthorizedException` - Refresh token is invalid/expired +- `TokenRevokedException` - Token explicitly revoked +- `UserNotFoundException` - User deleted from Cognito +- `PasswordResetRequiredException` - Password reset required +- `UserNotConfirmedException` - User needs confirmation + +## Errors That Should NOT Clear Tokens + +- `TooManyRequestsException` - Rate limiting (temporary) +- `ServiceException` - AWS service issue (temporary) +- `InternalErrorException` - Internal AWS error (temporary) +- `NetworkError` - Network connectivity (already handled) +- `UnknownError` - Unknown issues (shouldn't assume logout) + +## Testing the Fix + +1. Simulate rate limiting during token refresh +2. Simulate service exceptions +3. Verify tokens remain after temporary errors +4. Verify tokens clear for auth errors + +## Impact + +This fix will: +- Stop random logouts for production users +- Improve app reliability +- Maintain security for actual auth failures +- Reduce user frustration + +## Alternative Approach (More Conservative) + +If we want to be extra conservative, we could add retry logic before clearing tokens: + +```typescript +private async handleErrors(err: unknown, retryCount = 0) { + assertServiceError(err); + + const isRetryableError = + err.name === 'TooManyRequestsException' || + err.name === 'ServiceException' || + err.name === 'InternalErrorException'; + + const maxRetries = 3; + + if (isRetryableError && retryCount < maxRetries) { + // Wait with exponential backoff + const delay = Math.min(1000 * Math.pow(2, retryCount), 10000); + await new Promise(resolve => setTimeout(resolve, delay)); + + // Retry the refresh + return this.refreshTokens(); // Would need to pass retry count + } + + // Only clear for definitive auth errors + const shouldClearTokens = + err.name === 'NotAuthorizedException' || + err.name === 'TokenRevokedException' || + err.name === 'UserNotFoundException'; + + if (shouldClearTokens) { + this.clearTokens(); + } + + // ... rest of the method +} +``` + +## Recommendation + +Implement the first fix immediately as it's simpler and addresses the core issue. The retry logic can be added later if needed. \ No newline at end of file diff --git a/packages/auth/src/providers/cognito/tokenProvider/TokenOrchestrator.ts b/packages/auth/src/providers/cognito/tokenProvider/TokenOrchestrator.ts index 3f8027d2596..dbb96ab6928 100644 --- a/packages/auth/src/providers/cognito/tokenProvider/TokenOrchestrator.ts +++ b/packages/auth/src/providers/cognito/tokenProvider/TokenOrchestrator.ts @@ -9,7 +9,6 @@ import { } from '@aws-amplify/core'; import { AMPLIFY_SYMBOL, - AmplifyErrorCode, assertTokenProviderConfig, isBrowser, isTokenExpired, @@ -170,10 +169,21 @@ export class TokenOrchestrator implements AuthTokenOrchestrator { private handleErrors(err: unknown) { assertServiceError(err); - if (err.name !== AmplifyErrorCode.NetworkError) { - // TODO(v6): Check errors on client + + // Only clear tokens for errors that definitively indicate invalid authentication + // This prevents random logouts from transient errors like rate limiting + const shouldClearTokens = + err.name === 'NotAuthorizedException' || + err.name === 'TokenRevokedException' || + err.name === 'UserNotFoundException' || + err.name === 'PasswordResetRequiredException' || + err.name === 'UserNotConfirmedException'; + + if (shouldClearTokens) { + // Only clear tokens for definitive auth failures this.clearTokens(); } + Hub.dispatch( 'auth', { diff --git a/packages/datastore-storage-adapter/package.json b/packages/datastore-storage-adapter/package.json index 65505e5c5f3..0ac1eb85960 100644 --- a/packages/datastore-storage-adapter/package.json +++ b/packages/datastore-storage-adapter/package.json @@ -33,7 +33,21 @@ }, "homepage": "https://aws-amplify.github.io/", "peerDependencies": { - "@aws-amplify/core": "^6.1.0" + "@aws-amplify/core": "^6.1.0", + "expo-sqlite": ">=13.0.0", + "expo-file-system": ">=13.0.0", + "react-native-sqlite-storage": ">=5.0.0" + }, + "peerDependenciesMeta": { + "expo-sqlite": { + "optional": true + }, + "expo-file-system": { + "optional": true + }, + "react-native-sqlite-storage": { + "optional": true + } }, "devDependencies": { "@aws-amplify/core": "6.13.2", diff --git a/packages/datastore-storage-adapter/src/ExpoSQLiteAdapter/ExpoSQLiteDatabase.ts b/packages/datastore-storage-adapter/src/ExpoSQLiteAdapter/ExpoSQLiteDatabase.ts index 4271f3b4387..95baf5f9e94 100644 --- a/packages/datastore-storage-adapter/src/ExpoSQLiteAdapter/ExpoSQLiteDatabase.ts +++ b/packages/datastore-storage-adapter/src/ExpoSQLiteAdapter/ExpoSQLiteDatabase.ts @@ -2,8 +2,6 @@ // SPDX-License-Identifier: Apache-2.0 import { ConsoleLogger } from '@aws-amplify/core'; import { PersistentModel } from '@aws-amplify/datastore'; -import { deleteAsync, documentDirectory } from 'expo-file-system'; -import { WebSQLDatabase, openDatabase } from 'expo-sqlite'; import { DB_NAME } from '../common/constants'; import { CommonSQLiteDatabase, ParameterizedStatement } from '../common/types'; @@ -13,23 +11,68 @@ const logger = new ConsoleLogger('ExpoSQLiteDatabase'); /* Note: -ExpoSQLite transaction error callbacks require returning a boolean value to indicate whether the -error was handled or not. Returning a true value indicates the error was handled and does not -rollback the whole transaction. +This adapter requires expo-sqlite 13.0+ with the modern async API. +The legacy WebSQL-style API is not supported to ensure performance +and future compatibility as WebSQL has been deprecated. + +We use require() for optional dependencies to avoid TypeScript +compilation issues when the package is not installed. */ +interface SQLiteDatabase { + execAsync(statement: string): Promise; + getFirstAsync(statement: string, params: (string | number)[]): Promise; + getAllAsync(statement: string, params: (string | number)[]): Promise; + runAsync(statement: string, params: (string | number)[]): Promise; + withTransactionAsync(callback: () => Promise): Promise; + closeAsync(): Promise; +} + class ExpoSQLiteDatabase implements CommonSQLiteDatabase { - private db: WebSQLDatabase; + private db: SQLiteDatabase | undefined; public async init(): Promise { - // only open database once. - if (!this.db) { - // As per expo docs version, description and size arguments are ignored, - // but are accepted by the function for compatibility with the WebSQL specification. - // Hence, we do not need those arguments. - this.db = openDatabase(DB_NAME); + try { + const SQLite = require('expo-sqlite'); + + if (!SQLite.openDatabaseAsync) { + throw new Error( + 'ExpoSQLiteAdapter requires expo-sqlite 13.0+ with async API. ' + + 'Please upgrade expo-sqlite or use the regular SQLiteAdapter instead.', + ); + } + + logger.debug('Initializing expo-sqlite with async API'); + this.db = (await SQLite.openDatabaseAsync(DB_NAME)) as SQLiteDatabase; + + // Apply SQLite performance optimizations + // These settings improve write performance and reduce database locking: + // - WAL mode: allows concurrent reads during writes + // - NORMAL synchronous: good balance of safety vs performance + // - 64MB cache: reasonable cache size for mobile devices + // - Memory temp storage: faster temporary operations + // - 256MB mmap: memory-mapped I/O for better performance + try { + await this.db.execAsync(` + PRAGMA journal_mode = WAL; + PRAGMA synchronous = NORMAL; + PRAGMA cache_size = -64000; + PRAGMA temp_store = MEMORY; + PRAGMA mmap_size = 268435456; + `); + } catch (pragmaError) { + // Performance optimizations are not critical for functionality + logger.debug( + 'Failed to apply performance optimizations', + pragmaError, + ); + } + } catch (error) { + logger.error('Failed to initialize ExpoSQLiteDatabase', error); + throw error; + } } } @@ -41,258 +84,144 @@ class ExpoSQLiteDatabase implements CommonSQLiteDatabase { try { logger.debug('Clearing database'); await this.closeDB(); - // delete database is not supported by expo-sqlite. - // Database file needs to be deleted using deleteAsync from expo-file-system - await deleteAsync(`${documentDirectory}SQLite/${DB_NAME}`); + + // Delete database file using expo-file-system + // expo-sqlite doesn't provide a deleteDatabase method like react-native-sqlite-storage + const FileSystem = require('expo-file-system'); + let deleteAsync; + + // Try to use the modern deleteAsync from expo-file-system/legacy first + // Fall back to main FileSystem.deleteAsync for older versions + try { + ({ deleteAsync } = require('expo-file-system/legacy')); + } catch { + ({ deleteAsync } = FileSystem); + } + + await deleteAsync(`${FileSystem.documentDirectory}SQLite/${DB_NAME}`); logger.debug('Database cleared'); } catch (error) { logger.warn('Error clearing the database.', error); - // open database if it was closed earlier and this.db was set to undefined. - this.init(); + + if (!this.db) { + await this.init(); + } + + throw error; } } public async get( statement: string, params: (string | number)[], - ): Promise { - const results: T[] = await this.getAll(statement, params); + ): Promise { + if (!this.db) { + throw new Error('Database not initialized'); + } + + const result = await this.db.getFirstAsync(statement, params); - return results[0]; + return result as T | undefined; } - public getAll( + public async getAll( statement: string, params: (string | number)[], ): Promise { - return new Promise((resolve, reject) => { - this.db.readTransaction(transaction => { - transaction.executeSql( - statement, - params, - (_, result) => { - resolve(result.rows._array || []); - }, - (_, error) => { - reject(error); - logger.warn(error); - - return true; - }, - ); - }); - }); + if (!this.db) { + throw new Error('Database not initialized'); + } + + return this.db.getAllAsync(statement, params); } - public save(statement: string, params: (string | number)[]): Promise { - return new Promise((resolve, reject) => { - this.db.transaction(transaction => { - transaction.executeSql( - statement, - params, - () => { - resolve(null); - }, - (_, error) => { - reject(error); - logger.warn(error); - - return true; - }, - ); - }); - }); + public async save( + statement: string, + params: (string | number)[], + ): Promise { + if (!this.db) { + throw new Error('Database not initialized'); + } + + await this.db.runAsync(statement, params); } - public batchQuery( + public async batchQuery( queryParameterizedStatements = new Set(), ): Promise { - return new Promise((resolve, reject) => { - const resolveTransaction = resolve; - const rejectTransaction = reject; - this.db.transaction(async transaction => { - try { - const results: any[] = await Promise.all( - [...queryParameterizedStatements].map( - ([statement, params]) => - new Promise((_resolve, _reject) => { - transaction.executeSql( - statement, - params, - (_, result) => { - _resolve(result.rows._array[0]); - }, - (_, error) => { - _reject(error); - logger.warn(error); - - return true; - }, - ); - }), - ), - ); - resolveTransaction(results); - } catch (error) { - rejectTransaction(error); - logger.warn(error); + if (!this.db) { + throw new Error('Database not initialized'); + } + + const results: T[] = []; + await this.db.withTransactionAsync(async () => { + for (const [statement, params] of queryParameterizedStatements) { + const result = await this.db!.getAllAsync(statement, params); + if (result && result.length > 0) { + results.push(result[0]); } - }); + } }); + + return results; } - public batchSave( + public async batchSave( saveParameterizedStatements = new Set(), deleteParameterizedStatements?: Set, ): Promise { - return new Promise((resolve, reject) => { - const resolveTransaction = resolve; - const rejectTransaction = reject; - this.db.transaction(async transaction => { - try { - // await for all sql statements promises to resolve - await Promise.all( - [...saveParameterizedStatements].map( - ([statement, params]) => - new Promise((_resolve, _reject) => { - transaction.executeSql( - statement, - params, - () => { - _resolve(null); - }, - (_, error) => { - _reject(error); - logger.warn(error); - - return true; - }, - ); - }), - ), - ); - if (deleteParameterizedStatements) { - await Promise.all( - [...deleteParameterizedStatements].map( - ([statement, params]) => - new Promise((_resolve, _reject) => { - transaction.executeSql( - statement, - params, - () => { - _resolve(null); - }, - (_, error) => { - _reject(error); - logger.warn(error); - - return true; - }, - ); - }), - ), - ); - } - resolveTransaction(null); - } catch (error) { - rejectTransaction(error); - logger.warn(error); + if (!this.db) { + throw new Error('Database not initialized'); + } + + await this.db.withTransactionAsync(async () => { + for (const [statement, params] of saveParameterizedStatements) { + await this.db!.runAsync(statement, params); + } + if (deleteParameterizedStatements) { + for (const [statement, params] of deleteParameterizedStatements) { + await this.db!.runAsync(statement, params); } - }); + } }); } - public selectAndDelete( + public async selectAndDelete( queryParameterizedStatement: ParameterizedStatement, deleteParameterizedStatement: ParameterizedStatement, ): Promise { + if (!this.db) { + throw new Error('Database not initialized'); + } + const [queryStatement, queryParams] = queryParameterizedStatement; const [deleteStatement, deleteParams] = deleteParameterizedStatement; - return new Promise((resolve, reject) => { - const resolveTransaction = resolve; - const rejectTransaction = reject; - this.db.transaction(async transaction => { - try { - const result: T[] = await new Promise((_resolve, _reject) => { - transaction.executeSql( - queryStatement, - queryParams, - (_, sqlResult) => { - _resolve(sqlResult.rows._array || []); - }, - (_, error) => { - _reject(error); - logger.warn(error); - - return true; - }, - ); - }); - await new Promise((_resolve, _reject) => { - transaction.executeSql( - deleteStatement, - deleteParams, - () => { - _resolve(null); - }, - (_, error) => { - _reject(error); - logger.warn(error); - - return true; - }, - ); - }); - resolveTransaction(result); - } catch (error) { - rejectTransaction(error); - logger.warn(error); - } - }); + let results: T[] = []; + await this.db.withTransactionAsync(async () => { + results = await this.db!.getAllAsync(queryStatement, queryParams); + await this.db!.runAsync(deleteStatement, deleteParams); }); + + return results; } - private executeStatements(statements: string[]): Promise { - return new Promise((resolve, reject) => { - const resolveTransaction = resolve; - const rejectTransaction = reject; - this.db.transaction(async transaction => { - try { - await Promise.all( - statements.map( - statement => - new Promise((_resolve, _reject) => { - transaction.executeSql( - statement, - [], - () => { - _resolve(null); - }, - (_, error) => { - _reject(error); - - return true; - }, - ); - }), - ), - ); - resolveTransaction(null); - } catch (error) { - rejectTransaction(error); - logger.warn(error); - } - }); + private async executeStatements(statements: string[]): Promise { + if (!this.db) { + throw new Error('Database not initialized'); + } + + await this.db.withTransactionAsync(async () => { + for (const statement of statements) { + await this.db!.execAsync(statement); + } }); } - private async closeDB() { + private async closeDB(): Promise { if (this.db) { logger.debug('Closing Database'); - // closing database is not supported by expo-sqlite. - // Workaround is to access the private db variable and call the close() method. - await (this.db as any)._db.close(); + await this.db.closeAsync(); logger.debug('Database closed'); this.db = undefined; } diff --git a/packages/datastore-storage-adapter/src/index.ts b/packages/datastore-storage-adapter/src/index.ts index 19ba93a509b..3e7b2855d9d 100644 --- a/packages/datastore-storage-adapter/src/index.ts +++ b/packages/datastore-storage-adapter/src/index.ts @@ -1,5 +1,6 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 import SQLiteAdapter from './SQLiteAdapter/SQLiteAdapter'; +import ExpoSQLiteAdapter from './ExpoSQLiteAdapter/ExpoSQLiteAdapter'; -export { SQLiteAdapter }; +export { SQLiteAdapter, ExpoSQLiteAdapter };