Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

validate app state before performing potentially destructive actions: sync, save, download, upload #1348

Merged
merged 73 commits into from
Dec 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
73 commits
Select commit Hold shift + click to select a range
65a7536
create function that validates appState and returns list of errors as…
ichub Dec 11, 2023
144f2eb
handle state validation errors in inital state load
ichub Dec 11, 2023
0aa824c
commented some locations where validation logic should be added
ichub Dec 11, 2023
6c55e49
more validation
ichub Dec 11, 2023
dc0d6ff
added more validation
ichub Dec 11, 2023
3db82d9
implemented another validation point
ichub Dec 11, 2023
910a334
more validation
ichub Dec 11, 2023
0629d46
undo a change
ichub Dec 11, 2023
4502d33
add some validation logic inside of localstorage saving and loading o…
ichub Dec 11, 2023
91b5ca1
add a bunch of comments
ichub Dec 11, 2023
71dc18a
added account export button to the invalid user modal
ichub Dec 11, 2023
41bbdc1
add explanation to export button
ichub Dec 15, 2023
6067803
respond to some PR comments
ichub Dec 15, 2023
4268377
some cleanups
ichub Dec 15, 2023
e0710a3
slight refactor
ichub Dec 15, 2023
87e2ece
ensure user uuid is always uploaded with a validation error
ichub Dec 15, 2023
09ee405
more pr review comments
ichub Dec 15, 2023
d8537ca
more pr review comments
ichub Dec 15, 2023
36a242b
fixed another comment
ichub Dec 15, 2023
0535ebd
rename validateState to validateAppState
ichub Dec 15, 2023
f078f9d
consolidated all validation logic into one function
ichub Dec 15, 2023
e793c87
testing
ichub Dec 15, 2023
c79c4c7
undo extra file
ichub Dec 15, 2023
3e30ce2
fix bug
ichub Dec 15, 2023
a908239
fix bug
ichub Dec 15, 2023
2963009
updated comments
ichub Dec 15, 2023
5e9e822
extra validation logic in load initial app state
ichub Dec 15, 2023
6a37eed
early return from sync if userInvalid=true
ichub Dec 15, 2023
687e981
implement suggestion to make validation of state consistent between u…
ichub Dec 15, 2023
3570232
starting to add test for state validation function
ichub Dec 15, 2023
014a81c
update error message based on pr comment
ichub Dec 15, 2023
cebe1b9
update copy in response to feedback
ichub Dec 15, 2023
28aafad
cleaning up test
ichub Dec 18, 2023
82eb229
actually test
ichub Dec 18, 2023
21d0bfb
test logged out state
ichub Dec 18, 2023
f5331b6
more validation test cases
ichub Dec 18, 2023
06b1969
moved state validation out of sync and into uploadSerializedStorage
ichub Dec 18, 2023
b858d54
moved userinvalid check to top of dosync
ichub Dec 18, 2023
a463370
move pcd collection logic to top of validation function
ichub Dec 18, 2023
8f21814
add validationTag to validateState function
ichub Dec 18, 2023
00e5e49
console.log -> console.error
ichub Dec 18, 2023
c527bb0
unwrap errors object when logging to server
ichub Dec 18, 2023
44eb786
update validateState to be less confusing for cases when something is…
ichub Dec 18, 2023
215487b
fix test
ichub Dec 18, 2023
6fab637
testing logged out state
ichub Dec 18, 2023
2071644
one more test case
ichub Dec 18, 2023
5b01b79
cleanup
ichub Dec 18, 2023
e63a5f1
add case for missing identity
ichub Dec 18, 2023
430e506
actually propagate log tag
ichub Dec 18, 2023
f5d9125
more test cleanup
ichub Dec 18, 2023
7fbf563
more cleanup
ichub Dec 18, 2023
95ab85c
another test case
ichub Dec 18, 2023
69a6344
slight refactor
ichub Dec 18, 2023
e11aacd
more validation tests
ichub Dec 18, 2023
2c81bdb
state validation tests look good
ichub Dec 18, 2023
fcec42d
introduce another validation function - initial state
ichub Dec 18, 2023
5fce68d
add comments
ichub Dec 18, 2023
c49362b
remove encryption key check
ichub Dec 18, 2023
cfdc206
undo changes to rate limit service
ichub Dec 18, 2023
85d9e63
fix bug
ichub Dec 18, 2023
67e237c
Merge remote-tracking branch 'origin/main' into ivan/state-validation
ichub Dec 18, 2023
0b1e4fc
fix merge conflict
ichub Dec 18, 2023
0509323
pass in extra validation arguments to tryDeserializeNewStorage
ichub Dec 18, 2023
b7c881d
slight refactor of logValidationErrors
ichub Dec 19, 2023
c0a2128
prevent validation error from being reported by loading pcdcollection…
ichub Dec 19, 2023
0679204
Merge remote-tracking branch 'origin/main' into ivan/state-validation
ichub Dec 19, 2023
f4999f1
early return on upload validation error; this doesn't cause upload lo…
ichub Dec 19, 2023
c7dd380
update copy and styling of the invalid user modal to communicate better
ichub Dec 19, 2023
c9cff96
add mailto link in invalid user modal
ichub Dec 19, 2023
c4a6982
change the way we set userInvalid to also force the modal to open
ichub Dec 19, 2023
e7a843b
don't force userInvalid true
ichub Dec 19, 2023
e67b1d1
add comment explaining early return
ichub Dec 19, 2023
6adbf57
fixed build
ichub Dec 19, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions apps/passport-client/components/core/index.tsx
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -95,3 +96,7 @@ export function ZuLogo() {
<img draggable="false" src={icons.logo} width="160px" height="155px" />
);
}

