Skip to content

fix(backend): prevent double voting #1257

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.

Sign up for GitHub

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

Open
jwidan wants to merge 6 commits into
base: main
Choose a base branch
from
Open

fix(backend): prevent double voting #1257

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
538657c
fix(backend): prevent double voting
jwidan Mar 1, 2026
a4a7c1c
Fix build
jwidan Mar 1, 2026
5c3ff77
fix(backend): Convert CastVoteStore to atomic OCC
jwidan Mar 7, 2026
a74a75e
Merge branch 'Equal-Vote:main' into fix/prevent-double-voting
jwidan Mar 7, 2026
305dbd5
Clean up CastVoteStore database operations
jwidan Mar 7, 2026
38c9f29
refactor: throw distinct errors
jwidan Mar 9, 2026
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
61 changes: 35 additions & 26 deletions packages/backend/src/Controllers/Ballot/castVoteController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,11 @@ import { makeUniqueID, ID_LENGTHS, ID_PREFIXES } from "@equal-vote/star-vote-sha
const ElectionsModel = ServiceLocator.electionsDb();
const ElectionRollModel = ServiceLocator.electionRollDb();
const BallotModel = ServiceLocator.ballotsDb();
import { CastVoteEvent } from "../../Models/CastVoteStore";
const EventQueue = ServiceLocator.eventQueue();
const EmailService = ServiceLocator.emailService();
const AccountService = ServiceLocator.accountService();

type CastVoteEvent = {
requestId:Uid,
inputBallot:Ballot,
roll?:ElectionRoll,
userEmail?:string,
isBallotUpdate: boolean,
}

