Don't store voter emails/IPs on rolls for elections that don't authenticate

packages/backend/src/Controllers/Roll/voterRollUtils.ts

@@ -15,9 +15,11 @@ export async function getOrCreateElectionRoll(req: IRequest, election: Election,
     // Checks for existing election roll for user
     Logger.info(req, `getOrCreateElectionRoll`)
     const ip_hash = hashString(req.ip!)
-    // Get data that is used for voter authentication
+    // Get data that is used for voter authentication. Each of these holds the *value* to
+    // authenticate on, or null when this election doesn't authenticate on it — they are not
+    // booleans, and they're what both the roll lookup and the roll we write are built from.
     // NOTE: I'm ensuring that undefined is coaleced into null, that makes it compliant with the type when calling getElectionRoll
-    const require_ip_hash = (election.settings.voter_authentication.ip_address ? ip_hash : null) ?? null;
+    const auth_ip_hash = (election.settings.voter_authentication.ip_address ? ip_hash : null) ?? null;
     const email = election.settings.voter_authentication.email ? req.user?.email : null
 
     // Get voter ID if required and available, otherwise set to null
@@ -34,8 +36,8 @@ export async function getOrCreateElectionRoll(req: IRequest, election: Election,
     // This is an odd way of going about this, rather than getting a roll that matches all three we get all that match any of the fields and
     // check the output for a number of edge cases.
     var electionRollEntries = null
-    if ((require_ip_hash || email || voter_id)) {
-        electionRollEntries = await ElectionRollModel.getElectionRoll(String(election.election_id), voter_id, email, require_ip_hash, ctx);
+    if ((auth_ip_hash || email || voter_id)) {
+        electionRollEntries = await ElectionRollModel.getElectionRoll(String(election.election_id), voter_id, email, auth_ip_hash, ctx);
     }
 
     if (electionRollEntries == null) {
@@ -57,23 +59,30 @@ export async function getOrCreateElectionRoll(req: IRequest, election: Election,
             timestamp: Date.now(),
         }]
         // create_date / update_date / head are generated by the model in the insert transaction.
+        // Only persist the identifying fields this election actually authenticates on —
+        // i.e. the same values the lookup above ran against. Writing req.user.email /
+        // ip_hash unconditionally attached a real email address and IP hash to the
+        // voter's ballot_id even for elections configured to require neither.
         const roll: NewElectionRoll[] = [{
             election_id: String(election.election_id),
-            email: req.user?.email ? req.user.email : undefined,
+            email: email ?? undefined,
             voter_id: new_voter_id,
-            ip_hash: ip_hash,
+            ip_hash: auth_ip_hash ?? undefined,
             submitted: false,
             state: ElectionRollState.approved,
             history: history,
         }]
-        if ((require_ip_hash || email || voter_id)) {
+        if ((auth_ip_hash || email || voter_id)) {
             // Return the row the DB actually wrote — its update_date is the canonical value
             // that OCC will check against on the cast-vote path.
             const inserted = await ElectionRollModel.submitElectionRoll(roll, ctx, `User requesting Roll and is authorized`)
             return inserted[0];
         }
         else {
-            // Not persisted; downstream code that needs a real update_date should not reach here.
+            // Not persisted *here*, but the cast-vote path does reach this branch and
+            // CastVoteStore.submitBallotEvent inserts whatever roll it is handed — so this
+            // row does land in electionRollDB for e.g. open_open elections. Keep it free of
+            // anything the election didn't ask to authenticate on.
             return { ...roll[0], update_date: Date.now().toString(), head: true, create_date: new Date().toISOString() }
         }
     }

packages/backend/src/Models/__mocks__/CastVoteStore.ts

@@ -30,7 +30,17 @@ export default class CastVoteStore {
 
         if (event.roll) {
             event.roll.submitted = true;
-            await this._rollStore.update(event.roll, ctx, `User submits a ballot`);
+            // Mirror the real store: it archives the current head row if there is one and
+            // then inserts. For elections that authenticate on nothing, getOrCreateElectionRoll
+            // hands back a roll it never persisted, so there is no head row to update and the
+            // insert is what creates it. The mock used to no-op in that case, which hid
+            // everything the cast-vote path writes to the roll for open_open elections.
+            const existing = await this._rollStore.getByVoterID(event.roll.election_id, event.roll.voter_id, ctx);
+            if (existing) {
+                await this._rollStore.update(event.roll, ctx, `User submits a ballot`);
+            } else {
+                await this._rollStore.submitElectionRoll([event.roll], ctx, `User submits a ballot`);
+            }
         }
     }
 

packages/backend/src/test/openElectionRollPrivacy.test.ts

@@ -0,0 +1,146 @@
+require("dotenv").config();
+
+import { Election } from "@equal-vote/star-vote-shared/domain_model/Election";
+import { ElectionSettings } from "@equal-vote/star-vote-shared/domain_model/ElectionSettings";
+import { ElectionRoll } from "@equal-vote/star-vote-shared/domain_model/ElectionRoll";
+import ServiceLocator from "../ServiceLocator";
+import Logger from "../Services/Logging/Logger";
+import testInputs from "./testInputs";
+import { TestHelper } from "./TestHelper";
+
+const th = new TestHelper();
+const ctx = Logger.createContext("openElectionRollPrivacy");
+
+afterEach(() => {
+  jest.clearAllMocks();
+  th.afterEach();
+});
+
+// Asserts against the roll store rather than GET /rolls, because that endpoint refuses
+// to list rolls for open elections — which is exactly why this had gone unnoticed. The
+// row was still being written; nothing was reading it back.
+async function rollsFor(electionId: string): Promise<ElectionRoll[]> {
+  const rolls = await ServiceLocator.electionRollDb().getRollsByElectionID(electionId, ctx);
+  return rolls ?? [];
+}
+
+// getOrCreateElectionRoll used to copy req.user.email and hashString(req.ip) onto every
+// new roll regardless of the election's voter_authentication settings. The guard that
+// decides whether to persist the roll does consult those settings, but the cast-vote path
+// hands the roll to CastVoteStore.submitBallotEvent, which inserts it unconditionally — so
+// an election that authenticates on nothing still ended up with an email address and an IP
+// hash sitting next to the voter's ballot_id.
+describe("Open election roll only stores what the election authenticates on", () => {
+  // voter_access: 'open' + voter_authentication: {} — the "no authentication" mode
+  // (VoterAuthenticationMode 'open_open'). Alice is signed in, so req.user.email is
+  // populated even though the election never asks for it.
+  describe("open_open", () => {
+    var electionId = "";
+
+    test("Create election", async () => {
+      const response = await th.createElection(
+        testInputs.MultiRaceElection,
+        testInputs.user1token
+      );
+      expect(response.statusCode).toBe(200);
+      expect(response.election.settings.voter_authentication).toEqual({});
+      electionId = response.election.election_id;
+      th.testComplete();
+    });
+
+    test("Signed-in voter can cast a ballot", async () => {
+      const response = await th.submitBallot(
+        electionId,
+        testInputs.MultiRaceBallotValid2,
+        testInputs.user1token
+      );
+      expect(response.statusCode).toBe(200);
+      th.testComplete();
+    });
+
+    test("Roll records the ballot but neither the email nor the IP hash", async () => {
+      const rolls = await rollsFor(electionId);
+      expect(rolls.length).toBe(1);
+      expect(rolls[0].submitted).toBe(true);
+      expect(rolls[0].ballot_id).toBeTruthy();
+      expect(rolls[0].email).toBeFalsy();
+      expect(rolls[0].ip_hash).toBeFalsy();
+      th.testComplete();
+    });
+  });
+
+  // Control: same election with email authentication turned on. Here the email is what
+  // the election authenticates on, so it must still be stored.
+  describe("open + email authentication", () => {
+    var electionId = "";
+    const emailElection = {
+      ...testInputs.MultiRaceElection,
+      settings: {
+        ...testInputs.MultiRaceElection.settings,
+        voter_authentication: { email: true },
+      } as ElectionSettings,
+    } as Election;
+
+    test("Create election", async () => {
+      const response = await th.createElection(emailElection, testInputs.user1token);
+      expect(response.statusCode).toBe(200);
+      electionId = response.election.election_id;
+      th.testComplete();
+    });
+
+    test("Signed-in voter can cast a ballot", async () => {
+      const response = await th.submitBallot(
+        electionId,
+        testInputs.MultiRaceBallotValid2,
+        testInputs.user1token
+      );
+      expect(response.statusCode).toBe(200);
+      th.testComplete();
+    });
+
+    test("Roll still records the email", async () => {
+      const rolls = await rollsFor(electionId);
+      expect(rolls.length).toBe(1);
+      expect(rolls[0].email).toEqual("Alice@email.com");
+      th.testComplete();
+    });
+  });
+
+  // Control: IP-address authentication is what Election1 uses, and it's the only mode
+  // that should produce an ip_hash.
+  describe("open + ip_address authentication", () => {
+    var electionId = "";
+    const ipElection = {
+      ...testInputs.MultiRaceElection,
+      settings: {
+        ...testInputs.MultiRaceElection.settings,
+        voter_authentication: { ip_address: true },
+      } as ElectionSettings,
+    } as Election;
+
+    test("Create election", async () => {
+      const response = await th.createElection(ipElection, testInputs.user1token);
+      expect(response.statusCode).toBe(200);
+      electionId = response.election.election_id;
+      th.testComplete();
+    });
+
+    test("Signed-in voter can cast a ballot", async () => {
+      const response = await th.submitBallot(
+        electionId,
+        testInputs.MultiRaceBallotValid2,
+        testInputs.user1token
+      );
+      expect(response.statusCode).toBe(200);
+      th.testComplete();
+    });
+
+    test("Roll records the IP hash but not the email", async () => {
+      const rolls = await rollsFor(electionId);
+      expect(rolls.length).toBe(1);
+      expect(rolls[0].ip_hash).toBeTruthy();
+      expect(rolls[0].email).toBeFalsy();
+      th.testComplete();
+    });
+  });
+});