export const SupportLink = () => {
return <a href={`mailto:${ZUPASS_SUPPORT_EMAIL}`}>{ZUPASS_SUPPORT_EMAIL}</a>;
};
10 changes: 3 additions & 7 deletions apps/passport-client/components/modals/InfoModal.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { ZUPASS_GITHUB_REPOSITORY_URL, ZUPASS_SUPPORT_EMAIL } from "@pcd/util";
import { CenterColumn, Spacer, TextCenter } from "../core";
import { ZUPASS_GITHUB_REPOSITORY_URL } from "@pcd/util";
import { CenterColumn, Spacer, SupportLink, TextCenter } from "../core";
import { icons } from "../icons";

export function InfoModal() {
Expand All @@ -20,11 +20,7 @@ export function InfoModal() {
</TextCenter>
<Spacer h={16} />
<TextCenter>
For app support, contact{" "}
<a href={`mailto:${ZUPASS_SUPPORT_EMAIL}`}>
{ZUPASS_SUPPORT_EMAIL}
</a>
.
For app support, contact <SupportLink />.
</TextCenter>
</>
</CenterColumn>
Expand Down
52 changes: 44 additions & 8 deletions apps/passport-client/components/modals/InvalidUserModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,30 +2,66 @@ 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 onClick = 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]);

return (
<Container>
<H2>Invalid Zupass</H2>
<Spacer h={24} />
<p>Your Zupass is in an invalid state. This can happen when:</p>
<ul>
<li>You reset your account on another device.</li>
<li>Zupass application state becomes corrupted on this device.</li>
</ul>
<p>To resolve this, we recommend you either:</p>
<ul>
<li>Reload this page.</li>
<li>
Export your account data, log out of this account, and log in again.
</li>
</ul>
<p>
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.
If this issue persists, please contact us at <SupportLink />.
</p>
<Spacer h={24} />
<Button onClick={onClick}>Exit</Button>
<Spacer h={16} />
<AccountExportButton />
artwyman marked this conversation as resolved.
Show resolved Hide resolved
<Spacer h={16} />
<Button onClick={onLogoutClick}>Log Out</Button>
</Container>
);
}

const Container = styled.div`
padding: 24px;
p {
margin-bottom: 8px;
}
ul {
list-style: circle;
margin-bottom: 8px;
li {
margin-left: 32px;
}
}
`;
6 changes: 2 additions & 4 deletions apps/passport-client/components/screens/ServerErrorScreen.tsx
Original file line number Diff line number Diff line change
@@ -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";

