ReviewOS

craft-native/craft

"Local Auth Persistence" does not persist — the expiry is an in-memory field

#162
Closed glennmichael123 opened this 9 days ago ·
9 days ago

setAuthPersistence / checkAuthPersistence / clearAuthPersistence are backed by a single field:

// ==================== Local Auth Persistence ====================

private var authSessionExpiry: Long = 0L

setAuthPersistence(true, 86400) writes System.currentTimeMillis() + duration * 1000 into it and resolves with {enabled: true, duration: 86400, expiresAt: …}. Nothing writes it anywhere else — no SharedPreferences, no file, no EncryptedSharedPreferences (which this file already uses for secureSet).

So the session survives exactly as long as the process does. Android kills a backgrounded app routinely; when the user comes back, CraftBridge is constructed fresh, authSessionExpiry is 0L, and checkAuthPersistence resolves {isValid: false, remainingSeconds: 0} — the same answer it gives when persistence was never enabled at all.

Why this is worse than it sounds

The whole point of the feature is to skip re-authentication after the app has gone away. A page using it correctly — set a 24-hour window, check on launch — gets the benefit only while the app was never evicted, which is the case where the user would not have been asked to re-authenticate anyway.

And the failure is indistinguishable from the honest one: {isValid: false} is also what an expired session returns, so nothing in the reply says "I forgot".

The fix

The expiry is one long. Writing it beside the other secure values would make it survive:

securePrefs.edit().putLong("craft_auth_session_expiry", authSessionExpiry).apply()

and reading it back in checkAuthPersistence when the field is 0L. securePrefs is already lazy and already EncryptedSharedPreferences, which is the right store for this value — an attacker who can rewrite a plain preference can extend their own session.

clearAuthPersistence would remove the key as well as zeroing the field.

Not migrated to Zig because of this

android_dispatch serves nothing here. The state is a private field in CraftBridge, so Zig would have to keep its own copy and the two would disagree the moment either language answered a call — which is a worse bug than the one above. Once the expiry lives in a store both can read, the trio becomes portable.

Related: #127, #148, #154, #157, #159 — replies that are sent but do not mean what they appear to.

9 days ago

Implemented and pushed in cb9099c. Android auth persistence now stores the expiry timestamp in the existing EncryptedSharedPreferences, reads it on every check so it survives process recreation, and removes it when disabled or cleared. Added a generator regression test and updated the Android conformance rationale. Local Android/Zig suites passed, and exact-commit CI, SBOM/vulnerability scan, Binary Size, and Benchmark workflows all completed successfully.

Sign in to comment on this issue.