Move all password handling to EncryptedJournal

This commit should greatly simplify all password handling logic. No passwords are stored in the config dict anymore. Only the Encrypted Journals have any password related logic. I also had to remove some password fields from the test files, which shows how dangerous the previous approach was. A slight code change could've leaked passwords to the config file. However, I had to change the install progress a little bit to make this work. It will now not ask you for a password right away but rather if you want to encrypt or not. Only if you reply 'y' will it ask you for the password later on.

features/data/configs/bug153.yaml

@@ -6,7 +6,6 @@ highlight: true
 journals:
   default: features/journals/bug153.dayone
 linewrap: 80
-password: ''
 tagsymbols: '@'
 template: false
 timeformat: '%Y-%m-%d %H:%M'

features/data/configs/dayone.yaml

@@ -7,7 +7,6 @@ highlight: true
 journals:
   default: features/journals/dayone.dayone
 linewrap: 80
-password: ''
 tagsymbols: '@'
 timeformat: '%Y-%m-%d %H:%M'
 indent_character: "|"

features/data/configs/empty_folder.yaml

@@ -7,7 +7,6 @@ highlight: true
 journals:
   default: features/journals/empty_folder
 linewrap: 80
-password: ''
 tagsymbols: '@'
 timeformat: '%Y-%m-%d %H:%M'
 indent_character: "|"

features/data/configs/encrypted.yaml

@@ -7,7 +7,6 @@ highlight: true
 journals:
   default: features/journals/encrypted.journal
 linewrap: 80
-password: ''
 tagsymbols: '@'
 timeformat: '%Y-%m-%d %H:%M'
 indent_character: "|"

features/data/configs/markdown-headings-335.yaml

@@ -7,7 +7,6 @@ template: false
 journals:
   default: features/journals/markdown-headings-335.journal
 linewrap: 80
-password: ''
 tagsymbols: '@'
 timeformat: '%Y-%m-%d %H:%M'
 indent_character: "|"

features/data/configs/multiple.yaml

@@ -13,7 +13,6 @@ journals:
     encrypt: true
     journal: features/journals/new_encrypted.journal
 linewrap: 80
-password: ''
 tagsymbols: '@'
 timeformat: '%Y-%m-%d %H:%M'
 indent_character: "|"

features/data/configs/tags-216.yaml

@@ -7,7 +7,6 @@ template: false
 journals:
   default: features/journals/tags-216.journal
 linewrap: 80
-password: ''
 tagsymbols: '@'
 timeformat: '%Y-%m-%d %H:%M'
 indent_character: "|"

features/data/configs/tags-237.yaml

@@ -7,7 +7,6 @@ template: false
 journals:
   default: features/journals/tags-237.journal
 linewrap: 80
-password: ''
 tagsymbols: '@'
 timeformat: '%Y-%m-%d %H:%M'
 indent_character: "|"

features/data/configs/tags.yaml

@@ -7,7 +7,6 @@ template: false
 journals:
   default: features/journals/tags.journal
 linewrap: 80
-password: ''
 tagsymbols: '@'
 timeformat: '%Y-%m-%d %H:%M'
 indent_character: "|"

features/multiple_journals.feature

@@ -48,4 +48,4 @@ Feature: Multiple journals
         these three eyes
         n
         """
-	Then we should see the message "Journal 'new_encrypted' created"
+	Then we should see the message "Encrypted journal 'new_encrypted' created"

jrnl/EncryptedJournal.py

@@ -1,4 +1,5 @@
-from . import Journal, util
+from . import util
+from .Journal import Journal, LegacyJournal
 from cryptography.fernet import Fernet, InvalidToken
 from cryptography.hazmat.primitives import hashes, padding
 from cryptography.hazmat.primitives.ciphers import Cipher, algorithms, modes
@@ -9,6 +10,8 @@ import sys
 import os
 import base64
 import logging
+from typing import Optional
+
 
 log = logging.getLogger()
 
@@ -27,10 +30,11 @@ def make_key(password):
     return base64.urlsafe_b64encode(key)
 
 
-class EncryptedJournal(Journal.Journal):
+class EncryptedJournal(Journal):
     def __init__(self, name='default', **kwargs):
         super().__init__(name, **kwargs)
         self.config['encrypt'] = True
+        self.password = None
 
     def open(self, filename=None):
         """Opens the journal file defined in the config and parses it into a list of Entries.