// NOTE: discord isn't implemented yet, but that's the plan for the future
type BallotSubmitType = 'submitted_via_browser' | 'submitted_via_admin' | 'submitted_via_discord';
Expand Down Expand Up @@ -123,10 +117,10 @@ async function makeBallotEvent(req: IElectionRequest, targetElection: Election,
if(req.election.ballot_source !== 'prior_election') Logger.debug(req, "Submit Ballot:", inputBallot);

return {
requestId:req.contextId ? req.contextId : randomUUID(),
inputBallot,
roll,
userEmail:undefined,
requestId: req.contextId ? req.contextId : randomUUID(),
inputBallot: inputBallot as Ballot,
roll: roll || undefined,
userEmail: undefined,
isBallotUpdate: !!updatableBallot,
}
}
Expand Down Expand Up @@ -205,8 +199,21 @@ async function uploadBallotsController(req: IElectionRequest, res: Response, nex
req,
`Admin submits a ballot for prior election`
)
}else{
await (await EventQueue).publishBatch(castVoteEventQueue, events.filter(event => !('error' in event)));
} else {
const validEvents = events.filter((event: any) => !('error' in event)) as CastVoteEvent[];
const successfullySavedEvents: CastVoteEvent[] = [];
for (const event of validEvents) {
const ctx = Logger.createContext(event.requestId);
try {
await ServiceLocator.castVoteStore().submitBallotEvent(event, ctx);
successfullySavedEvents.push(event);
} catch (e: any) {
Logger.error(req, `Could not upload ballot for ${event.roll?.voter_id}: ${e.message}`);
Copy link
Collaborator

@jacksonloper jacksonloper Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This log isn't visible to an election admin. So, from their point of view, this will fail silently. I suppose we need some ux plan for handling this.

}
}
if (successfullySavedEvents.length > 0) {
await (await EventQueue).publishBatch(castVoteEventQueue, successfullySavedEvents);
}
}
}catch(err: any){
const msg = `Could not upload ballots`;
Expand Down Expand Up @@ -241,6 +248,17 @@ async function castVoteController(req: IElectionRequest, res: Response, next: Ne

event.userEmail = event.roll?.email ?? AccountService.extractUserFromRequest(req)?.email ?? req.body.receiptEmail;

const ctx = Logger.createContext(event.requestId);
try {
await ServiceLocator.castVoteStore().submitBallotEvent(event, ctx);
} catch (e: any) {
if (e.message === "ALREADY_VOTED") {
Logger.info(req, "Ballot Rejected. User has already voted.");
throw new BadRequest("User has already voted");
}
throw e;
}

await (await EventQueue).publish(castVoteEventQueue, event);

if(io != null){ // necessary for tests
Expand All @@ -261,24 +279,15 @@ async function castVoteController(req: IElectionRequest, res: Response, next: Ne
async function handleCastVoteEvent(job: { id: string; data: CastVoteEvent; }):Promise<void> {
const event = job.data;
const ctx = Logger.createContext(event.requestId);
let savedBallot;
if (event.isBallotUpdate) {
savedBallot = await BallotModel.updateBallot(event.inputBallot, ctx, `User updates a ballot`);
} else {
savedBallot = await BallotModel.getBallotByID(event.inputBallot.ballot_id, ctx);
if (!savedBallot){
savedBallot = await BallotModel.submitBallot(event.inputBallot, ctx, `User submits a ballot`);
}
}

if (event.roll != null) {
await ElectionRollModel.update(event.roll, ctx, `User submits a ballot`);
}
if (event.userEmail) {
const targetElection = await ElectionsModel.getElectionByID(event.inputBallot.election_id, ctx);
if (targetElection == null){
throw new InternalServerError("Target Election null: " + ctx.contextId);
}
const savedBallot = await BallotModel.getBallotByID(event.inputBallot.ballot_id, ctx);
if (!savedBallot) {
throw new InternalServerError("Ballot not found: " + event.inputBallot.ballot_id);
}
const url = ServiceLocator.globalData().mainUrl;
const receipt = Receipt(targetElection, event.userEmail, savedBallot, url, event.roll)
await EmailService.sendEmails([receipt])
Expand Down
2 changes: 1 addition & 1 deletion packages/backend/src/Controllers/Election/elections.controllers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ const electionExistsByID = async (req: any, res: any, next: any) => {
res.json({ exists: await ElectionsModel.electionExistsByID(req.params._id, req) })
}

const electionSpecificAuth = async (req: IRequest, res: any, next: any) => {
const electionSpecificAuth = async (req: IElectionRequest, res: any, next: any) => {
if (!req.election){
return next();
}
Expand Down
40 changes: 25 additions & 15 deletions packages/backend/src/Models/Ballots.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,24 +36,30 @@ export default class BallotsDB implements IBallotStore {
return this.makeSubmitBallotsQuery(ballot, ctx, reason, db) as Promise<Ballot>
}

async updateBallot(ballot: Ballot, ctx: ILoggingContext, reason: string): Promise<Ballot> {
async updateBallot(ballot: Ballot, ctx: ILoggingContext, reason: string, db?: Kysely<Database> | Transaction<Database>): Promise<Ballot> {
Logger.debug(ctx, `${tableName}.update`);
return this._postgresClient.transaction().execute( async (tx) => {
const update_response = await tx.updateTable(tableName)
const executeWork = async (activeDb: Kysely<Database> | Transaction<Database>) => {
await activeDb.updateTable(tableName)
.where('ballot_id', '=', ballot.ballot_id)
.where('election_id', '=', ballot.election_id)
.where('head', '=', true)
.set('head', false)
//TODO: replace with DB trigger
.set('update_date', Date.now().toString())
.execute();
return this.submitBallot(ballot, ctx, reason, tx);
});
return this.submitBallot(ballot, ctx, reason, activeDb);
};

if (db) {
return await executeWork(db);
} else {
return await this._postgresClient.transaction().execute(executeWork);
}
}

bulkSubmitBallots(ballots: Ballot[], ctx: ILoggingContext, reason: string): Promise<Ballot[]> {
bulkSubmitBallots(ballots: Ballot[], ctx: ILoggingContext, reason: string, db?: Kysely<Database> | Transaction<Database>): Promise<Ballot[]> {
Logger.debug(ctx, `${tableName}.bulkSubmit`) // removed ballot to make logging less noisy
return this.makeSubmitBallotsQuery(ballots, ctx, reason) as Promise<Ballot[]>
return this.makeSubmitBallotsQuery(ballots, ctx, reason, db) as Promise<Ballot[]>
}

private makeSubmitBallotsQuery(inputBallots: Ballot | Ballot[], ctx: ILoggingContext, reason: string,
Expand Down Expand Up @@ -94,21 +100,23 @@ export default class BallotsDB implements IBallotStore {
}


getBallotsByElectionID(election_id: string, ctx: ILoggingContext): Promise<Ballot[] | null> {
getBallotsByElectionID(election_id: string, ctx: ILoggingContext, db?: Kysely<Database> | Transaction<Database>): Promise<Ballot[] | null> {
Logger.debug(ctx, `${tableName}.getBallotsByElectionID ${election_id}`);
const client = db || this._postgresClient;

return this._postgresClient
return client
.selectFrom(tableName)
.selectAll()
.where('election_id', '=', election_id)
.where('head', '=', true)
.execute();
}

getBallotByVoterID(voter_id: string, election_id: string, ctx: ILoggingContext): Promise<Ballot | undefined> {
getBallotByVoterID(voter_id: string, election_id: string, ctx: ILoggingContext, db?: Kysely<Database> | Transaction<Database>): Promise<Ballot | undefined> {
Logger.debug(ctx, `${tableName}.getBallotByVoterID ${voter_id} ${election_id}`);
const client = db || this._postgresClient;

return this._postgresClient
return client
.selectFrom(tableName)
.innerJoin(electionRollTableName,
(join) => join
Expand All @@ -122,10 +130,11 @@ export default class BallotsDB implements IBallotStore {
.executeTakeFirst();
}

deleteAllBallotsForElectionID(election_id: string, ctx: ILoggingContext): Promise<boolean> {
deleteAllBallotsForElectionID(election_id: string, ctx: ILoggingContext, db?: Kysely<Database> | Transaction<Database>): Promise<boolean> {
Logger.debug(ctx, `${tableName}.deleteAllBallotsForElectionID ${election_id}`);
const client = db || this._postgresClient;

return this._postgresClient
return client
.deleteFrom(tableName)
.where('election_id', '=', election_id)
.returningAll()
Expand All @@ -134,10 +143,11 @@ export default class BallotsDB implements IBallotStore {
.catch(() => false);
}

delete(ballot_id: Uid, ctx: ILoggingContext, reason: string): Promise<boolean> {
delete(ballot_id: Uid, ctx: ILoggingContext, reason: string, db?: Kysely<Database> | Transaction<Database>): Promise<boolean> {
Logger.debug(ctx, `${tableName}.delete ${ballot_id}`);
const client = db || this._postgresClient;

return this._postgresClient
return client
.deleteFrom(tableName)
.where('ballot_id', '=', ballot_id)
.returningAll()
Expand Down
63 changes: 62 additions & 1 deletion packages/backend/src/Models/CastVoteStore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,80 @@ import { Ballot } from "@equal-vote/star-vote-shared/domain_model/Ballot";
import { ElectionRoll } from "@equal-vote/star-vote-shared/domain_model/ElectionRoll";
import { ILoggingContext } from "../Services/Logging/ILogger";
import Logger from "../Services/Logging/Logger";
import { Kysely } from "kysely";
import { Database } from "./Database";
import { Uid } from "@equal-vote/star-vote-shared/domain_model/Uid";
import ServiceLocator from "../ServiceLocator";

export type CastVoteEvent = {
requestId: Uid,
inputBallot: Ballot,
roll?: ElectionRoll,
userEmail?: string,
isBallotUpdate: boolean,
}
var pgFormat = require("pg-format");

export default class CastVoteStore {
_postgresClient;
_db: Kysely<Database>;
_ballotTableName: string;
_rollTableName: string;

constructor(postgresClient: any) {
constructor(postgresClient: any, db: Kysely<Database>) {
this._postgresClient = postgresClient;
this._db = db;
this._ballotTableName = "ballotDB";
this._rollTableName = "electionRollDB";
}

async submitBallotEvent(event: CastVoteEvent, ctx: ILoggingContext): Promise<void> {
return this._db.transaction().execute(async (trx) => {
if (event.roll) {
const currentRoll = await trx.selectFrom('electionRollDB')
.select(['submitted'])
.where('election_id', '=', event.roll.election_id)
.where('voter_id', '=', event.roll.voter_id)
.where('head', '=', true)
.forUpdate()
.executeTakeFirst();

if (currentRoll && currentRoll.submitted && !event.isBallotUpdate) {
throw new Error("ALREADY_VOTED");
}

if (!currentRoll) {
Copy link
Collaborator

@jacksonloper jacksonloper Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we have to do this twice?

Copy link
Collaborator Author

@jwidan jwidan Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

READ COMMITTED makes it necessary: https://www.postgresql.org/docs/current/transaction-iso.html#XACT-READ-COMMITTED

When a transaction uses this isolation level, a SELECT query (without a FOR UPDATE/SHARE clause) sees only data committed before the query began; it never sees either uncommitted data or changes committed by concurrent transactions during the query's execution. In effect, a SELECT query sees a snapshot of the database as of the instant the query begins to run.

To give an example,

Request A & Request B execute a query at roughly the same time. A new data snapshot is made at the start for each of them. Both snapshots see the current active row where head=true.

Request A gets the row lock, B attempts to get it but is blocked by A. B is now in a wait queue.

A finishes, updates the locked row (head=false), inserts a new row (w/ head=true), and commits.

After A releases the lock, B goes to lock the previously modified row (the new row doesn't exist in its snapshot), but the row no longer satisfies B's head=true. B's query returns undefined and interpreted as proof that voter has not voted yet and introduces another race condition where a duplicate ballot could be processed (but the window is probably smaller than what it was before this PR)

The second query makes a new snapshot so it can see the new state made by Request A.

Copy link
Collaborator

@jacksonloper jacksonloper Mar 5, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see. I don't think this will fix the race though, because there are other reasons we update the voter rows, not just to mark submitted. Let's see if I can get this right 😂.

  1. A locks the row for a vote.
  2. B wants to send an email to the voter, but A has the row locked. It waits.
  3. A commits, sets head=false, inserts a new row with head=true submitted=true.
  4. B unblocks. B is confused because the row it was waiting for has vanished (its head is false now). It says "whatever" and inserts a new row with head=true and submitted=false

Now there are two head=true rows 😂 . I have no idea what happens then.

You could certainly argue this is really a problem with the un-transactified-nature of email roll update and similar.

Its also not terribly concerning because realistically for nefariousness it would require coordination of the admin and the voter, and broadly we tend to trust the admins somewhat. Ofc if we add something in the future that the voter could contorl like "request that voter_id is resent to my email" and use the same patterns we have been using, then nefariousness could become a legit problem.

Still... hmm...

// Rescan to catch newly committed rows if a concurrent transaction modified our locked row while we waited.
const doubleCheckRoll = await trx.selectFrom('electionRollDB')
.select(['submitted'])
.where('election_id', '=', event.roll.election_id)
.where('voter_id', '=', event.roll.voter_id)
.where('head', '=', true)
.forUpdate()
.executeTakeFirst();

if (doubleCheckRoll && doubleCheckRoll.submitted && !event.isBallotUpdate) {
throw new Error("ALREADY_VOTED");
}
}
}

const BallotModel = ServiceLocator.ballotsDb();
const ElectionRollModel = ServiceLocator.electionRollDb();

if (event.isBallotUpdate) {
await BallotModel.updateBallot(event.inputBallot, ctx, `User updates a ballot`, trx);
} else {
await BallotModel.submitBallot(event.inputBallot, ctx, `User submits a ballot`, trx);
}

if (event.roll != null) {
event.roll.submitted = true;
await ElectionRollModel.update(event.roll, ctx, `User submits a ballot`, trx);
}
});
}

submitBallot(
Copy link
Collaborator

@jacksonloper jacksonloper Mar 6, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This submitBallot is dead code I guess. And was before your PR if I'm understanding things correctly.

ballot: Ballot,
roll: ElectionRoll,
Expand Down
24 changes: 16 additions & 8 deletions packages/backend/src/Models/ElectionRolls.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { ILoggingContext } from '../Services/Logging/ILogger';
import Logger from '../Services/Logging/Logger';
import { IElectionRollStore } from './IElectionRollStore';
import { Expression, Kysely } from 'kysely'
import { Expression, Kysely, Transaction } from 'kysely'
import { Database } from './Database';
import { ElectionRoll } from '@equal-vote/star-vote-shared/domain_model/ElectionRoll';
const tableName = 'electionRollDB';
Expand Down Expand Up @@ -148,28 +148,36 @@ export default class ElectionRollDB implements IElectionRollStore {
}))
}

update(election_roll: ElectionRoll, ctx: ILoggingContext, reason: string): Promise<ElectionRoll | null> {
async update(election_roll: ElectionRoll, ctx: ILoggingContext, reason: string, db?: Kysely<Database> | Transaction<Database>): Promise<ElectionRoll | null> {
Logger.debug(ctx, `${tableName}.updateRoll`);
Logger.debug(ctx, "", election_roll)
election_roll.update_date = Date.now().toString()
election_roll.head = true
// Transaction to insert updated roll and set old version's head to false
return this._postgresClient.transaction().execute(async (trx) => {
await trx.updateTable(tableName)

const executeWork = async (activeDb: Kysely<Database> | Transaction<Database>) => {
await activeDb.updateTable(tableName)
.where('election_id', '=', election_roll.election_id)
.where('voter_id', '=', election_roll.voter_id)
.where('head', '=', true)
.set('head', false)
.execute()

return await trx.insertInto(tableName)
return await activeDb.insertInto(tableName)
.values(election_roll)
.returningAll()
.executeTakeFirstOrThrow()
}).catch((reason: any) => {
};

try {
if (db) {
return await executeWork(db);
} else {
return await this._postgresClient.transaction().execute(executeWork);
}
} catch (reason: any) {
Logger.debug(ctx, ".get null");
return null;
})
}
}

delete(election_roll: ElectionRoll, ctx: ILoggingContext, reason: string): Promise<boolean> {
Expand Down
18 changes: 10 additions & 8 deletions packages/backend/src/Models/IBallotStore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
import { Ballot } from "@equal-vote/star-vote-shared/domain_model/Ballot";
import { Uid } from "@equal-vote/star-vote-shared/domain_model/Uid";
import { ILoggingContext } from "../Services/Logging/ILogger";
import { Kysely, Transaction } from 'kysely';
import { Database } from './Database';

export interface IBallotStore {
submitBallot: (ballot: Ballot, ctx:ILoggingContext, reason:string) => Promise<Ballot>;
updateBallot: (ballot: Ballot, ctx: ILoggingContext, reason: string) => Promise<Ballot>;
bulkSubmitBallots: (ballots: Ballot[], ctx:ILoggingContext, reason:string) => Promise<Ballot[]>;
getBallotByID: (ballot_id: string, ctx:ILoggingContext) => Promise<Ballot | null>;
getBallotsByElectionID: (election_id: string, ctx:ILoggingContext) => Promise<Ballot[] | null>;
getBallotByVoterID: (voter_id: string, election_id: string, ctx:ILoggingContext) => Promise<Ballot | undefined>;
delete(ballot_id: Uid, ctx:ILoggingContext, reason:string): Promise<boolean>;
deleteAllBallotsForElectionID: (election_id: string, ctx:ILoggingContext) => Promise<boolean>;
submitBallot: (ballot: Ballot, ctx: ILoggingContext, reason: string, db?: Kysely<Database> | Transaction<Database>) => Promise<Ballot>;
updateBallot: (ballot: Ballot, ctx: ILoggingContext, reason: string, db?: Kysely<Database> | Transaction<Database>) => Promise<Ballot>;
bulkSubmitBallots: (ballots: Ballot[], ctx: ILoggingContext, reason: string, db?: Kysely<Database> | Transaction<Database>) => Promise<Ballot[]>;
getBallotByID: (ballot_id: string, ctx: ILoggingContext, db?: Kysely<Database> | Transaction<Database>) => Promise<Ballot | null>;
getBallotsByElectionID: (election_id: string, ctx: ILoggingContext, db?: Kysely<Database> | Transaction<Database>) => Promise<Ballot[] | null>;
getBallotByVoterID: (voter_id: string, election_id: string, ctx: ILoggingContext, db?: Kysely<Database> | Transaction<Database>) => Promise<Ballot | undefined>;
delete(ballot_id: Uid, ctx: ILoggingContext, reason: string, db?: Kysely<Database> | Transaction<Database>): Promise<boolean>;
deleteAllBallotsForElectionID: (election_id: string, ctx: ILoggingContext, db?: Kysely<Database> | Transaction<Database>) => Promise<boolean>;
}
Loading