Add comprehensive plan for token-based voter login#452
Add comprehensive plan for token-based voter login#452
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive support for token-based voter authentication as an alternative to the traditional voter ID + password system. The implementation allows election administrators to choose between the two authentication methods via a new use_token_auth flag, with backward compatibility for existing elections.
Key changes:
- Adds
voting_tokenfield to Voter model anduse_token_authflag to Election model - Implements conditional authentication logic in login views to support both token-based and password-based authentication
- Updates email templates and UI to display appropriate authentication fields based on election configuration
- Includes new tests for token generation and uniqueness validation
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| helios/models.py | Adds use_token_auth boolean field to Election model, voting_token field to Voter model with unique constraint, and generate_voting_token() method. Updates VoterFile.process to conditionally generate tokens or passwords. |
| helios/migrations/0008_alter_voter_unique_together_election_use_token_auth_and_more.py | Database migration adding the new fields and updating unique_together constraint to include voting_token. |
| helios/views.py | Updates password_voter_login() to handle both token-based and password-based authentication with conditional logic based on election.use_token_auth. |
| helios/forms.py | Modifies VoterPasswordForm to include optional voting_token field alongside existing voter_id and password fields, all marked as not required. |
| helios/templates/_castconfirm_password.html | Adds conditional rendering logic to show either token input field or voter ID/password fields based on election authentication method. |
| helios/templates/email/vote_body.txt | Updates voter notification email to conditionally display voting token or voter ID/password based on election settings. |
| helios/templates/email/password_resend_body.txt | Updates credential resend email to show appropriate authentication information based on election configuration. |
| helios/templates/password_voter_resend.html | Updates resend credentials page with conditional text for token vs password authentication. |
| helios/tests.py | Adds three new test cases for token voter creation, token generation constraints, and token uniqueness per election. |
| TOKEN_LOGIN_PLAN.md | Comprehensive planning document detailing the implementation strategy, changes, migration safety, and implementation order for the token-based authentication feature. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Token-based authentication | ||
| if election.use_token_auth: | ||
| voting_token = password_login_form.cleaned_data.get('voting_token', '').strip() | ||
| if not voting_token: | ||
| raise Voter.DoesNotExist("No token provided") | ||
| voter = election.voter_set.get(voting_token=voting_token) | ||
| # Password-based authentication (legacy) | ||
| else: | ||
| voter_id = password_login_form.cleaned_data.get('voter_id', '').strip() | ||
| password = password_login_form.cleaned_data.get('password', '').strip() | ||
| if not voter_id or not password: | ||
| raise Voter.DoesNotExist("No voter_id or password provided") | ||
| voter = election.voter_set.get(voter_login_id=voter_id, voter_password=password) |
There was a problem hiding this comment.
The token-based authentication flow lacks test coverage for the login view integration. While tests exist for token generation and uniqueness, there are no tests verifying the actual login flow using a token through the password_voter_login view. Consider adding tests that verify successful token-based login, failed token-based login with invalid token, and that the correct authentication path is taken based on election.use_token_auth.
helios/views.py
Outdated
| raise Voter.DoesNotExist("No token provided") | ||
| voter = election.voter_set.get(voting_token=voting_token) | ||
| # Password-based authentication (legacy) | ||
| else: | ||
| voter_id = password_login_form.cleaned_data.get('voter_id', '').strip() | ||
| password = password_login_form.cleaned_data.get('password', '').strip() | ||
| if not voter_id or not password: | ||
| raise Voter.DoesNotExist("No voter_id or password provided") |
There was a problem hiding this comment.
The error messages passed to Voter.DoesNotExist are not user-facing and won't be displayed to voters. However, these messages ("No token provided", "No voter_id or password provided") could be logged or exposed in debug mode. The current implementation raises DoesNotExist with a custom message, but Django's DoesNotExist typically doesn't accept arguments. This could cause issues or confusing error messages. Consider logging these conditions separately and raising DoesNotExist without arguments.
| raise Voter.DoesNotExist("No token provided") | |
| voter = election.voter_set.get(voting_token=voting_token) | |
| # Password-based authentication (legacy) | |
| else: | |
| voter_id = password_login_form.cleaned_data.get('voter_id', '').strip() | |
| password = password_login_form.cleaned_data.get('password', '').strip() | |
| if not voter_id or not password: | |
| raise Voter.DoesNotExist("No voter_id or password provided") | |
| logging.warning( | |
| "Password voter login failed for election %s: no voting token provided.", | |
| election.uuid | |
| ) | |
| raise Voter.DoesNotExist() | |
| voter = election.voter_set.get(voting_token=voting_token) | |
| # Password-based authentication (legacy) | |
| else: | |
| voter_id = password_login_form.cleaned_data.get('voter_id', '').strip() | |
| password = password_login_form.cleaned_data.get('password', '').strip() | |
| if not voter_id or not password: | |
| logging.warning( | |
| "Password voter login failed for election %s: missing voter_id or password.", | |
| election.uuid | |
| ) | |
| raise Voter.DoesNotExist() |
| {% else %} | ||
| <input type="submit" class="button" value="log in" /> | ||
| {# Password-based authentication (legacy) #} | ||
| {{password_login_form.as_p}} |
There was a problem hiding this comment.
The template uses password_login_form.as_p for legacy authentication, which will render all form fields including the voting_token field (since it's part of the form). This could expose the token field to password-based elections. While it won't be visible if properly styled, it's better practice to explicitly render only the fields needed for password authentication (voter_id and password) rather than using as_p.
| {{password_login_form.as_p}} | |
| <p> | |
| <label for="{{ password_login_form.voter_id.id_for_label }}"> | |
| {{ password_login_form.voter_id.label }}: | |
| </label><br /> | |
| {{ password_login_form.voter_id }} | |
| </p> | |
| <p> | |
| <label for="{{ password_login_form.password.id_for_label }}"> | |
| {{ password_login_form.password.label }}: | |
| </label><br /> | |
| {{ password_login_form.password }} | |
| </p> |
| voter_password = models.CharField(max_length = 100, null=True) | ||
|
|
||
| # token-based authentication (alternative to voter_login_id + voter_password) | ||
| voting_token = models.CharField(max_length = 100, null=True) |
There was a problem hiding this comment.
The voting_token field should have a database index for performance. The authentication query filters by voting_token on every login attempt, which will perform a full table scan without an index. As the number of voters grows, this could significantly slow down authentication. Consider adding db_index=True to the voting_token field definition or creating an index in the migration.
| voting_token = models.CharField(max_length = 100, null=True) | |
| voting_token = models.CharField(max_length = 100, null=True, db_index=True) |
TOKEN_LOGIN_PLAN.md
Outdated
|
|
||
| **Election Model** (`helios/models.py`, Election class, line ~300): | ||
| ```python | ||
| use_token_auth = models.BooleanField(default=True) |
There was a problem hiding this comment.
The implementation of use_token_auth uses default=False in the migration and models.py, but the plan document at line 19 specifies default=True. This inconsistency could cause confusion. The actual implementation (default=False) is safer for backward compatibility with existing elections, but this discrepancy between the plan and implementation should be reconciled. Consider updating the plan document to match the implementation or adding a comment explaining why the default differs from the plan.
TOKEN_LOGIN_PLAN.md
Outdated
| def generate_voting_token(self, length=20): | ||
| """Generate a 20-character voting token""" | ||
| if self.voting_token: | ||
| raise Exception("voting token already exists") | ||
|
|
||
| self.voting_token = utils.random_string( | ||
| length, | ||
| alphabet='abcdefghjkmnopqrstuvwxyzABCDEFGHJKLMNPQRSTUVWXYZ23456789' | ||
| ) |
There was a problem hiding this comment.
The plan proposes storing voting_token as a raw, randomly generated string in the database and using it directly for authentication lookups. If the database (or a backup/log) is compromised, an attacker can immediately use these plaintext tokens to impersonate voters and cast ballots without any additional cracking effort. Treat voting_token as a password-equivalent by storing only a strong, salted hash in the model and performing authentication/uniqueness checks on the hashed value rather than the raw token.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
docs/2026-01-04-voter-tokens.md
Outdated
| - `password_voter_login()`: Branches authentication logic based on `election.use_token_auth` | ||
|
|
||
| **Forms:** | ||
| - `VoterPasswordForm`: Contains fields for both methods (all optional) |
There was a problem hiding this comment.
The documentation incorrectly states that "VoterPasswordForm: Contains fields for both methods (all optional)". This is not accurate based on the implementation. The code uses two separate forms: VoterPasswordForm for password-based auth and VoterTokenForm for token-based auth. The form selection is conditional based on election.use_token_auth, not a single form containing all optional fields.
| - `VoterPasswordForm`: Contains fields for both methods (all optional) | |
| - `VoterPasswordForm`: Handles traditional voter_login_id + voter_password authentication | |
| - `VoterTokenForm`: Handles single-token authentication when `election.use_token_auth` is True |
benadida
force-pushed
the
claude/token-login-voting-pf4bT
branch
from
81c9285 to
be6a181
Compare
- Add use_token_auth boolean to Election model - Keep both voter_login_id/voter_password and voting_token fields - New elections default to token auth, existing elections unchanged - Views/forms/templates adapt based on election setting - Safe migration, no breaking changes
- Add use_token_auth boolean field to Election model (default=True) - Add voting_token field to Voter model for single-token auth - Update unique_together constraint to include (election, voting_token) - Keep existing voter_login_id and voter_password fields for backwards compatibility
- Creates 20-character tokens (vs 10 for passwords) for better security - Uses same character set as passwords (no ambiguous characters) - Raises exception if token already exists (same pattern as generate_password) - Token provides ~117 bits of entropy for strong security
- Check election.use_token_auth to decide which credential type to generate - New elections (use_token_auth=True) get voting tokens - Legacy elections (use_token_auth=False) get voter_login_id + password - Ensures backwards compatibility for in-progress elections
- Modify VoterPasswordForm to accept voting_token (new) and voter_id+password (legacy) - Update password_voter_login() to check election.use_token_auth - Token-based elections: authenticate using voting_token only - Password-based elections: authenticate using voter_login_id + voter_password - Maintains backwards compatibility for existing elections
- Show single voting_token field for token-based elections - Show voter_id + password fields for password-based elections - Update error messages and help text appropriately - Conditional rendering based on election.use_token_auth
- Modify vote_body.txt to show voting_token for token-based elections - Modify password_resend_body.txt to show voting_token for token-based elections - Keep voter_login_id + voter_password for legacy password-based elections - Clear instructions to copy and paste token
- Conditional messaging based on election.use_token_auth - Token-based elections: mention 'voting token' instead of 'password' - Password-based elections: keep original messaging - Update page title to reflect credential type
- test_create_token_voter: verifies token generation works correctly - test_token_uniqueness_per_election: verifies tokens are unique - Tests verify 20-character token length (no dashes) - Tests verify token cannot be generated twice for same voter
- Set default=False for existing elections and tests - New elections can explicitly enable token auth - Maintains backward compatibility with existing voter workflows - Allows gradual migration to token-based authentication
- Remove first AlterUniqueTogether that was redundant - Keep only the final operation that adds voting_token constraint - Original (election, voter_login_id) constraint remains unchanged
…ntication - Move plan to docs/ directory with dated filename - Remove code snippets, focus on architecture and design decisions - Document security analysis, migration strategy, and deployment considerations - Remove old TOKEN_LOGIN_PLAN.md file
- Merge Migration Strategy and Backward Compatibility sections - Consolidate security analysis into single concise section - Remove repetitive statements about dual system support - Streamline component descriptions - Remove redundant Summary and Future Improvements sections - Reduce from 161 lines to 109 lines while retaining all key information
Replace VoterPasswordForm (which had all fields optional) with two dedicated forms: - VoterPasswordForm: For password-based auth (voter_id + password) - VoterTokenForm: For token-based auth (voting_token only) The login view now selects the appropriate form based on election.use_token_auth, simplifying validation logic and making the template cleaner (just renders login_form.as_p). Also add test_token_authentication_functional() to verify end-to-end token authentication flow with multiple voters.
The implementation now uses VoterPasswordForm and VoterTokenForm as separate form classes, rather than a single combined form. Also add reference to the functional test.
Implement new CSV format for token-based elections: token,<email>,<full name> Key features: - Auto-enables use_token_auth when token voters are uploaded - Prevents mixing token and password voters (in same CSV or election) - Simplified format with only email and name (no unique_id needed) - voter_login_id is set to email for token voters Changes: - VoterFile.itervoters(): Parse "token" rows with format validation - VoterFile.process(): Add mixing checks and auto-enable logic - voters_upload.html: Show both token and password formats - Add comprehensive tests for upload scenarios and validation Backward compatible: existing "password" CSV format still works.
After rebasing on master, which added migration 0008_add_election_soft_delete, our token auth migration has been renumbered from 0008 to 0009 and updated to depend on the soft delete migration.
benadida
force-pushed
the
claude/token-login-voting-pf4bT
branch
from
58f8b7d to
065709f
Compare
Changes: - Update _castconfirm_password.html to render explicit form fields instead of using as_p() for better webtest compatibility - Update one_election_cast_confirm view to select appropriate form (VoterTokenForm or VoterPasswordForm) based on election.use_token_auth - Pass login_form to template context for _castconfirm_password.html - Make LDAP imports optional in settings.py (wrapped in try/except) - Comment out python-ldap dependencies in pyproject.toml temporarily Fixes ElectionBlackboxTests.test_do_complete_election test failure. All 179 helios tests now passing.
There's no great reason to require username and password for voters. Just a voting token is enough. This makes voting easier, only one field to copy / paste.