From 65a7536efabc68a50006f5358bc0611f19173251 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 11 Dec 2023 12:29:40 -0800 Subject: [PATCH 01/71] create function that validates appState and returns list of errors as human interpretable strings --- apps/passport-client/src/validateState.ts | 74 +++++++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 apps/passport-client/src/validateState.ts diff --git a/apps/passport-client/src/validateState.ts b/apps/passport-client/src/validateState.ts new file mode 100644 index 0000000000..29416077a2 --- /dev/null +++ b/apps/passport-client/src/validateState.ts @@ -0,0 +1,74 @@ +import { + SemaphoreIdentityPCD, + SemaphoreIdentityPCDPackage +} from "@pcd/semaphore-identity-pcd"; +import { AppState } from "./state"; + +/** + * Determines whether the appState object contains valid data. If it does not, + * returns the set of things that are incorrect about it in an array of human + * interpretable strings. + */ +export function validateState(appState: AppState): string[] { + const validationErrors: string[] = []; + + if (!appState.self) { + validationErrors.push("missing 'self' field from app state"); + } + + if (!appState.identity) { + validationErrors.push("missing 'identity' field from app state"); + } + + if (!appState.encryptionKey) { + validationErrors.push("missing 'encryption' field from app state key"); + } + + if (!appState.pcds) { + validationErrors.push("missing 'pcds' field from app state"); + } + + if (appState.pcds.size() === 0) { + validationErrors.push("'pcds' field in app state contains no pcds"); + } + + const identityPCDFromCollection = appState.pcds.getPCDsByType( + SemaphoreIdentityPCDPackage.name + )[0] as SemaphoreIdentityPCD | undefined; + + if (!identityPCDFromCollection) { + validationErrors.push( + "'pcds' field in app state does not contain an identity PCD" + ); + } + + const identityFromPCDCollection = identityPCDFromCollection?.claim?.identity; + const commitmentOfIdentityPCDInCollection = + identityFromPCDCollection?.commitment?.toString(); + const commitmentFromSelfField = appState?.self?.commitment; + const commitmentFromIdentityField = + appState?.identity?.commitment?.toString(); + + if (commitmentOfIdentityPCDInCollection !== commitmentFromSelfField) { + validationErrors.push( + `commitment of identity pcd in collection (${commitmentOfIdentityPCDInCollection})` + + ` does not match commitment in 'self' field of app state (${commitmentFromSelfField})` + ); + } + + if (commitmentFromSelfField !== commitmentFromIdentityField) { + validationErrors.push( + `commitment in 'self' field of app state (${commitmentFromSelfField})` + + ` does not match commitment of 'identity' field of app state (${commitmentFromIdentityField})` + ); + } + + if (commitmentFromIdentityField !== commitmentOfIdentityPCDInCollection) { + validationErrors.push( + `commitment of 'identity' field of app state (${commitmentFromIdentityField})` + + ` does not match commitment of identity pcd in collection (${commitmentOfIdentityPCDInCollection})` + ); + } + + return validationErrors; +} From 144f2ebe3d693be12f8682a701ead31b955f4163 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 11 Dec 2023 12:36:32 -0800 Subject: [PATCH 02/71] handle state validation errors in inital state load --- apps/passport-client/pages/index.tsx | 14 +++++++++++++- apps/passport-client/src/validateState.ts | 15 +++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/apps/passport-client/pages/index.tsx b/apps/passport-client/pages/index.tsx index 3f7b7a661b..fde3b8b736 100644 --- a/apps/passport-client/pages/index.tsx +++ b/apps/passport-client/pages/index.tsx @@ -66,6 +66,10 @@ import { import { registerServiceWorker } from "../src/registerServiceWorker"; import { AppState, StateEmitter } from "../src/state"; import { pollUser } from "../src/user"; +import { + logAndUploadValidationErrors, + validateState +} from "../src/validateState"; class App extends React.Component { state = undefined as AppState | undefined; @@ -424,7 +428,7 @@ async function loadInitialState(): Promise { const persistentSyncStatus = loadPersistentSyncStatus(); - return { + const state: AppState = { self, encryptionKey, pcds, @@ -439,6 +443,14 @@ async function loadInitialState(): Promise { serverStorageRevision: persistentSyncStatus.serverStorageRevision, serverStorageHash: persistentSyncStatus.serverStorageHash }; + + const validationErrors = validateState(state); + if (validationErrors.length > 0) { + state.userInvalid = true; + logAndUploadValidationErrors(validationErrors); + } + + return state; } registerServiceWorker(); diff --git a/apps/passport-client/src/validateState.ts b/apps/passport-client/src/validateState.ts index 29416077a2..b5e92ce7e4 100644 --- a/apps/passport-client/src/validateState.ts +++ b/apps/passport-client/src/validateState.ts @@ -1,7 +1,9 @@ +import { requestLogToServer } from "@pcd/passport-interface"; import { SemaphoreIdentityPCD, SemaphoreIdentityPCDPackage } from "@pcd/semaphore-identity-pcd"; +import { appConfig } from "./appConfig"; import { AppState } from "./state"; /** @@ -72,3 +74,16 @@ export function validateState(appState: AppState): string[] { return validationErrors; } + +export async function logAndUploadValidationErrors( + errors: string[] +): Promise { + try { + console.log(`encountered state validation errors: `, errors); + await requestLogToServer(appConfig.zupassServer, "state-validation-error", { + errors + }); + } catch (e) { + console.log("error reporting errors", e); + } +} From 0aa824c4dc18ad7b47162ed28a8dcfd01dbc7f40 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 11 Dec 2023 13:01:41 -0800 Subject: [PATCH 03/71] commented some locations where validation logic should be added --- apps/passport-client/src/dispatch.ts | 2 ++ apps/passport-client/src/localstorage.ts | 2 ++ apps/passport-client/src/useSyncE2EEStorage.tsx | 3 ++- .../src/api/requestDownloadAndDecryptStorage.ts | 1 + 4 files changed, 7 insertions(+), 1 deletion(-) diff --git a/apps/passport-client/src/dispatch.ts b/apps/passport-client/src/dispatch.ts index a80fea8cee..122e5808f2 100644 --- a/apps/passport-client/src/dispatch.ts +++ b/apps/passport-client/src/dispatch.ts @@ -341,6 +341,7 @@ async function finishAccountCreation( state: AppState, update: ZuUpdate ) { + // validation point // Verify that the identity is correct. const { identity } = state; console.log("[ACCOUNT] Check user", identity, user); @@ -493,6 +494,7 @@ async function loadAfterLogin( storage: StorageWithRevision, update: ZuUpdate ) { + // validation point const { pcds, subscriptions, storageHash } = await deserializeStorage( storage.storage, await getPackages() diff --git a/apps/passport-client/src/localstorage.ts b/apps/passport-client/src/localstorage.ts index 2ee70bf98a..cda2becd02 100644 --- a/apps/passport-client/src/localstorage.ts +++ b/apps/passport-client/src/localstorage.ts @@ -13,6 +13,8 @@ import { Identity } from "@semaphore-protocol/identity"; import { z } from "zod"; import { getPackages } from "./pcdPackages"; +// validation points in this file + const OLD_PCDS_KEY = "pcds"; // deprecated const COLLECTION_KEY = "pcd_collection"; diff --git a/apps/passport-client/src/useSyncE2EEStorage.tsx b/apps/passport-client/src/useSyncE2EEStorage.tsx index 3c017c8482..619121e22d 100644 --- a/apps/passport-client/src/useSyncE2EEStorage.tsx +++ b/apps/passport-client/src/useSyncE2EEStorage.tsx @@ -116,7 +116,7 @@ export async function uploadStorage( } /** - * Uploads the state of this passport, in serialied form as produced by + * Uploads the state of this passport, in serialized form as produced by * serializeStorage(). */ export async function uploadSerializedStorage( @@ -131,6 +131,7 @@ export async function uploadSerializedStorage( encryptionKey ); + // validation point const uploadResult = await requestUploadEncryptedStorage( appConfig.zupassServer, blobKey, diff --git a/packages/passport-interface/src/api/requestDownloadAndDecryptStorage.ts b/packages/passport-interface/src/api/requestDownloadAndDecryptStorage.ts index a464e6484f..2c57e11ec9 100644 --- a/packages/passport-interface/src/api/requestDownloadAndDecryptStorage.ts +++ b/packages/passport-interface/src/api/requestDownloadAndDecryptStorage.ts @@ -83,6 +83,7 @@ export async function requestDownloadAndDecryptUpdatedStorage( encryptionKey: string, knownRevision: string | undefined ): Promise { + // validation point try { const encryptionKeyHash = await getHash(encryptionKey); const storageResult = await requestEncryptedStorage( From 6c55e49dfe6ce117352d4c3173fbc4c478af5f91 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 11 Dec 2023 13:14:17 -0800 Subject: [PATCH 04/71] more validation --- .../src/useSyncE2EEStorage.tsx | 11 ++++++++ apps/passport-client/src/validateState.ts | 25 +++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/apps/passport-client/src/useSyncE2EEStorage.tsx b/apps/passport-client/src/useSyncE2EEStorage.tsx index 619121e22d..f01eab8c5b 100644 --- a/apps/passport-client/src/useSyncE2EEStorage.tsx +++ b/apps/passport-client/src/useSyncE2EEStorage.tsx @@ -28,6 +28,10 @@ import { } from "./localstorage"; import { getPackages } from "./pcdPackages"; import { useOnStateChange } from "./subscribe"; +import { + logAndUploadValidationErrors, + validatePCDCollection +} from "./validateState"; export type UpdateBlobKeyStorageInfo = { revision: string; @@ -204,6 +208,13 @@ export async function downloadStorage( storageResult.value.storage, await getPackages() ); + + const validationErrors = validatePCDCollection(pcds); + if (validationErrors.length > 0) { + logAndUploadValidationErrors(validationErrors); + throw new Error("validation errors:\n" + validationErrors.join("\n")); + } + await savePCDs(pcds); await saveSubscriptions(subscriptions); savePersistentSyncStatus({ diff --git a/apps/passport-client/src/validateState.ts b/apps/passport-client/src/validateState.ts index b5e92ce7e4..7aaa482fbf 100644 --- a/apps/passport-client/src/validateState.ts +++ b/apps/passport-client/src/validateState.ts @@ -1,4 +1,5 @@ import { requestLogToServer } from "@pcd/passport-interface"; +import { PCDCollection } from "@pcd/pcd-collection"; import { SemaphoreIdentityPCD, SemaphoreIdentityPCDPackage @@ -75,6 +76,30 @@ export function validateState(appState: AppState): string[] { return validationErrors; } +export function validatePCDCollection(pcdCollection?: PCDCollection): string[] { + const validationErrors: string[] = []; + + if (!pcdCollection) { + validationErrors.push("missing 'pcds' field from app state"); + } + + if (pcdCollection.size() === 0) { + validationErrors.push("'pcds' field in app state contains no pcds"); + } + + const identityPCDFromCollection = pcdCollection.getPCDsByType( + SemaphoreIdentityPCDPackage.name + )[0] as SemaphoreIdentityPCD | undefined; + + if (!identityPCDFromCollection) { + validationErrors.push( + "'pcds' field in app state does not contain an identity PCD" + ); + } + + return validationErrors; +} + export async function logAndUploadValidationErrors( errors: string[] ): Promise { From dc0d6ff1677669189703153338a69005e7730841 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 11 Dec 2023 13:26:04 -0800 Subject: [PATCH 05/71] added more validation --- apps/passport-client/src/dispatch.ts | 7 ++++ .../src/useSyncE2EEStorage.tsx | 17 +++++++- apps/passport-client/src/validateState.ts | 40 +++++++++++++++---- 3 files changed, 54 insertions(+), 10 deletions(-) diff --git a/apps/passport-client/src/dispatch.ts b/apps/passport-client/src/dispatch.ts index 122e5808f2..5371a78d19 100644 --- a/apps/passport-client/src/dispatch.ts +++ b/apps/passport-client/src/dispatch.ts @@ -52,6 +52,7 @@ import { uploadStorage } from "./useSyncE2EEStorage"; import { assertUnreachable } from "./util"; +import { logAndUploadValidationErrors, validateUpload } from "./validateState"; export type Dispatcher = (action: Action) => void; @@ -758,6 +759,12 @@ async function doSync( state.pcds, state.subscriptions ); + const validationErrors = validateUpload(state.self, state.pcds); + if (validationErrors.length > 0) { + logAndUploadValidationErrors(validationErrors); + // TODO: what's the proper thing to do here? + } + if (state.serverStorageHash !== appStorage.storageHash) { console.log("[SYNC] sync action: upload"); // TODO(artwyman): Add serverStorageRevision input as knownRevision here, diff --git a/apps/passport-client/src/useSyncE2EEStorage.tsx b/apps/passport-client/src/useSyncE2EEStorage.tsx index f01eab8c5b..41e57fe536 100644 --- a/apps/passport-client/src/useSyncE2EEStorage.tsx +++ b/apps/passport-client/src/useSyncE2EEStorage.tsx @@ -30,7 +30,8 @@ import { getPackages } from "./pcdPackages"; import { useOnStateChange } from "./subscribe"; import { logAndUploadValidationErrors, - validatePCDCollection + validatePCDCollection, + validateUpload } from "./validateState"; export type UpdateBlobKeyStorageInfo = { @@ -111,6 +112,19 @@ export async function uploadStorage( pcds: PCDCollection, subscriptions: FeedSubscriptionManager ): Promise { + const validationErrors = validateUpload(user, pcds); + if (validationErrors.length > 0) { + logAndUploadValidationErrors(validationErrors); + return { + success: false, + error: { + name: "ValidationError", + detailedMessage: + "upload validation failed because\n: " + validationErrors.join("\n"), + code: undefined + } + }; + } const { serializedStorage, storageHash } = await serializeStorage( user, pcds, @@ -135,7 +149,6 @@ export async function uploadSerializedStorage( encryptionKey ); - // validation point const uploadResult = await requestUploadEncryptedStorage( appConfig.zupassServer, blobKey, diff --git a/apps/passport-client/src/validateState.ts b/apps/passport-client/src/validateState.ts index 7aaa482fbf..254915775f 100644 --- a/apps/passport-client/src/validateState.ts +++ b/apps/passport-client/src/validateState.ts @@ -1,4 +1,4 @@ -import { requestLogToServer } from "@pcd/passport-interface"; +import { User, requestLogToServer } from "@pcd/passport-interface"; import { PCDCollection } from "@pcd/pcd-collection"; import { SemaphoreIdentityPCD, @@ -76,24 +76,48 @@ export function validateState(appState: AppState): string[] { return validationErrors; } -export function validatePCDCollection(pcdCollection?: PCDCollection): string[] { +export function validatePCDCollection(pcds?: PCDCollection): string[] { const validationErrors: string[] = []; - if (!pcdCollection) { - validationErrors.push("missing 'pcds' field from app state"); + if (!pcds) { + validationErrors.push("pcd collection is absent"); } - if (pcdCollection.size() === 0) { - validationErrors.push("'pcds' field in app state contains no pcds"); + if (pcds.size() === 0) { + validationErrors.push("pcd collection is empty"); } - const identityPCDFromCollection = pcdCollection.getPCDsByType( + const identityPCDFromCollection = pcds.getPCDsByType( SemaphoreIdentityPCDPackage.name )[0] as SemaphoreIdentityPCD | undefined; if (!identityPCDFromCollection) { + validationErrors.push("pcd collection does not contain an identity pcd"); + } + + return validationErrors; +} + +export function validateUpload(user?: User, pcds?: PCDCollection): string[] { + const validationErrors: string[] = []; + + if (!user) { + validationErrors.push(`upload must include a user`); + } + + if (!pcds) { + validationErrors.push(`upload must include a pcd collection`); + } + + const identityPCDFromCollection = pcds.getPCDsByType( + SemaphoreIdentityPCDPackage.name + )[0] as SemaphoreIdentityPCD | undefined; + const commitmentFromPCDCollection = + identityPCDFromCollection?.claim?.identity?.commitment?.toString(); + + if (user?.commitment !== commitmentFromPCDCollection) { validationErrors.push( - "'pcds' field in app state does not contain an identity PCD" + "user commitment does not equal to commitment of identity pcd in pcd collection" ); } From 3db82d9f5d408063060fe63d33d6193755c0acc4 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 11 Dec 2023 13:32:17 -0800 Subject: [PATCH 06/71] implemented another validation point --- apps/passport-client/src/dispatch.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/apps/passport-client/src/dispatch.ts b/apps/passport-client/src/dispatch.ts index 5371a78d19..298d37ee55 100644 --- a/apps/passport-client/src/dispatch.ts +++ b/apps/passport-client/src/dispatch.ts @@ -52,7 +52,11 @@ import { uploadStorage } from "./useSyncE2EEStorage"; import { assertUnreachable } from "./util"; -import { logAndUploadValidationErrors, validateUpload } from "./validateState"; +import { + logAndUploadValidationErrors, + validatePCDCollection, + validateUpload +} from "./validateState"; export type Dispatcher = (action: Action) => void; @@ -495,11 +499,16 @@ async function loadAfterLogin( storage: StorageWithRevision, update: ZuUpdate ) { - // validation point const { pcds, subscriptions, storageHash } = await deserializeStorage( storage.storage, await getPackages() ); + const validationErrors = validatePCDCollection(pcds); + if (validationErrors.length > 0) { + logAndUploadValidationErrors(validationErrors); + // TODO: what's the right error message here? + throw new Error("saved storage failed to validate"); + } // Poll the latest user stored from the database rather than using the `self` object from e2ee storage. const userResponse = await requestUser( From 910a334a0d11be1f416ca06228491f9855ad1af1 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 11 Dec 2023 13:41:14 -0800 Subject: [PATCH 07/71] more validation --- apps/passport-client/src/dispatch.ts | 12 ++++--- apps/passport-client/src/validateState.ts | 38 +++++++++++++++++------ 2 files changed, 35 insertions(+), 15 deletions(-) diff --git a/apps/passport-client/src/dispatch.ts b/apps/passport-client/src/dispatch.ts index 298d37ee55..25bddfbc64 100644 --- a/apps/passport-client/src/dispatch.ts +++ b/apps/passport-client/src/dispatch.ts @@ -54,6 +54,7 @@ import { import { assertUnreachable } from "./util"; import { logAndUploadValidationErrors, + validateNewAccount, validatePCDCollection, validateUpload } from "./validateState"; @@ -346,18 +347,17 @@ async function finishAccountCreation( state: AppState, update: ZuUpdate ) { - // validation point // Verify that the identity is correct. - const { identity } = state; - console.log("[ACCOUNT] Check user", identity, user); - if (identity == null || identity.commitment.toString() !== user.commitment) { + const validationErrors = validateNewAccount(user, state); + if (validationErrors.length > 0) { + logAndUploadValidationErrors(validationErrors); update({ error: { title: "Invalid identity", message: "Something went wrong saving your Zupass. Contact support." } }); - return; // Don't save the bad identity. User must reset account. + return; } // Save PCDs to E2EE storage. @@ -373,6 +373,8 @@ async function finishAccountCreation( serverStorageRevision: uploadResult.value.revision, serverStorageHash: uploadResult.value.storageHash }); + } else { + // TODO: anything to add here? } // Save user to local storage. This is done last because it unblocks diff --git a/apps/passport-client/src/validateState.ts b/apps/passport-client/src/validateState.ts index 254915775f..89599d267f 100644 --- a/apps/passport-client/src/validateState.ts +++ b/apps/passport-client/src/validateState.ts @@ -12,30 +12,30 @@ import { AppState } from "./state"; * returns the set of things that are incorrect about it in an array of human * interpretable strings. */ -export function validateState(appState: AppState): string[] { +export function validateState(state: AppState): string[] { const validationErrors: string[] = []; - if (!appState.self) { + if (!state.self) { validationErrors.push("missing 'self' field from app state"); } - if (!appState.identity) { + if (!state.identity) { validationErrors.push("missing 'identity' field from app state"); } - if (!appState.encryptionKey) { + if (!state.encryptionKey) { validationErrors.push("missing 'encryption' field from app state key"); } - if (!appState.pcds) { + if (!state.pcds) { validationErrors.push("missing 'pcds' field from app state"); } - if (appState.pcds.size() === 0) { + if (state.pcds.size() === 0) { validationErrors.push("'pcds' field in app state contains no pcds"); } - const identityPCDFromCollection = appState.pcds.getPCDsByType( + const identityPCDFromCollection = state.pcds.getPCDsByType( SemaphoreIdentityPCDPackage.name )[0] as SemaphoreIdentityPCD | undefined; @@ -48,9 +48,8 @@ export function validateState(appState: AppState): string[] { const identityFromPCDCollection = identityPCDFromCollection?.claim?.identity; const commitmentOfIdentityPCDInCollection = identityFromPCDCollection?.commitment?.toString(); - const commitmentFromSelfField = appState?.self?.commitment; - const commitmentFromIdentityField = - appState?.identity?.commitment?.toString(); + const commitmentFromSelfField = state?.self?.commitment; + const commitmentFromIdentityField = state?.identity?.commitment?.toString(); if (commitmentOfIdentityPCDInCollection !== commitmentFromSelfField) { validationErrors.push( @@ -124,6 +123,25 @@ export function validateUpload(user?: User, pcds?: PCDCollection): string[] { return validationErrors; } +export function validateNewAccount(user: User, state: AppState): string[] { + const validationErrors: string[] = []; + + if (!state.identity) { + validationErrors.push("app state missing identity field"); + } + + const stateIdentityCommitment = state.identity?.commitment?.toString(); + const userIdentityCommitment = user?.commitment; + + if (stateIdentityCommitment !== userIdentityCommitment) { + validationErrors.push( + `app state identity (${stateIdentityCommitment}) does not match newly created user's commitment (${userIdentityCommitment})` + ); + } + + return validationErrors; +} + export async function logAndUploadValidationErrors( errors: string[] ): Promise { From 0629d46e065e831dc783eabdedd5a39c1a67b5b1 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 11 Dec 2023 13:43:36 -0800 Subject: [PATCH 08/71] undo a change --- .../src/api/requestDownloadAndDecryptStorage.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/passport-interface/src/api/requestDownloadAndDecryptStorage.ts b/packages/passport-interface/src/api/requestDownloadAndDecryptStorage.ts index 2c57e11ec9..a464e6484f 100644 --- a/packages/passport-interface/src/api/requestDownloadAndDecryptStorage.ts +++ b/packages/passport-interface/src/api/requestDownloadAndDecryptStorage.ts @@ -83,7 +83,6 @@ export async function requestDownloadAndDecryptUpdatedStorage( encryptionKey: string, knownRevision: string | undefined ): Promise { - // validation point try { const encryptionKeyHash = await getHash(encryptionKey); const storageResult = await requestEncryptedStorage( From 4502d33c5d49487b538493f0d24eda4de3fa468f Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 11 Dec 2023 13:48:12 -0800 Subject: [PATCH 09/71] add some validation logic inside of localstorage saving and loading of pcd collection --- apps/passport-client/src/localstorage.ts | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/apps/passport-client/src/localstorage.ts b/apps/passport-client/src/localstorage.ts index cda2becd02..26c3cdce85 100644 --- a/apps/passport-client/src/localstorage.ts +++ b/apps/passport-client/src/localstorage.ts @@ -12,6 +12,10 @@ import { SemaphoreSignaturePCD } from "@pcd/semaphore-signature-pcd"; import { Identity } from "@semaphore-protocol/identity"; import { z } from "zod"; import { getPackages } from "./pcdPackages"; +import { + logAndUploadValidationErrors, + validatePCDCollection +} from "./validateState"; // validation points in this file @@ -19,6 +23,11 @@ const OLD_PCDS_KEY = "pcds"; // deprecated const COLLECTION_KEY = "pcd_collection"; export async function savePCDs(pcds: PCDCollection): Promise { + const validationErrors = validatePCDCollection(pcds); + if (validationErrors.length > 0) { + logAndUploadValidationErrors(validationErrors); + throw new Error("couldn't save PCDs\n:" + validationErrors.join("\n")); + } const serialized = await pcds.serializeCollection(); window.localStorage[COLLECTION_KEY] = serialized; } @@ -33,10 +42,18 @@ export async function loadPCDs(): Promise { } const serializedCollection = window.localStorage[COLLECTION_KEY]; - return await PCDCollection.deserialize( + const collection = await PCDCollection.deserialize( await getPackages(), serializedCollection ?? "{}" ); + + const validationErrors = validatePCDCollection(collection); + if (validationErrors.length > 0) { + logAndUploadValidationErrors(validationErrors); + throw new Error("couldn't save PCDs\n:" + validationErrors.join("\n")); + } + + return collection; } export async function saveSubscriptions( From 91b5ca1e6785efe0f7295d987c0e6589d0672c0f Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 11 Dec 2023 14:11:55 -0800 Subject: [PATCH 10/71] add a bunch of comments --- apps/passport-client/src/validateState.ts | 24 ++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/apps/passport-client/src/validateState.ts b/apps/passport-client/src/validateState.ts index 89599d267f..cc1fd035f7 100644 --- a/apps/passport-client/src/validateState.ts +++ b/apps/passport-client/src/validateState.ts @@ -8,9 +8,10 @@ import { appConfig } from "./appConfig"; import { AppState } from "./state"; /** - * Determines whether the appState object contains valid data. If it does not, - * returns the set of things that are incorrect about it in an array of human - * interpretable strings. + * Determines whether the app's global state as represented by {@link AppState} object + * contains valid data. If it does not, returns the set of things that are incorrect about + * it in an array of human interpretable strings. If there are no errors, returns an + * empty array. */ export function validateState(state: AppState): string[] { const validationErrors: string[] = []; @@ -75,6 +76,11 @@ export function validateState(state: AppState): string[] { return validationErrors; } +/** + * Validates a {@link PCDCollection} by checking its contents. Does verify that the collection + * contains PCDs that are consistent with the rest of the application state. Returns a list of + * strings representing individual errors. If there are no errors, returns an empty array. + */ export function validatePCDCollection(pcds?: PCDCollection): string[] { const validationErrors: string[] = []; @@ -123,6 +129,13 @@ export function validateUpload(user?: User, pcds?: PCDCollection): string[] { return validationErrors; } +/** + * Validates whether a user returned by the Zupass server API is consistent + * with the local application state representation. Returns errors as strings, + * and returns an empty array if the two are not inconsistent. Does not validate + * {@link state} in its entirety, only that the {@link user} and {@link state} + * are consistent. + */ export function validateNewAccount(user: User, state: AppState): string[] { const validationErrors: string[] = []; @@ -142,6 +155,11 @@ export function validateNewAccount(user: User, state: AppState): string[] { return validationErrors; } +/** + * Logs validation errors to the console, and uploads them to the server so that + * we have records and are able to identify common types of errors. Does not leak + * sensitive information, such as decrypted versions of e2ee storage. + */ export async function logAndUploadValidationErrors( errors: string[] ): Promise { From 71dc18a00c8afbdd281ea2210a27501dae59fdd4 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 11 Dec 2023 14:13:29 -0800 Subject: [PATCH 11/71] added account export button to the invalid user modal --- .../passport-client/components/modals/InvalidUserModal.tsx | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/apps/passport-client/components/modals/InvalidUserModal.tsx b/apps/passport-client/components/modals/InvalidUserModal.tsx index 6d54040118..d95aacfdbd 100644 --- a/apps/passport-client/components/modals/InvalidUserModal.tsx +++ b/apps/passport-client/components/modals/InvalidUserModal.tsx @@ -3,11 +3,12 @@ import { useCallback } from "react"; import styled from "styled-components"; import { useDispatch } from "../../src/appHooks"; import { Button, H2 } from "../core"; +import { AccountExportButton } from "../shared/AccountExportButton"; export function InvalidUserModal() { const dispatch = useDispatch(); - const onClick = useCallback(() => { + const onExitClick = useCallback(() => { dispatch({ type: "reset-passport" }); }, [dispatch]); @@ -21,7 +22,9 @@ export function InvalidUserModal() { existing Zupass account onto this device.

- + + + ); } From 41bbdc131f34435fe50f0fc1a86bec9937846971 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Thu, 14 Dec 2023 19:23:31 -0800 Subject: [PATCH 12/71] add explanation to export button --- .../passport-client/components/modals/InvalidUserModal.tsx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/apps/passport-client/components/modals/InvalidUserModal.tsx b/apps/passport-client/components/modals/InvalidUserModal.tsx index d95aacfdbd..2c3f468c7e 100644 --- a/apps/passport-client/components/modals/InvalidUserModal.tsx +++ b/apps/passport-client/components/modals/InvalidUserModal.tsx @@ -21,7 +21,12 @@ export function InvalidUserModal() { one. Click the button below to log out. Then you'll be able to sync your existing Zupass account onto this device.

- + +

+ You can export your account data using the button below in case you need + it later. +

+ From 606780333ca7bba97109eaa2d25c804a3b9cd328 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Thu, 14 Dec 2023 19:28:29 -0800 Subject: [PATCH 13/71] respond to some PR comments --- apps/passport-client/pages/index.tsx | 9 ++------- apps/passport-client/src/dispatch.ts | 4 +--- apps/passport-client/src/localstorage.ts | 2 +- apps/passport-client/src/validateState.ts | 11 +++++++++++ 4 files changed, 15 insertions(+), 11 deletions(-) diff --git a/apps/passport-client/pages/index.tsx b/apps/passport-client/pages/index.tsx index fde3b8b736..1188dd4715 100644 --- a/apps/passport-client/pages/index.tsx +++ b/apps/passport-client/pages/index.tsx @@ -66,10 +66,7 @@ import { import { registerServiceWorker } from "../src/registerServiceWorker"; import { AppState, StateEmitter } from "../src/state"; import { pollUser } from "../src/user"; -import { - logAndUploadValidationErrors, - validateState -} from "../src/validateState"; +import { validateAndLogState } from "../src/validateState"; class App extends React.Component { state = undefined as AppState | undefined; @@ -444,10 +441,8 @@ async function loadInitialState(): Promise { serverStorageHash: persistentSyncStatus.serverStorageHash }; - const validationErrors = validateState(state); - if (validationErrors.length > 0) { + if (!validateAndLogState(state)) { state.userInvalid = true; - logAndUploadValidationErrors(validationErrors); } return state; diff --git a/apps/passport-client/src/dispatch.ts b/apps/passport-client/src/dispatch.ts index 25bddfbc64..5bd12faa78 100644 --- a/apps/passport-client/src/dispatch.ts +++ b/apps/passport-client/src/dispatch.ts @@ -357,7 +357,7 @@ async function finishAccountCreation( message: "Something went wrong saving your Zupass. Contact support." } }); - return; + return; // Don't save the bad identity. User must reset account. } // Save PCDs to E2EE storage. @@ -373,8 +373,6 @@ async function finishAccountCreation( serverStorageRevision: uploadResult.value.revision, serverStorageHash: uploadResult.value.storageHash }); - } else { - // TODO: anything to add here? } // Save user to local storage. This is done last because it unblocks diff --git a/apps/passport-client/src/localstorage.ts b/apps/passport-client/src/localstorage.ts index 26c3cdce85..2c8d3202e7 100644 --- a/apps/passport-client/src/localstorage.ts +++ b/apps/passport-client/src/localstorage.ts @@ -50,7 +50,7 @@ export async function loadPCDs(): Promise { const validationErrors = validatePCDCollection(collection); if (validationErrors.length > 0) { logAndUploadValidationErrors(validationErrors); - throw new Error("couldn't save PCDs\n:" + validationErrors.join("\n")); + throw new Error("couldn't load PCDs\n:" + validationErrors.join("\n")); } return collection; diff --git a/apps/passport-client/src/validateState.ts b/apps/passport-client/src/validateState.ts index cc1fd035f7..3b29e96634 100644 --- a/apps/passport-client/src/validateState.ts +++ b/apps/passport-client/src/validateState.ts @@ -7,6 +7,17 @@ import { import { appConfig } from "./appConfig"; import { AppState } from "./state"; +export function validateAndLogState(state: AppState): boolean { + const validationErrors = validateState(state); + + if (validationErrors.length > 0) { + logAndUploadValidationErrors(validationErrors); + return false; + } + + return true; +} + /** * Determines whether the app's global state as represented by {@link AppState} object * contains valid data. If it does not, returns the set of things that are incorrect about From 42683774cfb9a85889756b067dca2d84139f8c42 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Thu, 14 Dec 2023 19:31:03 -0800 Subject: [PATCH 14/71] some cleanups --- apps/passport-client/src/dispatch.ts | 1 + apps/passport-client/src/localstorage.ts | 2 -- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/apps/passport-client/src/dispatch.ts b/apps/passport-client/src/dispatch.ts index 5bd12faa78..d36862443f 100644 --- a/apps/passport-client/src/dispatch.ts +++ b/apps/passport-client/src/dispatch.ts @@ -768,6 +768,7 @@ async function doSync( state.pcds, state.subscriptions ); + const validationErrors = validateUpload(state.self, state.pcds); if (validationErrors.length > 0) { logAndUploadValidationErrors(validationErrors); diff --git a/apps/passport-client/src/localstorage.ts b/apps/passport-client/src/localstorage.ts index 2c8d3202e7..5018a968c2 100644 --- a/apps/passport-client/src/localstorage.ts +++ b/apps/passport-client/src/localstorage.ts @@ -17,8 +17,6 @@ import { validatePCDCollection } from "./validateState"; -// validation points in this file - const OLD_PCDS_KEY = "pcds"; // deprecated const COLLECTION_KEY = "pcd_collection"; From e0710a3f9c03ed78fad5eded68be303c5c56385b Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Thu, 14 Dec 2023 19:37:52 -0800 Subject: [PATCH 15/71] slight refactor --- apps/passport-client/src/dispatch.ts | 6 +-- apps/passport-client/src/localstorage.ts | 12 ++++-- .../src/useSyncE2EEStorage.tsx | 11 +++-- apps/passport-client/src/validateState.ts | 43 ++++++++++++++----- 4 files changed, 51 insertions(+), 21 deletions(-) diff --git a/apps/passport-client/src/dispatch.ts b/apps/passport-client/src/dispatch.ts index d36862443f..549a3d4dcf 100644 --- a/apps/passport-client/src/dispatch.ts +++ b/apps/passport-client/src/dispatch.ts @@ -349,7 +349,7 @@ async function finishAccountCreation( ) { // Verify that the identity is correct. const validationErrors = validateNewAccount(user, state); - if (validationErrors.length > 0) { + if (validationErrors.errors.length > 0) { logAndUploadValidationErrors(validationErrors); update({ error: { @@ -504,7 +504,7 @@ async function loadAfterLogin( await getPackages() ); const validationErrors = validatePCDCollection(pcds); - if (validationErrors.length > 0) { + if (validationErrors.errors.length > 0) { logAndUploadValidationErrors(validationErrors); // TODO: what's the right error message here? throw new Error("saved storage failed to validate"); @@ -770,7 +770,7 @@ async function doSync( ); const validationErrors = validateUpload(state.self, state.pcds); - if (validationErrors.length > 0) { + if (validationErrors.errors.length > 0) { logAndUploadValidationErrors(validationErrors); // TODO: what's the proper thing to do here? } diff --git a/apps/passport-client/src/localstorage.ts b/apps/passport-client/src/localstorage.ts index 5018a968c2..b1ac362b17 100644 --- a/apps/passport-client/src/localstorage.ts +++ b/apps/passport-client/src/localstorage.ts @@ -22,9 +22,11 @@ const COLLECTION_KEY = "pcd_collection"; export async function savePCDs(pcds: PCDCollection): Promise { const validationErrors = validatePCDCollection(pcds); - if (validationErrors.length > 0) { + if (validationErrors.errors.length > 0) { logAndUploadValidationErrors(validationErrors); - throw new Error("couldn't save PCDs\n:" + validationErrors.join("\n")); + throw new Error( + "couldn't save PCDs\n:" + validationErrors.errors.join("\n") + ); } const serialized = await pcds.serializeCollection(); window.localStorage[COLLECTION_KEY] = serialized; @@ -46,9 +48,11 @@ export async function loadPCDs(): Promise { ); const validationErrors = validatePCDCollection(collection); - if (validationErrors.length > 0) { + if (validationErrors.errors.length > 0) { logAndUploadValidationErrors(validationErrors); - throw new Error("couldn't load PCDs\n:" + validationErrors.join("\n")); + throw new Error( + "couldn't load PCDs\n:" + validationErrors.errors.join("\n") + ); } return collection; diff --git a/apps/passport-client/src/useSyncE2EEStorage.tsx b/apps/passport-client/src/useSyncE2EEStorage.tsx index 41e57fe536..90646726aa 100644 --- a/apps/passport-client/src/useSyncE2EEStorage.tsx +++ b/apps/passport-client/src/useSyncE2EEStorage.tsx @@ -113,14 +113,15 @@ export async function uploadStorage( subscriptions: FeedSubscriptionManager ): Promise { const validationErrors = validateUpload(user, pcds); - if (validationErrors.length > 0) { + if (validationErrors.errors.length > 0) { logAndUploadValidationErrors(validationErrors); return { success: false, error: { name: "ValidationError", detailedMessage: - "upload validation failed because\n: " + validationErrors.join("\n"), + "upload validation failed because\n: " + + validationErrors.errors.join("\n"), code: undefined } }; @@ -223,9 +224,11 @@ export async function downloadStorage( ); const validationErrors = validatePCDCollection(pcds); - if (validationErrors.length > 0) { + if (validationErrors.errors.length > 0) { logAndUploadValidationErrors(validationErrors); - throw new Error("validation errors:\n" + validationErrors.join("\n")); + throw new Error( + "validation errors:\n" + validationErrors.errors.join("\n") + ); } await savePCDs(pcds); diff --git a/apps/passport-client/src/validateState.ts b/apps/passport-client/src/validateState.ts index 3b29e96634..7866f11bfb 100644 --- a/apps/passport-client/src/validateState.ts +++ b/apps/passport-client/src/validateState.ts @@ -7,10 +7,15 @@ import { import { appConfig } from "./appConfig"; import { AppState } from "./state"; +export interface ValidationErrors { + errors: string[]; + userUUID: string; +} + export function validateAndLogState(state: AppState): boolean { const validationErrors = validateState(state); - if (validationErrors.length > 0) { + if (validationErrors.errors.length > 0) { logAndUploadValidationErrors(validationErrors); return false; } @@ -24,7 +29,7 @@ export function validateAndLogState(state: AppState): boolean { * it in an array of human interpretable strings. If there are no errors, returns an * empty array. */ -export function validateState(state: AppState): string[] { +export function validateState(state: AppState): ValidationErrors { const validationErrors: string[] = []; if (!state.self) { @@ -84,7 +89,10 @@ export function validateState(state: AppState): string[] { ); } - return validationErrors; + return { + errors: validationErrors, + userUUID: state.self?.uuid + }; } /** @@ -92,7 +100,7 @@ export function validateState(state: AppState): string[] { * contains PCDs that are consistent with the rest of the application state. Returns a list of * strings representing individual errors. If there are no errors, returns an empty array. */ -export function validatePCDCollection(pcds?: PCDCollection): string[] { +export function validatePCDCollection(pcds?: PCDCollection): ValidationErrors { const validationErrors: string[] = []; if (!pcds) { @@ -111,10 +119,16 @@ export function validatePCDCollection(pcds?: PCDCollection): string[] { validationErrors.push("pcd collection does not contain an identity pcd"); } - return validationErrors; + return { + errors: validationErrors, + userUUID: "" + }; } -export function validateUpload(user?: User, pcds?: PCDCollection): string[] { +export function validateUpload( + user?: User, + pcds?: PCDCollection +): ValidationErrors { const validationErrors: string[] = []; if (!user) { @@ -137,7 +151,10 @@ export function validateUpload(user?: User, pcds?: PCDCollection): string[] { ); } - return validationErrors; + return { + errors: validationErrors, + userUUID: user?.uuid + }; } /** @@ -147,7 +164,10 @@ export function validateUpload(user?: User, pcds?: PCDCollection): string[] { * {@link state} in its entirety, only that the {@link user} and {@link state} * are consistent. */ -export function validateNewAccount(user: User, state: AppState): string[] { +export function validateNewAccount( + user: User, + state: AppState +): ValidationErrors { const validationErrors: string[] = []; if (!state.identity) { @@ -163,7 +183,10 @@ export function validateNewAccount(user: User, state: AppState): string[] { ); } - return validationErrors; + return { + errors: validationErrors, + userUUID: user.uuid + }; } /** @@ -172,7 +195,7 @@ export function validateNewAccount(user: User, state: AppState): string[] { * sensitive information, such as decrypted versions of e2ee storage. */ export async function logAndUploadValidationErrors( - errors: string[] + errors: ValidationErrors ): Promise { try { console.log(`encountered state validation errors: `, errors); From 87e2ece979dab3c1b9faffc16e2b24cc596b5699 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Thu, 14 Dec 2023 19:49:39 -0800 Subject: [PATCH 16/71] ensure user uuid is always uploaded with a validation error --- apps/passport-client/src/validateState.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/apps/passport-client/src/validateState.ts b/apps/passport-client/src/validateState.ts index 7866f11bfb..d3e06168ef 100644 --- a/apps/passport-client/src/validateState.ts +++ b/apps/passport-client/src/validateState.ts @@ -5,11 +5,12 @@ import { SemaphoreIdentityPCDPackage } from "@pcd/semaphore-identity-pcd"; import { appConfig } from "./appConfig"; +import { loadSelf } from "./localstorage"; import { AppState } from "./state"; export interface ValidationErrors { errors: string[]; - userUUID: string; + userUUID?: string; } export function validateAndLogState(state: AppState): boolean { @@ -198,6 +199,8 @@ export async function logAndUploadValidationErrors( errors: ValidationErrors ): Promise { try { + const user = loadSelf(); + errors.userUUID = errors.userUUID ?? user.uuid; console.log(`encountered state validation errors: `, errors); await requestLogToServer(appConfig.zupassServer, "state-validation-error", { errors From 09ee40592331f3883f8c49b59fb275a31ffb71ea Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Thu, 14 Dec 2023 19:51:40 -0800 Subject: [PATCH 17/71] more pr review comments --- apps/passport-client/src/dispatch.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/passport-client/src/dispatch.ts b/apps/passport-client/src/dispatch.ts index 549a3d4dcf..600c6e5d36 100644 --- a/apps/passport-client/src/dispatch.ts +++ b/apps/passport-client/src/dispatch.ts @@ -772,7 +772,8 @@ async function doSync( const validationErrors = validateUpload(state.self, state.pcds); if (validationErrors.errors.length > 0) { logAndUploadValidationErrors(validationErrors); - // TODO: what's the proper thing to do here? + state.userInvalid = true; + return; } if (state.serverStorageHash !== appStorage.storageHash) { From d8537ca9f11e7eabb698d1c5563bf4650b600bf0 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Thu, 14 Dec 2023 19:53:21 -0800 Subject: [PATCH 18/71] more pr review comments --- apps/passport-client/src/dispatch.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/apps/passport-client/src/dispatch.ts b/apps/passport-client/src/dispatch.ts index 600c6e5d36..68457e64a6 100644 --- a/apps/passport-client/src/dispatch.ts +++ b/apps/passport-client/src/dispatch.ts @@ -373,6 +373,13 @@ async function finishAccountCreation( serverStorageRevision: uploadResult.value.revision, serverStorageHash: uploadResult.value.storageHash }); + } else if ( + !uploadResult.success && + uploadResult.error.name === "ValidationError" + ) { + update({ + userInvalid: true + }); } // Save user to local storage. This is done last because it unblocks From 36a242bc43b6df30b4b42e2d2be42a42685b61ba Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Thu, 14 Dec 2023 19:55:12 -0800 Subject: [PATCH 19/71] fixed another comment --- apps/passport-client/src/dispatch.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/apps/passport-client/src/dispatch.ts b/apps/passport-client/src/dispatch.ts index 68457e64a6..9492415869 100644 --- a/apps/passport-client/src/dispatch.ts +++ b/apps/passport-client/src/dispatch.ts @@ -776,14 +776,14 @@ async function doSync( state.subscriptions ); - const validationErrors = validateUpload(state.self, state.pcds); - if (validationErrors.errors.length > 0) { - logAndUploadValidationErrors(validationErrors); - state.userInvalid = true; - return; - } - if (state.serverStorageHash !== appStorage.storageHash) { + const validationErrors = validateUpload(state.self, state.pcds); + if (validationErrors.errors.length > 0) { + logAndUploadValidationErrors(validationErrors); + state.userInvalid = true; + return; + } + 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. From 0535ebd1eb08fc25c1f87bd058bf1551f3d71daa Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Thu, 14 Dec 2023 19:58:52 -0800 Subject: [PATCH 20/71] rename validateState to validateAppState --- apps/passport-client/src/localstorage.ts | 14 +++++++------- apps/passport-client/src/validateState.ts | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/apps/passport-client/src/localstorage.ts b/apps/passport-client/src/localstorage.ts index b1ac362b17..066d183ce5 100644 --- a/apps/passport-client/src/localstorage.ts +++ b/apps/passport-client/src/localstorage.ts @@ -47,13 +47,13 @@ export async function loadPCDs(): Promise { serializedCollection ?? "{}" ); - const validationErrors = validatePCDCollection(collection); - if (validationErrors.errors.length > 0) { - logAndUploadValidationErrors(validationErrors); - throw new Error( - "couldn't load PCDs\n:" + validationErrors.errors.join("\n") - ); - } + // const validationErrors = validatePCDCollection(collection); + // if (validationErrors.errors.length > 0) { + // logAndUploadValidationErrors(validationErrors); + // throw new Error( + // "couldn't load PCDs\n:" + validationErrors.errors.join("\n") + // ); + // } return collection; } diff --git a/apps/passport-client/src/validateState.ts b/apps/passport-client/src/validateState.ts index d3e06168ef..9bbec44e38 100644 --- a/apps/passport-client/src/validateState.ts +++ b/apps/passport-client/src/validateState.ts @@ -14,7 +14,7 @@ export interface ValidationErrors { } export function validateAndLogState(state: AppState): boolean { - const validationErrors = validateState(state); + const validationErrors = validateAppState(state); if (validationErrors.errors.length > 0) { logAndUploadValidationErrors(validationErrors); @@ -30,7 +30,7 @@ export function validateAndLogState(state: AppState): boolean { * it in an array of human interpretable strings. If there are no errors, returns an * empty array. */ -export function validateState(state: AppState): ValidationErrors { +export function validateAppState(state: AppState): ValidationErrors { const validationErrors: string[] = []; if (!state.self) { From f078f9d3ad5b7b2aeb92d2eac02ab49667930748 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Thu, 14 Dec 2023 20:22:46 -0800 Subject: [PATCH 21/71] consolidated all validation logic into one function --- apps/passport-client/pages/index.tsx | 4 +- .../pages/loadInitialState.tsx | 75 +++++++ apps/passport-client/src/dispatch.ts | 29 ++- apps/passport-client/src/localstorage.ts | 15 +- .../src/useSyncE2EEStorage.tsx | 24 +-- apps/passport-client/src/validateState.ts | 185 ++++++------------ 6 files changed, 160 insertions(+), 172 deletions(-) create mode 100644 apps/passport-client/pages/loadInitialState.tsx diff --git a/apps/passport-client/pages/index.tsx b/apps/passport-client/pages/index.tsx index 1188dd4715..df79ee4295 100644 --- a/apps/passport-client/pages/index.tsx +++ b/apps/passport-client/pages/index.tsx @@ -66,7 +66,7 @@ import { import { registerServiceWorker } from "../src/registerServiceWorker"; import { AppState, StateEmitter } from "../src/state"; import { pollUser } from "../src/user"; -import { validateAndLogState } from "../src/validateState"; +import { validateAndLogStateErrors } from "../src/validateState"; class App extends React.Component { state = undefined as AppState | undefined; @@ -441,7 +441,7 @@ async function loadInitialState(): Promise { serverStorageHash: persistentSyncStatus.serverStorageHash }; - if (!validateAndLogState(state)) { + if (!validateAndLogStateErrors(state.self, state.identity, state.pcds)) { state.userInvalid = true; } diff --git a/apps/passport-client/pages/loadInitialState.tsx b/apps/passport-client/pages/loadInitialState.tsx new file mode 100644 index 0000000000..0462a54801 --- /dev/null +++ b/apps/passport-client/pages/loadInitialState.tsx @@ -0,0 +1,75 @@ +import { createStorageBackedCredentialCache } from "@pcd/passport-interface"; +import { Identity } from "@semaphore-protocol/identity"; +import { + loadCheckedInOfflineDevconnectTickets, + loadEncryptionKey, + loadIdentity, + loadOfflineTickets, + loadPCDs, + loadPersistentSyncStatus, + loadSelf, + loadSubscriptions, + saveIdentity, + saveSubscriptions +} from "../src/localstorage"; +import { AppState } from "../src/state"; +import { validateAndLogStateErrors } from "../src/validateState"; + +// TODO: move to a separate file +export async function loadInitialState(): Promise { + let identity = loadIdentity(); + + if (identity == null) { + console.log("Generating a new Semaphore identity..."); + identity = new Identity(); + saveIdentity(identity); + } + + const self = loadSelf(); + const pcds = await loadPCDs(); + const encryptionKey = loadEncryptionKey(); + const subscriptions = await loadSubscriptions(); + const offlineTickets = loadOfflineTickets(); + const checkedInOfflineDevconnectTickets = + loadCheckedInOfflineDevconnectTickets(); + + subscriptions.updatedEmitter.listen(() => saveSubscriptions(subscriptions)); + + let modal = { modalType: "none" } as AppState["modal"]; + + if ( + // If on Zupass legacy login, ask user to set password + self != null && + encryptionKey == null && + self.salt == null + ) { + console.log("Asking existing user to set a password"); + modal = { modalType: "upgrade-account-modal" }; + } + + const credentialCache = createStorageBackedCredentialCache(); + + const persistentSyncStatus = loadPersistentSyncStatus(); + + const state: AppState = { + self, + encryptionKey, + pcds, + identity, + modal, + subscriptions, + resolvingSubscriptionId: undefined, + credentialCache, + offlineTickets, + checkedinOfflineDevconnectTickets: checkedInOfflineDevconnectTickets, + offline: !window.navigator.onLine, + serverStorageRevision: persistentSyncStatus.serverStorageRevision, + serverStorageHash: persistentSyncStatus.serverStorageHash + }; + + if (!validateAndLogStateErrors(state.self, state.identity, state.pcds)) { + state.userInvalid = true; + } + + return state; +} diff --git a/apps/passport-client/src/dispatch.ts b/apps/passport-client/src/dispatch.ts index 9492415869..066132a3e0 100644 --- a/apps/passport-client/src/dispatch.ts +++ b/apps/passport-client/src/dispatch.ts @@ -52,12 +52,7 @@ import { uploadStorage } from "./useSyncE2EEStorage"; import { assertUnreachable } from "./util"; -import { - logAndUploadValidationErrors, - validateNewAccount, - validatePCDCollection, - validateUpload -} from "./validateState"; +import { validateAndLogStateErrors } from "./validateState"; export type Dispatcher = (action: Action) => void; @@ -180,7 +175,12 @@ export async function dispatch( case "reset-passport": return resetPassport(state, update); case "load-after-login": - return loadAfterLogin(action.encryptionKey, action.storage, update); + return loadAfterLogin( + state, + action.encryptionKey, + action.storage, + update + ); case "set-modal": return update({ modal: action.modal @@ -348,9 +348,7 @@ async function finishAccountCreation( update: ZuUpdate ) { // Verify that the identity is correct. - const validationErrors = validateNewAccount(user, state); - if (validationErrors.errors.length > 0) { - logAndUploadValidationErrors(validationErrors); + if (!validateAndLogStateErrors(user, state.identity, state.pcds)) { update({ error: { title: "Invalid identity", @@ -364,6 +362,7 @@ async function finishAccountCreation( console.log("[ACCOUNT] Upload initial PCDs"); const uploadResult = await uploadStorage( user, + state.identity, state.pcds, state.subscriptions ); @@ -502,6 +501,7 @@ async function removePCD(state: AppState, update: ZuUpdate, pcdId: string) { } async function loadAfterLogin( + state: AppState, encryptionKey: string, storage: StorageWithRevision, update: ZuUpdate @@ -510,9 +510,8 @@ async function loadAfterLogin( storage.storage, await getPackages() ); - const validationErrors = validatePCDCollection(pcds); - if (validationErrors.errors.length > 0) { - logAndUploadValidationErrors(validationErrors); + + if (!validateAndLogStateErrors(state.self, state.identity, state.pcds)) { // TODO: what's the right error message here? throw new Error("saved storage failed to validate"); } @@ -777,9 +776,7 @@ async function doSync( ); if (state.serverStorageHash !== appStorage.storageHash) { - const validationErrors = validateUpload(state.self, state.pcds); - if (validationErrors.errors.length > 0) { - logAndUploadValidationErrors(validationErrors); + if (!validateAndLogStateErrors(state.self, state.identity, state.pcds)) { state.userInvalid = true; return; } diff --git a/apps/passport-client/src/localstorage.ts b/apps/passport-client/src/localstorage.ts index 066d183ce5..19a2bf1097 100644 --- a/apps/passport-client/src/localstorage.ts +++ b/apps/passport-client/src/localstorage.ts @@ -12,22 +12,19 @@ import { SemaphoreSignaturePCD } from "@pcd/semaphore-signature-pcd"; import { Identity } from "@semaphore-protocol/identity"; import { z } from "zod"; import { getPackages } from "./pcdPackages"; -import { - logAndUploadValidationErrors, - validatePCDCollection -} from "./validateState"; +import { validateAndLogStateErrors } from "./validateState"; const OLD_PCDS_KEY = "pcds"; // deprecated const COLLECTION_KEY = "pcd_collection"; export async function savePCDs(pcds: PCDCollection): Promise { - const validationErrors = validatePCDCollection(pcds); - if (validationErrors.errors.length > 0) { - logAndUploadValidationErrors(validationErrors); - throw new Error( - "couldn't save PCDs\n:" + validationErrors.errors.join("\n") + if (validateAndLogStateErrors(undefined, undefined, pcds, true)) { + console.log( + "PCD Collection failed to validate - not writing it to localstorage" ); + return; } + const serialized = await pcds.serializeCollection(); window.localStorage[COLLECTION_KEY] = serialized; } diff --git a/apps/passport-client/src/useSyncE2EEStorage.tsx b/apps/passport-client/src/useSyncE2EEStorage.tsx index 90646726aa..33bac9759f 100644 --- a/apps/passport-client/src/useSyncE2EEStorage.tsx +++ b/apps/passport-client/src/useSyncE2EEStorage.tsx @@ -13,6 +13,7 @@ import { serializeStorage } from "@pcd/passport-interface"; import { PCDCollection } from "@pcd/pcd-collection"; +import { Identity } from "@semaphore-protocol/identity"; import stringify from "fast-json-stable-stringify"; import { useCallback, useContext, useEffect } from "react"; import { appConfig } from "./appConfig"; @@ -28,11 +29,7 @@ import { } from "./localstorage"; import { getPackages } from "./pcdPackages"; import { useOnStateChange } from "./subscribe"; -import { - logAndUploadValidationErrors, - validatePCDCollection, - validateUpload -} from "./validateState"; +import { validateAndLogStateErrors } from "./validateState"; export type UpdateBlobKeyStorageInfo = { revision: string; @@ -109,19 +106,16 @@ export type UploadStorageResult = APIResult< */ export async function uploadStorage( user: User, + userIdentity: Identity, pcds: PCDCollection, subscriptions: FeedSubscriptionManager ): Promise { - const validationErrors = validateUpload(user, pcds); - if (validationErrors.errors.length > 0) { - logAndUploadValidationErrors(validationErrors); + if (!validateAndLogStateErrors(user, userIdentity, pcds)) { return { success: false, error: { name: "ValidationError", - detailedMessage: - "upload validation failed because\n: " + - validationErrors.errors.join("\n"), + detailedMessage: "upload validation failed", code: undefined } }; @@ -223,12 +217,8 @@ export async function downloadStorage( await getPackages() ); - const validationErrors = validatePCDCollection(pcds); - if (validationErrors.errors.length > 0) { - logAndUploadValidationErrors(validationErrors); - throw new Error( - "validation errors:\n" + validationErrors.errors.join("\n") - ); + if (!validateAndLogStateErrors(undefined, undefined, pcds, true)) { + throw new Error("downloaded e2ee state failed to validate"); } await savePCDs(pcds); diff --git a/apps/passport-client/src/validateState.ts b/apps/passport-client/src/validateState.ts index 9bbec44e38..096305c576 100644 --- a/apps/passport-client/src/validateState.ts +++ b/apps/passport-client/src/validateState.ts @@ -4,20 +4,25 @@ import { SemaphoreIdentityPCD, SemaphoreIdentityPCDPackage } from "@pcd/semaphore-identity-pcd"; +import { Identity } from "@semaphore-protocol/identity"; import { appConfig } from "./appConfig"; import { loadSelf } from "./localstorage"; -import { AppState } from "./state"; -export interface ValidationErrors { - errors: string[]; - userUUID?: string; -} - -export function validateAndLogState(state: AppState): boolean { - const validationErrors = validateAppState(state); +export function validateAndLogStateErrors( + self: User | undefined, + identity: Identity | undefined, + pcds: PCDCollection | undefined, + forceCheckPCDs?: boolean +): boolean { + const validationErrors = validateAppState( + self, + identity, + pcds, + forceCheckPCDs + ); if (validationErrors.errors.length > 0) { - logAndUploadValidationErrors(validationErrors); + logValidationErrors(validationErrors); return false; } @@ -30,44 +35,56 @@ export function validateAndLogState(state: AppState): boolean { * it in an array of human interpretable strings. If there are no errors, returns an * empty array. */ -export function validateAppState(state: AppState): ValidationErrors { +function validateAppState( + self: User | undefined, + identity: Identity | undefined, + pcds: PCDCollection | undefined, + forceCheckPCDs?: boolean +): ValidationErrors { const validationErrors: string[] = []; - if (!state.self) { - validationErrors.push("missing 'self' field from app state"); - } + const loggedOut = !self && !identity; - if (!state.identity) { - validationErrors.push("missing 'identity' field from app state"); + const identityPCDFromCollection = pcds?.getPCDsByType( + SemaphoreIdentityPCDPackage.name + )?.[0] as SemaphoreIdentityPCD | undefined; + + if (forceCheckPCDs || !loggedOut) { + if (!identityPCDFromCollection) { + validationErrors.push( + "'pcds' field in app state does not contain an identity PCD" + ); + } } - if (!state.encryptionKey) { - validationErrors.push("missing 'encryption' field from app state key"); + if (loggedOut) { + return { + errors: validationErrors, + userUUID: undefined + }; } - if (!state.pcds) { - validationErrors.push("missing 'pcds' field from app state"); + if (!self) { + validationErrors.push("missing 'self'"); } - if (state.pcds.size() === 0) { - validationErrors.push("'pcds' field in app state contains no pcds"); + if (!identity) { + validationErrors.push("missing 'identity'"); } - const identityPCDFromCollection = state.pcds.getPCDsByType( - SemaphoreIdentityPCDPackage.name - )[0] as SemaphoreIdentityPCD | undefined; + if (!pcds) { + validationErrors.push("missing 'pcds'"); + } - if (!identityPCDFromCollection) { - validationErrors.push( - "'pcds' field in app state does not contain an identity PCD" - ); + if (pcds.size() === 0) { + validationErrors.push("'pcds' contains no pcds"); } const identityFromPCDCollection = identityPCDFromCollection?.claim?.identity; const commitmentOfIdentityPCDInCollection = identityFromPCDCollection?.commitment?.toString(); - const commitmentFromSelfField = state?.self?.commitment; - const commitmentFromIdentityField = state?.identity?.commitment?.toString(); + const commitmentFromSelfField = self?.commitment; + const commitmentFromIdentityField = identity?.commitment?.toString(); if (commitmentOfIdentityPCDInCollection !== commitmentFromSelfField) { validationErrors.push( @@ -92,101 +109,7 @@ export function validateAppState(state: AppState): ValidationErrors { return { errors: validationErrors, - userUUID: state.self?.uuid - }; -} - -/** - * Validates a {@link PCDCollection} by checking its contents. Does verify that the collection - * contains PCDs that are consistent with the rest of the application state. Returns a list of - * strings representing individual errors. If there are no errors, returns an empty array. - */ -export function validatePCDCollection(pcds?: PCDCollection): ValidationErrors { - const validationErrors: string[] = []; - - if (!pcds) { - validationErrors.push("pcd collection is absent"); - } - - if (pcds.size() === 0) { - validationErrors.push("pcd collection is empty"); - } - - const identityPCDFromCollection = pcds.getPCDsByType( - SemaphoreIdentityPCDPackage.name - )[0] as SemaphoreIdentityPCD | undefined; - - if (!identityPCDFromCollection) { - validationErrors.push("pcd collection does not contain an identity pcd"); - } - - return { - errors: validationErrors, - userUUID: "" - }; -} - -export function validateUpload( - user?: User, - pcds?: PCDCollection -): ValidationErrors { - const validationErrors: string[] = []; - - if (!user) { - validationErrors.push(`upload must include a user`); - } - - if (!pcds) { - validationErrors.push(`upload must include a pcd collection`); - } - - const identityPCDFromCollection = pcds.getPCDsByType( - SemaphoreIdentityPCDPackage.name - )[0] as SemaphoreIdentityPCD | undefined; - const commitmentFromPCDCollection = - identityPCDFromCollection?.claim?.identity?.commitment?.toString(); - - if (user?.commitment !== commitmentFromPCDCollection) { - validationErrors.push( - "user commitment does not equal to commitment of identity pcd in pcd collection" - ); - } - - return { - errors: validationErrors, - userUUID: user?.uuid - }; -} - -/** - * Validates whether a user returned by the Zupass server API is consistent - * with the local application state representation. Returns errors as strings, - * and returns an empty array if the two are not inconsistent. Does not validate - * {@link state} in its entirety, only that the {@link user} and {@link state} - * are consistent. - */ -export function validateNewAccount( - user: User, - state: AppState -): ValidationErrors { - const validationErrors: string[] = []; - - if (!state.identity) { - validationErrors.push("app state missing identity field"); - } - - const stateIdentityCommitment = state.identity?.commitment?.toString(); - const userIdentityCommitment = user?.commitment; - - if (stateIdentityCommitment !== userIdentityCommitment) { - validationErrors.push( - `app state identity (${stateIdentityCommitment}) does not match newly created user's commitment (${userIdentityCommitment})` - ); - } - - return { - errors: validationErrors, - userUUID: user.uuid + userUUID: self?.uuid }; } @@ -195,9 +118,7 @@ export function validateNewAccount( * we have records and are able to identify common types of errors. Does not leak * sensitive information, such as decrypted versions of e2ee storage. */ -export async function logAndUploadValidationErrors( - errors: ValidationErrors -): Promise { +async function logValidationErrors(errors: ValidationErrors): Promise { try { const user = loadSelf(); errors.userUUID = errors.userUUID ?? user.uuid; @@ -209,3 +130,11 @@ export async function logAndUploadValidationErrors( console.log("error reporting errors", e); } } + +/** + * Uploaded to server in case of a state validation error. + */ +export interface ValidationErrors { + errors: string[]; + userUUID?: string; +} From e793c87b0942c38b16108f21882b9cd5d2f06c03 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Thu, 14 Dec 2023 20:46:38 -0800 Subject: [PATCH 22/71] testing --- apps/passport-client/src/dispatch.ts | 16 ++++-- apps/passport-client/src/validateState.ts | 9 +-- .../src/services/rateLimitService.ts | 57 +++++++++---------- 3 files changed, 43 insertions(+), 39 deletions(-) diff --git a/apps/passport-client/src/dispatch.ts b/apps/passport-client/src/dispatch.ts index 066132a3e0..e6e3332c98 100644 --- a/apps/passport-client/src/dispatch.ts +++ b/apps/passport-client/src/dispatch.ts @@ -511,11 +511,6 @@ async function loadAfterLogin( await getPackages() ); - if (!validateAndLogStateErrors(state.self, state.identity, state.pcds)) { - // TODO: what's the right error message here? - throw new Error("saved storage failed to validate"); - } - // Poll the latest user stored from the database rather than using the `self` object from e2ee storage. const userResponse = await requestUser( appConfig.zupassServer, @@ -533,6 +528,17 @@ async function loadAfterLogin( SemaphoreIdentityPCDTypeName )[0] as SemaphoreIdentityPCD; + if ( + !validateAndLogStateErrors( + userResponse.value, + identityPCD.claim.identity, + state.pcds + ) + ) { + update({ userInvalid: true }); + return; + } + let modal: AppState["modal"] = { modalType: "none" }; if (!identityPCD) { // TODO: handle error gracefully diff --git a/apps/passport-client/src/validateState.ts b/apps/passport-client/src/validateState.ts index 096305c576..b790bba804 100644 --- a/apps/passport-client/src/validateState.ts +++ b/apps/passport-client/src/validateState.ts @@ -43,7 +43,7 @@ function validateAppState( ): ValidationErrors { const validationErrors: string[] = []; - const loggedOut = !self && !identity; + const loggedOut = !self; const identityPCDFromCollection = pcds?.getPCDsByType( SemaphoreIdentityPCDPackage.name @@ -76,7 +76,7 @@ function validateAppState( validationErrors.push("missing 'pcds'"); } - if (pcds.size() === 0) { + if (pcds?.size() === 0) { validationErrors.push("'pcds' contains no pcds"); } @@ -107,8 +107,9 @@ function validateAppState( ); } + console.log("VALIDATION ERRORS", validationErrors); return { - errors: validationErrors, + errors: [], userUUID: self?.uuid }; } @@ -121,7 +122,7 @@ function validateAppState( async function logValidationErrors(errors: ValidationErrors): Promise { try { const user = loadSelf(); - errors.userUUID = errors.userUUID ?? user.uuid; + errors.userUUID = errors.userUUID ?? user?.uuid; console.log(`encountered state validation errors: `, errors); await requestLogToServer(appConfig.zupassServer, "state-validation-error", { errors diff --git a/apps/passport-server/src/services/rateLimitService.ts b/apps/passport-server/src/services/rateLimitService.ts index 085a88b724..588a6bc48b 100644 --- a/apps/passport-server/src/services/rateLimitService.ts +++ b/apps/passport-server/src/services/rateLimitService.ts @@ -1,12 +1,7 @@ import { ONE_HOUR_MS } from "@pcd/util"; -import { - checkRateLimit, - clearExpiredActions -} from "../database/queries/rateLimit"; +import { clearExpiredActions } from "../database/queries/rateLimit"; import { ApplicationContext } from "../types"; -import { logger } from "../util/logger"; import { RollbarService } from "./rollbarService"; -import { traced } from "./telemetryService"; export type RateLimitedActionType = "CHECK_EMAIL_TOKEN" | "REQUEST_EMAIL_TOKEN"; @@ -59,33 +54,35 @@ export class RateLimitService { actionType: RateLimitedActionType, actionId: string ): Promise { - return traced( - "RateLimitService", - "requestRateLimitedAction", - async (span) => { - span?.setAttribute("actionType", actionType); - span?.setAttribute("actionId", actionId); + return true; - const result = checkRateLimit( - this.context.dbPool, - actionType, - actionId - ); + // return traced( + // "RateLimitService", + // "requestRateLimitedAction", + // async (span) => { + // span?.setAttribute("actionType", actionType); + // span?.setAttribute("actionId", actionId); - if (!result) { - logger( - `[RATELIMIT] Action "${actionId}" of type "${actionType}" was rate-limited` - ); - this.rollbarService?.reportError( - new Error( - `Action "${actionId}" of type "${actionType}" was rate-limited` - ) - ); - } + // const result = checkRateLimit( + // this.context.dbPool, + // actionType, + // actionId + // ); - return result; - } - ); + // if (!result) { + // logger( + // `[RATELIMIT] Action "${actionId}" of type "${actionType}" was rate-limited` + // ); + // this.rollbarService?.reportError( + // new Error( + // `Action "${actionId}" of type "${actionType}" was rate-limited` + // ) + // ); + // } + + // return result; + // } + // ); } } From c79c4c7017285aae71c43939dc2b15c362184026 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Thu, 14 Dec 2023 20:47:53 -0800 Subject: [PATCH 23/71] undo extra file --- .../pages/loadInitialState.tsx | 75 ------------------- 1 file changed, 75 deletions(-) delete mode 100644 apps/passport-client/pages/loadInitialState.tsx diff --git a/apps/passport-client/pages/loadInitialState.tsx b/apps/passport-client/pages/loadInitialState.tsx deleted file mode 100644 index 0462a54801..0000000000 --- a/apps/passport-client/pages/loadInitialState.tsx +++ /dev/null @@ -1,75 +0,0 @@ -import { createStorageBackedCredentialCache } from "@pcd/passport-interface"; -import { Identity } from "@semaphore-protocol/identity"; -import { - loadCheckedInOfflineDevconnectTickets, - loadEncryptionKey, - loadIdentity, - loadOfflineTickets, - loadPCDs, - loadPersistentSyncStatus, - loadSelf, - loadSubscriptions, - saveIdentity, - saveSubscriptions -} from "../src/localstorage"; -import { AppState } from "../src/state"; -import { validateAndLogStateErrors } from "../src/validateState"; - -// TODO: move to a separate file -export async function loadInitialState(): Promise { - let identity = loadIdentity(); - - if (identity == null) { - console.log("Generating a new Semaphore identity..."); - identity = new Identity(); - saveIdentity(identity); - } - - const self = loadSelf(); - const pcds = await loadPCDs(); - const encryptionKey = loadEncryptionKey(); - const subscriptions = await loadSubscriptions(); - const offlineTickets = loadOfflineTickets(); - const checkedInOfflineDevconnectTickets = - loadCheckedInOfflineDevconnectTickets(); - - subscriptions.updatedEmitter.listen(() => saveSubscriptions(subscriptions)); - - let modal = { modalType: "none" } as AppState["modal"]; - - if ( - // If on Zupass legacy login, ask user to set password - self != null && - encryptionKey == null && - self.salt == null - ) { - console.log("Asking existing user to set a password"); - modal = { modalType: "upgrade-account-modal" }; - } - - const credentialCache = createStorageBackedCredentialCache(); - - const persistentSyncStatus = loadPersistentSyncStatus(); - - const state: AppState = { - self, - encryptionKey, - pcds, - identity, - modal, - subscriptions, - resolvingSubscriptionId: undefined, - credentialCache, - offlineTickets, - checkedinOfflineDevconnectTickets: checkedInOfflineDevconnectTickets, - offline: !window.navigator.onLine, - serverStorageRevision: persistentSyncStatus.serverStorageRevision, - serverStorageHash: persistentSyncStatus.serverStorageHash - }; - - if (!validateAndLogStateErrors(state.self, state.identity, state.pcds)) { - state.userInvalid = true; - } - - return state; -} From 3e30ce2178d6cfab147d25e35468c11dd87e9856 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Thu, 14 Dec 2023 20:49:23 -0800 Subject: [PATCH 24/71] fix bug --- apps/passport-client/src/dispatch.ts | 10 ++-------- apps/passport-client/src/localstorage.ts | 12 +++++------- 2 files changed, 7 insertions(+), 15 deletions(-) diff --git a/apps/passport-client/src/dispatch.ts b/apps/passport-client/src/dispatch.ts index e6e3332c98..5ac4c13788 100644 --- a/apps/passport-client/src/dispatch.ts +++ b/apps/passport-client/src/dispatch.ts @@ -175,12 +175,7 @@ export async function dispatch( case "reset-passport": return resetPassport(state, update); case "load-after-login": - return loadAfterLogin( - state, - action.encryptionKey, - action.storage, - update - ); + return loadAfterLogin(action.encryptionKey, action.storage, update); case "set-modal": return update({ modal: action.modal @@ -501,7 +496,6 @@ async function removePCD(state: AppState, update: ZuUpdate, pcdId: string) { } async function loadAfterLogin( - state: AppState, encryptionKey: string, storage: StorageWithRevision, update: ZuUpdate @@ -532,7 +526,7 @@ async function loadAfterLogin( !validateAndLogStateErrors( userResponse.value, identityPCD.claim.identity, - state.pcds + pcds ) ) { update({ userInvalid: true }); diff --git a/apps/passport-client/src/localstorage.ts b/apps/passport-client/src/localstorage.ts index 19a2bf1097..c666bfd82e 100644 --- a/apps/passport-client/src/localstorage.ts +++ b/apps/passport-client/src/localstorage.ts @@ -44,13 +44,11 @@ export async function loadPCDs(): Promise { serializedCollection ?? "{}" ); - // const validationErrors = validatePCDCollection(collection); - // if (validationErrors.errors.length > 0) { - // logAndUploadValidationErrors(validationErrors); - // throw new Error( - // "couldn't load PCDs\n:" + validationErrors.errors.join("\n") - // ); - // } + if (validateAndLogStateErrors(undefined, undefined, collection, true)) { + console.log( + "PCD Collection failed to validate when loading from localstorage" + ); + } return collection; } From a908239099947c7665769ffa0dd70036e40bfe1e Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Thu, 14 Dec 2023 20:57:57 -0800 Subject: [PATCH 25/71] fix bug --- apps/passport-client/src/localstorage.ts | 4 ++-- apps/passport-client/src/validateState.ts | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/apps/passport-client/src/localstorage.ts b/apps/passport-client/src/localstorage.ts index c666bfd82e..ca11a2205a 100644 --- a/apps/passport-client/src/localstorage.ts +++ b/apps/passport-client/src/localstorage.ts @@ -18,7 +18,7 @@ const OLD_PCDS_KEY = "pcds"; // deprecated const COLLECTION_KEY = "pcd_collection"; export async function savePCDs(pcds: PCDCollection): Promise { - if (validateAndLogStateErrors(undefined, undefined, pcds, true)) { + if (!validateAndLogStateErrors(undefined, undefined, pcds, true)) { console.log( "PCD Collection failed to validate - not writing it to localstorage" ); @@ -44,7 +44,7 @@ export async function loadPCDs(): Promise { serializedCollection ?? "{}" ); - if (validateAndLogStateErrors(undefined, undefined, collection, true)) { + if (!validateAndLogStateErrors(undefined, undefined, collection, true)) { console.log( "PCD Collection failed to validate when loading from localstorage" ); diff --git a/apps/passport-client/src/validateState.ts b/apps/passport-client/src/validateState.ts index b790bba804..f265d25ebd 100644 --- a/apps/passport-client/src/validateState.ts +++ b/apps/passport-client/src/validateState.ts @@ -107,9 +107,8 @@ function validateAppState( ); } - console.log("VALIDATION ERRORS", validationErrors); return { - errors: [], + errors: validationErrors, userUUID: self?.uuid }; } From 296300960805779dbd51ce8d14059c48a8325b51 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Thu, 14 Dec 2023 21:01:38 -0800 Subject: [PATCH 26/71] updated comments --- apps/passport-client/src/validateState.ts | 26 +++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/apps/passport-client/src/validateState.ts b/apps/passport-client/src/validateState.ts index f265d25ebd..c0b0b26bda 100644 --- a/apps/passport-client/src/validateState.ts +++ b/apps/passport-client/src/validateState.ts @@ -8,6 +8,13 @@ import { Identity } from "@semaphore-protocol/identity"; import { appConfig } from "./appConfig"; import { loadSelf } from "./localstorage"; +/** + * Validates the application state using {@link validateAppState}. In the case + * that the state is invalid, returns `true`, and concurrently uploads the validation + * errors to the server for further inspection. + * + * In the case there are no validation errors, returns `false`. + */ export function validateAndLogStateErrors( self: User | undefined, identity: Identity | undefined, @@ -30,10 +37,13 @@ export function validateAndLogStateErrors( } /** - * Determines whether the app's global state as represented by {@link AppState} object - * contains valid data. If it does not, returns the set of things that are incorrect about - * it in an array of human interpretable strings. If there are no errors, returns an - * empty array. + * Determines whether the app's global state contains valid data. If it does not, + * returns the set of things that are incorrect about it in a {@link ValidationErrors} + * object. If there were no validation errors the result will contain an empty array + * of errors. + * + * The provided {@link PCDCollection} is not checked unless either this function + * determines the user is logged in or the {@link forceCheckPCDs} argument is `true`. */ function validateAppState( self: User | undefined, @@ -135,6 +145,14 @@ async function logValidationErrors(errors: ValidationErrors): Promise { * Uploaded to server in case of a state validation error. */ export interface ValidationErrors { + /** + * Human readable non-sensitive-information-leaking errors. If this array is empty, + * it represents a state that has no errors. + */ errors: string[]; + + /** + * Used to identify the user on the server-side. + */ userUUID?: string; } From 5e9e822d477fdcd27e3077305d04096e61826457 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Fri, 15 Dec 2023 15:22:53 -0800 Subject: [PATCH 27/71] extra validation logic in load initial app state --- apps/passport-client/pages/index.tsx | 27 ++++++++++++++++++++++- apps/passport-client/src/validateState.ts | 4 +++- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/apps/passport-client/pages/index.tsx b/apps/passport-client/pages/index.tsx index df79ee4295..e404a56950 100644 --- a/apps/passport-client/pages/index.tsx +++ b/apps/passport-client/pages/index.tsx @@ -66,7 +66,10 @@ import { import { registerServiceWorker } from "../src/registerServiceWorker"; import { AppState, StateEmitter } from "../src/state"; import { pollUser } from "../src/user"; -import { validateAndLogStateErrors } from "../src/validateState"; +import { + logValidationErrors, + validateAndLogStateErrors +} from "../src/validateState"; class App extends React.Component { state = undefined as AppState | undefined; @@ -443,6 +446,28 @@ async function loadInitialState(): Promise { if (!validateAndLogStateErrors(state.self, state.identity, state.pcds)) { state.userInvalid = true; + } else { + const extraValidationErrors = []; + + // this case covers a logged in user. the only way the app can get a 'self' + // is by requesting one from the server, to do which one has to be logged in. + if (state.self) { + if (!state.encryptionKey) { + extraValidationErrors.push(`logged in user missing encryption key`); + } + + if (!state.identity) { + extraValidationErrors.push(`logged in user missing identity`); + } + } + + if (extraValidationErrors.length > 0) { + logValidationErrors({ + errors: extraValidationErrors, + userUUID: self?.uuid + }); + state.userInvalid = true; + } } return state; diff --git a/apps/passport-client/src/validateState.ts b/apps/passport-client/src/validateState.ts index c0b0b26bda..4590b4a35f 100644 --- a/apps/passport-client/src/validateState.ts +++ b/apps/passport-client/src/validateState.ts @@ -128,7 +128,9 @@ function validateAppState( * we have records and are able to identify common types of errors. Does not leak * sensitive information, such as decrypted versions of e2ee storage. */ -async function logValidationErrors(errors: ValidationErrors): Promise { +export async function logValidationErrors( + errors: ValidationErrors +): Promise { try { const user = loadSelf(); errors.userUUID = errors.userUUID ?? user?.uuid; From 6a37eed8b367acbc03fc16944fb435454f016509 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Fri, 15 Dec 2023 15:27:03 -0800 Subject: [PATCH 28/71] early return from sync if userInvalid=true --- apps/passport-client/src/dispatch.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/apps/passport-client/src/dispatch.ts b/apps/passport-client/src/dispatch.ts index 5ac4c13788..749bda3b9a 100644 --- a/apps/passport-client/src/dispatch.ts +++ b/apps/passport-client/src/dispatch.ts @@ -775,6 +775,11 @@ async function doSync( state.subscriptions ); + if (state.userInvalid) { + console.log("[SYNC] userInvalid=true, exiting sync"); + return; + } + if (state.serverStorageHash !== appStorage.storageHash) { if (!validateAndLogStateErrors(state.self, state.identity, state.pcds)) { state.userInvalid = true; From 687e9816eff3a7c32902a79b4e1839a9336e8322 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Fri, 15 Dec 2023 15:31:54 -0800 Subject: [PATCH 29/71] implement suggestion to make validation of state consistent between uploadSerializedStorage and uploadStorage --- apps/passport-client/src/dispatch.ts | 3 ++ .../src/useSyncE2EEStorage.tsx | 36 ++++++++++++------- 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/apps/passport-client/src/dispatch.ts b/apps/passport-client/src/dispatch.ts index 749bda3b9a..2b5a6d9a1b 100644 --- a/apps/passport-client/src/dispatch.ts +++ b/apps/passport-client/src/dispatch.ts @@ -790,6 +790,9 @@ async function doSync( // 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( + state.self, + state.identity, + state.pcds, appStorage.serializedStorage, appStorage.storageHash ); diff --git a/apps/passport-client/src/useSyncE2EEStorage.tsx b/apps/passport-client/src/useSyncE2EEStorage.tsx index 33bac9759f..03aadb06ee 100644 --- a/apps/passport-client/src/useSyncE2EEStorage.tsx +++ b/apps/passport-client/src/useSyncE2EEStorage.tsx @@ -110,32 +110,44 @@ export async function uploadStorage( pcds: PCDCollection, subscriptions: FeedSubscriptionManager ): Promise { - if (!validateAndLogStateErrors(user, userIdentity, pcds)) { - return { - success: false, - error: { - name: "ValidationError", - detailedMessage: "upload validation failed", - code: undefined - } - }; - } const { serializedStorage, storageHash } = await serializeStorage( user, pcds, subscriptions ); - return uploadSerializedStorage(serializedStorage, storageHash); + return uploadSerializedStorage( + user, + userIdentity, + pcds, + serializedStorage, + storageHash + ); } /** * Uploads the state of this passport, in serialized form as produced by - * serializeStorage(). + * serializeStorage(). The parameters {@link user}, {@link userIdentity}, and + * {@link pcds} are used only to validate the consistency between the three + * before attempting an upload, to help prevent uploading inconsistent state. */ export async function uploadSerializedStorage( + user: User, + userIdentity: Identity, + pcds: PCDCollection, serializedStorage: SyncedEncryptedStorage, storageHash: string ): Promise { + if (!validateAndLogStateErrors(user, userIdentity, pcds)) { + return { + success: false, + error: { + name: "ValidationError", + detailedMessage: "upload validation failed", + code: undefined + } + }; + } + const encryptionKey = loadEncryptionKey(); const blobKey = await getHash(encryptionKey); From 35702323e336529f1e569f0856e50e775a8f4478 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Fri, 15 Dec 2023 15:52:03 -0800 Subject: [PATCH 30/71] starting to add test for state validation function --- apps/passport-client/src/validateState.ts | 2 +- .../test/stateValidation.spec.ts | 31 +++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 apps/passport-client/test/stateValidation.spec.ts diff --git a/apps/passport-client/src/validateState.ts b/apps/passport-client/src/validateState.ts index 4590b4a35f..836b901591 100644 --- a/apps/passport-client/src/validateState.ts +++ b/apps/passport-client/src/validateState.ts @@ -45,7 +45,7 @@ export function validateAndLogStateErrors( * The provided {@link PCDCollection} is not checked unless either this function * determines the user is logged in or the {@link forceCheckPCDs} argument is `true`. */ -function validateAppState( +export function validateAppState( self: User | undefined, identity: Identity | undefined, pcds: PCDCollection | undefined, diff --git a/apps/passport-client/test/stateValidation.spec.ts b/apps/passport-client/test/stateValidation.spec.ts new file mode 100644 index 0000000000..cbd22be759 --- /dev/null +++ b/apps/passport-client/test/stateValidation.spec.ts @@ -0,0 +1,31 @@ +import { PCDCrypto } from "@pcd/passport-crypto"; +import { ZupassUserJson } from "@pcd/passport-interface"; +import { PCDCollection } from "@pcd/pcd-collection"; +import { SemaphoreIdentityPCDPackage } from "@pcd/semaphore-identity-pcd"; +import { Identity } from "@semaphore-protocol/identity"; +import { v4 as uuid } from "uuid"; +import { randomEmail } from "../src/util"; +import { validateAppState } from "../src/validateState"; + +describe("validateAppState", async function () { + it("validateState works properly", async function () { + const identity = new Identity(); + const crypto = await PCDCrypto.newInstance(); + const password = "testpassword123!@#asdf"; + const saltAndEncryptionKey = + await crypto.generateSaltAndEncryptionKey(password); + const id = uuid(); + + const self: ZupassUserJson = { + commitment: identity.commitment.toString(), + email: randomEmail(), + salt: saltAndEncryptionKey.salt, + terms_agreed: 1, + uuid: id + }; + const pcdPackages = [SemaphoreIdentityPCDPackage]; + const collection = new PCDCollection(pcdPackages); + + validateAppState(self, identity, collection); + }); +}); From 014a81c2c3fdc30cfc5992c9541f6400cc4238b9 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Fri, 15 Dec 2023 15:52:49 -0800 Subject: [PATCH 31/71] update error message based on pr comment --- apps/passport-client/src/useSyncE2EEStorage.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/passport-client/src/useSyncE2EEStorage.tsx b/apps/passport-client/src/useSyncE2EEStorage.tsx index 03aadb06ee..5ec54edcf3 100644 --- a/apps/passport-client/src/useSyncE2EEStorage.tsx +++ b/apps/passport-client/src/useSyncE2EEStorage.tsx @@ -142,7 +142,7 @@ export async function uploadSerializedStorage( success: false, error: { name: "ValidationError", - detailedMessage: "upload validation failed", + detailedMessage: "validation before upload failed", code: undefined } }; From cebe1b91856e7c7de3c706c0a917525b288a6f2f Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Fri, 15 Dec 2023 15:53:45 -0800 Subject: [PATCH 32/71] update copy in response to feedback --- .../components/modals/InvalidUserModal.tsx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/apps/passport-client/components/modals/InvalidUserModal.tsx b/apps/passport-client/components/modals/InvalidUserModal.tsx index 2c3f468c7e..e70df2d084 100644 --- a/apps/passport-client/components/modals/InvalidUserModal.tsx +++ b/apps/passport-client/components/modals/InvalidUserModal.tsx @@ -17,14 +17,14 @@ export function InvalidUserModal() {

Invalid Zupass

- You've reset your Zupass account on another device, invalidating this - one. Click the button below to log out. Then you'll be able to sync your - existing Zupass account onto this device. + You've reset your Zupass account on another device, or your app has + ended up in an invalid state. Click the button below to log out. Then + you'll be able to sync your existing Zupass account onto this device.

- You can export your account data using the button below in case you need - it later. + You can export a copy of your local account data using the button below + in case you need it later.

From 28aafade89b167d3214032157510dc1461be9411 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 18 Dec 2023 11:05:15 -0800 Subject: [PATCH 33/71] cleaning up test --- .../test/stateValidation.spec.ts | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/apps/passport-client/test/stateValidation.spec.ts b/apps/passport-client/test/stateValidation.spec.ts index cbd22be759..15b34b7c36 100644 --- a/apps/passport-client/test/stateValidation.spec.ts +++ b/apps/passport-client/test/stateValidation.spec.ts @@ -8,24 +8,29 @@ import { randomEmail } from "../src/util"; import { validateAppState } from "../src/validateState"; describe("validateAppState", async function () { + const crypto = await PCDCrypto.newInstance(); + const pcdPackages = [SemaphoreIdentityPCDPackage]; + it("validateState works properly", async function () { const identity = new Identity(); - const crypto = await PCDCrypto.newInstance(); - const password = "testpassword123!@#asdf"; - const saltAndEncryptionKey = - await crypto.generateSaltAndEncryptionKey(password); - const id = uuid(); - + const saltAndEncryptionKey = await crypto.generateSaltAndEncryptionKey( + "testpassword123!@#asdf" + ); const self: ZupassUserJson = { commitment: identity.commitment.toString(), email: randomEmail(), salt: saltAndEncryptionKey.salt, terms_agreed: 1, - uuid: id + uuid: uuid() }; - const pcdPackages = [SemaphoreIdentityPCDPackage]; - const collection = new PCDCollection(pcdPackages); + const pcds = new PCDCollection(pcdPackages); + pcds.add( + await SemaphoreIdentityPCDPackage.prove({ + identity + }) + ); + const errors = validateAppState(self, identity, pcds); - validateAppState(self, identity, collection); + console.log(errors); }); }); From 82eb229208926181560bfdd3aab2b58d22957b2e Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 18 Dec 2023 11:08:08 -0800 Subject: [PATCH 34/71] actually test --- apps/passport-client/package.json | 1 + .../test/stateValidation.spec.ts | 5 +-- yarn.lock | 32 +++++++++++++++++-- 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/apps/passport-client/package.json b/apps/passport-client/package.json index 5f45361384..6704945ff7 100644 --- a/apps/passport-client/package.json +++ b/apps/passport-client/package.json @@ -69,6 +69,7 @@ "@esbuild-plugins/node-globals-polyfill": "^0.2.3", "@esbuild-plugins/node-modules-polyfill": "^0.2.2", "@pcd/eslint-config-custom": "*", + "@types/chai": "^4.3.11", "@types/email-validator": "^1.0.6", "@types/express": "^4.17.17", "@types/mocha": "^10.0.1", diff --git a/apps/passport-client/test/stateValidation.spec.ts b/apps/passport-client/test/stateValidation.spec.ts index 15b34b7c36..f3ea7c49b8 100644 --- a/apps/passport-client/test/stateValidation.spec.ts +++ b/apps/passport-client/test/stateValidation.spec.ts @@ -3,6 +3,7 @@ import { ZupassUserJson } from "@pcd/passport-interface"; import { PCDCollection } from "@pcd/pcd-collection"; import { SemaphoreIdentityPCDPackage } from "@pcd/semaphore-identity-pcd"; import { Identity } from "@semaphore-protocol/identity"; +import { expect } from "chai"; import { v4 as uuid } from "uuid"; import { randomEmail } from "../src/util"; import { validateAppState } from "../src/validateState"; @@ -30,7 +31,7 @@ describe("validateAppState", async function () { }) ); const errors = validateAppState(self, identity, pcds); - - console.log(errors); + expect(errors.errors.length).to.eq(0); + expect(errors.userUUID).to.eq(self.uuid); }); }); diff --git a/yarn.lock b/yarn.lock index baacd97ae5..3d4c69caa4 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4172,6 +4172,11 @@ resolved "https://registry.yarnpkg.com/@types/chai/-/chai-4.3.6.tgz#7b489e8baf393d5dd1266fb203ddd4ea941259e6" integrity sha512-VOVRLM1mBxIRxydiViqPcKn6MIxZytrbMpd6RJLIWKxUNr3zux8no0Oc7kJx0WAPIitgZ0gkrDS+btlqQpubpw== +"@types/chai@^4.3.11": + version "4.3.11" + resolved "https://registry.yarnpkg.com/@types/chai/-/chai-4.3.11.tgz#e95050bf79a932cb7305dd130254ccdf9bde671c" + integrity sha512-qQR1dr2rGIHYlJulmr8Ioq3De0Le9E4MJ5AiaeAETJJpndT1uUNHsGFK3L/UIu+rbkQSdj8J/w2bCsBZc/Y5fQ== + "@types/circomlibjs@0.1.4": version "0.1.4" resolved "https://registry.yarnpkg.com/@types/circomlibjs/-/circomlibjs-0.1.4.tgz#162e6ab09a802c689f304c328e604ae302263811" @@ -4698,7 +4703,7 @@ dependencies: "@types/react" "*" -"@types/react@*", "@types/react@18.0.22", "@types/react@18.2.20", "@types/react@^18.0.22", "@types/react@^18.2.6": +"@types/react@*", "@types/react@^18.0.22", "@types/react@^18.2.6": version "18.2.21" resolved "https://registry.yarnpkg.com/@types/react/-/react-18.2.21.tgz#774c37fd01b522d0b91aed04811b58e4e0514ed9" integrity sha512-neFKG/sBAwGxHgXiIxnbm3/AAVQ/cMRS93hvBpg8xYRbeQSPVABp9U2bRnPf0iI4+Ucdv3plSxKK+3CW2ENJxA== @@ -4707,6 +4712,24 @@ "@types/scheduler" "*" csstype "^3.0.2" +"@types/react@18.0.22": + version "18.0.22" + resolved "https://registry.yarnpkg.com/@types/react/-/react-18.0.22.tgz#97782d995d999617de116cf61f437f1351036fc7" + integrity sha512-4yWc5PyCkZN8ke8K9rQHkTXxHIWHxLzzW6RI1kXVoepkD3vULpKzC2sDtAMKn78h92BRYuzf+7b/ms7ajE6hFw== + dependencies: + "@types/prop-types" "*" + "@types/scheduler" "*" + csstype "^3.0.2" + +"@types/react@18.2.20": + version "18.2.20" + resolved "https://registry.yarnpkg.com/@types/react/-/react-18.2.20.tgz#1605557a83df5c8a2cc4eeb743b3dfc0eb6aaeb2" + integrity sha512-WKNtmsLWJM/3D5mG4U84cysVY31ivmyw85dE84fOCk5Hx78wezB/XEjVPWl2JTZ5FkEeaTJf+VgUAUn3PE7Isw== + dependencies: + "@types/prop-types" "*" + "@types/scheduler" "*" + csstype "^3.0.2" + "@types/request@^2.48.8": version "2.48.8" resolved "https://registry.yarnpkg.com/@types/request/-/request-2.48.8.tgz#0b90fde3b655ab50976cb8c5ac00faca22f5a82c" @@ -15465,7 +15488,12 @@ typedoc@^0.25.1: minimatch "^9.0.3" shiki "^0.14.1" -typescript@5.1.6, typescript@^4.5.2, typescript@^4.9.5: +typescript@5.1.6: + version "5.1.6" + resolved "https://registry.yarnpkg.com/typescript/-/typescript-5.1.6.tgz#02f8ac202b6dad2c0dd5e0913745b47a37998274" + integrity sha512-zaWCozRZ6DLEWAWFrVDz1H6FVXzUSfTy5FUMWsQlU8Ym5JP9eO4xkTIROFCQvhQf61z6O/G6ugw3SgAnvvm+HA== + +typescript@^4.5.2, typescript@^4.9.5: version "4.9.5" resolved "https://registry.yarnpkg.com/typescript/-/typescript-4.9.5.tgz#095979f9bcc0d09da324d58d03ce8f8374cbe65a" integrity sha512-1FXk9E2Hm+QzZQ7z+McJiHL4NW1F2EzMu9Nq9i3zAaGqibafqYwCVU6WyWAuyQRRzOlxou8xZSyXLEN8oKj24g== From 21d0bfb86751d492cbd2a8a0006e95c83340a39f Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 18 Dec 2023 11:09:41 -0800 Subject: [PATCH 35/71] test logged out state --- apps/passport-client/test/stateValidation.spec.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/apps/passport-client/test/stateValidation.spec.ts b/apps/passport-client/test/stateValidation.spec.ts index f3ea7c49b8..3f4dce4b17 100644 --- a/apps/passport-client/test/stateValidation.spec.ts +++ b/apps/passport-client/test/stateValidation.spec.ts @@ -12,7 +12,7 @@ describe("validateAppState", async function () { const crypto = await PCDCrypto.newInstance(); const pcdPackages = [SemaphoreIdentityPCDPackage]; - it("validateState works properly", async function () { + it("validateState returns no errors on valid logged in state", async function () { const identity = new Identity(); const saltAndEncryptionKey = await crypto.generateSaltAndEncryptionKey( "testpassword123!@#asdf" @@ -34,4 +34,10 @@ describe("validateAppState", async function () { expect(errors.errors.length).to.eq(0); expect(errors.userUUID).to.eq(self.uuid); }); + + it("validateState returns no errors on valid logged out state", async function () { + const errors = validateAppState(undefined, undefined, undefined); + expect(errors.errors.length).to.eq(0); + expect(errors.userUUID).to.eq(undefined); + }); }); From f5331b6161aa1fc00cebb85f3525cef3ca321e3f Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 18 Dec 2023 11:10:39 -0800 Subject: [PATCH 36/71] more validation test cases --- .../test/stateValidation.spec.ts | 25 ++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/apps/passport-client/test/stateValidation.spec.ts b/apps/passport-client/test/stateValidation.spec.ts index 3f4dce4b17..a18f6fb07d 100644 --- a/apps/passport-client/test/stateValidation.spec.ts +++ b/apps/passport-client/test/stateValidation.spec.ts @@ -12,6 +12,12 @@ describe("validateAppState", async function () { const crypto = await PCDCrypto.newInstance(); const pcdPackages = [SemaphoreIdentityPCDPackage]; + it("validateState returns no errors on valid logged out state", async function () { + const errors = validateAppState(undefined, undefined, undefined); + expect(errors.errors.length).to.eq(0); + expect(errors.userUUID).to.eq(undefined); + }); + it("validateState returns no errors on valid logged in state", async function () { const identity = new Identity(); const saltAndEncryptionKey = await crypto.generateSaltAndEncryptionKey( @@ -35,9 +41,22 @@ describe("validateAppState", async function () { expect(errors.userUUID).to.eq(self.uuid); }); - it("validateState returns no errors on valid logged out state", async function () { - const errors = validateAppState(undefined, undefined, undefined); + it("validateState returns no errors on valid logged in state", async function () { + const identity = new Identity(); + const saltAndEncryptionKey = await crypto.generateSaltAndEncryptionKey( + "testpassword123!@#asdf" + ); + const self: ZupassUserJson = { + commitment: identity.commitment.toString(), + email: randomEmail(), + salt: saltAndEncryptionKey.salt, + terms_agreed: 1, + uuid: uuid() + }; + const pcds = new PCDCollection(pcdPackages); + // deliberately create empty pcd collection + const errors = validateAppState(self, identity, pcds); expect(errors.errors.length).to.eq(0); - expect(errors.userUUID).to.eq(undefined); + expect(errors.userUUID).to.eq(self.uuid); }); }); From 06b19696f244edcbee8b37ef4c89304f8dcb0f61 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 18 Dec 2023 11:14:14 -0800 Subject: [PATCH 37/71] moved state validation out of sync and into uploadSerializedStorage --- apps/passport-client/src/dispatch.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/apps/passport-client/src/dispatch.ts b/apps/passport-client/src/dispatch.ts index 2b5a6d9a1b..f22570e165 100644 --- a/apps/passport-client/src/dispatch.ts +++ b/apps/passport-client/src/dispatch.ts @@ -781,11 +781,6 @@ async function doSync( } if (state.serverStorageHash !== appStorage.storageHash) { - if (!validateAndLogStateErrors(state.self, state.identity, state.pcds)) { - state.userInvalid = true; - return; - } - 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. @@ -802,9 +797,15 @@ async function doSync( serverStorageHash: upRes.value.storageHash }; } else { - return { + const res: Partial = { completedFirstSync: true }; + + if (upRes.error.name === "ValidationError") { + res.userInvalid = true; + } + + return res; } } From b858d544109812c97a08dc06d065b14893164de6 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 18 Dec 2023 11:17:46 -0800 Subject: [PATCH 38/71] moved userinvalid check to top of dosync --- apps/passport-client/src/dispatch.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/apps/passport-client/src/dispatch.ts b/apps/passport-client/src/dispatch.ts index f22570e165..9a4e69d71c 100644 --- a/apps/passport-client/src/dispatch.ts +++ b/apps/passport-client/src/dispatch.ts @@ -691,6 +691,10 @@ async function doSync( console.log("[SYNC] no encryption key, can't sync"); return undefined; } + if (state.userInvalid) { + console.log("[SYNC] userInvalid=true, exiting sync"); + return undefined; + } // If we haven't downloaded from storage, do that first. After that we'll // download again when requested to poll, but only after the first full sync @@ -775,11 +779,6 @@ async function doSync( state.subscriptions ); - if (state.userInvalid) { - console.log("[SYNC] userInvalid=true, exiting sync"); - return; - } - if (state.serverStorageHash !== appStorage.storageHash) { console.log("[SYNC] sync action: upload"); // TODO(artwyman): Add serverStorageRevision input as knownRevision here, From a463370147927c4c21f060c2eeb1d43626aaf0ed Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 18 Dec 2023 11:24:15 -0800 Subject: [PATCH 39/71] move pcd collection logic to top of validation function --- apps/passport-client/src/validateState.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/apps/passport-client/src/validateState.ts b/apps/passport-client/src/validateState.ts index 836b901591..b6f0f4a961 100644 --- a/apps/passport-client/src/validateState.ts +++ b/apps/passport-client/src/validateState.ts @@ -60,6 +60,14 @@ export function validateAppState( )?.[0] as SemaphoreIdentityPCD | undefined; if (forceCheckPCDs || !loggedOut) { + if (!pcds) { + validationErrors.push("missing 'pcds'"); + } + + if (pcds?.size() === 0) { + validationErrors.push("'pcds' contains no pcds"); + } + if (!identityPCDFromCollection) { validationErrors.push( "'pcds' field in app state does not contain an identity PCD" @@ -82,14 +90,6 @@ export function validateAppState( validationErrors.push("missing 'identity'"); } - if (!pcds) { - validationErrors.push("missing 'pcds'"); - } - - if (pcds?.size() === 0) { - validationErrors.push("'pcds' contains no pcds"); - } - const identityFromPCDCollection = identityPCDFromCollection?.claim?.identity; const commitmentOfIdentityPCDInCollection = identityFromPCDCollection?.commitment?.toString(); From 8f21814ac794b6b2ed0dac73e5340474be8dc6b3 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 18 Dec 2023 11:25:45 -0800 Subject: [PATCH 40/71] add validationTag to validateState function --- apps/passport-client/pages/index.tsx | 9 ++++++++- apps/passport-client/src/dispatch.ts | 10 +++++++++- apps/passport-client/src/localstorage.ts | 14 ++++++++++++-- .../src/useSyncE2EEStorage.tsx | 19 +++++++++++++++++-- apps/passport-client/src/validateState.ts | 6 ++++++ .../test/stateValidation.spec.ts | 6 +++--- 6 files changed, 55 insertions(+), 9 deletions(-) diff --git a/apps/passport-client/pages/index.tsx b/apps/passport-client/pages/index.tsx index e404a56950..31ab26aaa7 100644 --- a/apps/passport-client/pages/index.tsx +++ b/apps/passport-client/pages/index.tsx @@ -444,7 +444,14 @@ async function loadInitialState(): Promise { serverStorageHash: persistentSyncStatus.serverStorageHash }; - if (!validateAndLogStateErrors(state.self, state.identity, state.pcds)) { + if ( + !validateAndLogStateErrors( + "loadInitialState", + state.self, + state.identity, + state.pcds + ) + ) { state.userInvalid = true; } else { const extraValidationErrors = []; diff --git a/apps/passport-client/src/dispatch.ts b/apps/passport-client/src/dispatch.ts index 9a4e69d71c..080686ad9a 100644 --- a/apps/passport-client/src/dispatch.ts +++ b/apps/passport-client/src/dispatch.ts @@ -343,7 +343,14 @@ async function finishAccountCreation( update: ZuUpdate ) { // Verify that the identity is correct. - if (!validateAndLogStateErrors(user, state.identity, state.pcds)) { + if ( + !validateAndLogStateErrors( + "finishAccountCreation", + user, + state.identity, + state.pcds + ) + ) { update({ error: { title: "Invalid identity", @@ -524,6 +531,7 @@ async function loadAfterLogin( if ( !validateAndLogStateErrors( + "loadAfterLogin", userResponse.value, identityPCD.claim.identity, pcds diff --git a/apps/passport-client/src/localstorage.ts b/apps/passport-client/src/localstorage.ts index ca11a2205a..28a88a5154 100644 --- a/apps/passport-client/src/localstorage.ts +++ b/apps/passport-client/src/localstorage.ts @@ -18,7 +18,9 @@ const OLD_PCDS_KEY = "pcds"; // deprecated const COLLECTION_KEY = "pcd_collection"; export async function savePCDs(pcds: PCDCollection): Promise { - if (!validateAndLogStateErrors(undefined, undefined, pcds, true)) { + if ( + !validateAndLogStateErrors("savePCDs", undefined, undefined, pcds, true) + ) { console.log( "PCD Collection failed to validate - not writing it to localstorage" ); @@ -44,7 +46,15 @@ export async function loadPCDs(): Promise { serializedCollection ?? "{}" ); - if (!validateAndLogStateErrors(undefined, undefined, collection, true)) { + if ( + !validateAndLogStateErrors( + "loadPCDs", + undefined, + undefined, + collection, + true + ) + ) { console.log( "PCD Collection failed to validate when loading from localstorage" ); diff --git a/apps/passport-client/src/useSyncE2EEStorage.tsx b/apps/passport-client/src/useSyncE2EEStorage.tsx index 5ec54edcf3..ad95dfd2da 100644 --- a/apps/passport-client/src/useSyncE2EEStorage.tsx +++ b/apps/passport-client/src/useSyncE2EEStorage.tsx @@ -137,7 +137,14 @@ export async function uploadSerializedStorage( serializedStorage: SyncedEncryptedStorage, storageHash: string ): Promise { - if (!validateAndLogStateErrors(user, userIdentity, pcds)) { + if ( + !validateAndLogStateErrors( + "uploadSerializedStorage", + user, + userIdentity, + pcds + ) + ) { return { success: false, error: { @@ -229,7 +236,15 @@ export async function downloadStorage( await getPackages() ); - if (!validateAndLogStateErrors(undefined, undefined, pcds, true)) { + if ( + !validateAndLogStateErrors( + "downloadStorage", + undefined, + undefined, + pcds, + true + ) + ) { throw new Error("downloaded e2ee state failed to validate"); } diff --git a/apps/passport-client/src/validateState.ts b/apps/passport-client/src/validateState.ts index b6f0f4a961..bb4deb7f91 100644 --- a/apps/passport-client/src/validateState.ts +++ b/apps/passport-client/src/validateState.ts @@ -16,12 +16,14 @@ import { loadSelf } from "./localstorage"; * In the case there are no validation errors, returns `false`. */ export function validateAndLogStateErrors( + validationTag: string, self: User | undefined, identity: Identity | undefined, pcds: PCDCollection | undefined, forceCheckPCDs?: boolean ): boolean { const validationErrors = validateAppState( + validationTag, self, identity, pcds, @@ -44,8 +46,12 @@ export function validateAndLogStateErrors( * * The provided {@link PCDCollection} is not checked unless either this function * determines the user is logged in or the {@link forceCheckPCDs} argument is `true`. + * + * Depending on where this function is called, pass in a unique {@link validationTag}, so + * that on the backend we can figure out where the validation failed. */ export function validateAppState( + validationTag: string, self: User | undefined, identity: Identity | undefined, pcds: PCDCollection | undefined, diff --git a/apps/passport-client/test/stateValidation.spec.ts b/apps/passport-client/test/stateValidation.spec.ts index a18f6fb07d..6b476d6dfe 100644 --- a/apps/passport-client/test/stateValidation.spec.ts +++ b/apps/passport-client/test/stateValidation.spec.ts @@ -13,7 +13,7 @@ describe("validateAppState", async function () { const pcdPackages = [SemaphoreIdentityPCDPackage]; it("validateState returns no errors on valid logged out state", async function () { - const errors = validateAppState(undefined, undefined, undefined); + const errors = validateAppState("test", undefined, undefined, undefined); expect(errors.errors.length).to.eq(0); expect(errors.userUUID).to.eq(undefined); }); @@ -36,7 +36,7 @@ describe("validateAppState", async function () { identity }) ); - const errors = validateAppState(self, identity, pcds); + const errors = validateAppState("test", self, identity, pcds); expect(errors.errors.length).to.eq(0); expect(errors.userUUID).to.eq(self.uuid); }); @@ -55,7 +55,7 @@ describe("validateAppState", async function () { }; const pcds = new PCDCollection(pcdPackages); // deliberately create empty pcd collection - const errors = validateAppState(self, identity, pcds); + const errors = validateAppState("test", self, identity, pcds); expect(errors.errors.length).to.eq(0); expect(errors.userUUID).to.eq(self.uuid); }); From 00e5e496341ea0423a853dc41b57fcf55f4b9e37 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 18 Dec 2023 11:30:17 -0800 Subject: [PATCH 41/71] console.log -> console.error --- apps/passport-client/src/localstorage.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/passport-client/src/localstorage.ts b/apps/passport-client/src/localstorage.ts index 28a88a5154..8102f304ef 100644 --- a/apps/passport-client/src/localstorage.ts +++ b/apps/passport-client/src/localstorage.ts @@ -21,7 +21,7 @@ export async function savePCDs(pcds: PCDCollection): Promise { if ( !validateAndLogStateErrors("savePCDs", undefined, undefined, pcds, true) ) { - console.log( + console.error( "PCD Collection failed to validate - not writing it to localstorage" ); return; @@ -55,7 +55,7 @@ export async function loadPCDs(): Promise { true ) ) { - console.log( + console.error( "PCD Collection failed to validate when loading from localstorage" ); } From c527bb0bc719b517122b435dfa2c924aa54c9d6a Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 18 Dec 2023 11:32:28 -0800 Subject: [PATCH 42/71] unwrap errors object when logging to server --- apps/passport-client/src/validateState.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/passport-client/src/validateState.ts b/apps/passport-client/src/validateState.ts index bb4deb7f91..4e5c3227fb 100644 --- a/apps/passport-client/src/validateState.ts +++ b/apps/passport-client/src/validateState.ts @@ -142,7 +142,7 @@ export async function logValidationErrors( errors.userUUID = errors.userUUID ?? user?.uuid; console.log(`encountered state validation errors: `, errors); await requestLogToServer(appConfig.zupassServer, "state-validation-error", { - errors + ...errors }); } catch (e) { console.log("error reporting errors", e); From 44eb78657ce4ba23e54225beb6e752d75d6ea75e Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 18 Dec 2023 11:42:31 -0800 Subject: [PATCH 43/71] update validateState to be less confusing for cases when something is undefined --- apps/passport-client/src/validateState.ts | 47 +++++++++++++++-------- 1 file changed, 31 insertions(+), 16 deletions(-) diff --git a/apps/passport-client/src/validateState.ts b/apps/passport-client/src/validateState.ts index 4e5c3227fb..67a527b664 100644 --- a/apps/passport-client/src/validateState.ts +++ b/apps/passport-client/src/validateState.ts @@ -102,25 +102,40 @@ export function validateAppState( const commitmentFromSelfField = self?.commitment; const commitmentFromIdentityField = identity?.commitment?.toString(); - if (commitmentOfIdentityPCDInCollection !== commitmentFromSelfField) { - validationErrors.push( - `commitment of identity pcd in collection (${commitmentOfIdentityPCDInCollection})` + - ` does not match commitment in 'self' field of app state (${commitmentFromSelfField})` - ); + if (commitmentFromSelfField === undefined) { + validationErrors.push(`'self' missing a commitment`); } - if (commitmentFromSelfField !== commitmentFromIdentityField) { - validationErrors.push( - `commitment in 'self' field of app state (${commitmentFromSelfField})` + - ` does not match commitment of 'identity' field of app state (${commitmentFromIdentityField})` - ); - } + if ( + commitmentFromSelfField === undefined || + commitmentOfIdentityPCDInCollection === undefined || + commitmentFromIdentityField === undefined + ) { + // these cases are validated earlier in this function + } else { + // in 'else' block we check that the commitments from all three + // places that the user's commitment exists match - in the self, the + // identity, and in the pcd collection + + if (commitmentOfIdentityPCDInCollection !== commitmentFromSelfField) { + validationErrors.push( + `commitment of identity pcd in collection (${commitmentOfIdentityPCDInCollection})` + + ` does not match commitment in 'self' field of app state (${commitmentFromSelfField})` + ); + } + if (commitmentFromSelfField !== commitmentFromIdentityField) { + validationErrors.push( + `commitment in 'self' field of app state (${commitmentFromSelfField})` + + ` does not match commitment of 'identity' field of app state (${commitmentFromIdentityField})` + ); + } - if (commitmentFromIdentityField !== commitmentOfIdentityPCDInCollection) { - validationErrors.push( - `commitment of 'identity' field of app state (${commitmentFromIdentityField})` + - ` does not match commitment of identity pcd in collection (${commitmentOfIdentityPCDInCollection})` - ); + if (commitmentFromIdentityField !== commitmentOfIdentityPCDInCollection) { + validationErrors.push( + `commitment of 'identity' field of app state (${commitmentFromIdentityField})` + + ` does not match commitment of identity pcd in collection (${commitmentOfIdentityPCDInCollection})` + ); + } } return { From 215487be2be7959012fa205d848eb48b1cb440d6 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 18 Dec 2023 11:44:52 -0800 Subject: [PATCH 44/71] fix test --- apps/passport-client/src/validateState.ts | 2 -- apps/passport-client/test/stateValidation.spec.ts | 7 +++++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/apps/passport-client/src/validateState.ts b/apps/passport-client/src/validateState.ts index 67a527b664..7b055ea238 100644 --- a/apps/passport-client/src/validateState.ts +++ b/apps/passport-client/src/validateState.ts @@ -58,9 +58,7 @@ export function validateAppState( forceCheckPCDs?: boolean ): ValidationErrors { const validationErrors: string[] = []; - const loggedOut = !self; - const identityPCDFromCollection = pcds?.getPCDsByType( SemaphoreIdentityPCDPackage.name )?.[0] as SemaphoreIdentityPCD | undefined; diff --git a/apps/passport-client/test/stateValidation.spec.ts b/apps/passport-client/test/stateValidation.spec.ts index 6b476d6dfe..fdaba4bdc6 100644 --- a/apps/passport-client/test/stateValidation.spec.ts +++ b/apps/passport-client/test/stateValidation.spec.ts @@ -41,7 +41,7 @@ describe("validateAppState", async function () { expect(errors.userUUID).to.eq(self.uuid); }); - it("validateState returns no errors on valid logged in state", async function () { + it("validateState returns errors for situation where the pcd collection is empty", async function () { const identity = new Identity(); const saltAndEncryptionKey = await crypto.generateSaltAndEncryptionKey( "testpassword123!@#asdf" @@ -56,7 +56,10 @@ describe("validateAppState", async function () { const pcds = new PCDCollection(pcdPackages); // deliberately create empty pcd collection const errors = validateAppState("test", self, identity, pcds); - expect(errors.errors.length).to.eq(0); + expect(errors.errors).to.deep.eq([ + "'pcds' contains no pcds", + "'pcds' field in app state does not contain an identity PCD" + ]); expect(errors.userUUID).to.eq(self.uuid); }); }); From 6fab637fee2e9867273dd740e41bb86c167d5293 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 18 Dec 2023 11:50:53 -0800 Subject: [PATCH 45/71] testing logged out state --- .../test/stateValidation.spec.ts | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/apps/passport-client/test/stateValidation.spec.ts b/apps/passport-client/test/stateValidation.spec.ts index fdaba4bdc6..2933bda516 100644 --- a/apps/passport-client/test/stateValidation.spec.ts +++ b/apps/passport-client/test/stateValidation.spec.ts @@ -6,7 +6,7 @@ import { Identity } from "@semaphore-protocol/identity"; import { expect } from "chai"; import { v4 as uuid } from "uuid"; import { randomEmail } from "../src/util"; -import { validateAppState } from "../src/validateState"; +import { ValidationErrors, validateAppState } from "../src/validateState"; describe("validateAppState", async function () { const crypto = await PCDCrypto.newInstance(); @@ -18,6 +18,26 @@ describe("validateAppState", async function () { expect(errors.userUUID).to.eq(undefined); }); + it("validateState returns errors for logged out state with invalid PCD collections", async function () { + expect( + validateAppState( + "test", + undefined, + undefined, + (() => { + return new PCDCollection(pcdPackages); + })(), + true + ) + ).to.deep.eq({ + errors: [ + "'pcds' contains no pcds", + "'pcds' field in app state does not contain an identity PCD" + ], + userUUID: undefined + } satisfies ValidationErrors); + }); + it("validateState returns no errors on valid logged in state", async function () { const identity = new Identity(); const saltAndEncryptionKey = await crypto.generateSaltAndEncryptionKey( From 20716440214c250b13100744ab0c646e3bba19ad Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 18 Dec 2023 11:59:09 -0800 Subject: [PATCH 46/71] one more test case --- .../test/stateValidation.spec.ts | 50 ++++++++++++++++++- 1 file changed, 48 insertions(+), 2 deletions(-) diff --git a/apps/passport-client/test/stateValidation.spec.ts b/apps/passport-client/test/stateValidation.spec.ts index 2933bda516..432524d432 100644 --- a/apps/passport-client/test/stateValidation.spec.ts +++ b/apps/passport-client/test/stateValidation.spec.ts @@ -1,6 +1,8 @@ +import { EdDSAPCD, EdDSAPCDPackage } from "@pcd/eddsa-pcd"; import { PCDCrypto } from "@pcd/passport-crypto"; import { ZupassUserJson } from "@pcd/passport-interface"; import { PCDCollection } from "@pcd/pcd-collection"; +import { ArgumentTypeName } from "@pcd/pcd-types"; import { SemaphoreIdentityPCDPackage } from "@pcd/semaphore-identity-pcd"; import { Identity } from "@semaphore-protocol/identity"; import { expect } from "chai"; @@ -8,9 +10,26 @@ import { v4 as uuid } from "uuid"; import { randomEmail } from "../src/util"; import { ValidationErrors, validateAppState } from "../src/validateState"; +function newEdSAPCD(): Promise { + return EdDSAPCDPackage.prove({ + message: { + value: ["0x12345", "0x54321", "0xdeadbeef"], + argumentType: ArgumentTypeName.StringArray + }, + privateKey: { + value: "0001020304050607080900010203040506070809000102030405060708090001", + argumentType: ArgumentTypeName.String + }, + id: { + value: undefined, + argumentType: ArgumentTypeName.String + } + }); +} + describe("validateAppState", async function () { const crypto = await PCDCrypto.newInstance(); - const pcdPackages = [SemaphoreIdentityPCDPackage]; + const pcdPackages = [SemaphoreIdentityPCDPackage, EdDSAPCDPackage]; it("validateState returns no errors on valid logged out state", async function () { const errors = validateAppState("test", undefined, undefined, undefined); @@ -24,7 +43,7 @@ describe("validateAppState", async function () { "test", undefined, undefined, - (() => { + await (async () => { return new PCDCollection(pcdPackages); })(), true @@ -36,6 +55,33 @@ describe("validateAppState", async function () { ], userUUID: undefined } satisfies ValidationErrors); + + expect( + validateAppState( + "test", + undefined, + undefined, + await (async () => { + const collection = new PCDCollection(pcdPackages); + collection.add(await newEdSAPCD()); + return collection; + })(), + true + ) + ).to.deep.eq({ + errors: ["'pcds' field in app state does not contain an identity PCD"], + userUUID: undefined + } satisfies ValidationErrors); + + expect( + validateAppState("test", undefined, undefined, undefined, true) + ).to.deep.eq({ + errors: [ + "'pcds' field in app state does not contain an identity PCD", + "missing 'pcds'" + ], + userUUID: undefined + } satisfies ValidationErrors); }); it("validateState returns no errors on valid logged in state", async function () { From 5b01b79a5f28c908d7c372a5ddd2fc4caef543b7 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 18 Dec 2023 12:08:20 -0800 Subject: [PATCH 47/71] cleanup --- .../test/stateValidation.spec.ts | 54 +++++++++++++------ 1 file changed, 38 insertions(+), 16 deletions(-) diff --git a/apps/passport-client/test/stateValidation.spec.ts b/apps/passport-client/test/stateValidation.spec.ts index 432524d432..e124b4b9e7 100644 --- a/apps/passport-client/test/stateValidation.spec.ts +++ b/apps/passport-client/test/stateValidation.spec.ts @@ -31,13 +31,13 @@ describe("validateAppState", async function () { const crypto = await PCDCrypto.newInstance(); const pcdPackages = [SemaphoreIdentityPCDPackage, EdDSAPCDPackage]; - it("validateState returns no errors on valid logged out state", async function () { + it("logged out ; no errors", async function () { const errors = validateAppState("test", undefined, undefined, undefined); expect(errors.errors.length).to.eq(0); expect(errors.userUUID).to.eq(undefined); }); - it("validateState returns errors for logged out state with invalid PCD collections", async function () { + it("logged out ; forceCheckPCDs=true; all error states caught", async function () { expect( validateAppState( "test", @@ -77,14 +77,14 @@ describe("validateAppState", async function () { validateAppState("test", undefined, undefined, undefined, true) ).to.deep.eq({ errors: [ - "'pcds' field in app state does not contain an identity PCD", - "missing 'pcds'" + "missing 'pcds'", + "'pcds' field in app state does not contain an identity PCD" ], userUUID: undefined } satisfies ValidationErrors); }); - it("validateState returns no errors on valid logged in state", async function () { + it("logged in ; no errors", async function () { const identity = new Identity(); const saltAndEncryptionKey = await crypto.generateSaltAndEncryptionKey( "testpassword123!@#asdf" @@ -102,12 +102,13 @@ describe("validateAppState", async function () { identity }) ); - const errors = validateAppState("test", self, identity, pcds); - expect(errors.errors.length).to.eq(0); - expect(errors.userUUID).to.eq(self.uuid); + expect(validateAppState("test", self, identity, pcds)).to.deep.eq({ + userUUID: self.uuid, + errors: [] + } satisfies ValidationErrors); }); - it("validateState returns errors for situation where the pcd collection is empty", async function () { + it("logged in ; empty pcd collection ; errors", async function () { const identity = new Identity(); const saltAndEncryptionKey = await crypto.generateSaltAndEncryptionKey( "testpassword123!@#asdf" @@ -120,12 +121,33 @@ describe("validateAppState", async function () { uuid: uuid() }; const pcds = new PCDCollection(pcdPackages); - // deliberately create empty pcd collection - const errors = validateAppState("test", self, identity, pcds); - expect(errors.errors).to.deep.eq([ - "'pcds' contains no pcds", - "'pcds' field in app state does not contain an identity PCD" - ]); - expect(errors.userUUID).to.eq(self.uuid); + expect(validateAppState("test", self, identity, pcds)).to.deep.eq({ + userUUID: self.uuid, + errors: [ + "'pcds' contains no pcds", + "'pcds' field in app state does not contain an identity PCD" + ] + } satisfies ValidationErrors); + }); + + it("logged in ; missing pcd collection ; errors", async function () { + const identity = new Identity(); + const saltAndEncryptionKey = await crypto.generateSaltAndEncryptionKey( + "testpassword123!@#asdf" + ); + const self: ZupassUserJson = { + commitment: identity.commitment.toString(), + email: randomEmail(), + salt: saltAndEncryptionKey.salt, + terms_agreed: 1, + uuid: uuid() + }; + expect(validateAppState("test", self, identity, undefined)).to.deep.eq({ + userUUID: self.uuid, + errors: [ + "missing 'pcds'", + "'pcds' field in app state does not contain an identity PCD" + ] + } satisfies ValidationErrors); }); }); From e63a5f1d67e0432672d34d70ca594b9f3e2f4a5a Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 18 Dec 2023 12:11:56 -0800 Subject: [PATCH 48/71] add case for missing identity --- apps/passport-client/src/validateState.ts | 4 ---- .../test/stateValidation.spec.ts | 24 +++++++++++++++++++ 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/apps/passport-client/src/validateState.ts b/apps/passport-client/src/validateState.ts index 7b055ea238..e908c6d22b 100644 --- a/apps/passport-client/src/validateState.ts +++ b/apps/passport-client/src/validateState.ts @@ -86,10 +86,6 @@ export function validateAppState( }; } - if (!self) { - validationErrors.push("missing 'self'"); - } - if (!identity) { validationErrors.push("missing 'identity'"); } diff --git a/apps/passport-client/test/stateValidation.spec.ts b/apps/passport-client/test/stateValidation.spec.ts index e124b4b9e7..031b7a7f4b 100644 --- a/apps/passport-client/test/stateValidation.spec.ts +++ b/apps/passport-client/test/stateValidation.spec.ts @@ -150,4 +150,28 @@ describe("validateAppState", async function () { ] } satisfies ValidationErrors); }); + + it("logged in ; missing identity ; errors", async function () { + const identity = new Identity(); + const saltAndEncryptionKey = await crypto.generateSaltAndEncryptionKey( + "testpassword123!@#asdf" + ); + const self: ZupassUserJson = { + commitment: identity.commitment.toString(), + email: randomEmail(), + salt: saltAndEncryptionKey.salt, + terms_agreed: 1, + uuid: uuid() + }; + const pcds = new PCDCollection(pcdPackages); + pcds.add( + await SemaphoreIdentityPCDPackage.prove({ + identity + }) + ); + expect(validateAppState("test", self, undefined, pcds)).to.deep.eq({ + userUUID: self.uuid, + errors: ["missing 'identity'"] + } satisfies ValidationErrors); + }); }); From 430e506458c5e761a0e6148a502c3de83dc22a13 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 18 Dec 2023 12:17:02 -0800 Subject: [PATCH 49/71] actually propagate log tag --- apps/passport-client/pages/index.tsx | 1 + apps/passport-client/src/validateState.ts | 30 ++++++---- .../test/stateValidation.spec.ts | 56 +++++++++++-------- 3 files changed, 52 insertions(+), 35 deletions(-) diff --git a/apps/passport-client/pages/index.tsx b/apps/passport-client/pages/index.tsx index 31ab26aaa7..efce8fd3cb 100644 --- a/apps/passport-client/pages/index.tsx +++ b/apps/passport-client/pages/index.tsx @@ -470,6 +470,7 @@ async function loadInitialState(): Promise { if (extraValidationErrors.length > 0) { logValidationErrors({ + tag: "afterLoadInitialState", errors: extraValidationErrors, userUUID: self?.uuid }); diff --git a/apps/passport-client/src/validateState.ts b/apps/passport-client/src/validateState.ts index e908c6d22b..973b55d575 100644 --- a/apps/passport-client/src/validateState.ts +++ b/apps/passport-client/src/validateState.ts @@ -16,14 +16,14 @@ import { loadSelf } from "./localstorage"; * In the case there are no validation errors, returns `false`. */ export function validateAndLogStateErrors( - validationTag: string, + tag: string, self: User | undefined, identity: Identity | undefined, pcds: PCDCollection | undefined, forceCheckPCDs?: boolean ): boolean { const validationErrors = validateAppState( - validationTag, + tag, self, identity, pcds, @@ -40,23 +40,23 @@ export function validateAndLogStateErrors( /** * Determines whether the app's global state contains valid data. If it does not, - * returns the set of things that are incorrect about it in a {@link ValidationErrors} + * returns the set of things that are incorrect about it in a {@link ErrorReport} * object. If there were no validation errors the result will contain an empty array * of errors. * * The provided {@link PCDCollection} is not checked unless either this function * determines the user is logged in or the {@link forceCheckPCDs} argument is `true`. * - * Depending on where this function is called, pass in a unique {@link validationTag}, so + * Depending on where this function is called, pass in a unique {@link tag}, so * that on the backend we can figure out where the validation failed. */ export function validateAppState( - validationTag: string, + tag: string, self: User | undefined, identity: Identity | undefined, pcds: PCDCollection | undefined, forceCheckPCDs?: boolean -): ValidationErrors { +): ErrorReport { const validationErrors: string[] = []; const loggedOut = !self; const identityPCDFromCollection = pcds?.getPCDsByType( @@ -82,7 +82,8 @@ export function validateAppState( if (loggedOut) { return { errors: validationErrors, - userUUID: undefined + userUUID: undefined, + tag }; } @@ -134,7 +135,8 @@ export function validateAppState( return { errors: validationErrors, - userUUID: self?.uuid + userUUID: self?.uuid, + tag }; } @@ -143,9 +145,7 @@ export function validateAppState( * we have records and are able to identify common types of errors. Does not leak * sensitive information, such as decrypted versions of e2ee storage. */ -export async function logValidationErrors( - errors: ValidationErrors -): Promise { +export async function logValidationErrors(errors: ErrorReport): Promise { try { const user = loadSelf(); errors.userUUID = errors.userUUID ?? user?.uuid; @@ -161,7 +161,7 @@ export async function logValidationErrors( /** * Uploaded to server in case of a state validation error. */ -export interface ValidationErrors { +export interface ErrorReport { /** * Human readable non-sensitive-information-leaking errors. If this array is empty, * it represents a state that has no errors. @@ -172,4 +172,10 @@ export interface ValidationErrors { * Used to identify the user on the server-side. */ userUUID?: string; + + /** + * Uniquely identifies the location in the code from which this error report + * was initiated. + */ + tag: string; } diff --git a/apps/passport-client/test/stateValidation.spec.ts b/apps/passport-client/test/stateValidation.spec.ts index 031b7a7f4b..f3267db878 100644 --- a/apps/passport-client/test/stateValidation.spec.ts +++ b/apps/passport-client/test/stateValidation.spec.ts @@ -8,7 +8,7 @@ import { Identity } from "@semaphore-protocol/identity"; import { expect } from "chai"; import { v4 as uuid } from "uuid"; import { randomEmail } from "../src/util"; -import { ValidationErrors, validateAppState } from "../src/validateState"; +import { ErrorReport, validateAppState } from "../src/validateState"; function newEdSAPCD(): Promise { return EdDSAPCDPackage.prove({ @@ -32,7 +32,7 @@ describe("validateAppState", async function () { const pcdPackages = [SemaphoreIdentityPCDPackage, EdDSAPCDPackage]; it("logged out ; no errors", async function () { - const errors = validateAppState("test", undefined, undefined, undefined); + const errors = validateAppState(TAG_STR, undefined, undefined, undefined); expect(errors.errors.length).to.eq(0); expect(errors.userUUID).to.eq(undefined); }); @@ -40,7 +40,7 @@ describe("validateAppState", async function () { it("logged out ; forceCheckPCDs=true; all error states caught", async function () { expect( validateAppState( - "test", + TAG_STR, undefined, undefined, await (async () => { @@ -53,12 +53,13 @@ describe("validateAppState", async function () { "'pcds' contains no pcds", "'pcds' field in app state does not contain an identity PCD" ], - userUUID: undefined - } satisfies ValidationErrors); + userUUID: undefined, + ...TAG + } satisfies ErrorReport); expect( validateAppState( - "test", + TAG_STR, undefined, undefined, await (async () => { @@ -70,18 +71,20 @@ describe("validateAppState", async function () { ) ).to.deep.eq({ errors: ["'pcds' field in app state does not contain an identity PCD"], - userUUID: undefined - } satisfies ValidationErrors); + userUUID: undefined, + ...TAG + } satisfies ErrorReport); expect( - validateAppState("test", undefined, undefined, undefined, true) + validateAppState(TAG_STR, undefined, undefined, undefined, true) ).to.deep.eq({ errors: [ "missing 'pcds'", "'pcds' field in app state does not contain an identity PCD" ], - userUUID: undefined - } satisfies ValidationErrors); + userUUID: undefined, + ...TAG + } satisfies ErrorReport); }); it("logged in ; no errors", async function () { @@ -102,10 +105,11 @@ describe("validateAppState", async function () { identity }) ); - expect(validateAppState("test", self, identity, pcds)).to.deep.eq({ + expect(validateAppState(TAG_STR, self, identity, pcds)).to.deep.eq({ userUUID: self.uuid, - errors: [] - } satisfies ValidationErrors); + errors: [], + ...TAG + } satisfies ErrorReport); }); it("logged in ; empty pcd collection ; errors", async function () { @@ -121,13 +125,14 @@ describe("validateAppState", async function () { uuid: uuid() }; const pcds = new PCDCollection(pcdPackages); - expect(validateAppState("test", self, identity, pcds)).to.deep.eq({ + expect(validateAppState(TAG_STR, self, identity, pcds)).to.deep.eq({ userUUID: self.uuid, errors: [ "'pcds' contains no pcds", "'pcds' field in app state does not contain an identity PCD" - ] - } satisfies ValidationErrors); + ], + ...TAG + } satisfies ErrorReport); }); it("logged in ; missing pcd collection ; errors", async function () { @@ -142,13 +147,14 @@ describe("validateAppState", async function () { terms_agreed: 1, uuid: uuid() }; - expect(validateAppState("test", self, identity, undefined)).to.deep.eq({ + expect(validateAppState(TAG_STR, self, identity, undefined)).to.deep.eq({ userUUID: self.uuid, errors: [ "missing 'pcds'", "'pcds' field in app state does not contain an identity PCD" - ] - } satisfies ValidationErrors); + ], + ...TAG + } satisfies ErrorReport); }); it("logged in ; missing identity ; errors", async function () { @@ -169,9 +175,13 @@ describe("validateAppState", async function () { identity }) ); - expect(validateAppState("test", self, undefined, pcds)).to.deep.eq({ + expect(validateAppState(TAG_STR, self, undefined, pcds)).to.deep.eq({ userUUID: self.uuid, - errors: ["missing 'identity'"] - } satisfies ValidationErrors); + errors: ["missing 'identity'"], + ...TAG + } satisfies ErrorReport); }); }); + +const TAG_STR = "test"; +const TAG = { tag: TAG_STR }; From f5d9125e3a5628afa6feea7ff18cd469bee0c3bd Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 18 Dec 2023 12:19:53 -0800 Subject: [PATCH 50/71] more test cleanup --- apps/passport-client/test/stateValidation.spec.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/apps/passport-client/test/stateValidation.spec.ts b/apps/passport-client/test/stateValidation.spec.ts index f3267db878..056bbc838f 100644 --- a/apps/passport-client/test/stateValidation.spec.ts +++ b/apps/passport-client/test/stateValidation.spec.ts @@ -32,9 +32,13 @@ describe("validateAppState", async function () { const pcdPackages = [SemaphoreIdentityPCDPackage, EdDSAPCDPackage]; it("logged out ; no errors", async function () { - const errors = validateAppState(TAG_STR, undefined, undefined, undefined); - expect(errors.errors.length).to.eq(0); - expect(errors.userUUID).to.eq(undefined); + expect( + validateAppState(TAG_STR, undefined, undefined, undefined) + ).to.deep.eq({ + errors: [], + userUUID: undefined, + ...TAG + } satisfies ErrorReport); }); it("logged out ; forceCheckPCDs=true; all error states caught", async function () { From 7fbf5631d9a629d80a1a25c8e3d0cd47fa351ddd Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 18 Dec 2023 12:20:47 -0800 Subject: [PATCH 51/71] more cleanup --- apps/passport-client/test/stateValidation.spec.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/apps/passport-client/test/stateValidation.spec.ts b/apps/passport-client/test/stateValidation.spec.ts index 056bbc838f..ab532ead9e 100644 --- a/apps/passport-client/test/stateValidation.spec.ts +++ b/apps/passport-client/test/stateValidation.spec.ts @@ -47,9 +47,7 @@ describe("validateAppState", async function () { TAG_STR, undefined, undefined, - await (async () => { - return new PCDCollection(pcdPackages); - })(), + new PCDCollection(pcdPackages), true ) ).to.deep.eq({ From 95ab85cca27ea800718352278c67e32bb67faafe Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 18 Dec 2023 12:22:23 -0800 Subject: [PATCH 52/71] another test case --- .../test/stateValidation.spec.ts | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/apps/passport-client/test/stateValidation.spec.ts b/apps/passport-client/test/stateValidation.spec.ts index ab532ead9e..fd210b38fc 100644 --- a/apps/passport-client/test/stateValidation.spec.ts +++ b/apps/passport-client/test/stateValidation.spec.ts @@ -183,6 +183,32 @@ describe("validateAppState", async function () { ...TAG } satisfies ErrorReport); }); + + it("logged in ; self missing commitment ; errors", async function () { + const identity = new Identity(); + const saltAndEncryptionKey = await crypto.generateSaltAndEncryptionKey( + "testpassword123!@#asdf" + ); + const self: ZupassUserJson = { + commitment: identity.commitment.toString(), + email: randomEmail(), + salt: saltAndEncryptionKey.salt, + terms_agreed: 1, + uuid: uuid() + }; + const pcds = new PCDCollection(pcdPackages); + pcds.add( + await SemaphoreIdentityPCDPackage.prove({ + identity + }) + ); + delete self.commitment; + expect(validateAppState(TAG_STR, self, identity, pcds)).to.deep.eq({ + userUUID: self.uuid, + errors: ["'self' missing a commitment"], + ...TAG + } satisfies ErrorReport); + }); }); const TAG_STR = "test"; From 69a6344b7f59206c01c3fe187aef181b9ae2a82f Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 18 Dec 2023 12:26:42 -0800 Subject: [PATCH 53/71] slight refactor --- .../test/stateValidation.spec.ts | 59 +++++++++++++------ 1 file changed, 42 insertions(+), 17 deletions(-) diff --git a/apps/passport-client/test/stateValidation.spec.ts b/apps/passport-client/test/stateValidation.spec.ts index fd210b38fc..3268bfdd10 100644 --- a/apps/passport-client/test/stateValidation.spec.ts +++ b/apps/passport-client/test/stateValidation.spec.ts @@ -30,6 +30,12 @@ function newEdSAPCD(): Promise { describe("validateAppState", async function () { const crypto = await PCDCrypto.newInstance(); const pcdPackages = [SemaphoreIdentityPCDPackage, EdDSAPCDPackage]; + const identity1 = new Identity( + '["0xaa5fa3165e1ca129bd7a2b3bada18c5f81350faacf2edff59cf44eeba2e2d","0xa944ed90153fc2be5759a0d18eda47266885aea0966ef4dbb96ff979c29ed4"]' + ); + const identity2 = new Identity( + '["0x8526e030dbd593833f24bf73b60f0bcc58690c590b9953acc741f2eb71394d","0x520e4ae6f5d5e4526dd517e61defe16f90bd4aef72b41394285e77463e0c69"]' + ); it("logged out ; no errors", async function () { expect( @@ -90,12 +96,11 @@ describe("validateAppState", async function () { }); it("logged in ; no errors", async function () { - const identity = new Identity(); const saltAndEncryptionKey = await crypto.generateSaltAndEncryptionKey( "testpassword123!@#asdf" ); const self: ZupassUserJson = { - commitment: identity.commitment.toString(), + commitment: identity1.commitment.toString(), email: randomEmail(), salt: saltAndEncryptionKey.salt, terms_agreed: 1, @@ -104,10 +109,10 @@ describe("validateAppState", async function () { const pcds = new PCDCollection(pcdPackages); pcds.add( await SemaphoreIdentityPCDPackage.prove({ - identity + identity: identity1 }) ); - expect(validateAppState(TAG_STR, self, identity, pcds)).to.deep.eq({ + expect(validateAppState(TAG_STR, self, identity1, pcds)).to.deep.eq({ userUUID: self.uuid, errors: [], ...TAG @@ -115,19 +120,18 @@ describe("validateAppState", async function () { }); it("logged in ; empty pcd collection ; errors", async function () { - const identity = new Identity(); const saltAndEncryptionKey = await crypto.generateSaltAndEncryptionKey( "testpassword123!@#asdf" ); const self: ZupassUserJson = { - commitment: identity.commitment.toString(), + commitment: identity1.commitment.toString(), email: randomEmail(), salt: saltAndEncryptionKey.salt, terms_agreed: 1, uuid: uuid() }; const pcds = new PCDCollection(pcdPackages); - expect(validateAppState(TAG_STR, self, identity, pcds)).to.deep.eq({ + expect(validateAppState(TAG_STR, self, identity1, pcds)).to.deep.eq({ userUUID: self.uuid, errors: [ "'pcds' contains no pcds", @@ -138,18 +142,17 @@ describe("validateAppState", async function () { }); it("logged in ; missing pcd collection ; errors", async function () { - const identity = new Identity(); const saltAndEncryptionKey = await crypto.generateSaltAndEncryptionKey( "testpassword123!@#asdf" ); const self: ZupassUserJson = { - commitment: identity.commitment.toString(), + commitment: identity1.commitment.toString(), email: randomEmail(), salt: saltAndEncryptionKey.salt, terms_agreed: 1, uuid: uuid() }; - expect(validateAppState(TAG_STR, self, identity, undefined)).to.deep.eq({ + expect(validateAppState(TAG_STR, self, identity1, undefined)).to.deep.eq({ userUUID: self.uuid, errors: [ "missing 'pcds'", @@ -160,12 +163,11 @@ describe("validateAppState", async function () { }); it("logged in ; missing identity ; errors", async function () { - const identity = new Identity(); const saltAndEncryptionKey = await crypto.generateSaltAndEncryptionKey( "testpassword123!@#asdf" ); const self: ZupassUserJson = { - commitment: identity.commitment.toString(), + commitment: identity1.commitment.toString(), email: randomEmail(), salt: saltAndEncryptionKey.salt, terms_agreed: 1, @@ -174,7 +176,7 @@ describe("validateAppState", async function () { const pcds = new PCDCollection(pcdPackages); pcds.add( await SemaphoreIdentityPCDPackage.prove({ - identity + identity: identity1 }) ); expect(validateAppState(TAG_STR, self, undefined, pcds)).to.deep.eq({ @@ -185,12 +187,11 @@ describe("validateAppState", async function () { }); it("logged in ; self missing commitment ; errors", async function () { - const identity = new Identity(); const saltAndEncryptionKey = await crypto.generateSaltAndEncryptionKey( "testpassword123!@#asdf" ); const self: ZupassUserJson = { - commitment: identity.commitment.toString(), + commitment: identity1.commitment.toString(), email: randomEmail(), salt: saltAndEncryptionKey.salt, terms_agreed: 1, @@ -199,16 +200,40 @@ describe("validateAppState", async function () { const pcds = new PCDCollection(pcdPackages); pcds.add( await SemaphoreIdentityPCDPackage.prove({ - identity + identity: identity1 }) ); delete self.commitment; - expect(validateAppState(TAG_STR, self, identity, pcds)).to.deep.eq({ + expect(validateAppState(TAG_STR, self, identity1, pcds)).to.deep.eq({ userUUID: self.uuid, errors: ["'self' missing a commitment"], ...TAG } satisfies ErrorReport); }); + + it("logged in ; no errors", async function () { + const saltAndEncryptionKey = await crypto.generateSaltAndEncryptionKey( + "testpassword123!@#asdf" + ); + const self: ZupassUserJson = { + commitment: identity1.commitment.toString(), + email: randomEmail(), + salt: saltAndEncryptionKey.salt, + terms_agreed: 1, + uuid: uuid() + }; + const pcds = new PCDCollection(pcdPackages); + pcds.add( + await SemaphoreIdentityPCDPackage.prove({ + identity: identity1 + }) + ); + expect(validateAppState(TAG_STR, self, identity1, pcds)).to.deep.eq({ + userUUID: self.uuid, + errors: [], + ...TAG + } satisfies ErrorReport); + }); }); const TAG_STR = "test"; From e11aacdbdf5f34d26dcad9fea7e3b9d026cea1b2 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 18 Dec 2023 12:37:09 -0800 Subject: [PATCH 54/71] more validation tests --- .../test/stateValidation.spec.ts | 106 +++++++++++++++++- 1 file changed, 100 insertions(+), 6 deletions(-) diff --git a/apps/passport-client/test/stateValidation.spec.ts b/apps/passport-client/test/stateValidation.spec.ts index 3268bfdd10..8a284fc1d9 100644 --- a/apps/passport-client/test/stateValidation.spec.ts +++ b/apps/passport-client/test/stateValidation.spec.ts @@ -31,11 +31,20 @@ describe("validateAppState", async function () { const crypto = await PCDCrypto.newInstance(); const pcdPackages = [SemaphoreIdentityPCDPackage, EdDSAPCDPackage]; const identity1 = new Identity( - '["0xaa5fa3165e1ca129bd7a2b3bada18c5f81350faacf2edff59cf44eeba2e2d","0xa944ed90153fc2be5759a0d18eda47266885aea0966ef4dbb96ff979c29ed4"]' + '["0xaa5fa3165e1ca129bd7a2b3bada18c5f81350faacf2edff59cf44eeba2e2d",' + + '"0xa944ed90153fc2be5759a0d18eda47266885aea0966ef4dbb96ff979c29ed4"]' ); + const commitment1 = identity1.commitment; const identity2 = new Identity( - '["0x8526e030dbd593833f24bf73b60f0bcc58690c590b9953acc741f2eb71394d","0x520e4ae6f5d5e4526dd517e61defe16f90bd4aef72b41394285e77463e0c69"]' + '["0x8526e030dbd593833f24bf73b60f0bcc58690c590b9953acc741f2eb71394d",' + + '"0x520e4ae6f5d5e4526dd517e61defe16f90bd4aef72b41394285e77463e0c69"]' ); + const commitment2 = identity2.commitment; + const identity3 = new Identity( + '["0x4837c6f88904d1dfefcb7dc6486e95c06cda6eb76d76a9888167c0993e40f0",' + + '"0x956f9e03b0cc324045d24f9b20531e547272fab8e8ee2f96c0cf2e50311468"]' + ); + const commitment3 = identity3.commitment; it("logged out ; no errors", async function () { expect( @@ -47,7 +56,7 @@ describe("validateAppState", async function () { } satisfies ErrorReport); }); - it("logged out ; forceCheckPCDs=true; all error states caught", async function () { + it("logged out ; forceCheckPCDs=true; test of all error states", async function () { expect( validateAppState( TAG_STR, @@ -211,12 +220,12 @@ describe("validateAppState", async function () { } satisfies ErrorReport); }); - it("logged in ; no errors", async function () { + it("logged in ; self commitment wrong ; errors", async function () { const saltAndEncryptionKey = await crypto.generateSaltAndEncryptionKey( "testpassword123!@#asdf" ); const self: ZupassUserJson = { - commitment: identity1.commitment.toString(), + commitment: identity2.commitment.toString(), email: randomEmail(), salt: saltAndEncryptionKey.salt, terms_agreed: 1, @@ -230,7 +239,92 @@ describe("validateAppState", async function () { ); expect(validateAppState(TAG_STR, self, identity1, pcds)).to.deep.eq({ userUUID: self.uuid, - errors: [], + errors: [ + `commitment of identity pcd in collection (${commitment1}) does not match commitment in 'self' field of app state (${commitment2})`, + `commitment in 'self' field of app state (${commitment2}) does not match commitment of 'identity' field of app state (${commitment1})` + ], + ...TAG + } satisfies ErrorReport); + }); + + it("logged in ; pcd collection identity wrong ; errors", async function () { + const saltAndEncryptionKey = await crypto.generateSaltAndEncryptionKey( + "testpassword123!@#asdf" + ); + const self: ZupassUserJson = { + commitment: identity1.commitment.toString(), + email: randomEmail(), + salt: saltAndEncryptionKey.salt, + terms_agreed: 1, + uuid: uuid() + }; + const pcds = new PCDCollection(pcdPackages); + pcds.add( + await SemaphoreIdentityPCDPackage.prove({ + identity: identity2 + }) + ); + expect(validateAppState(TAG_STR, self, identity1, pcds)).to.deep.eq({ + userUUID: self.uuid, + errors: [ + `commitment of identity pcd in collection (${commitment2}) does not match commitment in 'self' field of app state (${commitment1})`, + `commitment of 'identity' field of app state (${commitment1}) does not match commitment of identity pcd in collection (${commitment2})` + ], + ...TAG + } satisfies ErrorReport); + }); + + it("logged in ; appState identity wrong ; errors", async function () { + const saltAndEncryptionKey = await crypto.generateSaltAndEncryptionKey( + "testpassword123!@#asdf" + ); + const self: ZupassUserJson = { + commitment: identity1.commitment.toString(), + email: randomEmail(), + salt: saltAndEncryptionKey.salt, + terms_agreed: 1, + uuid: uuid() + }; + const pcds = new PCDCollection(pcdPackages); + pcds.add( + await SemaphoreIdentityPCDPackage.prove({ + identity: identity1 + }) + ); + expect(validateAppState(TAG_STR, self, identity2, pcds)).to.deep.eq({ + userUUID: self.uuid, + errors: [ + `commitment in 'self' field of app state (${commitment1}) does not match commitment of 'identity' field of app state (${commitment2})`, + `commitment of 'identity' field of app state (${commitment2}) does not match commitment of identity pcd in collection (${commitment1})` + ], + ...TAG + } satisfies ErrorReport); + }); + + it("logged in ; all identities mistmatched ; errors", async function () { + const saltAndEncryptionKey = await crypto.generateSaltAndEncryptionKey( + "testpassword123!@#asdf" + ); + const self: ZupassUserJson = { + commitment: identity1.commitment.toString(), + email: randomEmail(), + salt: saltAndEncryptionKey.salt, + terms_agreed: 1, + uuid: uuid() + }; + const pcds = new PCDCollection(pcdPackages); + pcds.add( + await SemaphoreIdentityPCDPackage.prove({ + identity: identity2 + }) + ); + expect(validateAppState(TAG_STR, self, identity3, pcds)).to.deep.eq({ + userUUID: self.uuid, + errors: [ + `commitment of identity pcd in collection (${commitment2}) does not match commitment in 'self' field of app state (${commitment1})`, + `commitment in 'self' field of app state (${commitment1}) does not match commitment of 'identity' field of app state (${commitment3})`, + `commitment of 'identity' field of app state (${commitment3}) does not match commitment of identity pcd in collection (${commitment2})` + ], ...TAG } satisfies ErrorReport); }); From 2c81bdbce501321de533b22dd2a7ed960f4fb071 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 18 Dec 2023 12:38:35 -0800 Subject: [PATCH 55/71] state validation tests look good --- .../test/stateValidation.spec.ts | 30 ++----------------- 1 file changed, 3 insertions(+), 27 deletions(-) diff --git a/apps/passport-client/test/stateValidation.spec.ts b/apps/passport-client/test/stateValidation.spec.ts index 8a284fc1d9..a8595a0335 100644 --- a/apps/passport-client/test/stateValidation.spec.ts +++ b/apps/passport-client/test/stateValidation.spec.ts @@ -29,6 +29,9 @@ function newEdSAPCD(): Promise { describe("validateAppState", async function () { const crypto = await PCDCrypto.newInstance(); + const saltAndEncryptionKey = await crypto.generateSaltAndEncryptionKey( + "testpassword123!@#asdf" + ); const pcdPackages = [SemaphoreIdentityPCDPackage, EdDSAPCDPackage]; const identity1 = new Identity( '["0xaa5fa3165e1ca129bd7a2b3bada18c5f81350faacf2edff59cf44eeba2e2d",' + @@ -105,9 +108,6 @@ describe("validateAppState", async function () { }); it("logged in ; no errors", async function () { - const saltAndEncryptionKey = await crypto.generateSaltAndEncryptionKey( - "testpassword123!@#asdf" - ); const self: ZupassUserJson = { commitment: identity1.commitment.toString(), email: randomEmail(), @@ -129,9 +129,6 @@ describe("validateAppState", async function () { }); it("logged in ; empty pcd collection ; errors", async function () { - const saltAndEncryptionKey = await crypto.generateSaltAndEncryptionKey( - "testpassword123!@#asdf" - ); const self: ZupassUserJson = { commitment: identity1.commitment.toString(), email: randomEmail(), @@ -151,9 +148,6 @@ describe("validateAppState", async function () { }); it("logged in ; missing pcd collection ; errors", async function () { - const saltAndEncryptionKey = await crypto.generateSaltAndEncryptionKey( - "testpassword123!@#asdf" - ); const self: ZupassUserJson = { commitment: identity1.commitment.toString(), email: randomEmail(), @@ -172,9 +166,6 @@ describe("validateAppState", async function () { }); it("logged in ; missing identity ; errors", async function () { - const saltAndEncryptionKey = await crypto.generateSaltAndEncryptionKey( - "testpassword123!@#asdf" - ); const self: ZupassUserJson = { commitment: identity1.commitment.toString(), email: randomEmail(), @@ -196,9 +187,6 @@ describe("validateAppState", async function () { }); it("logged in ; self missing commitment ; errors", async function () { - const saltAndEncryptionKey = await crypto.generateSaltAndEncryptionKey( - "testpassword123!@#asdf" - ); const self: ZupassUserJson = { commitment: identity1.commitment.toString(), email: randomEmail(), @@ -221,9 +209,6 @@ describe("validateAppState", async function () { }); it("logged in ; self commitment wrong ; errors", async function () { - const saltAndEncryptionKey = await crypto.generateSaltAndEncryptionKey( - "testpassword123!@#asdf" - ); const self: ZupassUserJson = { commitment: identity2.commitment.toString(), email: randomEmail(), @@ -248,9 +233,6 @@ describe("validateAppState", async function () { }); it("logged in ; pcd collection identity wrong ; errors", async function () { - const saltAndEncryptionKey = await crypto.generateSaltAndEncryptionKey( - "testpassword123!@#asdf" - ); const self: ZupassUserJson = { commitment: identity1.commitment.toString(), email: randomEmail(), @@ -275,9 +257,6 @@ describe("validateAppState", async function () { }); it("logged in ; appState identity wrong ; errors", async function () { - const saltAndEncryptionKey = await crypto.generateSaltAndEncryptionKey( - "testpassword123!@#asdf" - ); const self: ZupassUserJson = { commitment: identity1.commitment.toString(), email: randomEmail(), @@ -302,9 +281,6 @@ describe("validateAppState", async function () { }); it("logged in ; all identities mistmatched ; errors", async function () { - const saltAndEncryptionKey = await crypto.generateSaltAndEncryptionKey( - "testpassword123!@#asdf" - ); const self: ZupassUserJson = { commitment: identity1.commitment.toString(), email: randomEmail(), From fcec42d4a0f255dffc48a2a9e96f0df82fd10081 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 18 Dec 2023 13:01:24 -0800 Subject: [PATCH 56/71] introduce another validation function - initial state --- apps/passport-client/pages/index.tsx | 37 +----- apps/passport-client/src/dispatch.ts | 6 +- apps/passport-client/src/localstorage.ts | 6 +- .../src/useSyncE2EEStorage.tsx | 6 +- apps/passport-client/src/validateState.ts | 125 ++++++++++++------ .../test/stateValidation.spec.ts | 30 +++-- 6 files changed, 109 insertions(+), 101 deletions(-) diff --git a/apps/passport-client/pages/index.tsx b/apps/passport-client/pages/index.tsx index efce8fd3cb..2b306495b0 100644 --- a/apps/passport-client/pages/index.tsx +++ b/apps/passport-client/pages/index.tsx @@ -66,10 +66,7 @@ import { import { registerServiceWorker } from "../src/registerServiceWorker"; import { AppState, StateEmitter } from "../src/state"; import { pollUser } from "../src/user"; -import { - logValidationErrors, - validateAndLogStateErrors -} from "../src/validateState"; +import { validateAndLogInitialAppState } from "../src/validateState"; class App extends React.Component { state = undefined as AppState | undefined; @@ -444,38 +441,8 @@ async function loadInitialState(): Promise { serverStorageHash: persistentSyncStatus.serverStorageHash }; - if ( - !validateAndLogStateErrors( - "loadInitialState", - state.self, - state.identity, - state.pcds - ) - ) { + if (!validateAndLogInitialAppState("loadInitialState", state)) { state.userInvalid = true; - } else { - const extraValidationErrors = []; - - // this case covers a logged in user. the only way the app can get a 'self' - // is by requesting one from the server, to do which one has to be logged in. - if (state.self) { - if (!state.encryptionKey) { - extraValidationErrors.push(`logged in user missing encryption key`); - } - - if (!state.identity) { - extraValidationErrors.push(`logged in user missing identity`); - } - } - - if (extraValidationErrors.length > 0) { - logValidationErrors({ - tag: "afterLoadInitialState", - errors: extraValidationErrors, - userUUID: self?.uuid - }); - state.userInvalid = true; - } } return state; diff --git a/apps/passport-client/src/dispatch.ts b/apps/passport-client/src/dispatch.ts index 080686ad9a..001dd25452 100644 --- a/apps/passport-client/src/dispatch.ts +++ b/apps/passport-client/src/dispatch.ts @@ -52,7 +52,7 @@ import { uploadStorage } from "./useSyncE2EEStorage"; import { assertUnreachable } from "./util"; -import { validateAndLogStateErrors } from "./validateState"; +import { validateAndLogRunningAppState } from "./validateState"; export type Dispatcher = (action: Action) => void; @@ -344,7 +344,7 @@ async function finishAccountCreation( ) { // Verify that the identity is correct. if ( - !validateAndLogStateErrors( + !validateAndLogRunningAppState( "finishAccountCreation", user, state.identity, @@ -530,7 +530,7 @@ async function loadAfterLogin( )[0] as SemaphoreIdentityPCD; if ( - !validateAndLogStateErrors( + !validateAndLogRunningAppState( "loadAfterLogin", userResponse.value, identityPCD.claim.identity, diff --git a/apps/passport-client/src/localstorage.ts b/apps/passport-client/src/localstorage.ts index 8102f304ef..0d7ea2a358 100644 --- a/apps/passport-client/src/localstorage.ts +++ b/apps/passport-client/src/localstorage.ts @@ -12,14 +12,14 @@ import { SemaphoreSignaturePCD } from "@pcd/semaphore-signature-pcd"; import { Identity } from "@semaphore-protocol/identity"; import { z } from "zod"; import { getPackages } from "./pcdPackages"; -import { validateAndLogStateErrors } from "./validateState"; +import { validateAndLogRunningAppState } from "./validateState"; const OLD_PCDS_KEY = "pcds"; // deprecated const COLLECTION_KEY = "pcd_collection"; export async function savePCDs(pcds: PCDCollection): Promise { if ( - !validateAndLogStateErrors("savePCDs", undefined, undefined, pcds, true) + !validateAndLogRunningAppState("savePCDs", undefined, undefined, pcds, true) ) { console.error( "PCD Collection failed to validate - not writing it to localstorage" @@ -47,7 +47,7 @@ export async function loadPCDs(): Promise { ); if ( - !validateAndLogStateErrors( + !validateAndLogRunningAppState( "loadPCDs", undefined, undefined, diff --git a/apps/passport-client/src/useSyncE2EEStorage.tsx b/apps/passport-client/src/useSyncE2EEStorage.tsx index ad95dfd2da..5d664c116a 100644 --- a/apps/passport-client/src/useSyncE2EEStorage.tsx +++ b/apps/passport-client/src/useSyncE2EEStorage.tsx @@ -29,7 +29,7 @@ import { } from "./localstorage"; import { getPackages } from "./pcdPackages"; import { useOnStateChange } from "./subscribe"; -import { validateAndLogStateErrors } from "./validateState"; +import { validateAndLogRunningAppState } from "./validateState"; export type UpdateBlobKeyStorageInfo = { revision: string; @@ -138,7 +138,7 @@ export async function uploadSerializedStorage( storageHash: string ): Promise { if ( - !validateAndLogStateErrors( + !validateAndLogRunningAppState( "uploadSerializedStorage", user, userIdentity, @@ -237,7 +237,7 @@ export async function downloadStorage( ); if ( - !validateAndLogStateErrors( + !validateAndLogRunningAppState( "downloadStorage", undefined, undefined, diff --git a/apps/passport-client/src/validateState.ts b/apps/passport-client/src/validateState.ts index 973b55d575..d67b6fd8c5 100644 --- a/apps/passport-client/src/validateState.ts +++ b/apps/passport-client/src/validateState.ts @@ -7,22 +7,30 @@ import { import { Identity } from "@semaphore-protocol/identity"; import { appConfig } from "./appConfig"; import { loadSelf } from "./localstorage"; +import { AppState } from "./state"; -/** - * Validates the application state using {@link validateAppState}. In the case - * that the state is invalid, returns `true`, and concurrently uploads the validation - * errors to the server for further inspection. - * - * In the case there are no validation errors, returns `false`. - */ -export function validateAndLogStateErrors( +export function validateAndLogInitialAppState( + tag: string, + state: AppState +): boolean { + const validationErrors = validateInitialAppState(tag, state); + + if (validationErrors.errors.length > 0) { + logValidationErrors(validationErrors); + return false; + } + + return true; +} + +export function validateAndLogRunningAppState( tag: string, self: User | undefined, identity: Identity | undefined, pcds: PCDCollection | undefined, forceCheckPCDs?: boolean ): boolean { - const validationErrors = validateAppState( + const validationErrors = validateRunningAppState( tag, self, identity, @@ -38,26 +46,67 @@ export function validateAndLogStateErrors( return true; } -/** - * Determines whether the app's global state contains valid data. If it does not, - * returns the set of things that are incorrect about it in a {@link ErrorReport} - * object. If there were no validation errors the result will contain an empty array - * of errors. - * - * The provided {@link PCDCollection} is not checked unless either this function - * determines the user is logged in or the {@link forceCheckPCDs} argument is `true`. - * - * Depending on where this function is called, pass in a unique {@link tag}, so - * that on the backend we can figure out where the validation failed. - */ -export function validateAppState( +export function validateInitialAppState( + tag: string, + appState: AppState | undefined +): ErrorReport { + return { + errors: getInitialAppStateValidationErrors(appState), + userUUID: appState?.self?.uuid, + tag + }; +} + +export function validateRunningAppState( tag: string, self: User | undefined, identity: Identity | undefined, pcds: PCDCollection | undefined, forceCheckPCDs?: boolean ): ErrorReport { - const validationErrors: string[] = []; + return { + errors: getRunningAppStateValidationErrors( + self, + identity, + pcds, + forceCheckPCDs + ), + userUUID: self?.uuid, + tag + }; +} + +export function getInitialAppStateValidationErrors(state: AppState): string[] { + const errors = [ + ...getRunningAppStateValidationErrors( + state.self, + state.identity, + state.pcds + ) + ]; + + // this case covers a logged in user. the only way the app can get a 'self' + // is by requesting one from the server, to do which one has to be logged in. + if (state.self) { + if (!state.encryptionKey) { + errors.push(`logged in user missing encryption key`); + } + + if (!state.identity) { + errors.push(`logged in user missing identity`); + } + } + + return errors; +} + +export function getRunningAppStateValidationErrors( + self: User | undefined, + identity: Identity | undefined, + pcds: PCDCollection | undefined, + forceCheckPCDs?: boolean +): string[] { + const errors: string[] = []; const loggedOut = !self; const identityPCDFromCollection = pcds?.getPCDsByType( SemaphoreIdentityPCDPackage.name @@ -65,30 +114,24 @@ export function validateAppState( if (forceCheckPCDs || !loggedOut) { if (!pcds) { - validationErrors.push("missing 'pcds'"); + errors.push("missing 'pcds'"); } if (pcds?.size() === 0) { - validationErrors.push("'pcds' contains no pcds"); + errors.push("'pcds' contains no pcds"); } if (!identityPCDFromCollection) { - validationErrors.push( - "'pcds' field in app state does not contain an identity PCD" - ); + errors.push("'pcds' field in app state does not contain an identity PCD"); } } if (loggedOut) { - return { - errors: validationErrors, - userUUID: undefined, - tag - }; + errors; } if (!identity) { - validationErrors.push("missing 'identity'"); + errors.push("missing 'identity'"); } const identityFromPCDCollection = identityPCDFromCollection?.claim?.identity; @@ -98,7 +141,7 @@ export function validateAppState( const commitmentFromIdentityField = identity?.commitment?.toString(); if (commitmentFromSelfField === undefined) { - validationErrors.push(`'self' missing a commitment`); + errors.push(`'self' missing a commitment`); } if ( @@ -113,31 +156,27 @@ export function validateAppState( // identity, and in the pcd collection if (commitmentOfIdentityPCDInCollection !== commitmentFromSelfField) { - validationErrors.push( + errors.push( `commitment of identity pcd in collection (${commitmentOfIdentityPCDInCollection})` + ` does not match commitment in 'self' field of app state (${commitmentFromSelfField})` ); } if (commitmentFromSelfField !== commitmentFromIdentityField) { - validationErrors.push( + errors.push( `commitment in 'self' field of app state (${commitmentFromSelfField})` + ` does not match commitment of 'identity' field of app state (${commitmentFromIdentityField})` ); } if (commitmentFromIdentityField !== commitmentOfIdentityPCDInCollection) { - validationErrors.push( + errors.push( `commitment of 'identity' field of app state (${commitmentFromIdentityField})` + ` does not match commitment of identity pcd in collection (${commitmentOfIdentityPCDInCollection})` ); } } - return { - errors: validationErrors, - userUUID: self?.uuid, - tag - }; + return errors; } /** diff --git a/apps/passport-client/test/stateValidation.spec.ts b/apps/passport-client/test/stateValidation.spec.ts index a8595a0335..f9148597ca 100644 --- a/apps/passport-client/test/stateValidation.spec.ts +++ b/apps/passport-client/test/stateValidation.spec.ts @@ -8,7 +8,7 @@ import { Identity } from "@semaphore-protocol/identity"; import { expect } from "chai"; import { v4 as uuid } from "uuid"; import { randomEmail } from "../src/util"; -import { ErrorReport, validateAppState } from "../src/validateState"; +import { ErrorReport, validateRunningAppState } from "../src/validateState"; function newEdSAPCD(): Promise { return EdDSAPCDPackage.prove({ @@ -51,7 +51,7 @@ describe("validateAppState", async function () { it("logged out ; no errors", async function () { expect( - validateAppState(TAG_STR, undefined, undefined, undefined) + validateRunningAppState(TAG_STR, undefined, undefined, undefined) ).to.deep.eq({ errors: [], userUUID: undefined, @@ -61,7 +61,7 @@ describe("validateAppState", async function () { it("logged out ; forceCheckPCDs=true; test of all error states", async function () { expect( - validateAppState( + validateRunningAppState( TAG_STR, undefined, undefined, @@ -78,7 +78,7 @@ describe("validateAppState", async function () { } satisfies ErrorReport); expect( - validateAppState( + validateRunningAppState( TAG_STR, undefined, undefined, @@ -96,7 +96,7 @@ describe("validateAppState", async function () { } satisfies ErrorReport); expect( - validateAppState(TAG_STR, undefined, undefined, undefined, true) + validateRunningAppState(TAG_STR, undefined, undefined, undefined, true) ).to.deep.eq({ errors: [ "missing 'pcds'", @@ -121,7 +121,7 @@ describe("validateAppState", async function () { identity: identity1 }) ); - expect(validateAppState(TAG_STR, self, identity1, pcds)).to.deep.eq({ + expect(validateRunningAppState(TAG_STR, self, identity1, pcds)).to.deep.eq({ userUUID: self.uuid, errors: [], ...TAG @@ -137,7 +137,7 @@ describe("validateAppState", async function () { uuid: uuid() }; const pcds = new PCDCollection(pcdPackages); - expect(validateAppState(TAG_STR, self, identity1, pcds)).to.deep.eq({ + expect(validateRunningAppState(TAG_STR, self, identity1, pcds)).to.deep.eq({ userUUID: self.uuid, errors: [ "'pcds' contains no pcds", @@ -155,7 +155,9 @@ describe("validateAppState", async function () { terms_agreed: 1, uuid: uuid() }; - expect(validateAppState(TAG_STR, self, identity1, undefined)).to.deep.eq({ + expect( + validateRunningAppState(TAG_STR, self, identity1, undefined) + ).to.deep.eq({ userUUID: self.uuid, errors: [ "missing 'pcds'", @@ -179,7 +181,7 @@ describe("validateAppState", async function () { identity: identity1 }) ); - expect(validateAppState(TAG_STR, self, undefined, pcds)).to.deep.eq({ + expect(validateRunningAppState(TAG_STR, self, undefined, pcds)).to.deep.eq({ userUUID: self.uuid, errors: ["missing 'identity'"], ...TAG @@ -201,7 +203,7 @@ describe("validateAppState", async function () { }) ); delete self.commitment; - expect(validateAppState(TAG_STR, self, identity1, pcds)).to.deep.eq({ + expect(validateRunningAppState(TAG_STR, self, identity1, pcds)).to.deep.eq({ userUUID: self.uuid, errors: ["'self' missing a commitment"], ...TAG @@ -222,7 +224,7 @@ describe("validateAppState", async function () { identity: identity1 }) ); - expect(validateAppState(TAG_STR, self, identity1, pcds)).to.deep.eq({ + expect(validateRunningAppState(TAG_STR, self, identity1, pcds)).to.deep.eq({ userUUID: self.uuid, errors: [ `commitment of identity pcd in collection (${commitment1}) does not match commitment in 'self' field of app state (${commitment2})`, @@ -246,7 +248,7 @@ describe("validateAppState", async function () { identity: identity2 }) ); - expect(validateAppState(TAG_STR, self, identity1, pcds)).to.deep.eq({ + expect(validateRunningAppState(TAG_STR, self, identity1, pcds)).to.deep.eq({ userUUID: self.uuid, errors: [ `commitment of identity pcd in collection (${commitment2}) does not match commitment in 'self' field of app state (${commitment1})`, @@ -270,7 +272,7 @@ describe("validateAppState", async function () { identity: identity1 }) ); - expect(validateAppState(TAG_STR, self, identity2, pcds)).to.deep.eq({ + expect(validateRunningAppState(TAG_STR, self, identity2, pcds)).to.deep.eq({ userUUID: self.uuid, errors: [ `commitment in 'self' field of app state (${commitment1}) does not match commitment of 'identity' field of app state (${commitment2})`, @@ -294,7 +296,7 @@ describe("validateAppState", async function () { identity: identity2 }) ); - expect(validateAppState(TAG_STR, self, identity3, pcds)).to.deep.eq({ + expect(validateRunningAppState(TAG_STR, self, identity3, pcds)).to.deep.eq({ userUUID: self.uuid, errors: [ `commitment of identity pcd in collection (${commitment2}) does not match commitment in 'self' field of app state (${commitment1})`, From 5fce68d5a59090cfa098c81bafb6482ae5ae0534 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 18 Dec 2023 13:08:14 -0800 Subject: [PATCH 57/71] add comments --- apps/passport-client/src/validateState.ts | 26 +++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/apps/passport-client/src/validateState.ts b/apps/passport-client/src/validateState.ts index d67b6fd8c5..507abd66dd 100644 --- a/apps/passport-client/src/validateState.ts +++ b/apps/passport-client/src/validateState.ts @@ -9,6 +9,11 @@ import { appConfig } from "./appConfig"; import { loadSelf } from "./localstorage"; import { AppState } from "./state"; +/** + * Returns `true` if {@link validateInitialAppState} returns no errors, and `false` + * otherwise. In the case that errors were encountered, uploads an error report to + * the Zupass backend. + */ export function validateAndLogInitialAppState( tag: string, state: AppState @@ -23,6 +28,11 @@ export function validateAndLogInitialAppState( return true; } +/** + * Returns `true` if {@link validateRunningAppState} returns no errors, and `false` + * otherwise. In the case that errors were encountered, uploads an error report to + * the Zupass backend. + */ export function validateAndLogRunningAppState( tag: string, self: User | undefined, @@ -46,6 +56,10 @@ export function validateAndLogRunningAppState( return true; } +/** + * Validates state of a Zupass application that is just starting up (using the running + * app state checks alongside some extra ones), and returns an {@link ErrorReport}. + */ export function validateInitialAppState( tag: string, appState: AppState | undefined @@ -57,6 +71,9 @@ export function validateInitialAppState( }; } +/** + * Validates state of a running Zupass application and returns an {@link ErrorReport}. + */ export function validateRunningAppState( tag: string, self: User | undefined, @@ -76,6 +93,11 @@ export function validateRunningAppState( }; } +/** + * Validates state of a Zupass application that is just starting up. Uses + * {@link getRunningAppStateValidationErrors}, and performs some extra checks + * on top of it. + */ export function getInitialAppStateValidationErrors(state: AppState): string[] { const errors = [ ...getRunningAppStateValidationErrors( @@ -100,6 +122,10 @@ export function getInitialAppStateValidationErrors(state: AppState): string[] { return errors; } +/** + * Validates state of a running Zupass application. Returns a list of errors + * represented by strings. If the state is not invalid, returns an empty list. + */ export function getRunningAppStateValidationErrors( self: User | undefined, identity: Identity | undefined, From c49362bf174a8d3e57083f489bbef5e7b0f01a66 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 18 Dec 2023 13:17:18 -0800 Subject: [PATCH 58/71] remove encryption key check --- apps/passport-client/src/validateState.ts | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/apps/passport-client/src/validateState.ts b/apps/passport-client/src/validateState.ts index 507abd66dd..258ab0962b 100644 --- a/apps/passport-client/src/validateState.ts +++ b/apps/passport-client/src/validateState.ts @@ -57,8 +57,8 @@ export function validateAndLogRunningAppState( } /** - * Validates state of a Zupass application that is just starting up (using the running - * app state checks alongside some extra ones), and returns an {@link ErrorReport}. + * Validates state of a Zupass application that is just starting up (using the + * {@link validateRunningAppState} alongside some extra ones), and returns an {@link ErrorReport}. */ export function validateInitialAppState( tag: string, @@ -110,13 +110,8 @@ export function getInitialAppStateValidationErrors(state: AppState): string[] { // this case covers a logged in user. the only way the app can get a 'self' // is by requesting one from the server, to do which one has to be logged in. if (state.self) { - if (!state.encryptionKey) { - errors.push(`logged in user missing encryption key`); - } - - if (!state.identity) { - errors.push(`logged in user missing identity`); - } + // TODO: any extra checks that need to happen on immediate app startup should + // be put here. } return errors; From cfdc206a009baf04aebdbcbe34d35b3f6aa3db42 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 18 Dec 2023 13:20:16 -0800 Subject: [PATCH 59/71] undo changes to rate limit service --- .../src/services/rateLimitService.ts | 57 ++++++++++--------- 1 file changed, 30 insertions(+), 27 deletions(-) diff --git a/apps/passport-server/src/services/rateLimitService.ts b/apps/passport-server/src/services/rateLimitService.ts index 588a6bc48b..085a88b724 100644 --- a/apps/passport-server/src/services/rateLimitService.ts +++ b/apps/passport-server/src/services/rateLimitService.ts @@ -1,7 +1,12 @@ import { ONE_HOUR_MS } from "@pcd/util"; -import { clearExpiredActions } from "../database/queries/rateLimit"; +import { + checkRateLimit, + clearExpiredActions +} from "../database/queries/rateLimit"; import { ApplicationContext } from "../types"; +import { logger } from "../util/logger"; import { RollbarService } from "./rollbarService"; +import { traced } from "./telemetryService"; export type RateLimitedActionType = "CHECK_EMAIL_TOKEN" | "REQUEST_EMAIL_TOKEN"; @@ -54,35 +59,33 @@ export class RateLimitService { actionType: RateLimitedActionType, actionId: string ): Promise { - return true; + return traced( + "RateLimitService", + "requestRateLimitedAction", + async (span) => { + span?.setAttribute("actionType", actionType); + span?.setAttribute("actionId", actionId); - // return traced( - // "RateLimitService", - // "requestRateLimitedAction", - // async (span) => { - // span?.setAttribute("actionType", actionType); - // span?.setAttribute("actionId", actionId); + const result = checkRateLimit( + this.context.dbPool, + actionType, + actionId + ); - // const result = checkRateLimit( - // this.context.dbPool, - // actionType, - // actionId - // ); + if (!result) { + logger( + `[RATELIMIT] Action "${actionId}" of type "${actionType}" was rate-limited` + ); + this.rollbarService?.reportError( + new Error( + `Action "${actionId}" of type "${actionType}" was rate-limited` + ) + ); + } - // if (!result) { - // logger( - // `[RATELIMIT] Action "${actionId}" of type "${actionType}" was rate-limited` - // ); - // this.rollbarService?.reportError( - // new Error( - // `Action "${actionId}" of type "${actionType}" was rate-limited` - // ) - // ); - // } - - // return result; - // } - // ); + return result; + } + ); } } From 85d9e63122ec1a506c8c54f4a3d566ba2f52050f Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 18 Dec 2023 13:27:23 -0800 Subject: [PATCH 60/71] fix bug --- apps/passport-client/src/validateState.ts | 2 +- yarn.lock | 27 ++--------------------- 2 files changed, 3 insertions(+), 26 deletions(-) diff --git a/apps/passport-client/src/validateState.ts b/apps/passport-client/src/validateState.ts index 258ab0962b..45a66bbd63 100644 --- a/apps/passport-client/src/validateState.ts +++ b/apps/passport-client/src/validateState.ts @@ -148,7 +148,7 @@ export function getRunningAppStateValidationErrors( } if (loggedOut) { - errors; + return errors; } if (!identity) { diff --git a/yarn.lock b/yarn.lock index 3d4c69caa4..4862a74822 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4703,7 +4703,7 @@ dependencies: "@types/react" "*" -"@types/react@*", "@types/react@^18.0.22", "@types/react@^18.2.6": +"@types/react@*", "@types/react@18.0.22", "@types/react@18.2.20", "@types/react@^18.0.22", "@types/react@^18.2.6": version "18.2.21" resolved "https://registry.yarnpkg.com/@types/react/-/react-18.2.21.tgz#774c37fd01b522d0b91aed04811b58e4e0514ed9" integrity sha512-neFKG/sBAwGxHgXiIxnbm3/AAVQ/cMRS93hvBpg8xYRbeQSPVABp9U2bRnPf0iI4+Ucdv3plSxKK+3CW2ENJxA== @@ -4712,24 +4712,6 @@ "@types/scheduler" "*" csstype "^3.0.2" -"@types/react@18.0.22": - version "18.0.22" - resolved "https://registry.yarnpkg.com/@types/react/-/react-18.0.22.tgz#97782d995d999617de116cf61f437f1351036fc7" - integrity sha512-4yWc5PyCkZN8ke8K9rQHkTXxHIWHxLzzW6RI1kXVoepkD3vULpKzC2sDtAMKn78h92BRYuzf+7b/ms7ajE6hFw== - dependencies: - "@types/prop-types" "*" - "@types/scheduler" "*" - csstype "^3.0.2" - -"@types/react@18.2.20": - version "18.2.20" - resolved "https://registry.yarnpkg.com/@types/react/-/react-18.2.20.tgz#1605557a83df5c8a2cc4eeb743b3dfc0eb6aaeb2" - integrity sha512-WKNtmsLWJM/3D5mG4U84cysVY31ivmyw85dE84fOCk5Hx78wezB/XEjVPWl2JTZ5FkEeaTJf+VgUAUn3PE7Isw== - dependencies: - "@types/prop-types" "*" - "@types/scheduler" "*" - csstype "^3.0.2" - "@types/request@^2.48.8": version "2.48.8" resolved "https://registry.yarnpkg.com/@types/request/-/request-2.48.8.tgz#0b90fde3b655ab50976cb8c5ac00faca22f5a82c" @@ -15488,12 +15470,7 @@ typedoc@^0.25.1: minimatch "^9.0.3" shiki "^0.14.1" -typescript@5.1.6: - version "5.1.6" - resolved "https://registry.yarnpkg.com/typescript/-/typescript-5.1.6.tgz#02f8ac202b6dad2c0dd5e0913745b47a37998274" - integrity sha512-zaWCozRZ6DLEWAWFrVDz1H6FVXzUSfTy5FUMWsQlU8Ym5JP9eO4xkTIROFCQvhQf61z6O/G6ugw3SgAnvvm+HA== - -typescript@^4.5.2, typescript@^4.9.5: +typescript@5.1.6, typescript@^4.5.2, typescript@^4.9.5: version "4.9.5" resolved "https://registry.yarnpkg.com/typescript/-/typescript-4.9.5.tgz#095979f9bcc0d09da324d58d03ce8f8374cbe65a" integrity sha512-1FXk9E2Hm+QzZQ7z+McJiHL4NW1F2EzMu9Nq9i3zAaGqibafqYwCVU6WyWAuyQRRzOlxou8xZSyXLEN8oKj24g== From 0b1e4fc20d54a2d0fca123a36a0e3647ceba3e50 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 18 Dec 2023 14:39:38 -0800 Subject: [PATCH 61/71] fix merge conflict --- apps/passport-client/src/useSyncE2EEStorage.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/passport-client/src/useSyncE2EEStorage.tsx b/apps/passport-client/src/useSyncE2EEStorage.tsx index a895998899..9afcd2324e 100644 --- a/apps/passport-client/src/useSyncE2EEStorage.tsx +++ b/apps/passport-client/src/useSyncE2EEStorage.tsx @@ -133,7 +133,8 @@ export async function uploadStorage( userIdentity, pcds, serializedStorage, - storageHash + storageHash, + knownRevision ); } From 050932310f2528bf66ef835a58cd391d64eb72c2 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 18 Dec 2023 14:45:32 -0800 Subject: [PATCH 62/71] pass in extra validation arguments to tryDeserializeNewStorage --- apps/passport-client/src/dispatch.ts | 1 + apps/passport-client/src/useSyncE2EEStorage.tsx | 15 +++++++++++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/apps/passport-client/src/dispatch.ts b/apps/passport-client/src/dispatch.ts index 5c2a0284b2..d12c3b428b 100644 --- a/apps/passport-client/src/dispatch.ts +++ b/apps/passport-client/src/dispatch.ts @@ -752,6 +752,7 @@ async function doSync( state.serverStorageRevision, state.serverStorageHash, state.self, + state.identity, state.pcds, state.subscriptions ); diff --git a/apps/passport-client/src/useSyncE2EEStorage.tsx b/apps/passport-client/src/useSyncE2EEStorage.tsx index 9afcd2324e..33236e3db8 100644 --- a/apps/passport-client/src/useSyncE2EEStorage.tsx +++ b/apps/passport-client/src/useSyncE2EEStorage.tsx @@ -261,6 +261,7 @@ export async function downloadAndMergeStorage( knownServerRevision: string | undefined, knownServerHash: string | undefined, appSelf: User, + appIdentity: Identity, appPCDs: PCDCollection, appSubscriptions: FeedSubscriptionManager ): Promise { @@ -290,6 +291,8 @@ export async function downloadAndMergeStorage( // Deserialize downloaded storage, which becomes the default new state if no // merge is necessary. const downloaded = await tryDeserializeNewStorage( + appSelf, + appIdentity, storageResult.value.storage ); if (downloaded === undefined) { @@ -352,7 +355,12 @@ export async function downloadAndMergeStorage( }; } +/** + * {@link appSelf} and {@link appIdentity} are used solely for validation purposes. + */ export async function tryDeserializeNewStorage( + appSelf: User, + appIdentity: Identity, storage: SyncedEncryptedStorage ): Promise< | undefined @@ -371,10 +379,9 @@ export async function tryDeserializeNewStorage( if ( !validateAndLogRunningAppState( "downloadStorage", - undefined, - undefined, - pcds, - true + appSelf, + appIdentity, + pcds ) ) { throw new Error("downloaded e2ee state failed to validate"); From b7c881da763f2a74f8d12b8f091fc423c8939414 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 18 Dec 2023 16:44:32 -0800 Subject: [PATCH 63/71] slight refactor of logValidationErrors --- apps/passport-client/src/validateState.ts | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/apps/passport-client/src/validateState.ts b/apps/passport-client/src/validateState.ts index 45a66bbd63..1a5774c236 100644 --- a/apps/passport-client/src/validateState.ts +++ b/apps/passport-client/src/validateState.ts @@ -205,13 +205,20 @@ export function getRunningAppStateValidationErrors( * we have records and are able to identify common types of errors. Does not leak * sensitive information, such as decrypted versions of e2ee storage. */ -export async function logValidationErrors(errors: ErrorReport): Promise { +export async function logValidationErrors( + errorReport: ErrorReport +): Promise { + if (errorReport?.errors?.length === 0) { + console.log(`not logging empty error report`); + return; + } + try { const user = loadSelf(); - errors.userUUID = errors.userUUID ?? user?.uuid; - console.log(`encountered state validation errors: `, errors); + errorReport.userUUID = errorReport.userUUID ?? user?.uuid; + console.log(`encountered state validation errors: `, errorReport); await requestLogToServer(appConfig.zupassServer, "state-validation-error", { - ...errors + ...errorReport }); } catch (e) { console.log("error reporting errors", e); From c0a21289de551598f34ebe06b91d0b06b02bfe63 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 18 Dec 2023 16:47:01 -0800 Subject: [PATCH 64/71] prevent validation error from being reported by loading pcdcollection from localstorage for logged out users --- apps/passport-client/pages/index.tsx | 2 +- apps/passport-client/src/localstorage.ts | 7 +++++-- apps/passport-client/src/useSyncE2EEStorage.tsx | 2 +- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/apps/passport-client/pages/index.tsx b/apps/passport-client/pages/index.tsx index 49ff323e80..763f6015ee 100644 --- a/apps/passport-client/pages/index.tsx +++ b/apps/passport-client/pages/index.tsx @@ -404,7 +404,7 @@ async function loadInitialState(): Promise { } const self = loadSelf(); - const pcds = await loadPCDs(); + const pcds = await loadPCDs(self); const encryptionKey = loadEncryptionKey(); const subscriptions = await loadSubscriptions(); const offlineTickets = loadOfflineTickets(); diff --git a/apps/passport-client/src/localstorage.ts b/apps/passport-client/src/localstorage.ts index 0d7ea2a358..323f461d7c 100644 --- a/apps/passport-client/src/localstorage.ts +++ b/apps/passport-client/src/localstorage.ts @@ -31,7 +31,10 @@ export async function savePCDs(pcds: PCDCollection): Promise { window.localStorage[COLLECTION_KEY] = serialized; } -export async function loadPCDs(): Promise { +/** + * {@link self} argument used only to modify validation behavior. + */ +export async function loadPCDs(self?: User): Promise { const oldPCDs = window.localStorage[OLD_PCDS_KEY]; if (oldPCDs) { const collection = new PCDCollection(await getPackages()); @@ -52,7 +55,7 @@ export async function loadPCDs(): Promise { undefined, undefined, collection, - true + self !== undefined ) ) { console.error( diff --git a/apps/passport-client/src/useSyncE2EEStorage.tsx b/apps/passport-client/src/useSyncE2EEStorage.tsx index 33236e3db8..e1fdecfe85 100644 --- a/apps/passport-client/src/useSyncE2EEStorage.tsx +++ b/apps/passport-client/src/useSyncE2EEStorage.tsx @@ -57,7 +57,7 @@ export async function updateBlobKeyForEncryptedStorage( ): Promise { const oldUser = loadSelf(); const newUser = { ...oldUser, salt: newSalt }; - const pcds = await loadPCDs(); + const pcds = await loadPCDs(oldUser); const subscriptions = await loadSubscriptions(); const { serializedStorage, storageHash } = await serializeStorage( From f4999f14d85bc1448f19d563613be7470ae6e4ff Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 18 Dec 2023 17:03:47 -0800 Subject: [PATCH 65/71] early return on upload validation error; this doesn't cause upload loop b/c there is an even earlier early return that exits the sync code in the case that the userInvalid flat is set --- apps/passport-client/src/dispatch.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/apps/passport-client/src/dispatch.ts b/apps/passport-client/src/dispatch.ts index 996d9f7e13..d8c4176312 100644 --- a/apps/passport-client/src/dispatch.ts +++ b/apps/passport-client/src/dispatch.ts @@ -842,6 +842,10 @@ async function doSync( serverStorageHash: upRes.value.storageHash }; } else { + if (upRes.error.name === "ValidationError") { + return { userInvalid: true }; + } + // Upload failed. Update AppState if necessary, but not unnecessarily. // AppState updates will trigger another upload attempt. const needExtraDownload = upRes.error.name === "Conflict"; @@ -860,9 +864,6 @@ async function doSync( if (needExtraDownload && !state.extraDownloadRequested) { updates.extraDownloadRequested = true; } - if (upRes.error.name === "ValidationError") { - updates.userInvalid = true; - } return updates; } From c7dd380f847c317a81330e3dd81bdf1072823d5c Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 18 Dec 2023 17:18:06 -0800 Subject: [PATCH 66/71] update copy and styling of the invalid user modal to communicate better --- .../components/modals/InvalidUserModal.tsx | 44 ++++++++++++++----- apps/passport-client/pages/index.tsx | 3 +- 2 files changed, 34 insertions(+), 13 deletions(-) diff --git a/apps/passport-client/components/modals/InvalidUserModal.tsx b/apps/passport-client/components/modals/InvalidUserModal.tsx index e70df2d084..25015ba28f 100644 --- a/apps/passport-client/components/modals/InvalidUserModal.tsx +++ b/apps/passport-client/components/modals/InvalidUserModal.tsx @@ -7,8 +7,15 @@ import { AccountExportButton } from "../shared/AccountExportButton"; export function InvalidUserModal() { const dispatch = useDispatch(); - - const onExitClick = useCallback(() => { + const onLogoutClick = useCallback(() => { + if ( + !confirm( + "Are you sure you want to log out? " + + "We recommend that you export your account before doing so." + ) + ) { + return; + } dispatch({ type: "reset-passport" }); }, [dispatch]); @@ -16,24 +23,37 @@ export function InvalidUserModal() {

Invalid Zupass

-

- You've reset your Zupass account on another device, or your app has - ended up in an invalid state. Click the button below to log out. Then - you'll be able to sync your existing Zupass account onto this device. -

+

Your Zupass is in an invalid state. This can happen because:

+
    +
  • You reset your account on another device.
    • +
  • Zupass application state was corrupted on this device.
    • +
+

To resolve this, we recommend you either:

+
    +
  • Reload this page.
    • +
  • + Export your account data, log out of this account, and log in again. +
    • +
-

- You can export a copy of your local account data using the button below - in case you need it later. -

- + ); } const Container = styled.div` padding: 24px; + p { + margin-bottom: 8px; + } + ul { + list-style: circle; + margin-bottom: 8px; + li { + margin-left: 32px; + } + } `; diff --git a/apps/passport-client/pages/index.tsx b/apps/passport-client/pages/index.tsx index 15cb8259aa..a2f80acd76 100644 --- a/apps/passport-client/pages/index.tsx +++ b/apps/passport-client/pages/index.tsx @@ -443,8 +443,9 @@ async function loadInitialState(): Promise { importScreen: undefined }; - if (!validateAndLogInitialAppState("loadInitialState", state)) { + if (!validateAndLogInitialAppState("loadInitialState", state) || true) { state.userInvalid = true; + state.modal = { modalType: "invalid-participant" }; } return state; From c9cff96819b7b49315e63959807be189bc0d243f Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 18 Dec 2023 17:26:52 -0800 Subject: [PATCH 67/71] add mailto link in invalid user modal --- apps/passport-client/components/core/index.tsx | 5 +++++ .../components/modals/InfoModal.tsx | 8 ++------ .../components/modals/InvalidUserModal.tsx | 16 ++++++++++++---- .../components/screens/ServerErrorScreen.tsx | 6 ++---- .../components/screens/TermsScreen.tsx | 2 +- .../components/shared/PrivacyNotice.tsx | 5 ++--- 6 files changed, 24 insertions(+), 18 deletions(-) diff --git a/apps/passport-client/components/core/index.tsx b/apps/passport-client/components/core/index.tsx index 7a16fc4eb4..2edf7313db 100644 --- a/apps/passport-client/components/core/index.tsx +++ b/apps/passport-client/components/core/index.tsx @@ -1,4 +1,5 @@ import { Spacer } from "@pcd/passport-ui"; +import { ZUPASS_SUPPORT_EMAIL } from "@pcd/util"; import styled from "styled-components"; import { icons } from "../icons"; import { Button } from "./Button"; @@ -95,3 +96,7 @@ export function ZuLogo() { ); } + +export const SupportLink = () => { + return {ZUPASS_SUPPORT_EMAIL}; +}; diff --git a/apps/passport-client/components/modals/InfoModal.tsx b/apps/passport-client/components/modals/InfoModal.tsx index decc25eb81..dd8533a26a 100644 --- a/apps/passport-client/components/modals/InfoModal.tsx +++ b/apps/passport-client/components/modals/InfoModal.tsx @@ -1,4 +1,4 @@ -import { ZUPASS_GITHUB_REPOSITORY_URL, ZUPASS_SUPPORT_EMAIL } from "@pcd/util"; +import { ZUPASS_GITHUB_REPOSITORY_URL } from "@pcd/util"; import { CenterColumn, Spacer, TextCenter } from "../core"; import { icons } from "../icons"; @@ -20,11 +20,7 @@ export function InfoModal() { - For app support, contact{" "} - - {ZUPASS_SUPPORT_EMAIL} - - . + For app support, contact . diff --git a/apps/passport-client/components/modals/InvalidUserModal.tsx b/apps/passport-client/components/modals/InvalidUserModal.tsx index 25015ba28f..40a1f5a17a 100644 --- a/apps/passport-client/components/modals/InvalidUserModal.tsx +++ b/apps/passport-client/components/modals/InvalidUserModal.tsx @@ -2,9 +2,15 @@ import { Spacer } from "@pcd/passport-ui"; import { useCallback } from "react"; import styled from "styled-components"; import { useDispatch } from "../../src/appHooks"; -import { Button, H2 } from "../core"; +import { Button, H2, SupportLink } from "../core"; import { AccountExportButton } from "../shared/AccountExportButton"; +/** + * A Zupass client can sometimes end up with invalid local state. When that happens, + * we generally set {@link AppState.userInvalid} to true, and display this modal by + * setting {@link AppState.modal} to `{ modalType: "invalid-participant" }`. This modal + * explains what's going on + suggest paths to resolve the problem. + */ export function InvalidUserModal() { const dispatch = useDispatch(); const onLogoutClick = useCallback(() => { @@ -23,10 +29,10 @@ export function InvalidUserModal() {

Invalid Zupass

-

Your Zupass is in an invalid state. This can happen because:

+

Your Zupass is in an invalid state. This can happen when:

  • You reset your account on another device.
    • -
  • Zupass application state was corrupted on this device.
    • +
  • Zupass application state becomes corrupted on this device.

To resolve this, we recommend you either:

    @@ -35,7 +41,9 @@ export function InvalidUserModal() { Export your account data, log out of this account, and log in again.
- +

+ If this issue persists, please contact us at . +

diff --git a/apps/passport-client/components/screens/ServerErrorScreen.tsx b/apps/passport-client/components/screens/ServerErrorScreen.tsx index 11ffcd2af0..fb860bebcf 100644 --- a/apps/passport-client/components/screens/ServerErrorScreen.tsx +++ b/apps/passport-client/components/screens/ServerErrorScreen.tsx @@ -1,6 +1,5 @@ -import { ZUPASS_SUPPORT_EMAIL } from "@pcd/util"; import { useSearchParams } from "react-router-dom"; -import { CenterColumn, H1, Spacer, TextCenter } from "../core"; +import { CenterColumn, H1, Spacer, SupportLink, TextCenter } from "../core"; import { LinkButton } from "../core/Button"; import { AppContainer } from "../shared/AppContainer"; @@ -23,8 +22,7 @@ export function ServerErrorScreen() { {description} {!!description && } - For support, please send a message to{" "} - {ZUPASS_SUPPORT_EMAIL}. + For support, please send a message to . Return to Zupass diff --git a/apps/passport-client/components/screens/TermsScreen.tsx b/apps/passport-client/components/screens/TermsScreen.tsx index c6c22bedb7..14f8bfb703 100644 --- a/apps/passport-client/components/screens/TermsScreen.tsx +++ b/apps/passport-client/components/screens/TermsScreen.tsx @@ -1,5 +1,5 @@ import styled, { createGlobalStyle } from "styled-components"; -import { PrivacyNoticeText } from "../shared/PrivacyNotice"; +import { PrivacyNoticeText } from "../shared/PrivacyNoticeText"; export function TermsScreen() { return ( diff --git a/apps/passport-client/components/shared/PrivacyNotice.tsx b/apps/passport-client/components/shared/PrivacyNotice.tsx index 62f373d975..d4c9dcfa84 100644 --- a/apps/passport-client/components/shared/PrivacyNotice.tsx +++ b/apps/passport-client/components/shared/PrivacyNotice.tsx @@ -1,6 +1,6 @@ import { useCallback, useState } from "react"; import styled from "styled-components"; -import { Button, Spacer } from "../core"; +import { Button, Spacer, SupportLink } from "../core"; export function PrivacyNoticeText() { return ( @@ -244,8 +244,7 @@ export function PrivacyNoticeText() {

8. HOW TO CONTACT US

Should you have any questions about our privacy practices or this - Privacy Notice, please email us at{" "} - support@zupass.org. + Privacy Notice, please email us at .

); From c4a698236be05a2a8d60b433d31dc55d7992814b Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 18 Dec 2023 17:32:41 -0800 Subject: [PATCH 68/71] change the way we set userInvalid to also force the modal to open --- apps/passport-client/src/dispatch.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/apps/passport-client/src/dispatch.ts b/apps/passport-client/src/dispatch.ts index d8c4176312..5014b70e03 100644 --- a/apps/passport-client/src/dispatch.ts +++ b/apps/passport-client/src/dispatch.ts @@ -396,9 +396,8 @@ async function finishAccountCreation( !uploadResult.success && uploadResult.error.name === "ValidationError" ) { - update({ - userInvalid: true - }); + userInvalid(update); + return; } // Save user to local storage. This is done last because it unblocks @@ -555,7 +554,7 @@ async function loadAfterLogin( pcds ) ) { - update({ userInvalid: true }); + userInvalid(update); return; } @@ -843,7 +842,8 @@ async function doSync( }; } else { if (upRes.error.name === "ValidationError") { - return { userInvalid: true }; + userInvalid(update); + return; } // Upload failed. Update AppState if necessary, but not unnecessarily. From e7a843bf21fcdf03fdf82483b4a6dbebda70594a Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 18 Dec 2023 17:34:16 -0800 Subject: [PATCH 69/71] don't force userInvalid true --- apps/passport-client/pages/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/passport-client/pages/index.tsx b/apps/passport-client/pages/index.tsx index a2f80acd76..f46115e573 100644 --- a/apps/passport-client/pages/index.tsx +++ b/apps/passport-client/pages/index.tsx @@ -443,7 +443,7 @@ async function loadInitialState(): Promise { importScreen: undefined }; - if (!validateAndLogInitialAppState("loadInitialState", state) || true) { + if (!validateAndLogInitialAppState("loadInitialState", state)) { state.userInvalid = true; state.modal = { modalType: "invalid-participant" }; } From e67b1d1d54fd298b5df11ab67cde06111f84fe95 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 18 Dec 2023 17:35:39 -0800 Subject: [PATCH 70/71] add comment explaining early return --- apps/passport-client/src/dispatch.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/apps/passport-client/src/dispatch.ts b/apps/passport-client/src/dispatch.ts index 5014b70e03..e29ebb4fa1 100644 --- a/apps/passport-client/src/dispatch.ts +++ b/apps/passport-client/src/dispatch.ts @@ -842,6 +842,9 @@ async function doSync( }; } else { if (upRes.error.name === "ValidationError") { + // early return on upload validation error; this doesn't cause upload + // loop b/c there is an even earlier early return that exits the sync + // code in the case that the userInvalid flag is set userInvalid(update); return; } From 6adbf57008990e63d388aa6a0868ad3e712d1445 Mon Sep 17 00:00:00 2001 From: Ivan Chub Date: Mon, 18 Dec 2023 17:38:35 -0800 Subject: [PATCH 71/71] fixed build --- apps/passport-client/components/modals/InfoModal.tsx | 2 +- apps/passport-client/components/screens/TermsScreen.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/passport-client/components/modals/InfoModal.tsx b/apps/passport-client/components/modals/InfoModal.tsx index dd8533a26a..1a02cd298b 100644 --- a/apps/passport-client/components/modals/InfoModal.tsx +++ b/apps/passport-client/components/modals/InfoModal.tsx @@ -1,5 +1,5 @@ import { ZUPASS_GITHUB_REPOSITORY_URL } from "@pcd/util"; -import { CenterColumn, Spacer, TextCenter } from "../core"; +import { CenterColumn, Spacer, SupportLink, TextCenter } from "../core"; import { icons } from "../icons"; export function InfoModal() { diff --git a/apps/passport-client/components/screens/TermsScreen.tsx b/apps/passport-client/components/screens/TermsScreen.tsx index 14f8bfb703..c6c22bedb7 100644 --- a/apps/passport-client/components/screens/TermsScreen.tsx +++ b/apps/passport-client/components/screens/TermsScreen.tsx @@ -1,5 +1,5 @@ import styled, { createGlobalStyle } from "styled-components"; -import { PrivacyNoticeText } from "../shared/PrivacyNoticeText"; +import { PrivacyNoticeText } from "../shared/PrivacyNotice"; export function TermsScreen() { return (