@@ -38,27 +42,17 @@ class EncryptedJournal(Journal.Journal):
         filename = filename or self.config['journal']
 
         if not os.path.exists(filename):
-            password = util.create_password()
+            self.create_file(filename)
-            if password:
+            self.password = util.create_password(self.name)
-                if util.yesno("Do you want to store the password in your keychain?", default=True):
+            print(f"Encrypted journal '{self.name}' created at {filename}", file=sys.stderr)
-                    util.set_keychain(self.name, password)
+
-                else:
+        text = self._load(filename)
-                    util.set_keychain(self.name, None)
-                self.config['password'] = password
-                text = ""
-                self._store(filename, text)
-                print(f"[Journal '{self.name}' created at {filename}]", file=sys.stderr)
-            else:
-                print("No password supplied for encrypted journal", file=sys.stderr)
-                sys.exit(1)
-        else:
-            text = self._load(filename)
         self.entries = self._parse(text)
         self.sort()
         log.debug("opened %s with %d entries", self.__class__.__name__, len(self))
         return self
 
-    def _load(self, filename, password=None):
+    def _load(self, filename):
         """Loads an encrypted journal from a file and tries to decrypt it.
         If password is not provided, will look for password in the keychain
         and otherwise ask the user to enter a password up to three times.
@@ -67,50 +61,52 @@ class EncryptedJournal(Journal.Journal):
         with open(filename, 'rb') as f:
             journal_encrypted = f.read()
 
-        def validate_password(password):
+        def decrypt_journal(password):
             key = make_key(password)
             try:
                 plain = Fernet(key).decrypt(journal_encrypted).decode('utf-8')
-                self.config['password'] = password
+                self.password = password
                 return plain
             except (InvalidToken, IndexError):
                 return None
-        if password:
+
-            return validate_password(password)
+        if self.password:
-        return util.get_password(keychain=self.name, validator=validate_password)
+            return decrypt_journal(self.password)
+
+        return util.decrypt_content(keychain=self.name, decrypt_func=decrypt_journal)
 
     def _store(self, filename, text):
-        key = make_key(self.config['password'])
+        key = make_key(self.password)
         journal = Fernet(key).encrypt(text.encode('utf-8'))
         with open(filename, 'wb') as f:
             f.write(journal)
 
     @classmethod
-    def _create(cls, filename, password):
+    def from_journal(cls, other: Journal):
-        key = make_key(password)
+        new_journal = super().from_journal(other)
-        dummy = Fernet(key).encrypt(b"")
+        new_journal.password = other.password if hasattr(other, "password") else util.create_password(other.name)
-        with open(filename, 'wb') as f:
+        return new_journal
-            f.write(dummy)
 
 
-class LegacyEncryptedJournal(Journal.LegacyJournal):
+class LegacyEncryptedJournal(LegacyJournal):
     """Legacy class to support opening journals encrypted with the jrnl 1.x
     standard. You'll not be able to save these journals anymore."""
     def __init__(self, name='default', **kwargs):
         super().__init__(name, **kwargs)
         self.config['encrypt'] = True
+        self.password = None
 
-    def _load(self, filename, password=None):
+    def _load(self, filename):
         with open(filename, 'rb') as f:
             journal_encrypted = f.read()
         iv, cipher = journal_encrypted[:16], journal_encrypted[16:]
 
-        def validate_password(password):
+        def decrypt_journal(password):
             decryption_key = hashlib.sha256(password.encode('utf-8')).digest()
             decryptor = Cipher(algorithms.AES(decryption_key), modes.CBC(iv), default_backend()).decryptor()
             try:
                 plain_padded = decryptor.update(cipher) + decryptor.finalize()
-                self.config['password'] = password
+                self.password = password
                 if plain_padded[-1] in (" ", 32):
                     # Ancient versions of jrnl. Do not judge me.
                     return plain_padded.decode('utf-8').rstrip(" ")
@@ -120,6 +116,6 @@ class LegacyEncryptedJournal(Journal.LegacyJournal):
                     return plain.decode('utf-8')
             except ValueError:
                 return None
-        if password:
+        if self.password:
-            return validate_password(password)
+            return decrypt_journal(self.password)
-        return util.get_password(keychain=self.name, validator=validate_password)
+        return util.decrypt_content(keychain=self.name, decrypt_func=decrypt_journal)

