diff --git a/packages/backend/src/Controllers/Roll/voterRollUtils.ts b/packages/backend/src/Controllers/Roll/voterRollUtils.ts index 1b976011..14903ced 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) { @@ -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() } } } diff --git a/packages/backend/src/Models/__mocks__/CastVoteStore.ts b/packages/backend/src/Models/__mocks__/CastVoteStore.ts index 6dd64547..c0b07892 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 00000000..8c1f19ad --- /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(); + }); + }); +});