Expand All @@ -23,8 +22,7 @@ export function ServerErrorScreen() {
<Spacer h={24} />
{description}
{!!description && <Spacer h={24} />}
For support, please send a message to{" "}
<a href={`mailto:${ZUPASS_SUPPORT_EMAIL}`}>{ZUPASS_SUPPORT_EMAIL}</a>.
For support, please send a message to <SupportLink />.
<Spacer h={24} />
<LinkButton to="/">Return to Zupass</LinkButton>
<Spacer h={24} />
Expand Down
5 changes: 2 additions & 3 deletions apps/passport-client/components/shared/PrivacyNotice.tsx
Original file line number Diff line number Diff line change
@@ -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 (
Expand Down Expand Up @@ -244,8 +244,7 @@ export function PrivacyNoticeText() {
<h3>8. HOW TO CONTACT US</h3>
<p>
Should you have any questions about our privacy practices or this
Privacy Notice, please email us at{" "}
<a href="mailto:[email protected]">[email protected]</a>.
Privacy Notice, please email us at <SupportLink />.
</p>
</Prose>
);
Expand Down
1 change: 1 addition & 0 deletions apps/passport-client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,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",
Expand Down
12 changes: 10 additions & 2 deletions apps/passport-client/pages/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ import {
import { registerServiceWorker } from "../src/registerServiceWorker";
import { AppState, StateEmitter } from "../src/state";
import { pollUser } from "../src/user";
import { validateAndLogInitialAppState } from "../src/validateState";

class App extends React.Component<object, AppState> {
state = undefined as AppState | undefined;
Expand Down Expand Up @@ -402,7 +403,7 @@ async function loadInitialState(): Promise<AppState> {
}

const self = loadSelf();
const pcds = await loadPCDs();
const pcds = await loadPCDs(self);
const encryptionKey = loadEncryptionKey();
const subscriptions = await loadSubscriptions();
const offlineTickets = loadOfflineTickets();
Expand All @@ -425,7 +426,7 @@ async function loadInitialState(): Promise<AppState> {

const persistentSyncStatus = loadPersistentSyncStatus();

return {
const state: AppState = {
ichub marked this conversation as resolved.
Show resolved Hide resolved
self,
encryptionKey,
pcds,
Expand All @@ -441,6 +442,13 @@ async function loadInitialState(): Promise<AppState> {
serverStorageHash: persistentSyncStatus.serverStorageHash,
importScreen: undefined
};

ichub marked this conversation as resolved.
Show resolved Hide resolved
if (!validateAndLogInitialAppState("loadInitialState", state)) {
state.userInvalid = true;
state.modal = { modalType: "invalid-participant" };
}

return state;
}

registerServiceWorker();
Expand Down
51 changes: 47 additions & 4 deletions apps/passport-client/src/dispatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ import {
uploadStorage
} from "./useSyncE2EEStorage";
import { assertUnreachable } from "./util";
import { validateAndLogRunningAppState } from "./validateState";

export type Dispatcher = (action: Action) => void;

Expand Down Expand Up @@ -356,16 +357,21 @@ async function finishAccountCreation(
update: ZuUpdate
) {
// Verify that the identity is correct.
const { identity } = state;
console.log("[ACCOUNT] Check user", identity, user);
if (identity == null || identity.commitment.toString() !== user.commitment) {
if (
!validateAndLogRunningAppState(
"finishAccountCreation",
user,
state.identity,
state.pcds
)
) {
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.
ichub marked this conversation as resolved.
Show resolved Hide resolved
return; // Don't save the bad identity. User must reset account.
}

// Save PCDs to E2EE storage. knownRevision=undefined is the way to create
Expand All @@ -375,6 +381,7 @@ async function finishAccountCreation(
console.log("[ACCOUNT] Upload initial PCDs");
const uploadResult = await uploadStorage(
user,
state.identity,
state.pcds,
state.subscriptions,
undefined // knownRevision
Expand All @@ -385,6 +392,12 @@ async function finishAccountCreation(
serverStorageRevision: uploadResult.value.revision,
serverStorageHash: uploadResult.value.storageHash
});
} else if (
!uploadResult.success &&
uploadResult.error.name === "ValidationError"
) {
userInvalid(update);
return;
}

// Save user to local storage. This is done last because it unblocks
Expand Down Expand Up @@ -533,6 +546,18 @@ async function loadAfterLogin(
SemaphoreIdentityPCDTypeName
)[0] as SemaphoreIdentityPCD;

if (
!validateAndLogRunningAppState(
"loadAfterLogin",
userResponse.value,
identityPCD.claim.identity,
pcds
)
) {
userInvalid(update);
return;
}

let modal: AppState["modal"] = { modalType: "none" };
if (!identityPCD) {
// TODO: handle error gracefully
Expand Down Expand Up @@ -705,6 +730,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
Expand All @@ -722,6 +751,7 @@ async function doSync(
state.serverStorageRevision,
state.serverStorageHash,
state.self,
state.identity,
state.pcds,
state.subscriptions
);
Expand Down Expand Up @@ -794,9 +824,13 @@ async function doSync(
state.pcds,
state.subscriptions
);

ichub marked this conversation as resolved.
Show resolved Hide resolved
if (state.serverStorageHash !== appStorage.storageHash) {
console.log("[SYNC] sync action: upload");
const upRes = await uploadSerializedStorage(
state.self,
state.identity,
state.pcds,
appStorage.serializedStorage,
appStorage.storageHash,
state.serverStorageRevision
Expand All @@ -807,6 +841,14 @@ async function doSync(
serverStorageHash: upRes.value.storageHash
};
} 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;
}

// Upload failed. Update AppState if necessary, but not unnecessarily.
// AppState updates will trigger another upload attempt.
const needExtraDownload = upRes.error.name === "Conflict";
Expand All @@ -825,6 +867,7 @@ async function doSync(
if (needExtraDownload && !state.extraDownloadRequested) {
updates.extraDownloadRequested = true;
}

return updates;
}
}
Expand Down
33 changes: 31 additions & 2 deletions apps/passport-client/src/localstorage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,29 @@ import { SemaphoreSignaturePCD } from "@pcd/semaphore-signature-pcd";
import { Identity } from "@semaphore-protocol/identity";
import { z } from "zod";
import { getPackages } from "./pcdPackages";
import { validateAndLogRunningAppState } from "./validateState";

const OLD_PCDS_KEY = "pcds"; // deprecated
const COLLECTION_KEY = "pcd_collection";

export async function savePCDs(pcds: PCDCollection): Promise<void> {
ichub marked this conversation as resolved.
Show resolved Hide resolved
if (
!validateAndLogRunningAppState("savePCDs", undefined, undefined, pcds, true)
) {
console.error(
"PCD Collection failed to validate - not writing it to localstorage"
);
return;
}

const serialized = await pcds.serializeCollection();
window.localStorage[COLLECTION_KEY] = serialized;
}

export async function loadPCDs(): Promise<PCDCollection> {
/**
* {@link self} argument used only to modify validation behavior.
*/
export async function loadPCDs(self?: User): Promise<PCDCollection> {
const oldPCDs = window.localStorage[OLD_PCDS_KEY];
if (oldPCDs) {
const collection = new PCDCollection(await getPackages());
Expand All @@ -31,10 +44,26 @@ export async function loadPCDs(): Promise<PCDCollection> {
}

const serializedCollection = window.localStorage[COLLECTION_KEY];
return await PCDCollection.deserialize(
const collection = await PCDCollection.deserialize(
await getPackages(),
serializedCollection ?? "{}"
);

if (
!validateAndLogRunningAppState(
"loadPCDs",
undefined,
undefined,
collection,
self !== undefined
)
) {
console.error(
"PCD Collection failed to validate when loading from localstorage"
);
}

return collection;
}

export async function saveSubscriptions(
Expand Down
Loading