diff --git a/apps/passport-client/components/modals/RequireAddPasswordModal.tsx b/apps/passport-client/components/modals/RequireAddPasswordModal.tsx index de497b7794..c26a1d9070 100644 --- a/apps/passport-client/components/modals/RequireAddPasswordModal.tsx +++ b/apps/passport-client/components/modals/RequireAddPasswordModal.tsx @@ -1,9 +1,15 @@ import { sleep } from "@pcd/util"; import { useCallback, useState } from "react"; import styled from "styled-components"; -import { useDispatch, useSelf, useUpdate } from "../../src/appHooks"; +import { + useDispatch, + useSelf, + useServerStorageRevision, + useUpdate +} from "../../src/appHooks"; import { loadEncryptionKey } from "../../src/localstorage"; import { setPassword } from "../../src/password"; +import { useSyncE2EEStorage } from "../../src/useSyncE2EEStorage"; import { BigInput, H2, Spacer } from "../core"; import { NewPasswordForm } from "../shared/NewPasswordForm"; import { ScreenLoader } from "../shared/ScreenLoader"; @@ -13,10 +19,12 @@ import { ScreenLoader } from "../shared/ScreenLoader"; * and have never created a password. It asks them to create a password. */ export function RequireAddPasswordModal() { + useSyncE2EEStorage(); const [loading, setLoading] = useState(false); const dispatch = useDispatch(); const update = useUpdate(); const self = useSelf(); + const serverStorageRevision = useServerStorageRevision(); const [newPassword, setNewPassword] = useState(""); const [confirmPassword, setConfirmPassword] = useState(""); @@ -29,18 +37,25 @@ export function RequireAddPasswordModal() { await sleep(); try { const currentEncryptionKey = loadEncryptionKey(); - await setPassword(newPassword, currentEncryptionKey, dispatch, update); + await setPassword( + newPassword, + currentEncryptionKey, + serverStorageRevision, + dispatch, + update + ); dispatch({ type: "set-modal", modal: { modalType: "none" } }); } catch (e) { - setError("Couldn't set a password - try again later"); + console.log("error setting password", e); + setError(e.message); } finally { setLoading(false); } - }, [loading, newPassword, dispatch, update]); + }, [loading, newPassword, serverStorageRevision, dispatch, update]); if (loading) { return ; diff --git a/apps/passport-client/components/modals/UpgradeAccountModal.tsx b/apps/passport-client/components/modals/UpgradeAccountModal.tsx index 28c741e928..44926d9ee7 100644 --- a/apps/passport-client/components/modals/UpgradeAccountModal.tsx +++ b/apps/passport-client/components/modals/UpgradeAccountModal.tsx @@ -1,9 +1,15 @@ import { sleep } from "@pcd/util"; import { useCallback, useState } from "react"; import styled from "styled-components"; -import { useDispatch, useSelf, useUpdate } from "../../src/appHooks"; +import { + useDispatch, + useSelf, + useServerStorageRevision, + useUpdate +} from "../../src/appHooks"; import { loadEncryptionKey } from "../../src/localstorage"; import { setPassword } from "../../src/password"; +import { useSyncE2EEStorage } from "../../src/useSyncE2EEStorage"; import { BigInput, H2, Spacer } from "../core"; import { NewPasswordForm } from "../shared/NewPasswordForm"; import { ScreenLoader } from "../shared/ScreenLoader"; @@ -13,10 +19,12 @@ import { ScreenLoader } from "../shared/ScreenLoader"; * and have never created a password. It asks them to create a password. */ export function UpgradeAccountModal() { + useSyncE2EEStorage(); const [loading, setLoading] = useState(false); const dispatch = useDispatch(); const update = useUpdate(); const self = useSelf(); + const serverStorageRevision = useServerStorageRevision(); const [newPassword, setNewPassword] = useState(""); const [confirmPassword, setConfirmPassword] = useState(""); @@ -29,18 +37,25 @@ export function UpgradeAccountModal() { await sleep(); try { const currentEncryptionKey = loadEncryptionKey(); - await setPassword(newPassword, currentEncryptionKey, dispatch, update); + await setPassword( + newPassword, + currentEncryptionKey, + serverStorageRevision, + dispatch, + update + ); dispatch({ type: "set-modal", modal: { modalType: "changed-password" } }); } catch (e) { - setError("Couldn't set a password - try again later"); + console.log("error setting password", e); + setError(e.message); } finally { setLoading(false); } - }, [loading, newPassword, dispatch, update]); + }, [loading, newPassword, serverStorageRevision, dispatch, update]); if (loading) { return ; diff --git a/apps/passport-client/components/screens/ChangePasswordScreen.tsx b/apps/passport-client/components/screens/ChangePasswordScreen.tsx index 16910a0d77..f8a9ca6092 100644 --- a/apps/passport-client/components/screens/ChangePasswordScreen.tsx +++ b/apps/passport-client/components/screens/ChangePasswordScreen.tsx @@ -7,10 +7,12 @@ import { useDispatch, useHasSetupPassword, useSelf, + useServerStorageRevision, useUpdate } from "../../src/appHooks"; import { loadEncryptionKey } from "../../src/localstorage"; import { setPassword } from "../../src/password"; +import { useSyncE2EEStorage } from "../../src/useSyncE2EEStorage"; import { CenterColumn, H2, HR, Spacer, TextCenter } from "../core"; import { LinkButton } from "../core/Button"; import { RippleLoader } from "../core/RippleLoader"; @@ -20,8 +22,11 @@ import { NewPasswordForm } from "../shared/NewPasswordForm"; import { PasswordInput } from "../shared/PasswordInput"; export function ChangePasswordScreen() { + useSyncE2EEStorage(); const self = useSelf(); const hasSetupPassword = useHasSetupPassword(); + const serverStorageRevision = useServerStorageRevision(); + // We want the `isChangePassword` state to persist on future renders, // otherwise we may show the invalid copy on the "finished" screen // after a password is set for the first time. @@ -66,7 +71,13 @@ export function ChangePasswordScreen() { saltResult.value ); } - await setPassword(newPassword, currentEncryptionKey, dispatch, update); + await setPassword( + newPassword, + currentEncryptionKey, + serverStorageRevision, + dispatch, + update + ); setFinished(true); @@ -79,6 +90,7 @@ export function ChangePasswordScreen() { }, [ currentPassword, newPassword, + serverStorageRevision, dispatch, update, loading, diff --git a/apps/passport-client/src/appHooks.ts b/apps/passport-client/src/appHooks.ts index 489435c650..faa381e08a 100644 --- a/apps/passport-client/src/appHooks.ts +++ b/apps/passport-client/src/appHooks.ts @@ -106,6 +106,10 @@ export function useIsDownloaded(): boolean | undefined { return useSelector((s) => s.downloadedPCDs, []); } +export function useServerStorageRevision(): string | undefined { + return useSelector((s) => s.serverStorageRevision, []); +} + export function useUserForcedToLogout(): boolean { const userForcedToLogout = useSelector( (s) => !!s.userInvalid || !!s.anotherDeviceChangedPassword, diff --git a/apps/passport-client/src/dispatch.ts b/apps/passport-client/src/dispatch.ts index a80fea8cee..2aa4bc8673 100644 --- a/apps/passport-client/src/dispatch.ts +++ b/apps/passport-client/src/dispatch.ts @@ -47,7 +47,7 @@ import { hasPendingRequest } from "./sessionStorage"; import { AppError, AppState, GetState, StateEmitter } from "./state"; import { hasSetupPassword } from "./user"; import { - downloadStorage, + downloadAndMergeStorage, uploadSerializedStorage, uploadStorage } from "./useSyncE2EEStorage"; @@ -354,12 +354,16 @@ async function finishAccountCreation( return; // Don't save the bad identity. User must reset account. } - // Save PCDs to E2EE storage. + // Save PCDs to E2EE storage. knownRevision=undefined is the way to create + // a new entry. It would also overwrite any conflicting data which may + // already exist, but that should be impossible for a new account with + // a new encryption key. console.log("[ACCOUNT] Upload initial PCDs"); const uploadResult = await uploadStorage( user, state.pcds, - state.subscriptions + state.subscriptions, + undefined // knownRevision ); if (uploadResult.success) { update({ @@ -531,6 +535,7 @@ async function loadAfterLogin( modal = { modalType: "upgrade-account-modal" }; } + console.log(`[SYNC] saving state at login: revision ${storage.revision}`); await savePCDs(pcds); await saveSubscriptions(subscriptions); savePersistentSyncStatus({ @@ -569,7 +574,13 @@ async function handlePasswordChangeOnOtherTab(update: ZuUpdate) { const encryptionKey = loadEncryptionKey(); return update({ self, - encryptionKey + encryptionKey, + + // Extra download helps to get our in-memory sync state up-to-date faster. + // The password change on the other tab is an upload of a new revision so + // a download is necessary. Otherwise loading our new self object above + // will make this tab think it needs to upload and cause a conflict. + extraDownloadRequested: true }); } @@ -656,9 +667,16 @@ let syncInProgress = false; let skippedSyncUpdates = 0; /** - * Does the real work of sync(), inside of reentrancy protection. - * Returns the changes to be made to AppState. If changes were made, - * this function should be run again. + * Does the real work of sync(), inside of reentrancy protection. If action + * is needed, this function takes one action and returns, expecting to be + * run again until no further action is needed. + * + * Returns the changes to be made to AppState, which the caller is expected + * to applly via update(). If the result is defined, this function should be + * run again. + * + * Further calls to update() will also occur inside of this function, to update + * fields which allow the UI to track progress. */ async function doSync( state: AppState, @@ -686,15 +704,21 @@ async function doSync( // Download user's E2EE storage, which includes both PCDs and subscriptions. // We'll skip this if it fails, or if the server indicates no changes based // on the last revision we downloaded. - const dlRes = await downloadStorage(state.serverStorageRevision); + const dlRes = await downloadAndMergeStorage( + state.serverStorageRevision, + state.serverStorageHash, + state.self, + state.pcds, + state.subscriptions + ); if (dlRes.success && dlRes.value != null) { - const { pcds, subscriptions, revision, storageHash } = dlRes.value; + const { pcds, subscriptions, serverRevision, serverHash } = dlRes.value; return { downloadedPCDs: true, pcds, subscriptions, - serverStorageRevision: revision, - serverStorageHash: storageHash, + serverStorageRevision: serverRevision, + serverStorageHash: serverHash, extraDownloadRequested: false }; } else { @@ -758,18 +782,24 @@ async function doSync( ); if (state.serverStorageHash !== appStorage.storageHash) { console.log("[SYNC] sync action: upload"); - // TODO(artwyman): Add serverStorageRevision input as knownRevision here, - // but only after we're able to respond to a conflict by downloading. const upRes = await uploadSerializedStorage( appStorage.serializedStorage, - appStorage.storageHash + appStorage.storageHash, + state.serverStorageRevision ); if (upRes.success) { return { serverStorageRevision: upRes.value.revision, serverStorageHash: upRes.value.storageHash }; + } else if (upRes.error.name === "Conflict") { + // Conflicts are resolved at download time, so ensure another download. + return { + completedFirstSync: true, + extraDownloadRequested: true + }; } else { + // We completed a first attempt at sync, even if it failed. We'll retry. return { completedFirstSync: true }; diff --git a/apps/passport-client/src/password.ts b/apps/passport-client/src/password.ts index d376d73b85..91f92f12f8 100644 --- a/apps/passport-client/src/password.ts +++ b/apps/passport-client/src/password.ts @@ -23,6 +23,7 @@ export const PASSWORD_MINIMUM_LENGTH = 8; export const setPassword = async ( newPassword: string, currentEncryptionKey: HexString, + knownServerStorageRevision: string | undefined, dispatch: Dispatcher, update: ZuUpdate ) => { @@ -32,18 +33,24 @@ export const setPassword = async ( const res = await updateBlobKeyForEncryptedStorage( currentEncryptionKey, newEncryptionKey, - newSalt + newSalt, + knownServerStorageRevision ); - // Meaning password is incorrect, as old row is not found - if (!res.success && res.error.name === "PasswordIncorrect") { - throw new Error( - "Incorrect password. If you've lost your password, reset your account below." - ); - } - if (!res.success) { - throw new Error(`Request failed with message ${res.error}`); + if (res.error.name === "PasswordIncorrect") { + throw new Error( + "Incorrect password. If you've lost your password, reset your account below." + ); + } else if (res.error.name === "Conflict") { + update({ extraDownloadRequested: true }); + throw new Error(`Cannot change password while PCDs are syncing. + Wait for download to complete or reload the page and try again.`); + } else { + throw new Error( + `Failed to set password - try again later. Request failed with message ${res.error}` + ); + } } dispatch({ diff --git a/apps/passport-client/src/state.ts b/apps/passport-client/src/state.ts index 0aeac205a7..5135b4621e 100644 --- a/apps/passport-client/src/state.ts +++ b/apps/passport-client/src/state.ts @@ -58,21 +58,59 @@ export interface AppState { anotherDeviceChangedPassword?: boolean; // Dynamic (in-memory-only) state-machine for sync of E2EE encrypted data. + // The background sync will always perform all steps (download, fetch feeds, + // upload) on its initial run, after which it will repeat each step only + // when requested (for download and feeds), or when the hash of stored + // state changes (for upload). // TODO(artwyman): The parts of this not needed by the rest of the app // might be better stored elsewhere, to avoid issues with reentrancy // and stale snapshots delivered via dispatch(). + + // (Dynamic sync state) Output variable indicating whether the first attempt + // to download from E2EE storage has completed (whether success or failure). + // Also used within the sync engine to avoid repeating this attempt. downloadedPCDs?: boolean; + + // (Dynamic sync state) Output variable indicating whether the first attempt + // to fetch PCDs from subscription feeds has completed (whether success or + // failure). + // Also used within the sync engine to avoid repeating this attempt. loadedIssuedPCDs?: boolean; - loadingIssuedPCDs?: boolean; // Used only to update UI + + // (Dynamic sync state) Output variable indicating when a fetch from + // subscription feeds is in progress. + // Only used to update UI, not to control the behavior of the sync itself. + loadingIssuedPCDs?: boolean; + + // (Dynamic sync state) Output variable indicating when all stages of the + // initial sync are complete. + // Also used within the sync engine so that the behavior of future syncs + // differs from the first. completedFirstSync?: boolean; + + // (Dynamic sync state) Input variable to indicate to the sync engine that + // it should download again. Will trigger at most one download, after which + // it will be set back to false (whether the download succeeded or failed). extraDownloadRequested?: boolean; + + // (Dynamic sync state) Input variable to indicate to the sync engine that + // it should fetch subscription feeds again. Will trigger at most one fetch, + // after which it will be set back to false (whether the fetch succeeded or + // failed). extraSubscriptionFetchRequested?: boolean; // Persistent sync state-machine fields, saved in local storage as a // PersistentSyncStatus object. This is structured to allow for more - // fields to be added later. See the docs in that type for the meaning of - // individual fields. + // fields to be added later. + + // (Persistent sync state) The revision (assigned by the server) of the most + // recent storage uploaded or downloaded. Represents the most recent + // point where we know our state was the same as the server. serverStorageRevision?: string; + + // (Persistent sync state) The hash (calculated by the client) of the most + // recent storage uploaded or downloaded. Represents the most recent + // point where we know our state was the same as the server. serverStorageHash?: string; knownTicketTypes?: KnownTicketType[]; diff --git a/apps/passport-client/src/useSyncE2EEStorage.tsx b/apps/passport-client/src/useSyncE2EEStorage.tsx index 3c017c8482..1f5aa1c406 100644 --- a/apps/passport-client/src/useSyncE2EEStorage.tsx +++ b/apps/passport-client/src/useSyncE2EEStorage.tsx @@ -9,6 +9,7 @@ import { deserializeStorage, requestChangeBlobKey, requestDownloadAndDecryptUpdatedStorage, + requestLogToServer, requestUploadEncryptedStorage, serializeStorage } from "@pcd/passport-interface"; @@ -29,6 +30,14 @@ import { import { getPackages } from "./pcdPackages"; import { useOnStateChange } from "./subscribe"; +// Temporary feature flag to allow the sync-merge code to be on the main branch +// before it's fully complete. When this is set to false, the behavior should +// remain as it was before sync-merge was implemented at all. Uploads will +// always overwrite existing contents, and downloads will always take new +// contents without merging. +// TODO(artwyman): Remove this when #1342 is complete. +const ENABLE_SYNC_MERGE = false; + export type UpdateBlobKeyStorageInfo = { revision: string; storageHash: string; @@ -41,7 +50,8 @@ export type UpdateBlobKeyResult = APIResult< export async function updateBlobKeyForEncryptedStorage( oldEncryptionKey: string, newEncryptionKey: string, - newSalt: string + newSalt: string, + knownServerStorageRevision?: string ): Promise { const oldUser = loadSelf(); const newUser = { ...oldUser, salt: newSalt }; @@ -61,16 +71,14 @@ export async function updateBlobKeyForEncryptedStorage( const oldBlobKey = await getHash(oldEncryptionKey); const newBlobKey = await getHash(newEncryptionKey); - // TODO(artwyman): Pass in knownRevison here, but only once this code is - // ready to respond to a conflict. For now, without a revision, this - // password change could clobber PCD changed which haven't downloaded yet. const changeResult = await requestChangeBlobKey( appConfig.zupassServer, oldBlobKey, newBlobKey, newUser.uuid, newSalt, - encryptedStorage + encryptedStorage, + ENABLE_SYNC_MERGE ? knownServerStorageRevision : undefined ); if (changeResult.success) { console.log( @@ -101,27 +109,37 @@ export type UploadStorageResult = APIResult< /** * Uploads the state of this passport which is contained in localstorage * to the server, end to end encrypted. + * + * If knownRevision is specified, it is used to abort the upload in + * case of conflict. If it is undefined, the upload will overwrite + * any revision. */ export async function uploadStorage( user: User, pcds: PCDCollection, - subscriptions: FeedSubscriptionManager + subscriptions: FeedSubscriptionManager, + knownRevision?: string ): Promise { const { serializedStorage, storageHash } = await serializeStorage( user, pcds, subscriptions ); - return uploadSerializedStorage(serializedStorage, storageHash); + return uploadSerializedStorage(serializedStorage, storageHash, knownRevision); } /** * Uploads the state of this passport, in serialied form as produced by * serializeStorage(). + * + * If knownRevision is specified, it is used to abort the upload in + * case of conflict. If it is undefined, the upload will overwrite + * any revision. */ export async function uploadSerializedStorage( serializedStorage: SyncedEncryptedStorage, - storageHash: string + storageHash: string, + knownRevision?: string ): Promise { const encryptionKey = loadEncryptionKey(); const blobKey = await getHash(encryptionKey); @@ -134,9 +152,8 @@ export async function uploadSerializedStorage( const uploadResult = await requestUploadEncryptedStorage( appConfig.zupassServer, blobKey, - encryptedStorage - // TODO(artwyman): Expose access to knownRevision when a caller is ready - // to handle conflicts. + encryptedStorage, + ENABLE_SYNC_MERGE ? knownRevision : undefined ); if (uploadResult.success) { @@ -151,18 +168,50 @@ export async function uploadSerializedStorage( success: true, value: { revision: uploadResult.value.revision, storageHash: storageHash } }; + } else if (uploadResult.error.name === "Conflict") { + console.warn("[SYNC] conflict uploading e2ee storage", uploadResult.error); + return { success: false, error: uploadResult.error }; } else { console.error("[SYNC] failed to upload e2ee storage", uploadResult.error); return { success: false, error: uploadResult.error }; } } +export type MergeableFields = { + pcds: PCDCollection; + subscriptions: FeedSubscriptionManager; +}; + +export type MergeStorageResult = APIResult; + +/** + * Merge the contents of local and remote states, both of which have potential + * changes from a common base state. + * + * TODO(artwyman): Describe merge algorithm. + */ +export async function mergeStorage( + _localFields: MergeableFields, + remoteFields: MergeableFields, + self: User +): Promise { + console.error( + "[SYNC] sync conflict needs merge! Keeping only remote state." + ); + // TODO(artwyman): Refactor this out to implement and test real merge. + requestLogToServer(appConfig.zupassServer, "sync-merge", { + user: self.uuid + // TODO(artwyman): more details for tracking. + }); + return { value: remoteFields, success: true }; +} + export type SyncStorageResult = APIResult< { pcds: PCDCollection; subscriptions: FeedSubscriptionManager; - revision: string; - storageHash: string; + serverRevision: string; + serverHash: string; } | null, null >; @@ -173,11 +222,16 @@ export type SyncStorageResult = APIResult< * result if the download detects that storage is unchanged from the * last saved revision. */ -export async function downloadStorage( - knownServerRevision: string | undefined +export async function downloadAndMergeStorage( + knownServerRevision: string | undefined, + knownServerHash: string | undefined, + appSelf: User, + appPCDs: PCDCollection, + appSubscriptions: FeedSubscriptionManager ): Promise { console.log("[SYNC] downloading e2ee storage"); + // Download latest revision from server, if it's newer than what's known. const encryptionKey = loadEncryptionKey(); const storageResult = await requestDownloadAndDecryptUpdatedStorage( appConfig.zupassServer, @@ -198,32 +252,94 @@ export async function downloadStorage( return { value: null, success: true }; } + // Deserialize downloaded storage, which becomes the default new state if no + // merge is necessary. + const downloaded = await tryDeserializeNewStorage( + storageResult.value.storage + ); + if (downloaded === undefined) { + return { error: null, success: false }; + } + const { dlPCDs, dlSubscriptions, dlServerHash } = downloaded; + + // Check if local app state has changes since the last server revision, in + // which case a merge is necessary. Otherwise we keep the downloaded state. + let [newPCDs, newSubscriptions] = [dlPCDs, dlSubscriptions]; + if ( + ENABLE_SYNC_MERGE && + knownServerRevision !== undefined && + knownServerHash !== undefined + ) { + const appStorage = await serializeStorage( + appSelf, + appPCDs, + appSubscriptions + ); + if (appStorage.storageHash !== knownServerHash) { + const mergeResult = await mergeStorage( + { pcds: appPCDs, subscriptions: appSubscriptions }, + { + pcds: downloaded.dlPCDs, + subscriptions: downloaded.dlSubscriptions + }, + appSelf + ); + if (!mergeResult.success) { + console.error( + "[SYNC] unable to merge new e2ee storage", + mergeResult.error + ); + return { error: null, success: false }; + } + ({ pcds: newPCDs, subscriptions: newSubscriptions } = mergeResult.value); + } + } + + // We've successfully either accepted a new state, or created a merged state. + // Save and return results. + await savePCDs(newPCDs); + await saveSubscriptions(newSubscriptions); + savePersistentSyncStatus({ + serverStorageRevision: storageResult.value.revision, + serverStorageHash: dlServerHash + }); + console.log( + `[SYNC] downloaded e2ee storage (revision ${storageResult.value.revision})` + ); + return { + value: { + pcds: newPCDs, + subscriptions: newSubscriptions, + serverRevision: storageResult.value.revision, + serverHash: dlServerHash + }, + success: true + }; +} + +export async function tryDeserializeNewStorage( + storage: SyncedEncryptedStorage +): Promise< + | undefined + | { + dlPCDs: PCDCollection; + dlSubscriptions: FeedSubscriptionManager; + dlServerHash: string; + } +> { try { const { pcds, subscriptions, storageHash } = await deserializeStorage( - storageResult.value.storage, + storage, await getPackages() ); - await savePCDs(pcds); - await saveSubscriptions(subscriptions); - savePersistentSyncStatus({ - serverStorageRevision: storageResult.value.revision, - serverStorageHash: storageHash - }); - console.log( - `[SYNC] downloaded e2ee storage (revision ${storageResult.value.revision})` - ); return { - value: { - pcds, - subscriptions, - revision: storageResult.value.revision, - storageHash: storageHash - }, - success: true + dlPCDs: pcds, + dlSubscriptions: subscriptions, + dlServerHash: storageHash }; } catch (e) { console.error("[SYNC] uploaded storage is corrupted - ignoring it", e); - return { error: null, success: false }; + return undefined; } }