diff --git a/releases/Next-ChangeLog.md b/releases/Next-ChangeLog.md index 87ff4f2d..c1f66a42 100644 --- a/releases/Next-ChangeLog.md +++ b/releases/Next-ChangeLog.md @@ -59,6 +59,7 @@ This lists the new changes that have not yet been published in a normal release. ## 1.4.xQ - 2065-04-xx +- New Feature: Secure Notes & Passwords UX groups - Bugfix: Teleporting a multisig PSBT file (without signing it first) sent stale data instead of the selected file - Bugfix: Fix export UX message after teleport PSBT import & sign - Bugfix: BIP-21 QR `amount` rendered with wrong decimal scaling on the Payment Address screen (e.g. `amount=1.1` was shown as `1.00000001 BTC`) diff --git a/shared/notes.py b/shared/notes.py index bbdb74a0..bee03c36 100644 --- a/shared/notes.py +++ b/shared/notes.py @@ -131,9 +131,7 @@ class NotesMenu(MenuSystem): else: wipe_if_deltamode() - rv = [] - for note in NoteContent.get_all(): - rv.append(MenuItem('%d: %s' % (note.idx+1, note.title), menu=note.make_menu)) + rv = cls.construct_note_items(readonly=False) rv.extend(news) @@ -151,16 +149,34 @@ class NotesMenu(MenuSystem): # When only allowed to view, no export/add new/delete. wipe_if_deltamode() - rv = [] - for note in NoteContent.get_all(): - rv.append(MenuItem('%d: %s' % (note.idx+1, note.title), - menu=note.make_menu, arg=True)) # readonly=True + rv = cls.construct_note_items(readonly=True) if not rv: rv.append(MenuItem('(none saved yet)')) return rv + @classmethod + def construct_note_items(cls, readonly=False): + rv = [] + by_group = {} + + for note in NoteContent.get_all(): + item = MenuItem('%d: %s' % (note.idx+1, note.title), + menu=note.make_menu, arg=readonly) + group = note.group + if group: + if group not in by_group: + by_group[group] = [] + by_group[group].append(item) + else: + rv.append(item) + + for group in sorted(by_group): + rv.append(MenuItem('↳ ' + group, menu=NoteGroupMenu(group, readonly))) + + return rv + @classmethod async def export_all(cls, *a): await start_export(NoteContent.get_all()) @@ -231,10 +247,28 @@ class NotesMenu(MenuSystem): @classmethod async def drill_to(cls, menu, item): # make it so looks like we drilled down into the new note - menu.goto_idx(item.idx) + label = '%d: %s' % (item.idx+1, item.title) + group = item.group + if group: + cls.goto_exact_label(menu, '↳ ' + group) + gm = NoteGroupMenu(group) + cls.goto_exact_label(gm, label) + the_ux.push(gm) + else: + cls.goto_exact_label(menu, label) + m = await item._make_menu() the_ux.push(MenuSystem(m)) + @staticmethod + def goto_exact_label(menu, label): + for i, mi in enumerate(menu.items): + if mi.label == label: + menu.goto_idx(i) + return True + + return False + class NoteContentBase: def __init__(self, json={}, idx=-1): @@ -250,9 +284,15 @@ class NoteContentBase: return PasswordContent(j, idx) if 'user' in j else NoteContent(j, idx) def serialize(self): - return {fld:getattr(self, fld, '') for fld in self.flds} + res = {} + for fld in self.flds: + val = getattr(self, fld, '') + # user field is necessary for proper password identification in constructor + if not val and (fld != "user"): + continue + res[fld] = val - to_json = serialize + return res @classmethod def get_all(cls): @@ -278,6 +318,15 @@ class NoteContentBase: settings.put('notes', [n.serialize() for n in notes]) settings.save() + @classmethod + def get_groups(cls): + groups = set() + for note in cls.get_all(): + if note.group: + groups.add(note.group) + + return sorted(groups) + async def delete(self, *a): # Remove note ok = await ux_confirm("Everything about this note/password will be lost.") @@ -298,6 +347,11 @@ class NoteContentBase: the_ux.pop() m = the_ux.top_of_stack() m.update_contents() + parent = the_ux.parent_of(m) + if parent: + parent.update_contents() + if isinstance(m, NoteGroupMenu) and not m.has_notes(): + the_ux.pop() await ux_dramatic_pause('Deleted.', 3) @@ -335,6 +389,11 @@ class NoteContentBase: # update parent parent = the_ux.parent_of(menu) parent.update_contents() + grandparent = the_ux.parent_of(parent) + if grandparent: + grandparent.update_contents() + if isinstance(parent, NoteGroupMenu) and not parent.has_notes(): + the_ux.stack.remove(parent) else: menu.update_contents() @@ -368,9 +427,73 @@ class NoteContentBase: predicate=2 <= len(self.misc) <= MSG_SIGNING_MAX_LENGTH) +class NoteGroupMenu(MenuSystem): + def __init__(self, group, readonly=False): + self.group = group + self.readonly = readonly + super().__init__(self.construct()) + + def construct(self): + items = [] + for note in NoteContent.get_all(): + if note.group == self.group: + items.append(MenuItem('%d: %s' % (note.idx+1, note.title), + menu=note.make_menu, arg=self.readonly)) + + return items or [MenuItem('(none)')] + + def has_notes(self): + return any(note.group == self.group for note in NoteContent.get_all()) + + def update_contents(self): + self.replace_items(self.construct()) + + +class GroupPickerMenu(MenuSystem): + def __init__(self, current=''): + self.result = None + self.current = current + + groups = NoteContentBase.get_groups() + chosen = 0 + items = [MenuItem('(none)', f=self.picked, arg='')] + + for group in groups: + if group == self.current: + chosen = len(items) + items.append(MenuItem(group, f=self.picked, arg=group)) + + items.append(MenuItem('New Group', f=self.new_group)) + + super().__init__(items, chosen=chosen) + + async def picked(self, menu, idx, mi): + assert menu == self + self.result = mi.arg + the_ux.pop() + + async def new_group(self, menu, idx, mi): + group = await ux_input_text('', max_len=ONE_LINE, confirm_exit=False, + prompt='Group', placeholder='(optional)') + if group is None: + self.result = None + else: + self.result = group + + the_ux.pop() + + @classmethod + async def pick(cls, current=''): + m = cls(current) + the_ux.push(m) + await m.interact() + + return current if m.result is None else m.result + + class PasswordContent(NoteContentBase): # "Passwords" have a few more fields and are more structured - flds = ['title', 'user', 'password', 'site', 'misc' ] + flds = ['title', 'user', 'password', 'site', 'misc', 'group'] type_label = 'password' async def _make_menu(self, readonly=False): @@ -483,6 +606,8 @@ class PasswordContent(NoteContentBase): if misc is None: misc = self.misc + group = await GroupPickerMenu.pick(self.group) + if self.idx != -1: # confirm changes, don't for new records chgs = [] @@ -494,6 +619,8 @@ class PasswordContent(NoteContentBase): chgs.append('Username') if self.misc != misc: chgs.append('Other Notes') + if self.group != group: + chgs.append('Group') if not chgs: await ux_dramatic_pause('No changes.', 3) @@ -507,6 +634,7 @@ class PasswordContent(NoteContentBase): self.user = user self.site = site self.misc = misc + self.group = group await self._save_ux(menu) return self @@ -514,7 +642,7 @@ class PasswordContent(NoteContentBase): class NoteContent(NoteContentBase): # Pure "notes" have just a title and free-form text - flds = ['title', 'misc'] + flds = ['title', 'misc', 'group'] type_label = 'note' async def _make_menu(self, readonly=False): @@ -560,6 +688,8 @@ class NoteContent(NoteContentBase): if misc is None: misc = self.misc + group = await GroupPickerMenu.pick(self.group) + if self.idx != -1: # confirm changes, don't for new records chgs = [] @@ -567,6 +697,8 @@ class NoteContent(NoteContentBase): chgs.append('Title') if self.misc != misc: chgs.append('Note Text') + if self.group != group: + chgs.append('Group') if not chgs: await ux_dramatic_pause('No changes.', 3) @@ -579,6 +711,7 @@ class NoteContent(NoteContentBase): self.title = title self.misc = misc + self.group = group await self._save_ux(menu) diff --git a/testing/test_notes.py b/testing/test_notes.py index 6eb01e7a..3fd5c473 100644 --- a/testing/test_notes.py +++ b/testing/test_notes.py @@ -98,7 +98,7 @@ def build_note(goto_notes, pick_menu_item, enter_text, cap_menu, cap_story, need_keypress, cap_screen_qr, readback_bbqr, nfc_read_text, press_select, press_cancel, is_headless, nfc_disabled): - def doit(n_title, n_body): + def doit(n_title, n_body, group=None): # we don't try to preserve leading/trailing spaces on note bodies n_body= n_body.strip() @@ -107,6 +107,11 @@ def build_note(goto_notes, pick_menu_item, enter_text, cap_menu, cap_story, # create enter_text(n_title) enter_text(n_body, multiline=True) + if group: + pick_menu_item('New Group') + enter_text(group) + else: + pick_menu_item('(none)') # view time.sleep(0.1) @@ -172,6 +177,10 @@ def build_note(goto_notes, pick_menu_item, enter_text, cap_menu, cap_story, obj = obj[0] assert obj['title'] == n_title assert obj['misc'] == n_body + if group: + assert obj['group'] == group + else: + assert 'group' not in obj # back to top notes menu press_select() @@ -185,7 +194,8 @@ def build_password(goto_notes, pick_menu_item, enter_text, cap_menu, cap_story, cap_text_box, settings_get, settings_set, scan_a_qr, press_select, press_cancel, is_headless): - def doit(n_title, n_user=None, n_pw='secret', n_site=None, n_body=None, key_pw=None): + def doit(n_title, n_user=None, n_pw='secret', n_site=None, n_body=None, + key_pw=None, group=None): goto_notes('New Password') enter_text(n_title) if n_user: @@ -221,6 +231,11 @@ def build_password(goto_notes, pick_menu_item, enter_text, cap_menu, cap_story, enter_text(n_body, multiline=True) else: press_cancel() + if group: + pick_menu_item('New Group') + enter_text(group) + else: + pick_menu_item('(none)') # view time.sleep(0.1) @@ -282,7 +297,7 @@ def change_password(goto_notes, pick_menu_item, enter_text, cap_story, cap_menu): def doit(id_title, new_title=None, new_username=None, new_site=None, - new_misc=None, new_password=None, old_password=None): + new_misc=None, new_password=None, new_group=None): goto_notes() m = cap_menu() found = [i for i in m if f': {id_title}' in i] @@ -313,6 +328,18 @@ def change_password(goto_notes, pick_menu_item, enter_text, cap_story, need_in_story.append('Other Notes') else: press_cancel() + if new_group is not None: + if new_group: + if new_group in cap_menu(): + pick_menu_item(new_group) + else: + pick_menu_item('New Group') + enter_text(new_group) + else: + pick_menu_item('(none)') + need_in_story.append('Group') + else: + press_cancel() # approve change time.sleep(0.1) @@ -349,6 +376,8 @@ def change_password(goto_notes, pick_menu_item, enter_text, cap_story, assert note['misc'] == new_misc if new_password: assert note['password'] == new_password + if new_group is not None: + assert note.get('group', '') == new_group return doit @@ -363,7 +392,7 @@ def test_build_note(n_title, n_body, build_note, delete_note): @pytest.mark.parametrize('size', [ 4000, 30000]) @pytest.mark.parametrize('encoding', '2Z' ) def test_huge_notes(size, encoding, goto_notes, enter_text, cap_menu, need_keypress, - scan_a_qr, settings_set, settings_get): + scan_a_qr, settings_set, settings_get, pick_menu_item): # Since we don't limit note sizes, by request of NVK ... test them @@ -387,6 +416,7 @@ def test_huge_notes(size, encoding, goto_notes, enter_text, cap_menu, need_keypr time.sleep(2.0 / len(parts)) # just so we can watch time.sleep(.5) # decompression time in some cases + pick_menu_item('(none)') m = cap_menu() assert 'Export' in m @@ -479,6 +509,168 @@ def test_sort_by_title(goto_notes, pick_menu_item, cap_story, need_keypress, set assert sorted((i['title'] for i in after), key=lambda i:i.lower()) \ == [i['title'] for i in after] + +def test_grouped_note_menu(settings_set, settings_get, goto_notes, cap_menu, + pick_menu_item, build_note, press_cancel, press_select): + settings_set('notes', []) + settings_set('secnap', True) + + build_note('group-note', 'body', group='Work') + notes = settings_get('notes') + assert notes[-1]['group'] == 'Work' + + goto_notes() + m = cap_menu() + assert '↳ Work' in m + assert not any(': group-note' in i for i in m) + + press_select() + m = cap_menu() + assert '1: group-note' in m + press_cancel() + + +def test_grouped_password_menu(settings_set, settings_get, goto_notes, cap_menu, + pick_menu_item, build_password, press_cancel, press_select): + settings_set('notes', []) + settings_set('secnap', True) + + build_password('group-pw', n_pw='secret', group='Accounts') + press_cancel() + notes = settings_get('notes') + assert notes[-1]['group'] == 'Accounts' + + goto_notes() + m = cap_menu() + assert '↳ Accounts' in m + assert not any(': group-pw' in i for i in m) + + press_select() + m = cap_menu() + assert '1: group-pw' in m + press_cancel() + + +def test_grouped_and_ungrouped_menu(settings_set, goto_notes, cap_menu, + pick_menu_item, press_cancel): + settings_set('secnap', True) + settings_set('notes', [ + {'title': 'loose-note', 'misc': 'aaa'}, + {'title': 'work-note', 'misc': 'bbb', 'group': 'Work'}, + {'title': 'work-pw', 'misc': '', 'password': 'secret', 'site': '', + 'user': '', 'group': 'Work'}, + ]) + + goto_notes() + m = cap_menu() + assert '1: loose-note' in m + assert '↳ Work' in m + assert not any(': work-note' in i for i in m) + assert not any(': work-pw' in i for i in m) + + pick_menu_item('↳ Work') + m = cap_menu() + assert '2: work-note' in m + assert '3: work-pw' in m + press_cancel() + + +def test_new_grouped_note_cancel_lands_in_group(settings_set, goto_notes, cap_menu, + pick_menu_item, enter_text, press_cancel): + settings_set('notes', []) + settings_set('secnap', True) + + goto_notes('New Note') + enter_text('new-note') + enter_text('body', multiline=True) + pick_menu_item('New Group') + enter_text('Work') + + assert '"new-note"' in cap_menu() + + press_cancel() + m = cap_menu() + assert '1: new-note' in m + assert 'New Note' not in m + + press_cancel() + m = cap_menu() + assert '↳ Work' in m + assert '1: new-note' not in m + + +def test_edit_note_group_moves(settings_set, settings_get, goto_notes, cap_menu, + pick_menu_item, enter_text, press_select, + press_cancel, cap_story): + + settings_set('secnap', True) + settings_set('notes', [{'title': 'move-note', 'misc': 'body'}]) + + goto_notes() + pick_menu_item('1: move-note') + pick_menu_item('Edit') + press_select() # unchanged title + press_cancel() # unchanged note body + pick_menu_item('New Group') + enter_text('Work') + time.sleep(.1) + title, story = cap_story() + assert "SURE" in title + assert 'Group' in story + press_select() + time.sleep(.1) + + goto_notes() + m = cap_menu() + assert '↳ Work' in m + assert '1: move-note' not in m + + press_select() + pick_menu_item('1: move-note') + pick_menu_item('Edit') + press_select() + press_cancel() + pick_menu_item('New Group') + enter_text('Home') + time.sleep(.1) + title, story = cap_story() + assert 'SURE' in title + assert 'Group' in story + press_select() + + goto_notes() + m = cap_menu() + assert '↳ Work' not in m + assert '↳ Home' in m + + press_select() + pick_menu_item('1: move-note') + pick_menu_item('Edit') + press_select() + press_cancel() + pick_menu_item('(none)') + time.sleep(.1) + title, story = cap_story() + assert 'SURE' in title + assert 'Group' in story + press_select() + + press_cancel() + m = cap_menu() + assert '↳ Home' not in m + assert '1: move-note' in m + assert '(none)' not in m + + +def test_old_records_without_group(settings_set, settings_get, goto_notes, cap_menu): + settings_set('secnap', True) + settings_set('notes', [{'title': 'old-note', 'misc': 'body'}]) + + goto_notes() + assert '1: old-note' in cap_menu() + assert settings_get('notes')[0].get('group', '') == '' + + def test_top_import(goto_notes, cap_menu, cap_story, need_keypress, settings_get, settings_set, scan_a_qr, need_some_notes): # make some @@ -705,7 +897,7 @@ def test_send_password_menu_item(need_some_passwords, goto_notes, cap_menu, pick settings_set('du', 1) goto_notes() - pick_menu_item('1: A') + pick_menu_item([i for i in cap_menu() if i.endswith(': A')][0]) time.sleep(.2) m = cap_menu() assert 'Send Password' not in m @@ -713,7 +905,7 @@ def test_send_password_menu_item(need_some_passwords, goto_notes, cap_menu, pick settings_set('du', 0) goto_notes() - pick_menu_item('1: A') + pick_menu_item([i for i in cap_menu() if i.endswith(': A')][0]) time.sleep(.2) m = cap_menu() assert 'Send Password' in m @@ -738,6 +930,7 @@ def test_password_cancel_stores_empty_not_none(goto_notes, need_keypress, press_ press_cancel() # cancel password field - bug, stores None press_select() # skip site press_cancel() # exit misc + pick_menu_item('(none)') # no group time.sleep(0.2)