jrnl/Journal.py

@@ -41,6 +41,7 @@ class Journal:
         # Set up date parser
         self.search_tags = None  # Store tags we're highlighting
         self.name = name
+        self.entries = []
 
     def __len__(self):
         """Returns the number of entries"""
@@ -69,8 +70,8 @@ class Journal:
         filename = filename or self.config['journal']
 
         if not os.path.exists(filename):
+            self.create_file(filename)
             print(f"[Journal '{self.name}' created at {filename}]", file=sys.stderr)
-            self._create(filename)
         
         text = self._load(filename)
         self.entries = self._parse(text)
@@ -92,6 +93,11 @@ class Journal:
                 return False
         return True
 
+    @staticmethod
+    def create_file(filename):
+        with open(filename, "w"):
+            pass
+
     def _to_text(self):
         return "\n".join([str(e) for e in self.entries])
 
@@ -101,10 +107,6 @@ class Journal:
     def _store(self, filename, text):
         raise NotImplementedError
 
-    @classmethod
-    def _create(cls, filename):
-        raise NotImplementedError
-
     def _parse(self, journal_txt):
         """Parses a journal that's stored in a string and returns a list of entries"""
 
@@ -274,11 +276,6 @@ class Journal:
 
 
 class PlainJournal(Journal):
-    @classmethod
-    def _create(cls, filename):
-        with open(filename, "a", encoding="utf-8"):
-            pass
-
     def _load(self, filename):
         with open(filename, "r", encoding="utf-8") as f:
             return f.read()

jrnl/cli.py

