feat: Add TOTP/2FA support for credential management#58
Merged
Conversation
Implement TOTP (Time-based One-Time Password) functionality per RFC 6238: Core TOTP Implementation (internal/vault/totp.go): - ParseTOTPURI: Parse otpauth:// URIs or raw base32 secrets - ValidateTOTPSecret: Validate base32 format - GenerateTOTPCode: Generate 6/8 digit codes with SHA1/SHA256/SHA512 - Credential methods: HasTOTP(), GetTOTPCode(), SetTOTPFromURI(), ClearTOTP() CLI Commands Updated: - add: --totp-uri and --totp flags for adding TOTP during credential creation - get: --totp flag to display TOTP code with countdown and clipboard support - update: --totp-uri and --clear-totp flags for modifying TOTP configuration Vault Service: - Extended Credential struct with TOTP fields (secret, algorithm, digits, period, issuer) - Extended UpdateOpts with TOTP fields and ClearTOTP boolean - Added GetTOTPCode() method with audit logging - Extended CredentialMetadata with HasTOTP and TOTPIssuer Security/Audit: - Added TOTP-specific audit events: EventTOTPAccess, EventTOTPAdd, EventTOTPUpdate, EventTOTPClear - Audit logging integrated into TOTP operations Dependencies: - Added github.com/pquerna/otp v1.5.0 for RFC 6238 TOTP implementation Tests: - Added comprehensive unit tests for TOTP functionality (18 tests) - All existing tests pass Generated with Claude Code Co-Authored-By: Claude <[email protected]>
- Add GetTOTPCode() to VaultService interface and AppState - Add TOTP display in DetailView (shows issuer when available) - Add 't' key binding to copy TOTP code to clipboard - Update help dialog with TOTP shortcut - Update test mocks with GetTOTPCode method - Add QR code scanning support in CLI get command Phase 3 (TUI Integration) of TOTP/2FA support - in progress. Generated with Claude Code Co-Authored-By: Claude <[email protected]>
- Add GetTOTPCode to testVaultService in detail_test.go - Add GetTOTPCode to MockVaultService in state_test.go Completes TUI test mock updates for TOTP integration. Generated with Claude Code Co-Authored-By: Claude <[email protected]>
- Add TOTP Secret/URI input field to AddForm - Add TOTP Secret/URI input field to EditForm with status indicator - Add Clear TOTP checkbox to EditForm (shown when credential has TOTP) - Update AppState UpdateCredentialOpts with TOTP fields - Parse TOTP input using vault.ParseTOTPURI on form submission - Support both base32 secrets and otpauth:// URIs Completes Phase 3 TUI Integration for TOTP/2FA support. Generated with Claude Code Co-Authored-By: Claude <[email protected]>
- Add TOTP/2FA Support to Key Features list - Remove TOTP from Roadmap (feature is shipped) Generated with Claude Code Co-Authored-By: Claude <[email protected]>
- Add --totp, --totp-uri flags to add command documentation - Add --totp, --totp-qr, --totp-qr-file flags to get command documentation - Add --totp-uri, --clear-totp flags to update command documentation - Add TOTP usage examples for CLI commands - Add 't' key shortcut to TUI Actions table Generated with Claude Code Co-Authored-By: Claude <[email protected]>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Collaborator
Author
|
@greptile can you take a look and see what's causing the errors and issues |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
- Add bounds check for key.Period() conversion (max 300 seconds) - Add bounds check for cred.TOTPPeriod before conversion - Add #nosec G115 directives with justification for bounded conversions - Fix errcheck: properly ignore resp.Body.Close() error - Fix staticcheck: refactor base32 validation for clarity Addresses CodeQL/gosec security warnings for uint64->int conversions. Generated with Claude Code Co-Authored-By: Claude <[email protected]>
arimxyer
force-pushed
the
feature/totp-support
branch
from
43e3be1 to
46e29ac
Compare
The previous implementation used totp.Generate() which re-encoded the already base32-encoded secret, causing QR codes to contain a different secret than what's stored and used for code generation. This fix builds the otpauth:// URI manually, using the stored secret directly without re-encoding. This ensures that: - QR codes scanned by authenticator apps generate matching codes - The secret in the URI matches the secret used for code generation Added comprehensive tests to verify secret consistency between URI generation and TOTP code generation. Generated with Claude Code Co-Authored-By: Claude <[email protected]>
Addresses issues identified in Oracle code review: - Use %20 for spaces instead of + in query params for max authenticator app compatibility - Fix test flakiness by using same timestamp for both code generations - Add validation for algorithm (SHA1/SHA256/SHA512 only) - Add validation for digits (6 or 8 only) - Add validation for period (1-300 seconds range) - Add error when both Service and Username are empty (no account identity) - Normalize secret (trim whitespace, uppercase) before building URI - Add comprehensive tests for all edge cases Generated with Claude Code Co-Authored-By: Claude <[email protected]>
- New comprehensive TOTP & 2FA guide (docs/02-guides/totp-guide.md) - Explains how Service/Username map to authenticator app display - Documents fallback behavior and best practices - Added tips in command-reference.md for TOTP URI labeling - Updated navigation links in index pages Generated with Claude Code Co-Authored-By: Claude <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Features
CLI Commands
pass-cli add --totp- Interactive prompt for TOTP secretpass-cli add --totp-uri- Direct TOTP URI inputpass-cli get --totp- Generate and display TOTP code with countdownpass-cli get --totp-qr- Display QR code in terminal (for adding to another device)pass-cli get --totp-qr-file- Export QR code to PNG filepass-cli update --totp-uri- Update TOTP configurationpass-cli update --clear-totp- Remove TOTP from credentialTUI Integration
tkey binding to copy TOTP code to clipboardCore Functionality
Dependencies Added
github.com/pquerna/otp v1.5.0- TOTP generationgithub.com/mdp/qrterminal/v3 v3.2.1- Terminal QR codesgithub.com/skip2/go-qrcode- PNG QR exportTesting
Linear Issue
Closes ARI-52