Detect and analyze background script reload events #720
Conversation
everdimension
changed the title
|
📦 build.zip [updated at Feb 27, 5:09:25 PM UTC] |
| @@ -46,27 +61,29 @@ export class Account extends EventEmitter<AccountEvents> { | |||
| isPendingNewUser: boolean; | |||
|
|
|||
| private static async writeCurrentUser(user: User) { | |||
| await browserStorage.set(currentUserKey, user); | |||
| await BrowserStorage.set(currentUserKey, user); | |||
There was a problem hiding this comment.
User data contains salt:
export interface User {
id: string;
salt: string;
}
I'm not sure how risky it is to store the salt in browser storage or if we can keep user data in session storage
There was a problem hiding this comment.
Encrypted storage cannot be unlocked without it, this is part of necessary data
There was a problem hiding this comment.
Is it possible to make the salt a hash of the password instead of using crypto.getRandomValues(new Uint8Array(32)), so that we wouldn’t need to store it in browser storage (it could be stored in the session store).
But I’m still not sure if this is a good idea
| @@ -10,7 +10,7 @@ export function registerPersistentRoute(value: string) { | |||
| } | |||
|
|
|||
| export async function resetPersistedRoutes() { | |||
| return browserStorage.remove('routeRestoration'); | |||
| return BrowserStorage.remove('routeRestoration'); | |||
There was a problem hiding this comment.
Wdyt about moving this one to the session storage? May be there is no point of restoring the route after browser was reopened?
| } | ||
|
|
||
| private static async removeCredentials() { | ||
| return await browserStorage.remove(credentialsKey); | ||
| await SessionStorage.remove(credentialsKey); | ||
| await LoginActivity.recordLogout(); | ||
| } | ||
|
|
||
| static async readCurrentUser() { |
There was a problem hiding this comment.
Let's move these 2 functions to the top to place it near the writeCurrentUser?
| @@ -152,6 +169,16 @@ export class Account extends EventEmitter<AccountEvents> { | |||
| await this.setUser(user, { password }, { isNewUser: false }); | |||
| } | |||
|
|
|||
| isAuthenticated(): boolean { | |||
There was a problem hiding this comment.
Let's move this function below the getUser?
| ? installedEvent.reason | ||
| : isIntentionalBrowserRestart | ||
| ? 'browser restart' | ||
| : 're-enabled by user'; |
There was a problem hiding this comment.
Do we consider background script reload by itself as an unlikely reason?
There was a problem hiding this comment.
I expect such reloads to be caught by onInstalled listener and come with installedEvent.reason
| return; | ||
| } | ||
| const hasExistingUser = await account.hasExistingUser(); | ||
| const didNotOnboardYet = !hasExistingUser; |
| } | ||
|
|
||
| const sessionExpiry = await estimateSessionExpiry(); | ||
| const { previousVersion } = packageVersionStore; |
There was a problem hiding this comment.
Should we use getState here?
No description provided.