Skip to content

Commit cabd8ed

Browse files
Merge pull request #1067 from shogun444/refactor/964-secure-storage-item-options
refactor(secureStorage): replace isSensitive param with options
2 parents a648ec3 + c964166 commit cabd8ed

2 files changed

Lines changed: 49 additions & 46 deletions

File tree

src/__tests__/services/secureStorage.test.ts

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -428,17 +428,6 @@ describe('SecureStorage - Keychain/Keystore Verification #140', () => {
428428
expect(secureStorage.STORAGE_KEYS.REFRESH_TOKEN).toBe('teachlink_refresh_token');
429429
expect(secureStorage.STORAGE_KEYS.USER_DATA).toBe('teachlink_user_data');
430430
});
431-
432-
it('should identify sensitive keys', () => {
433-
expect(secureStorage.STORAGE_SENSITIVE_KEYS).toBeDefined();
434-
expect(secureStorage.STORAGE_SENSITIVE_KEYS.has('teachlink_access_token')).toBe(
435-
true,
436-
);
437-
expect(secureStorage.STORAGE_SENSITIVE_KEYS.has('teachlink_refresh_token')).toBe(
438-
true,
439-
);
440-
expect(secureStorage.STORAGE_SENSITIVE_KEYS.has('teachlink_user_data')).toBe(true);
441-
});
442431
});
443432

444433
// ─── Security Summary ───────────────────────────────────────────────────

src/services/secureStorage.ts

Lines changed: 49 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,6 @@ const KEYS = {
4444
INSTALL_UUID: 'teachlink_install_uuid',
4545
} as const;
4646

