Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,9 @@ describe('Voice transcription section', () => {
it('rolls back the switch and toasts when the write fails', async () => {
voiceInputControllerMock.supportsOnDevice.mockReturnValue(false);
voiceNetworkConsentMock.readVoiceNetworkConsent.mockResolvedValue('unset');
voiceNetworkConsentMock.writeVoiceNetworkConsent.mockRejectedValue(new Error('boom'));
// The real write resolves `false` on a keychain failure (it reports at
// warning level and stays total); the row owns the rollback.
voiceNetworkConsentMock.writeVoiceNetworkConsent.mockResolvedValue(false);
const renderer = mountVoiceControl();
await TestRenderer.act(flush);

Expand Down
5 changes: 2 additions & 3 deletions apps/mobile/src/components/consent/consent-details.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,8 @@ export function VoiceTranscriptionControl() {
const value: 'granted' | 'declined' = next ? 'granted' : 'declined';
const previous = consent;
setConsent(value);
try {
await writeVoiceNetworkConsent(userId, value);
} catch {
const stored = await writeVoiceNetworkConsent(userId, value);
if (!stored) {
// Roll back the optimistic flip so the switch reflects the stored value.
setConsent(previous);
toast.error(t('consent.couldNotSaveChoice'));
Expand Down
2 changes: 1 addition & 1 deletion apps/mobile/src/glanceable-ios/adopt-activity.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * as SecureStore from 'expo-secure-store';
import * as SecureStore from '@/lib/auth/secure-store';

// The adoption hands an update token straight to the delivery, and it runs from
// the root layout's import. The route group that would otherwise install the
Expand Down
2 changes: 1 addition & 1 deletion apps/mobile/src/i18n/return-target.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * as SecureStore from 'expo-secure-store';
import * as SecureStore from '@/lib/auth/secure-store';

import { LANGUAGE_RETURN_TARGET_KEY } from '@/lib/storage-keys';

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * as SecureStore from 'expo-secure-store';
import * as SecureStore from '@/lib/auth/secure-store';
import * as z from 'zod';

import { PICKER_LAUNCH_CONTEXT_KEY } from '@/lib/storage-keys';
Expand Down
2 changes: 1 addition & 1 deletion apps/mobile/src/lib/app-unlock-context.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * as SecureStore from 'expo-secure-store';
import * as SecureStore from '@/lib/auth/secure-store';
import {
createContext,
type ReactNode,
Expand Down
11 changes: 6 additions & 5 deletions apps/mobile/src/lib/auth/account-metadata-write.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import * as SecureStore from 'expo-secure-store';

import { currentAuthEpoch, isCurrentAuthEpoch } from '@/lib/auth/auth-epoch';
import { deleteStoredValue, writeStoredValue } from '@/lib/auth/secure-store-value';
import { chainSave } from '@/lib/hooks/save-chain';

/**
Expand All @@ -19,10 +18,12 @@ export async function writeAccountMetadata(key: string, write: () => Promise<voi
});
}

/** Epoch-fenced, per-key serialized write of one string value. */
/** Epoch-fenced, per-key serialized write of one string value. A failed write
* is reported at warning level with the stable write fingerprint and still
* rejects, so a preference write can surface its own recoverable state. */
export async function setAccountMetadata(key: string, value: string): Promise<void> {
await writeAccountMetadata(key, async () => {
await SecureStore.setItemAsync(key, value);
await writeStoredValue(key, value);
});
}

Expand All @@ -34,6 +35,6 @@ export async function setAccountMetadata(key: string, value: string): Promise<vo
*/
export async function deleteAccountMetadata(key: string): Promise<void> {
await chainSave(key, async () => {
await SecureStore.deleteItemAsync(key);
await deleteStoredValue(key);
});
}
2 changes: 1 addition & 1 deletion apps/mobile/src/lib/auth/admission.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as AppIntegrity from '@expo/app-integrity';
import { CryptoDigestAlgorithm, CryptoEncoding, digestStringAsync } from 'expo-crypto';
import * as SecureStore from 'expo-secure-store';
import * as SecureStore from '@/lib/auth/secure-store';
import { Platform } from 'react-native';
import * as z from 'zod';

Expand Down
29 changes: 17 additions & 12 deletions apps/mobile/src/lib/auth/auth-context.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
/* eslint-disable max-lines -- sign-out teardown ordering, stale sign-in fencing, and the consent-outcome clear are kept together with the provider mount */
import * as SecureStore from 'expo-secure-store';
import { z } from 'zod';
import {
createContext,
Expand Down Expand Up @@ -43,6 +42,11 @@ import {
setSignOutTeardownActive,
} from '@/lib/auth/token-owner';
import { readStoredValueWithRetry } from '@/lib/auth/secure-store-read';
import {
deleteStoredValue,
readStoredValue,
readStoredValueSafe,
} from '@/lib/auth/secure-store-value';
import { type AuthSignOutCause, reportAuthBranch } from '@/lib/auth/sign-out-telemetry';
import { chainSave } from '@/lib/hooks/save-chain';
import { clearAgentModelPreference } from '@/lib/hooks/use-persisted-agent-model';
Expand Down Expand Up @@ -93,9 +97,13 @@ import { purgePostHogPersistence } from '@/lib/telemetry/posthog-storage';
import { AppState } from 'react-native';
import { beginAuthenticatedOwner, markRestoredAuthenticatedOwner } from '@/lib/context-scope';

// Pre-load tokens at module level so they're available before React mounts
export const preloadedAuthToken = SecureStore.getItemAsync(AUTH_TOKEN_KEY);
const preloadedRefreshToken = SecureStore.getItemAsync(REFRESH_TOKEN_KEY);
// Pre-load tokens at module level so they're available before React mounts.
// The raw throwing read is deliberate: a rejection here must reach the
// bootstrap read below, not settle as "nothing stored", so the retry helper —
// not this preload — owns the failure outcome (it reports the exhausted read at
// warning level with the stable read fingerprint).
export const preloadedAuthToken = readStoredValue(AUTH_TOKEN_KEY);
const preloadedRefreshToken = readStoredValue(REFRESH_TOKEN_KEY);
// A keychain failure at process start rejects these before any consumer can
// await them, and the runtime would report that as an unhandled rejection
// before AuthProvider even mounts. Observe it here; the bootstrap read below
Expand Down Expand Up @@ -503,13 +511,10 @@ export function AuthProvider({ children }: { readonly children: ReactNode }) {
// deletion are members of the same always-attempted batch.
await Promise.allSettled([
writeCredentials(async () => {
await SecureStore.deleteItemAsync(AUTH_TOKEN_KEY, IOS_BEARER_SECURE_STORE_OPTIONS);
await SecureStore.deleteItemAsync(REFRESH_TOKEN_KEY, IOS_BEARER_SECURE_STORE_OPTIONS);
await SecureStore.deleteItemAsync(
TOKEN_EXPIRES_AT_KEY,
IOS_BEARER_SECURE_STORE_OPTIONS
);
await SecureStore.deleteItemAsync(LEGACY_EXCHANGE_DONE_KEY);
await deleteStoredValue(AUTH_TOKEN_KEY, IOS_BEARER_SECURE_STORE_OPTIONS);
await deleteStoredValue(REFRESH_TOKEN_KEY, IOS_BEARER_SECURE_STORE_OPTIONS);
await deleteStoredValue(TOKEN_EXPIRES_AT_KEY, IOS_BEARER_SECURE_STORE_OPTIONS);
await deleteStoredValue(LEGACY_EXCHANGE_DONE_KEY);
}),
deleteAccountMetadata(ACTIVE_USER_ID_KEY),
deleteAccountMetadata(ORGANIZATION_STORAGE_KEY),
Expand Down Expand Up @@ -641,7 +646,7 @@ export function AuthProvider({ children }: { readonly children: ReactNode }) {

void (async () => {
try {
const expiresAtStr = await SecureStore.getItemAsync(TOKEN_EXPIRES_AT_KEY);
const expiresAtStr = await readStoredValueSafe(TOKEN_EXPIRES_AT_KEY);
if (!expiresAtStr) {
return;
}
Expand Down
71 changes: 71 additions & 0 deletions apps/mobile/src/lib/auth/credentials.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,77 @@ describe('bearer credential writes', () => {
expect(SecureStore.deleteItemAsync).toHaveBeenCalledWith(REFRESH_TOKEN_KEY, expectedOptions);
expect(SecureStore.deleteItemAsync).toHaveBeenCalledWith(TOKEN_EXPIRES_AT_KEY, expectedOptions);
});

it('clears the keys committed before a later write rejection and rethrows the failure', async () => {
store.set(AUTH_TOKEN_KEY, 'old-token');
store.set(REFRESH_TOKEN_KEY, 'old-refresh');
store.set(TOKEN_EXPIRES_AT_KEY, '999');

// The auth-token write commits; the refresh-token write then rejects.
vi.mocked(SecureStore.setItemAsync)
.mockImplementationOnce(async (key: string, value: string) => {
store.set(key, value);
await Promise.resolve();
})
.mockImplementationOnce(async () => {
await Promise.resolve();
throw new Error('keychain write failed');
});

await expect(
persistSignInCredentialsAtEpoch('new-token', 'new-refresh', { expiresIn: 3600 })
).rejects.toThrow('keychain write failed');

// The partial set is gone: no half-written session survives the failure.
expect(store.has(AUTH_TOKEN_KEY)).toBe(false);
expect(store.has(REFRESH_TOKEN_KEY)).toBe(false);
expect(store.has(TOKEN_EXPIRES_AT_KEY)).toBe(false);
expect(SecureStore.deleteItemAsync).toHaveBeenCalledWith(AUTH_TOKEN_KEY, expectedOptions);
expect(SecureStore.deleteItemAsync).toHaveBeenCalledWith(REFRESH_TOKEN_KEY, expectedOptions);
expect(SecureStore.deleteItemAsync).toHaveBeenCalledWith(TOKEN_EXPIRES_AT_KEY, expectedOptions);
});

it('rethrows the write failure when the partial-set cleanup also fails', async () => {
vi.mocked(SecureStore.setItemAsync)
.mockImplementationOnce(async (key: string, value: string) => {
store.set(key, value);
await Promise.resolve();
})
.mockImplementationOnce(async () => {
await Promise.resolve();
throw new Error('keychain write failed');
});
// The first cleanup delete rejects too: the original failure must still
// surface. A one-shot rejection keeps the shared mock clean for the rest
// of the suite (the sequential cleanup stops at the first rejection).
vi.mocked(SecureStore.deleteItemAsync).mockRejectedValueOnce(
new Error('cleanup delete failed')
);

await expect(
persistSignInCredentialsAtEpoch('new-token', 'new-refresh', { expiresIn: 3600 })
).rejects.toThrow('keychain write failed');
});

it('leaves the previous session intact when the first write of an attempt rejects', async () => {
store.set(AUTH_TOKEN_KEY, 'old-token');
store.set(REFRESH_TOKEN_KEY, 'old-refresh');
store.set(TOKEN_EXPIRES_AT_KEY, '999');

// The very first operation rejects, so nothing of this attempt committed.
vi.mocked(SecureStore.setItemAsync).mockRejectedValueOnce(new Error('keychain unavailable'));

await expect(
persistSignInCredentialsAtEpoch('new-token', 'new-refresh', { expiresIn: 3600 })
).rejects.toThrow('keychain unavailable');

// No partial set exists, so nothing is cleared: a transient keychain
// failure stays retryable instead of destroying the stored session.
expect(store.get(AUTH_TOKEN_KEY)).toBe('old-token');
expect(store.get(REFRESH_TOKEN_KEY)).toBe('old-refresh');
expect(store.get(TOKEN_EXPIRES_AT_KEY)).toBe('999');
expect(SecureStore.deleteItemAsync).not.toHaveBeenCalled();
});
});

describe('refresh rotation', () => {
Expand Down
53 changes: 40 additions & 13 deletions apps/mobile/src/lib/auth/credentials.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
readStoredValueRetryingNull,
readStoredValueWithRetry,
} from '@/lib/auth/secure-store-read';
import { deleteStoredValue, writeStoredValue } from '@/lib/auth/secure-store-value';
import { getActiveToken, isSignOutTeardownActive, setActiveToken } from '@/lib/auth/token-owner';
import { chainSave } from '@/lib/hooks/save-chain';
import { AUTH_TOKEN_KEY, REFRESH_TOKEN_KEY, TOKEN_EXPIRES_AT_KEY } from '@/lib/storage-keys';
Expand Down Expand Up @@ -101,20 +102,30 @@ export async function persistSignInCredentialsAtEpoch(
// a newer sign-in or sign-out: their own credential write is queued
// strictly behind this one.
const clearPartialCredentials = async (): Promise<void> => {
await SecureStore.deleteItemAsync(AUTH_TOKEN_KEY, IOS_BEARER_SECURE_STORE_OPTIONS);
await SecureStore.deleteItemAsync(REFRESH_TOKEN_KEY, IOS_BEARER_SECURE_STORE_OPTIONS);
await SecureStore.deleteItemAsync(TOKEN_EXPIRES_AT_KEY, IOS_BEARER_SECURE_STORE_OPTIONS);
await deleteStoredValue(AUTH_TOKEN_KEY, IOS_BEARER_SECURE_STORE_OPTIONS);
await deleteStoredValue(REFRESH_TOKEN_KEY, IOS_BEARER_SECURE_STORE_OPTIONS);
await deleteStoredValue(TOKEN_EXPIRES_AT_KEY, IOS_BEARER_SECURE_STORE_OPTIONS);
};

// Whether any credential key of this attempt reached the keychain. A later
// rejection then knows the store may hold a partial set and wipes it; a
// failure on the very first operation leaves the previous session untouched,
// so a transient keychain failure stays retryable instead of signing out.
let committedAny = false;

// Fence one credential operation: skip it when the epoch moved before the
// op, and clear the partial pair when it moved during the op.
const commitWrite = async (key: string, value?: string): Promise<boolean> => {
if (!isCurrentAuthEpoch(epoch)) {
return false;
}
// A keychain write/delete that rejects is reported at warning level with
// the stable operation fingerprint and then propagates, so sign-in lands
// on its persist-error state instead of publishing a half-written pair.
await (value === undefined
? SecureStore.deleteItemAsync(key, IOS_BEARER_SECURE_STORE_OPTIONS)
: SecureStore.setItemAsync(key, value, IOS_BEARER_SECURE_STORE_OPTIONS));
? deleteStoredValue(key, IOS_BEARER_SECURE_STORE_OPTIONS)
: writeStoredValue(key, value, IOS_BEARER_SECURE_STORE_OPTIONS));
Comment thread
iscekic marked this conversation as resolved.
committedAny = true;
if (!isCurrentAuthEpoch(epoch)) {
await clearPartialCredentials();
return false;
Expand All @@ -124,14 +135,30 @@ export async function persistSignInCredentialsAtEpoch(

let published = false;
await writeCredentials(async () => {
if (!(await commitWrite(AUTH_TOKEN_KEY, token))) {
return;
}
if (!(await commitWrite(REFRESH_TOKEN_KEY, hasPair ? refreshToken : undefined))) {
return;
}
if (!(await commitWrite(TOKEN_EXPIRES_AT_KEY, hasPair ? String(expiresAtMs) : undefined))) {
return;
try {
if (!(await commitWrite(AUTH_TOKEN_KEY, token))) {
return;
}
if (!(await commitWrite(REFRESH_TOKEN_KEY, hasPair ? refreshToken : undefined))) {
return;
}
if (!(await commitWrite(TOKEN_EXPIRES_AT_KEY, hasPair ? String(expiresAtMs) : undefined))) {
return;
}
} catch (error) {
// A later keychain operation rejected after an earlier one committed:
// the store now holds a partial credential set. Wipe it best-effort in
// the same serialized slot — so it cannot touch a newer sign-in's keys —
// then rethrow the original failure. A cleanup failure is swallowed so
// it cannot mask the write failure the caller owns.
if (committedAny) {
try {
await clearPartialCredentials();
} catch {
// Best effort: the caller must still see the original failure.
}
}
throw error;
}
// Every fenced operation passed its post-check and nothing awaited since
// the last one, so the epoch is still current: publish to the owner.
Expand Down
2 changes: 1 addition & 1 deletion apps/mobile/src/lib/auth/exchange-legacy-token.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * as SecureStore from 'expo-secure-store';
import * as SecureStore from '@/lib/auth/secure-store';

import { API_BASE_URL } from '@/lib/config';
import { currentAuthEpoch, isCurrentAuthEpoch } from '@/lib/auth/auth-epoch';
Expand Down
2 changes: 1 addition & 1 deletion apps/mobile/src/lib/auth/logout-cleanup.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * as SecureStore from 'expo-secure-store';
import * as SecureStore from '@/lib/auth/secure-store';
import * as Sentry from '@sentry/react-native';
import * as z from 'zod';

Expand Down
30 changes: 30 additions & 0 deletions apps/mobile/src/lib/auth/pending-external-auth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest';
import * as SecureStore from 'expo-secure-store';
import * as Sentry from '@sentry/react-native';

import { setTelemetrySink, type TelemetryEvent } from '@/lib/telemetry/error-sink';
import {
_resetPendingExternalAuthForTests,
clearPendingExternalAuth,
Expand Down Expand Up @@ -55,6 +56,35 @@ describe('pending-external-auth', () => {
expect(SecureStore.deleteItemAsync).not.toHaveBeenCalled();
});

it('treats a rejected read as absent instead of rejecting into the caller', async () => {
const events: TelemetryEvent[] = [];
setTelemetrySink(event => {
events.push(event);
});
try {
vi.mocked(SecureStore.getItemAsync).mockRejectedValueOnce(
new Error("Calling the 'getValueWithKeyAsync' function has failed")
);

// The mount effect `void`s this read: a rejection would be an unhandled
// error, so an unreadable record must read as absent.
await expect(readPendingExternalAuth()).resolves.toEqual({ kind: 'none' });
expect(SecureStore.deleteItemAsync).not.toHaveBeenCalled();

// The guarded store reports the failure once, at warning level, under the
// stable operation fingerprint — not the raw native message.
expect(events).toHaveLength(1);
expect(events[0]?.level).toBe('warning');
expect(events[0]?.fingerprint).toEqual(['secure-store-failure', 'read']);
expect(events[0]?.tags).toEqual({
'error.subsystem': 'secure_store',
'error.operation': 'read',
});
} finally {
setTelemetrySink(null);
}
});

it('returns stale for a record past the TTL without deleting', async () => {
const expired = { ...record, startedAt: Date.now() - PENDING_EXTERNAL_AUTH_TTL_MS - 1000 };
vi.mocked(SecureStore.getItemAsync).mockResolvedValue(JSON.stringify(expired));
Expand Down
16 changes: 14 additions & 2 deletions apps/mobile/src/lib/auth/pending-external-auth.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as Sentry from '@sentry/react-native';
import * as SecureStore from 'expo-secure-store';
import * as SecureStore from '@/lib/auth/secure-store';
import * as z from 'zod';

// Module-local key: this slice does not own storage-keys.ts, so the key is
Expand Down Expand Up @@ -60,8 +60,20 @@ export type PendingExternalAuthReadResult =
// Reads WITHOUT clearing. The caller must clear a `stale` record itself, after
// its own epoch check, so a concurrent `start()` write is never deleted by a
// restore read that observed the older record.
//
// Total: a rejected read is the same recoverable outcome as an absent record.
// The guarded store reports the failure at warning level with the stable read
// fingerprint before rethrowing, so catching here leaves exactly one event. It
// must not reject — the mount effect `void`s this read, so a rejection would
// surface as an unhandled error instead of the idle screen, and the person
// could still start sign-in again.
export async function readPendingExternalAuth(): Promise<PendingExternalAuthReadResult> {
const raw = await SecureStore.getItemAsync(PENDING_EXTERNAL_AUTH_KEY);
let raw: string | null = null;
try {
raw = await SecureStore.getItemAsync(PENDING_EXTERNAL_AUTH_KEY);
} catch {
return { kind: 'none' };
}
if (!raw) {
return { kind: 'none' };
}
Expand Down
Loading
Loading