From ff1b1e96a75ec701ae7507d9120b3746a1cba9fe Mon Sep 17 00:00:00 2001 From: Jackson Loper Date: Tue, 4 Aug 2026 13:51:18 -0400 Subject: [PATCH 1/2] Don't store voter emails/IPs on rolls for elections that don't authenticate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit getOrCreateElectionRoll computes `email` and `require_ip_hash` from the election's voter_authentication settings, correctly leaving them null when the election doesn't authenticate on them. But the roll it builds ignored those and copied `req.user.email` and `hashString(req.ip)` in unconditionally. The guard that decides whether to persist the roll does consult the settings, so this looked safe. It isn't: the cast-vote path reaches the "not persisted" branch and hands the roll to CastVoteStore.submitBallotEvent, which inserts whatever it's given. So an open_open election — voter_access 'open', voter_authentication {} , i.e. the mode that authenticates on nothing — still wrote the signed-in voter's email address and an IP hash next to their ballot_id. Use the already-computed values instead. ElectionRoll.ip_hash is documented as "set when voter_authentication.ip_address is enabled"; now it is. Receipt emails are unaffected: castVoteController falls back to extractUserFromRequest(req)?.email when the roll has no email. The mock CastVoteStore only called rollStore.update(), which no-ops when there's no head row to update — exactly the case here — so the whole class of bug was invisible to the suite. It now inserts when there's no head row, which is what the real store does. Co-Authored-By: Claude Opus 5 --- .../src/Controllers/Roll/voterRollUtils.ts | 13 +- .../src/Models/__mocks__/CastVoteStore.ts | 12 +- .../src/test/openElectionRollPrivacy.test.ts | 146 ++++++++++++++++++ 3 files changed, 167 insertions(+), 4 deletions(-) create mode 100644 packages/backend/src/test/openElectionRollPrivacy.test.ts diff --git a/packages/backend/src/Controllers/Roll/voterRollUtils.ts b/packages/backend/src/Controllers/Roll/voterRollUtils.ts index 1b9760117..c33dea07e 100644 --- a/packages/backend/src/Controllers/Roll/voterRollUtils.ts +++ b/packages/backend/src/Controllers/Roll/voterRollUtils.ts @@ -57,11 +57,15 @@ 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: require_ip_hash ?? undefined, submitted: false, state: ElectionRollState.approved, history: history, @@ -73,7 +77,10 @@ export async function getOrCreateElectionRoll(req: IRequest, election: Election, 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() } } } diff --git a/packages/backend/src/Models/__mocks__/CastVoteStore.ts b/packages/backend/src/Models/__mocks__/CastVoteStore.ts index 6dd64547f..c0b07892c 100644 --- a/packages/backend/src/Models/__mocks__/CastVoteStore.ts +++ b/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`); + } } } diff --git a/packages/backend/src/test/openElectionRollPrivacy.test.ts b/packages/backend/src/test/openElectionRollPrivacy.test.ts new file mode 100644 index 000000000..8c1f19ad7 --- /dev/null +++ b/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 { + 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(); + }); + }); +}); From ede5c23d52836c49a6548fe8e8298eaaa2533fe6 Mon Sep 17 00:00:00 2001 From: Jackson Loper Date: Tue, 4 Aug 2026 13:57:10 -0400 Subject: [PATCH 2/2] Rename require_ip_hash -> auth_ip_hash MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The name read as a boolean predicate, but the value is `string | null` — the hash to authenticate against, or null when the election doesn't authenticate on IP. It's passed straight into getElectionRoll's `ip_hash: string | null` parameter. `email` on the next line already follows the value-or-null convention; this now matches. No behavior change. Co-Authored-By: Claude Opus 5 --- .../backend/src/Controllers/Roll/voterRollUtils.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/backend/src/Controllers/Roll/voterRollUtils.ts b/packages/backend/src/Controllers/Roll/voterRollUtils.ts index c33dea07e..14903ced3 100644 --- a/packages/backend/src/Controllers/Roll/voterRollUtils.ts +++ b/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) { @@ -65,12 +67,12 @@ export async function getOrCreateElectionRoll(req: IRequest, election: Election, election_id: String(election.election_id), email: email ?? undefined, voter_id: new_voter_id, - ip_hash: require_ip_hash ?? undefined, + 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`)