secrets: Sort and filter credentials in the list command

Makefile

@@ -11,6 +11,7 @@ LINT_DIRS := $(PACKAGE_NAME) stubs
 
 RUFF ?= poetry run ruff
 MYPY ?= poetry run mypy
+PYTEST ?= poetry run pytest
 
 .PHONY: all
 all: install
@@ -42,9 +43,12 @@ check-typing:
 .PHONY: check
 check: check-format check-style check-typing
 
+# Tests that do not require a device. The tests requiring a device are run with
+# the secrets-test* targets below.
 .PHONY: test
 test:
 	$(PYTHON3_VENV) -m doctest pynitrokey/helpers.py
+	$(PYTEST) pynitrokey/test_cli_secrets.py
 
 # automatic code fixes
 .PHONY: fix

pynitrokey/cli/nk3/secrets.py

@@ -7,12 +7,14 @@
 import sys
 import typing
 from base64 import b32decode
-from typing import Any, Callable, List, Optional
+from typing import Any, Callable, Iterable, List, Optional, Sequence, Tuple
 
 import click
 from nitrokey.nk3.secrets_app import (
     ALGORITHM_TO_KIND,
     STRING_TO_KIND,
+    Kind,
+    ListItem,
     SecretsApp,
     SecretsAppException,
     SecretsAppExceptionID,
@@ -402,13 +404,163 @@ def ask_to_touch_if_needed() -> None:
     local_print("Please touch the device if it blinks", file=sys.stderr)
 
 
+KIND_TO_STRING = {kind: name for name, kind in STRING_TO_KIND.items()}
+KIND_TO_STRING[Kind.NotSet] = "PWS"
+
+LIST_KINDS = [KIND_TO_STRING[kind] for kind in [*STRING_TO_KIND.values(), Kind.NotSet]]
+
+LIST_SORT_KEYS = ["label", "kind"]
+
+
+def credential_label(credential: ListItem) -> str:
+    """Best-effort decoding of the credential label, used for sorting and matching."""
+    return credential.label.decode("utf-8", errors="replace")
+
+
+def format_label(label: bytes, hexa: bool = False) -> str:
+    """
+    Format a credential label for the output.  Labels are arbitrary byte strings, so those that are
+    not valid UTF-8 are printed as hex with a 0x prefix instead of failing to decode.  A name that
+    is literally spelled "0x..." is indistinguishable from that, so use --hexa, which prints every
+    name as hex, if the exact bytes matter.
+    """
+    if not hexa:
+        try:
+            return label.decode()
+        except UnicodeDecodeError:
+            pass
+    return f"0x{label.hex()}"
+
+
+def format_credential(credential: ListItem, hexa: bool = False) -> str:
+    kind = ListItem.get_type_name(credential.kind)
+    algorithm = ListItem.get_type_name(credential.algorithm)
+    return f"{format_label(credential.label, hexa)}\t{kind}/{algorithm}\t{credential.properties}"
+
+
+def credential_kind(credential: ListItem) -> str:
+    return KIND_TO_STRING.get(credential.kind, ListItem.get_type_name(credential.kind).upper())
+
+
+def filter_credentials(
+    credentials: Iterable[ListItem],
+    pattern: Optional[str] = None,
+    kinds: Sequence[str] = (),
+    touch_button: Optional[bool] = None,
+    pin_protected: Optional[bool] = None,
+    pws: Optional[bool] = None,
+) -> List[ListItem]:
+    """
+    Filter the given credentials.  All given criteria have to match (logical AND), while multiple
+    values for a single criterion match if any of them does (logical OR).  Criteria set to None (or
+    to an empty sequence) are ignored.
+    """
+    selected_kinds = {kind.upper() for kind in kinds}
+
+    def matches(credential: ListItem) -> bool:
+        if pattern is not None and pattern.lower() not in credential_label(credential).lower():
+            return False
+        if selected_kinds and credential_kind(credential) not in selected_kinds:
+            return False
+        if touch_button is not None and credential.properties.touch_required != touch_button:
+            return False
+        if pin_protected is not None and credential.properties.secret_encryption != pin_protected:
+            return False
+        if pws is not None and credential.properties.pws_data_exist != pws:
+            return False
+        return True
+
+    return [credential for credential in credentials if matches(credential)]
+
+
+def sort_credentials(
+    credentials: Iterable[ListItem], key: str = "label", reverse: bool = False
+) -> List[ListItem]:
+    """
+    Sort the given credentials by label or by kind.  Labels are compared case-insensitively, with
+    the raw label as a tie breaker.  Sorting by kind falls back to the label for equal kinds.
+    """
+
+    def by_label(credential: ListItem) -> Tuple[str, bytes]:
+        return (credential_label(credential).lower(), credential.label)
+
+    def by_kind(credential: ListItem) -> Tuple[str, str, bytes]:
+        return (credential_kind(credential), *by_label(credential))
+
+    if key == "label":
+        return sorted(credentials, key=by_label, reverse=reverse)
+    elif key == "kind":
+        return sorted(credentials, key=by_kind, reverse=reverse)
+    raise ValueError(f"Unsupported sort key: {key}")
+
+
 @secrets.command()
 @click.pass_obj
 @click.option(
-    "--hexa", "hexa", type=click.BOOL, help="Use hex representation", default=False, is_flag=True
+    "--hexa",
+    "hexa",
+    type=click.BOOL,
+    help="Print the credential names as hex",
+    default=False,
+    is_flag=True,
 )
-def list(ctx: Context, hexa: bool) -> None:
-    """List registered OTP credentials."""
+@click.option(
+    "--pattern",
+    "pattern",
+    type=click.STRING,
+    help="Only show credentials containing this text in their name (case insensitive)",
+    default=None,
+)
+@click.option(
+    "--kind",
+    "kinds",
+    type=click.Choice(choices=LIST_KINDS, case_sensitive=False),
+    help="Only show credentials of this kind. Can be given multiple times.",
+    multiple=True,
+)
+@click.option(
+    "--touch-button/--no-touch-button",
+    "touch_button",
+    help="Only show credentials that do (not) require a touch button press",
+    default=None,
+)
+@click.option(
+    "--pin-protected/--no-pin-protected",
+    "pin_protected",
+    help="Only show credentials that are (not) encrypted with the PIN",
+    default=None,
+)
+@click.option(
+    "--pws/--no-pws",
+    "pws",
+    help="Only show credentials that do (not) have Password Safe data",
+    default=None,
+)
+@click.option(
+    "--sort",
+    "sort_by",
+    type=click.Choice(choices=LIST_SORT_KEYS, case_sensitive=False),
+    help="Sort the credentials by this property",
+    default="label",
+    show_default=True,
+)
+@click.option("--reverse", "reverse", help="Reverse the sort order", default=False, is_flag=True)
+def list(
+    ctx: Context,
+    hexa: bool,
+    pattern: Optional[str],
+    kinds: Sequence[str],
+    touch_button: Optional[bool],
+    pin_protected: Optional[bool],
+    pws: Optional[bool],
+    sort_by: str,
+    reverse: bool,
+) -> None:
+    """List registered OTP credentials.
+
+    Without any options all credentials are listed, sorted by name. The credentials can be
+    restricted to a subset with the filtering options, which are combined with a logical AND.
+    """
     with ctx.connect_device() as device:
         app = SecretsApp(device)
         if app.is_pin_healthy():
@@ -421,11 +573,28 @@ def list(ctx: Context, hexa: bool) -> None:
             except click.Abort:
                 pass
 
-        credentials_list = sorted(app.list_with_properties(), key=lambda x: x.label)
+        all_credentials = app.list_with_properties()
+        credentials_list = sort_credentials(
+            filter_credentials(
+                all_credentials,
+                pattern=pattern,
+                kinds=kinds,
+                touch_button=touch_button,
+                pin_protected=pin_protected,
+                pws=pws,
+            ),
+            key=sort_by.lower(),
+            reverse=reverse,
+        )
         for i, credential in enumerate(credentials_list):
-            local_print(f"{i + 1:02}. {credential}")
+            local_print(f"{i + 1:02}. {format_credential(credential, hexa)}")
         if len(credentials_list) == 0:
-            local_print("No credentials found")
+            if all_credentials:
+                local_print(
+                    f"No credentials matching the filter found (out of {len(all_credentials)})"
+                )
+            else:
+                local_print("No credentials found")
 
 
 @secrets.command()

pynitrokey/test_cli_secrets.py

@@ -0,0 +1,155 @@
+# Copyright Nitrokey GmbH
+# SPDX-License-Identifier: Apache-2.0 OR MIT
+
+"""
+Tests for the sorting, filtering and formatting of the credentials in the secrets list command.
+These do not require a device.
+"""
+
+from typing import List, Optional, Sequence
+
+import pytest
+from nitrokey.nk3.secrets_app import Algorithm, Kind, ListItem, ListItemProperties
+
+from pynitrokey.cli.nk3.secrets import (
+    filter_credentials,
+    format_credential,
+    format_label,
+    sort_credentials,
+)
+
+
+def credential(
+    label: str,
+    kind: Kind = Kind.Totp,
+    touch_required: bool = False,
+    secret_encryption: bool = False,
+    pws_data_exist: bool = False,
+) -> ListItem:
+    return ListItem(
+        kind=kind,
+        algorithm=Algorithm.Sha1,
+        label=label.encode(),
+        properties=ListItemProperties(
+            touch_required=touch_required,
+            secret_encryption=secret_encryption,
+            pws_data_exist=pws_data_exist,
+        ),
+    )
+
+
+CREDENTIALS = [
+    credential("gitlab", kind=Kind.Totp, touch_required=True),
+    credential("Bank", kind=Kind.Hotp, secret_encryption=True),
+    credential("github", kind=Kind.Totp, pws_data_exist=True),
+    credential("archive", kind=Kind.NotSet, pws_data_exist=True),
+    credential("Login", kind=Kind.HotpReverse, touch_required=True, secret_encryption=True),
+]
+
+
+def labels(credentials: Sequence[ListItem]) -> List[str]:
+    return [item.label.decode() for item in credentials]
+
+
+def test_sort_by_label_is_case_insensitive() -> None:
+    assert labels(sort_credentials(CREDENTIALS)) == ["archive", "Bank", "github", "gitlab", "Login"]
+
+
+def test_sort_reverse() -> None:
+    assert labels(sort_credentials(CREDENTIALS, reverse=True)) == [
+        "Login",
+        "gitlab",
+        "github",
+        "Bank",
+        "archive",
+    ]
+
+
+def test_sort_by_kind_falls_back_to_label() -> None:
+    assert labels(sort_credentials(CREDENTIALS, key="kind")) == [
+        "Bank",
+        "Login",
+        "archive",
+        "github",
+        "gitlab",
+    ]
+
+
+def test_sort_rejects_unknown_key() -> None:
+    with pytest.raises(ValueError):
+        sort_credentials(CREDENTIALS, key="algorithm")
+
+
+def test_sort_handles_non_utf8_labels() -> None:
+    credentials = [ListItem(Kind.Totp, Algorithm.Sha1, b"\xff\xfe", CREDENTIALS[0].properties)]
+    assert sort_credentials(credentials + CREDENTIALS)[0].label == b"archive"
+
+
+def test_filter_without_criteria_keeps_all() -> None:
+    assert filter_credentials(CREDENTIALS) == CREDENTIALS
+
+
+def test_filter_by_pattern_is_case_insensitive_substring() -> None:
+    assert labels(filter_credentials(CREDENTIALS, pattern="GIT")) == ["gitlab", "github"]
+    assert labels(filter_credentials(CREDENTIALS, pattern="hub")) == ["github"]
+    assert filter_credentials(CREDENTIALS, pattern="missing") == []
+
+
+def test_filter_by_kind() -> None:
+    assert labels(filter_credentials(CREDENTIALS, kinds=["totp"])) == ["gitlab", "github"]
+    assert labels(filter_credentials(CREDENTIALS, kinds=["PWS"])) == ["archive"]
+    assert labels(filter_credentials(CREDENTIALS, kinds=["HOTP", "HOTP_REVERSE"])) == [
+        "Bank",
+        "Login",
+    ]
+
+
+@pytest.mark.parametrize(
+    ["touch_button", "pin_protected", "pws", "expected"],
+    [
+        (True, None, None, ["gitlab", "Login"]),
+        (False, None, None, ["Bank", "github", "archive"]),
+        (None, True, None, ["Bank", "Login"]),
+        (None, None, True, ["github", "archive"]),
+        (None, None, False, ["gitlab", "Bank", "Login"]),
+        (True, True, None, ["Login"]),
+        (True, None, True, []),
+    ],
+)
+def test_filter_by_properties(
+    touch_button: Optional[bool],
+    pin_protected: Optional[bool],
+    pws: Optional[bool],
+    expected: List[str],
+) -> None:
+    filtered = filter_credentials(
+        CREDENTIALS, touch_button=touch_button, pin_protected=pin_protected, pws=pws
+    )
+    assert labels(filtered) == expected
+
+
+def test_filter_combines_criteria_with_and() -> None:
+    filtered = filter_credentials(CREDENTIALS, pattern="git", kinds=["TOTP"], touch_button=True)
+    assert labels(filtered) == ["gitlab"]
+
+
+def test_format_label() -> None:
+    assert format_label(b"github") == "github"
+    assert format_label(b"") == ""
+    assert format_label("äöü".encode()) == "äöü"
+    assert format_label(b"github", hexa=True) == "0x676974687562"
+
+
+def test_format_label_does_not_fail_on_non_utf8_labels() -> None:
+    assert format_label(b"\xed\x4c\x2e") == "0xed4c2e"
+    assert format_label(b"\xed\x4c\x2e", hexa=True) == "0xed4c2e"
+
+
+def test_format_credential_keeps_the_established_output() -> None:
+    for item in CREDENTIALS:
+        assert format_credential(item) == str(item)
+
+
+def test_format_credential_uses_hex_labels_on_demand() -> None:
+    item = credential("github", kind=Kind.Totp, pws_data_exist=True)
+    assert format_credential(item, hexa=True) == "0x676974687562\tTotp/Sha1\tPWS data available"