mirror of
https://github.com/PR0M3TH3AN/SeedPass.git
synced 2026-11-05 20:32:05 +00:00
fix(python): TOTP codes must honor the entry's period and digits
TotpManager built pyotp.TOTP(secret) with library defaults (6 digits, 30s) and ignored what the entry recorded, so any imported or derived entry using other values produced codes the issuing service rejects. The service was configured from the entry's values -- those govern the code. - current_code_from_secret and current_code take period/digits - EntryManager.get_totp_code passes the entry's values for both the derived and imported paths - display_service, menu_handler (totp_list now carries digits) and the GUI updated at every call site - regression tests for imported and deterministic entries with custom parameters, asserting against pyotp with the matching interval/digits, and confirming default entries are byte-identical to before Found by scripts/cross_impl_check.py, which flagged it as a divergence against the TypeScript port. That allowance is now removed: the suite asserts TOTP equality and reports 30/30 with zero divergences. Co-Authored-By: Claude Fable 5 <[email protected]>
This commit is contained in:
1 parent
b4f20d417a
commit
5dcd779a6f
9 files changed
+132
-62
No files matched your search
@@ -87,8 +87,11 @@ class DisplayService:
|
||||
self.manager, "parent_seed", None
|
||||
)
|
||||
secret = self.manager.entry_manager.get_totp_secret(index, key)
|
||||
digits = int(entry.get("digits", 6))
|
||||
while True:
|
||||
code = TotpManager.current_code_from_secret(secret)
|
||||
code = TotpManager.current_code_from_secret(
|
||||
secret, period=period, digits=digits
|
||||
)
|
||||
if self.manager.secret_mode_enabled:
|
||||
if self._copy_to_clipboard(code):
|
||||
print(
|
||||
|
||||
@@ -1059,12 +1059,20 @@ class EntryManager:
|
||||
etype != EntryType.TOTP.value and kind != EntryType.TOTP.value
|
||||
):
|
||||
raise ValueError("Entry is not a TOTP entry")
|
||||
# The entry's own period/digits govern the code; the issuing service
|
||||
# was configured from those values, not from pyotp's defaults.
|
||||
period = int(entry.get("period", 30))
|
||||
digits = int(entry.get("digits", 6))
|
||||
if entry.get("deterministic", False) or "secret" not in entry:
|
||||
if parent_seed is None:
|
||||
raise ValueError("Seed required for derived TOTP")
|
||||
totp_index = int(entry.get("index", 0))
|
||||
return TotpManager.current_code(parent_seed, totp_index, timestamp)
|
||||
return TotpManager.current_code_from_secret(entry["secret"], timestamp)
|
||||
return TotpManager.current_code(
|
||||
parent_seed, totp_index, timestamp, period=period, digits=digits
|
||||
)
|
||||
return TotpManager.current_code_from_secret(
|
||||
entry["secret"], timestamp, period=period, digits=digits
|
||||
)
|
||||
|
||||
def get_totp_time_remaining(self, index: int) -> int:
|
||||
"""Return seconds remaining in the TOTP period for the given entry."""
|
||||
|
||||
@@ -157,15 +157,18 @@ class MenuHandler:
|
||||
)
|
||||
data = pm.entry_manager.vault.load_index()
|
||||
entries = data.get("entries", {})
|
||||
totp_list: list[tuple[str, int, int, bool]] = []
|
||||
# digits travels with period: both govern the code the issuing
|
||||
# service expects, and neither may fall back to pyotp defaults.
|
||||
totp_list: list[tuple[str, int, int, bool, int]] = []
|
||||
for idx_str, entry in entries.items():
|
||||
if pm._entry_type_str(entry) == EntryType.TOTP.value and not entry.get(
|
||||
"archived", entry.get("blacklisted", False)
|
||||
):
|
||||
label = entry.get("label", "")
|
||||
period = int(entry.get("period", 30))
|
||||
digits = int(entry.get("digits", 6))
|
||||
imported = "secret" in entry
|
||||
totp_list.append((label, int(idx_str), period, imported))
|
||||
totp_list.append((label, int(idx_str), period, imported, digits))
|
||||
|
||||
if not totp_list:
|
||||
pm.notify("No 2FA entries found.", level="WARNING")
|
||||
@@ -175,7 +178,7 @@ class MenuHandler:
|
||||
|
||||
totp_secrets: dict[int, str] = {}
|
||||
key = getattr(pm, "KEY_TOTP_DET", None) or getattr(pm, "parent_seed", None)
|
||||
for _, idx, _, _ in totp_list:
|
||||
for _, idx, _, _, _ in totp_list:
|
||||
try:
|
||||
totp_secrets[idx] = pm.entry_manager.get_totp_secret(idx, key)
|
||||
except Exception:
|
||||
@@ -196,10 +199,12 @@ class MenuHandler:
|
||||
imported_list = [t for t in totp_list if t[3]]
|
||||
if generated:
|
||||
print(colored("\nGenerated 2FA Codes:", "green"))
|
||||
for label, idx, period, _ in generated:
|
||||
for label, idx, period, _, digits in generated:
|
||||
secret = totp_secrets.get(idx)
|
||||
code = (
|
||||
TotpManager.current_code_from_secret(secret)
|
||||
TotpManager.current_code_from_secret(
|
||||
secret, period=period, digits=digits
|
||||
)
|
||||
if secret
|
||||
else "ERROR"
|
||||
)
|
||||
@@ -219,10 +224,12 @@ class MenuHandler:
|
||||
)
|
||||
if imported_list:
|
||||
print(colored("\nImported 2FA Codes:", "green"))
|
||||
for label, idx, period, _ in imported_list:
|
||||
for label, idx, period, _, digits in imported_list:
|
||||
secret = totp_secrets.get(idx)
|
||||
code = (
|
||||
TotpManager.current_code_from_secret(secret)
|
||||
TotpManager.current_code_from_secret(
|
||||
secret, period=period, digits=digits
|
||||
)
|
||||
if secret
|
||||
else "ERROR"
|
||||
)
|
||||
|
||||
@@ -32,19 +32,35 @@ class TotpManager:
|
||||
|
||||
@classmethod
|
||||
def current_code(
|
||||
cls, seed: Union[str, bytes], index: int, timestamp: int | None = None
|
||||
cls,
|
||||
seed: Union[str, bytes],
|
||||
index: int,
|
||||
timestamp: int | None = None,
|
||||
*,
|
||||
period: int = 30,
|
||||
digits: int = 6,
|
||||
) -> str:
|
||||
"""Return the TOTP code for the given seed/key and index."""
|
||||
"""Return the TOTP code for the given seed/key and index.
|
||||
|
||||
``period`` and ``digits`` must be the values recorded on the entry.
|
||||
Falling back to pyotp's defaults produces codes the issuing service
|
||||
rejects whenever an entry uses anything else.
|
||||
"""
|
||||
secret = cls.derive_secret(seed, index)
|
||||
totp = pyotp.TOTP(secret)
|
||||
if timestamp is None:
|
||||
return totp.now()
|
||||
return totp.at(timestamp)
|
||||
return cls.current_code_from_secret(
|
||||
secret, timestamp, period=period, digits=digits
|
||||
)
|
||||
|
||||
@staticmethod
|
||||
def current_code_from_secret(secret: str, timestamp: int | None = None) -> str:
|
||||
def current_code_from_secret(
|
||||
secret: str,
|
||||
timestamp: int | None = None,
|
||||
*,
|
||||
period: int = 30,
|
||||
digits: int = 6,
|
||||
) -> str:
|
||||
"""Return the TOTP code for a raw secret."""
|
||||
totp = pyotp.TOTP(secret)
|
||||
totp = pyotp.TOTP(secret, interval=int(period), digits=int(digits))
|
||||
return totp.now() if timestamp is None else totp.at(timestamp)
|
||||
|
||||
@staticmethod
|
||||
|
||||
@@ -403,6 +403,7 @@ class TotpViewerWindow(toga.Window):
|
||||
entry = self.entries.retrieve_entry(idx)
|
||||
|
||||
period = int(entry.get("period", 30)) if entry else 30
|
||||
digits = int(entry.get("digits", 6)) if entry else 6
|
||||
remaining = TotpManager.time_remaining(period)
|
||||
if hasattr(self.entries, "get_totp_secret"):
|
||||
if idx in self._secret_cache:
|
||||
@@ -414,7 +415,9 @@ class TotpViewerWindow(toga.Window):
|
||||
except Exception:
|
||||
# Skip entries where secret cannot be derived (e.g. no seed)
|
||||
continue
|
||||
code = TotpManager.current_code_from_secret(secret)
|
||||
code = TotpManager.current_code_from_secret(
|
||||
secret, period=period, digits=digits
|
||||
)
|
||||
else:
|
||||
# Compatibility path for services exposing code retrieval directly.
|
||||
code = self.entries.get_totp_code(idx)
|
||||
|
||||
@@ -99,3 +99,63 @@ def test_legacy_deterministic_entry(tmp_path):
|
||||
|
||||
exported = em.export_totp_entries(TEST_SEED)
|
||||
assert exported["entries"][0]["secret"] == TotpManager.derive_secret(TEST_SEED, 0)
|
||||
|
||||
|
||||
def test_totp_code_honors_entry_period_and_digits(tmp_path):
|
||||
"""An entry's recorded period/digits govern its code.
|
||||
|
||||
Regression: codes were generated with pyotp's defaults (6 digits, 30s)
|
||||
regardless of what the entry recorded, so any imported 8-digit or
|
||||
45-second secret produced codes the issuing service rejects. Found by
|
||||
scripts/cross_impl_check.py comparing against the TypeScript port.
|
||||
"""
|
||||
import pyotp
|
||||
|
||||
from helpers import TEST_PASSWORD, TEST_SEED, create_vault
|
||||
from seedpass.core.backup import BackupManager
|
||||
from seedpass.core.config_manager import ConfigManager
|
||||
from seedpass.core.entry_management import EntryManager
|
||||
|
||||
vault, _enc = create_vault(tmp_path, TEST_SEED, TEST_PASSWORD)
|
||||
cfg = ConfigManager(vault, tmp_path)
|
||||
entry_mgr = EntryManager(vault, BackupManager(tmp_path, cfg))
|
||||
|
||||
secret = "JBSWY3DPEHPK3PXP"
|
||||
timestamp = 1700000000
|
||||
|
||||
idx = entry_mgr.get_next_index()
|
||||
entry_mgr.add_totp("custom", secret=secret, period=45, digits=8)
|
||||
code = entry_mgr.get_totp_code(idx, timestamp=timestamp)
|
||||
expected = pyotp.TOTP(secret, interval=45, digits=8).at(timestamp)
|
||||
assert code == expected
|
||||
assert len(code) == 8
|
||||
|
||||
# Defaults still behave exactly as before
|
||||
idx_default = entry_mgr.get_next_index()
|
||||
entry_mgr.add_totp("plain", secret=secret)
|
||||
default_code = entry_mgr.get_totp_code(idx_default, timestamp=timestamp)
|
||||
assert default_code == pyotp.TOTP(secret).at(timestamp)
|
||||
assert len(default_code) == 6
|
||||
|
||||
|
||||
def test_deterministic_totp_code_honors_entry_period_and_digits(tmp_path):
|
||||
"""The same rule applies to derived (deterministic) TOTP entries."""
|
||||
import pyotp
|
||||
|
||||
from helpers import TEST_PASSWORD, TEST_SEED, create_vault
|
||||
from seedpass.core.backup import BackupManager
|
||||
from seedpass.core.config_manager import ConfigManager
|
||||
from seedpass.core.entry_management import EntryManager
|
||||
from seedpass.core.totp import TotpManager
|
||||
|
||||
vault, _enc = create_vault(tmp_path, TEST_SEED, TEST_PASSWORD)
|
||||
cfg = ConfigManager(vault, tmp_path)
|
||||
entry_mgr = EntryManager(vault, BackupManager(tmp_path, cfg))
|
||||
|
||||
idx = entry_mgr.get_next_index()
|
||||
entry_mgr.add_totp("derived", TEST_SEED, deterministic=True, period=60, digits=8)
|
||||
timestamp = 1700000000
|
||||
code = entry_mgr.get_totp_code(idx, TEST_SEED, timestamp=timestamp)
|
||||
derived_secret = TotpManager.derive_secret(TEST_SEED, 0)
|
||||
assert code == pyotp.TOTP(derived_secret, interval=60, digits=8).at(timestamp)
|
||||
assert len(code) == 8
|
||||
Reference in new issue
Block a user