47-
// ─── Sensitive Keys (enforce Keychain/Keystore) ────────────────────────────────
48-
const SENSITIVE_KEYS = new Set([KEYS.ACCESS_TOKEN, KEYS.REFRESH_TOKEN, KEYS.USER_DATA]);
49-
5047
// ─── Options ──────────────────────────────────────────────────────────────────
5148
/**
5249
* Secure storage options configured for maximum security:
@@ -138,12 +135,7 @@ function auditLog(action: 'read' | 'write' | 'delete', key: string, tag: string)
138135
* Set item in encrypted secure storage
139136
* Throws error on failure - no silent fallback
140137
*/
141-
async function setItem(
142-
key: string,
143-
value: string,
144-
isSensitive: boolean = true,
145-
tag: string = 'unknown'
146-
): Promise<void> {
138+
async function setItem(key: string, value: string, tag: string = 'unknown'): Promise<void> {
147139
try {
148140
auditLog('write', key, tag);
149141
await SecureStore.setItemAsync(key, value, SECURE_OPTIONS);
@@ -158,13 +150,24 @@ async function setItem(
158150

159151
/**
160152
* Get item from encrypted secure storage
161-
* Throws error on failure - no silent fallback for sensitive data
162-
*/
163-
async function getItem(
164-
key: string,
165-
isSensitive: boolean = true,
166-
tag: string = 'unknown'
167-
): Promise<string | null> {
153+
* Throws error on failure unless {@link GetItemOptions.throwOnMissing} is false,
154+
* in which case a read error is swallowed and `null` is returned.
155+
*/
156+
interface GetItemOptions {
157+
/**
158+
* A unique identifier for the call-site (e.g., 'session-refresh').
159+
*/
160+
tag?: string;
161+
/**
162+
* When true (default), rethrow read errors. When false, swallow read errors
163+
* and return `null`. Used for non-sensitive reads where a missing value is
164+
* an expected outcome rather than a failure.
165+
*/
166+
throwOnMissing?: boolean;
167+
}
168+
169+
async function getItem(key: string, options: GetItemOptions = {}): Promise<string | null> {
170+
const { tag = 'unknown', throwOnMissing = true } = options;
168171
try {
169172
auditLog('read', key, tag);
170173
const value = await SecureStore.getItemAsync(key, SECURE_OPTIONS);
@@ -175,7 +178,7 @@ async function getItem(
175178
}`;
176179
logger.error(errorMsg, { key, platform: Platform.OS, tag });
177180

178-
if (isSensitive) {
181+
if (throwOnMissing) {
179182
throw error;
180183
}
181184

@@ -252,9 +255,9 @@ export async function saveTokens(
252255
}
253256

254257
await Promise.all([
255-
setItem(KEYS.ACCESS_TOKEN, accessToken, true, 'saveTokens'),
256-
setItem(KEYS.REFRESH_TOKEN, refreshToken, true, 'saveTokens'),
257-
setItem(KEYS.SESSION_EXPIRES_AT, String(expiresAt), false, 'saveTokens'),
258+
setItem(KEYS.ACCESS_TOKEN, accessToken, 'saveTokens'),
259+
setItem(KEYS.REFRESH_TOKEN, refreshToken, 'saveTokens'),
260+
setItem(KEYS.SESSION_EXPIRES_AT, String(expiresAt), 'saveTokens'),
258261
]);
259262

260263
tokenCache.set(KEYS.ACCESS_TOKEN, accessToken);
@@ -273,7 +276,7 @@ export async function saveTokens(
273276
export async function getAccessToken(): Promise<string | null> {
274277
const cached = tokenCache.get(KEYS.ACCESS_TOKEN);
275278
if (cached) return cached;
276-
const token = await getItem(KEYS.ACCESS_TOKEN, true, 'getAccessToken');
279+
const token = await getItem(KEYS.ACCESS_TOKEN, { tag: 'getAccessToken' });
277280
if (token) tokenCache.set(KEYS.ACCESS_TOKEN, token);
278281
return token;
279282
}
@@ -283,14 +286,17 @@ export async function getAccessToken(): Promise<string | null> {
283286
* Throws error if retrieval fails (sensitive data)
284287
*/
285288
export async function getRefreshToken(): Promise<string | null> {
286-
return getItem(KEYS.REFRESH_TOKEN, true, 'getRefreshToken');
289+
return getItem(KEYS.REFRESH_TOKEN, { tag: 'getRefreshToken' });
287290
}
288291

289292
/**
290293
* Get session expiration timestamp from secure storage
291294
*/
292295
export async function getSessionExpiresAt(): Promise<number | null> {
293-
const raw = await getItem(KEYS.SESSION_EXPIRES_AT, false, 'getSessionExpiresAt');
296+
const raw = await getItem(KEYS.SESSION_EXPIRES_AT, {
297+
tag: 'getSessionExpiresAt',
298+
throwOnMissing: false,
299+
});
294300
return raw ? Number(raw) : null;
295301
}
296302

@@ -319,15 +325,15 @@ export async function saveUserData(user: Record<string, unknown>): Promise<void>
319325
throw new Error('SecureStorage not initialized - cannot save user data');
320326
}
321327

322-
await setItem(KEYS.USER_DATA, JSON.stringify(user), true, 'saveUserData');
328+
await setItem(KEYS.USER_DATA, JSON.stringify(user), 'saveUserData');
323329
logger.info('✅ User data saved securely to Keychain/Keystore');
324330
}
325331

326332
/**
327333
* Get user profile data from encrypted storage
328334
*/
329335
export async function getUserData<T = Record<string, unknown>>(): Promise<T | null> {
330-
const raw = await getItem(KEYS.USER_DATA, true, 'getUserData');
336+
const raw = await getItem(KEYS.USER_DATA, { tag: 'getUserData' });
331337
if (!raw) return null;
332338
try {
333339
return JSON.parse(raw) as T;
@@ -351,15 +357,18 @@ export async function clearUserData(): Promise<void> {
351357
* Save biometric authentication preference to secure storage
352358
*/
353359
export async function setBiometricEnabled(enabled: boolean): Promise<void> {
354-
await setItem(KEYS.BIOMETRIC_ENABLED, enabled ? '1' : '0', false, 'setBiometricEnabled');
360+
await setItem(KEYS.BIOMETRIC_ENABLED, enabled ? '1' : '0', 'setBiometricEnabled');
355361
logger.info(`Biometric setting updated: ${enabled ? 'enabled' : 'disabled'}`);
356362
}
357363

358364
/**
359365
* Check if biometric authentication is enabled
360366
*/
361367
export async function isBiometricEnabled(): Promise<boolean> {
362-
const value = await getItem(KEYS.BIOMETRIC_ENABLED, false, 'isBiometricEnabled');
368+
const value = await getItem(KEYS.BIOMETRIC_ENABLED, {
369+
tag: 'isBiometricEnabled',
370+
throwOnMissing: false,
371+
});
363372
return value === '1';
364373
}
365374

@@ -386,7 +395,7 @@ export async function isBiometricEnabled(): Promise<boolean> {
386395
* @param enrollmentId A UUID that uniquely identifies the enrollment session.
387396
*/
388397
export async function saveBiometricEnrollmentId(enrollmentId: string): Promise<void> {
389-
await setItem(KEYS.BIOMETRIC_ENROLLMENT_ID, enrollmentId, false, 'saveBiometricEnrollmentId');
398+
await setItem(KEYS.BIOMETRIC_ENROLLMENT_ID, enrollmentId, 'saveBiometricEnrollmentId');
390399
logger.info('Biometric enrollment id saved to secure storage');
391400
}
392401

@@ -396,7 +405,10 @@ export async function saveBiometricEnrollmentId(enrollmentId: string): Promise<v
396405
* @returns The enrollment id, or `null` if none has been stored.
397406
*/
398407
export async function getBiometricEnrollmentId(): Promise<string | null> {
399-
return getItem(KEYS.BIOMETRIC_ENROLLMENT_ID, false, 'getBiometricEnrollmentId');
408+
return getItem(KEYS.BIOMETRIC_ENROLLMENT_ID, {
409+
tag: 'getBiometricEnrollmentId',
410+
throwOnMissing: false,
411+
});
400412
}
401413

402414
/**
@@ -522,30 +534,33 @@ export async function verifyBiometricOnReinstall(): Promise<void> {
522534
* Save remembered email to secure storage
523535
*/
524536
export async function saveRememberedEmail(email: string): Promise<void> {
525-
await setItem(KEYS.REMEMBERED_EMAIL, email, false, 'saveRememberedEmail');
537+
await setItem(KEYS.REMEMBERED_EMAIL, email, 'saveRememberedEmail');
526538
logger.info('Email address remembered in secure storage');
527539
}
528540

529541
/**
530542
* Get remembered email from secure storage
531543
*/
532544
export async function getRememberedEmail(): Promise<string | null> {
533-
return getItem(KEYS.REMEMBERED_EMAIL, false, 'getRememberedEmail');
545+
return getItem(KEYS.REMEMBERED_EMAIL, { tag: 'getRememberedEmail', throwOnMissing: false });
534546
}
535547

536548
/**
537549
* Save remember-me preference to secure storage
538550
*/
539551
export async function setRememberMe(enabled: boolean): Promise<void> {
540-
await setItem(KEYS.REMEMBER_ME, enabled ? '1' : '0', false, 'setRememberMe');
552+
await setItem(KEYS.REMEMBER_ME, enabled ? '1' : '0', 'setRememberMe');
541553
logger.info(`Remember me setting updated: ${enabled ? 'enabled' : 'disabled'}`);
542554
}
543555

544556
/**
545557
* Check if remember-me is enabled
546558
*/
547559
export async function isRememberMeEnabled(): Promise<boolean> {
548-
const value = await getItem(KEYS.REMEMBER_ME, false, 'isRememberMeEnabled');
560+
const value = await getItem(KEYS.REMEMBER_ME, {
561+
tag: 'isRememberMeEnabled',
562+
throwOnMissing: false,
563+
});
549564
return value === '1';
550565
}
551566

@@ -650,9 +665,8 @@ export async function refreshAccessToken(): Promise<RefreshTokenResponse> {
650665
// ─── Export manifest (for verification in tests) ────────────────────────────────
651666

652667
export const STORAGE_KEYS = KEYS;
653-
export const STORAGE_SENSITIVE_KEYS = SENSITIVE_KEYS;
654668

655669
// ─── Test Helpers ─────────────────────────────────────────────────────────────
656670
export function __resetSecureStorageVerification__(): void {
657671
isSecureStorageVerified = false;
658-
}
672+
}

0 commit comments

Comments
 (0)