From 33b0de862fba37c1c82d76163d446426d003ea42 Mon Sep 17 00:00:00 2001 From: Jonathan Wren Date: Sat, 5 Nov 2022 18:43:39 -0700 Subject: [PATCH] Fix for new test from develop branch There was a new test added for re-encrypting a journal. This updates the refactor to match the old (previously untested) behavior of jrnl. Co-authored-by: Micah Jerome Ellison --- jrnl/commands.py | 9 ++++-- jrnl/encryption/BaseEncryption.py | 3 ++ jrnl/encryption/BasePasswordEncryption.py | 37 ++++++++++++++++++----- jrnl/encryption/Jrnlv2Encryption.py | 6 +++- tests/bdd/features/encrypt.feature | 20 ++++++++++++ tests/lib/given_steps.py | 16 +++++++--- 6 files changed, 75 insertions(+), 16 deletions(-) diff --git a/jrnl/commands.py b/jrnl/commands.py index a52489c6..912b21cc 100644 --- a/jrnl/commands.py +++ b/jrnl/commands.py @@ -107,13 +107,16 @@ def postconfig_encrypt( ) # If journal is encrypted, create new password + logging.debug("Clearing encryption method...") + if journal.config["encrypt"] is True: logging.debug("Journal already encrypted. Re-encrypting...") print(f"Journal {journal.name} is already encrypted. Create a new password.") + journal.encryption_method.clear() + else: + journal.config["encrypt"] = True + journal.encryption_method = None - logging.debug("Clearing encryption method...") - journal.encryption_method = None - journal.config["encrypt"] = True journal.write(args.filename) print_msg( diff --git a/jrnl/encryption/BaseEncryption.py b/jrnl/encryption/BaseEncryption.py index c69ddfb7..8fc0fa76 100644 --- a/jrnl/encryption/BaseEncryption.py +++ b/jrnl/encryption/BaseEncryption.py @@ -17,6 +17,9 @@ class BaseEncryption(ABC): self._journal_name: str = journal_name self._config: dict = config + def clear(self) -> None: + pass + def encrypt(self, text: str) -> bytes: logging.debug("encrypting") return self._encrypt(text) diff --git a/jrnl/encryption/BasePasswordEncryption.py b/jrnl/encryption/BasePasswordEncryption.py index 4defdf53..f2642263 100644 --- a/jrnl/encryption/BasePasswordEncryption.py +++ b/jrnl/encryption/BasePasswordEncryption.py @@ -19,30 +19,51 @@ class BasePasswordEncryption(BaseEncryption): self._attempts: int = 0 self._max_attempts: int = 3 self._password: str = "" - - # Check keyring first for password. - # That way we'll have it. - if keyring_pw := get_keyring_password(self._journal_name): - self.password = keyring_pw + self._check_keyring: bool = True @property - def password(self) -> str: + def check_keyring(self) -> bool: + return self._check_keyring + + @check_keyring.setter + def check_keyring(self, value: bool) -> None: + self._check_keyring = value + + @property + def password(self) -> str | None: return self._password @password.setter def password(self, value: str) -> None: self._password = value + def clear(self): + self.password = None + self.check_keyring = False + def encrypt(self, text: str) -> bytes: logging.debug("encrypting") if not self.password: - self.password = create_password(self._journal_name) + if self.check_keyring and ( + keyring_pw := get_keyring_password(self._journal_name) + ): + self.password = keyring_pw + + if not self.password: + self.password = create_password(self._journal_name) + return self._encrypt(text) def decrypt(self, text: bytes) -> str: logging.debug("decrypting") if not self.password: - self._prompt_password() + if self.check_keyring and ( + keyring_pw := get_keyring_password(self._journal_name) + ): + self.password = keyring_pw + + if not self.password: + self._prompt_password() while (result := self._decrypt(text)) is None: self._prompt_password() diff --git a/jrnl/encryption/Jrnlv2Encryption.py b/jrnl/encryption/Jrnlv2Encryption.py index aae10a37..c78c5412 100644 --- a/jrnl/encryption/Jrnlv2Encryption.py +++ b/jrnl/encryption/Jrnlv2Encryption.py @@ -26,11 +26,15 @@ class Jrnlv2Encryption(BasePasswordEncryption): return self._password @password.setter - def password(self, value): + def password(self, value: str | None): self._password = value self._make_key() def _make_key(self) -> None: + if self._password is None: + # Password was removed after being set + self._key = None + return password = self.password.encode(self._encoding) kdf = PBKDF2HMAC( algorithm=hashes.SHA256(), diff --git a/tests/bdd/features/encrypt.feature b/tests/bdd/features/encrypt.feature index b5ef1126..8442ca66 100644 --- a/tests/bdd/features/encrypt.feature +++ b/tests/bdd/features/encrypt.feature @@ -48,6 +48,7 @@ Feature: Encrypting and decrypting journals Scenario: Encrypt journal twice and get prompted each time Given we use the config "simple.yaml" + And we don't have a keyring When we run "jrnl --encrypt" and enter swordfish swordfish @@ -56,7 +57,26 @@ Feature: Encrypting and decrypting journals And the output should contain "Journal encrypted" When we run "jrnl --encrypt" and enter swordfish + tuna + tuna + y + Then we should get no error + And the output should contain "Journal default is already encrypted. Create a new password." + And we should be prompted for a password + And the config for journal "default" should contain "encrypt: true" + + Scenario: Encrypt journal twice and get prompted each time with keyring + Given we use the config "simple.yaml" + And we have a keyring + When we run "jrnl --encrypt" and enter swordfish + swordfish + y + Then we should get no error + And the output should contain "Journal encrypted" + When we run "jrnl --encrypt" and enter + tuna + tuna y Then we should get no error And the output should contain "Journal default is already encrypted. Create a new password." diff --git a/tests/lib/given_steps.py b/tests/lib/given_steps.py index 5ee88ab7..f0c691d1 100644 --- a/tests/lib/given_steps.py +++ b/tests/lib/given_steps.py @@ -17,6 +17,7 @@ from pytest_bdd.parsers import parse from jrnl import __version__ from jrnl.time import __get_pdt_calendar from tests.lib.fixtures import FailedKeyring +from tests.lib.fixtures import NoKeyring from tests.lib.fixtures import TestKeyring from tests.lib.helpers import get_fixture @@ -67,13 +68,20 @@ def now_is_str(date_str, mock_factories): ) +@given("we don't have a keyring", target_fixture="keyring") +def we_dont_have_keyring(keyring_type): + return NoKeyring() + + @given("we have a keyring", target_fixture="keyring") @given(parse("we have a {keyring_type} keyring"), target_fixture="keyring") def we_have_type_of_keyring(keyring_type): - if keyring_type == "failed": - return FailedKeyring() - else: - return TestKeyring() + match keyring_type: + case "failed": + return FailedKeyring() + + case _: + return TestKeyring() @given(parse('we use the config "{config_file}"'), target_fixture="config_path")