@@ -6,7 +6,8 @@
     license: MIT, see LICENSE for more details.
 """
 
-from . import Journal
+from .Journal import PlainJournal, open_journal
+from .EncryptedJournal import EncryptedJournal
 from . import util
 from . import install
 from . import plugins
@@ -77,28 +78,19 @@ def guess_mode(args, config):
 
 def encrypt(journal, filename=None):
     """ Encrypt into new file. If filename is not set, we encrypt the journal file itself. """
-    from . import EncryptedJournal
-
-    journal.config['password'] = util.create_password()
     journal.config['encrypt'] = True
 
-    new_journal = EncryptedJournal.EncryptedJournal(None, **journal.config)
+    new_journal = EncryptedJournal.from_journal(journal)
-    new_journal.entries = journal.entries
     new_journal.write(filename)
 
-    if util.yesno("Do you want to store the password in your keychain?", default=True):
-        util.set_keychain(journal.name, journal.config['password'])
-
     print("Journal encrypted to {}.".format(filename or new_journal.config['journal']), file=sys.stderr)
 
 
 def decrypt(journal, filename=None):
     """ Decrypts into new file. If filename is not set, we encrypt the journal file itself. """
     journal.config['encrypt'] = False
-    journal.config['password'] = ""
 
-    new_journal = Journal.PlainJournal(filename, **journal.config)
+    new_journal = PlainJournal.from_journal(journal)
-    new_journal.entries = journal.entries
     new_journal.write(filename)
     print("Journal decrypted to {}.".format(filename or new_journal.config['journal']), file=sys.stderr)
 
@@ -156,11 +148,12 @@ def run(manual_args=None):
 
     # If the first textual argument points to a journal file,
     # use this!
-    journal_name = args.text[0] if (args.text and args.text[0] in config['journals']) else 'default'
 
-    if journal_name != 'default':
+    journal_name = install.DEFAULT_JOURNAL_KEY
+    if args.text and args.text[0] in config['journals']:
+        journal_name = args.text[0]
         args.text = args.text[1:]
-    elif "default" not in config['journals']:
+    elif install.DEFAULT_JOURNAL_KEY not in config['journals']:
         print("No default journal configured.", file=sys.stderr)
         print(list_journals(config), file=sys.stderr)
         sys.exit(1)
@@ -211,7 +204,7 @@ def run(manual_args=None):
 
     # This is where we finally open the journal!
     try:
-        journal = Journal.open_journal(journal_name, config)
+        journal = open_journal(journal_name, config)
     except KeyboardInterrupt:
         print(f"[Interrupted while opening journal]", file=sys.stderr)
         sys.exit(1)

jrnl/install.py

@@ -19,6 +19,7 @@ if "win32" not in sys.platform:
 
 DEFAULT_CONFIG_NAME = 'jrnl.yaml'
 DEFAULT_JOURNAL_NAME = 'journal.txt'
+DEFAULT_JOURNAL_KEY = 'default'
 XDG_RESOURCE = 'jrnl'
 
 USER_HOME = os.path.expanduser('~')
@@ -45,7 +46,7 @@ def module_exists(module_name):
 default_config = {
     'version': __version__,
     'journals': {
-        "default": JOURNAL_FILE_PATH
+        DEFAULT_JOURNAL_KEY: JOURNAL_FILE_PATH
     },
     'editor': os.getenv('VISUAL') or os.getenv('EDITOR') or "",
     'encrypt': False,
@@ -118,32 +119,23 @@ def install():
     # Where to create the journal?
     path_query = f'Path to your journal file (leave blank for {JOURNAL_FILE_PATH}): '
     journal_path = input(path_query).strip() or JOURNAL_FILE_PATH
-    default_config['journals']['default'] = os.path.expanduser(os.path.expandvars(journal_path))
+    default_config['journals'][DEFAULT_JOURNAL_KEY] = os.path.expanduser(os.path.expandvars(journal_path))
 
-    path = os.path.split(default_config['journals']['default'])[0]  # If the folder doesn't exist, create it
+    path = os.path.split(default_config['journals'][DEFAULT_JOURNAL_KEY])[0]  # If the folder doesn't exist, create it
     try:
         os.makedirs(path)
     except OSError:
         pass
 
     # Encrypt it?
-    password = getpass.getpass("Enter password for journal (leave blank for no encryption): ")
+    encrypt = util.yesno("Do you want to encrypt your journal? You can always change this later", default=False)
-    if password:
+    if encrypt:
         default_config['encrypt'] = True
-        if util.yesno("Do you want to store the password in your keychain?", default=True):
-            util.set_keychain("default", password)
-        else:
-            util.set_keychain("default", None)
-        EncryptedJournal._create(default_config['journals']['default'], password)
         print("Journal will be encrypted.", file=sys.stderr)
-    else:
-        PlainJournal._create(default_config['journals']['default'])
 
-    config = default_config
+    save_config(default_config)
-    save_config(config)
+    return default_config
-    if password:
+
-        config['password'] = password
-    return config
 
 def autocomplete(text, state):
     expansions = glob.glob(os.path.expanduser(os.path.expandvars(text)) + '*')

jrnl/util.py

@@ -4,8 +4,10 @@ import sys
 import os
 import getpass as gp
 import yaml
+
 if "win32" in sys.platform:
     import colorama
+
     colorama.init()
 import re
 import tempfile
@@ -13,6 +15,7 @@ import subprocess
 import unicodedata
 import shlex
 import logging
+from typing import Optional, Callable
 
 log = logging.getLogger(__name__)
 
@@ -37,19 +40,29 @@ class UserAbort(Exception):
     pass
 
 
-def create_password():
+def create_password(journal_name: str, prompt: str = "Enter password for new journal: ") -> str:
     while True:
-        pw = gp.getpass("Enter password for new journal: ")
+        pw = gp.getpass(prompt)
-        if pw == gp.getpass("Enter password again: "):
+        if not pw:
-            return pw
+            print("Password can't be an empty string!", file=sys.stderr)
+            continue
+        elif pw == gp.getpass("Enter password again: "):
+            break
 
         print("Passwords did not match, please try again", file=sys.stderr)
 
+    if yesno("Do you want to store the password in your keychain?", default=True):
+        set_keychain(journal_name, pw)
+    else:
+        set_keychain(journal_name, None)
 
-def get_password(validator, keychain=None, max_attempts=3):
+    return pw
+
+
+def decrypt_content(decrypt_func: Callable[[str], Optional[str]], keychain: str = None, max_attempts: int = 3) -> str:
     pwd_from_keychain = keychain and get_keychain(keychain)
     password = pwd_from_keychain or gp.getpass()
-    result = validator(password)
+    result = decrypt_func(password)
     # Password is bad:
     if result is None and pwd_from_keychain:
         set_keychain(keychain, None)
@@ -57,7 +70,7 @@ def get_password(validator, keychain=None, max_attempts=3):
     while result is None and attempt < max_attempts:
         print("Wrong password, try again.", file=sys.stderr)
         password = gp.getpass()
-        result = validator(password)
+        result = decrypt_func(password)
         attempt += 1
     if result is not None:
         return result