From 0a104c88afa04e22906654bc6d7dbd8b19a012e9 Mon Sep 17 00:00:00 2001 From: BakerNet Date: Tue, 16 Apr 2024 09:49:21 -0700 Subject: [PATCH 01/15] add getcreds to cli for interface to get_credentials --- keyring/cli.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/keyring/cli.py b/keyring/cli.py index 34945a6f..ed21704e 100644 --- a/keyring/cli.py +++ b/keyring/cli.py @@ -12,6 +12,7 @@ get_password, set_keyring, set_password, + get_credential, ) from .util import platform_ @@ -41,7 +42,7 @@ def __init__(self): self.parser.add_argument( "--disable", action="store_true", help="Disable keyring and exit" ) - self.parser._operations = ["get", "set", "del", "diagnose"] + self.parser._operations = ["get", "set", "del", "diagnose", "getcreds"] self.parser.add_argument( 'operation', choices=self.parser._operations, @@ -81,7 +82,7 @@ def run(self, argv): def _check_args(self): if self.operation: - if self.service is None or self.username is None: + if self.service is None or (self.operation != "getcreds" && self.username is None): self.parser.error(f"{self.operation} requires service and username") def do_get(self): @@ -90,6 +91,13 @@ def do_get(self): raise SystemExit(1) print(password) + def do_getcreds(self): + creds = get_credential(self.service, self.username) + if creds is None: + raise SystemExit(1) + print(f"username: {creds.username}") + print(f"password: {creds.password}") + def do_set(self): password = self.input_password( f"Password for '{self.username}' in '{self.service}': " From 95e4bb47ebee5aa158e332f4ec9b1b12b2b7769e Mon Sep 17 00:00:00 2001 From: BakerNet Date: Tue, 16 Apr 2024 10:19:49 -0700 Subject: [PATCH 02/15] Syntax --- keyring/cli.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/keyring/cli.py b/keyring/cli.py index ed21704e..8a6cfcba 100644 --- a/keyring/cli.py +++ b/keyring/cli.py @@ -42,7 +42,7 @@ def __init__(self): self.parser.add_argument( "--disable", action="store_true", help="Disable keyring and exit" ) - self.parser._operations = ["get", "set", "del", "diagnose", "getcreds"] + self.parser._operations = ["get", "getcreds", "set", "del", "diagnose"] self.parser.add_argument( 'operation', choices=self.parser._operations, @@ -82,7 +82,7 @@ def run(self, argv): def _check_args(self): if self.operation: - if self.service is None or (self.operation != "getcreds" && self.username is None): + if self.service is None or (self.operation != "getcreds" and self.username is None): self.parser.error(f"{self.operation} requires service and username") def do_get(self): From 9773fc432ea9e6fccbb3331b95d8e22dc815241d Mon Sep 17 00:00:00 2001 From: BakerNet Date: Tue, 16 Apr 2024 10:27:26 -0700 Subject: [PATCH 03/15] ruff --- keyring/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/keyring/cli.py b/keyring/cli.py index 8a6cfcba..7e66205d 100644 --- a/keyring/cli.py +++ b/keyring/cli.py @@ -42,7 +42,7 @@ def __init__(self): self.parser.add_argument( "--disable", action="store_true", help="Disable keyring and exit" ) - self.parser._operations = ["get", "getcreds", "set", "del", "diagnose"] + self.parser._operations = ["get", "set", "del", "getcreds", "diagnose"] self.parser.add_argument( 'operation', choices=self.parser._operations, From 8ee16b6e37b6a125e2bd610e1d8676397535afed Mon Sep 17 00:00:00 2001 From: BakerNet Date: Tue, 16 Apr 2024 10:38:31 -0700 Subject: [PATCH 04/15] fix _check_args --- keyring/cli.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/keyring/cli.py b/keyring/cli.py index 7e66205d..023c8475 100644 --- a/keyring/cli.py +++ b/keyring/cli.py @@ -82,7 +82,10 @@ def run(self, argv): def _check_args(self): if self.operation: - if self.service is None or (self.operation != "getcreds" and self.username is None): + if self.operation == 'getcreds': + if self.service is None : + self.parser.error(f"{self.operation} requires service") + elif self.service is None or self.username is None: self.parser.error(f"{self.operation} requires service and username") def do_get(self): From 9a2d951c77464ec854b17773b1c761817f48b53f Mon Sep 17 00:00:00 2001 From: BakerNet Date: Tue, 16 Apr 2024 13:02:31 -0700 Subject: [PATCH 05/15] format --- keyring/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/keyring/cli.py b/keyring/cli.py index 023c8475..9a733d4d 100644 --- a/keyring/cli.py +++ b/keyring/cli.py @@ -83,7 +83,7 @@ def run(self, argv): def _check_args(self): if self.operation: if self.operation == 'getcreds': - if self.service is None : + if self.service is None: self.parser.error(f"{self.operation} requires service") elif self.service is None or self.username is None: self.parser.error(f"{self.operation} requires service and username") From 7830a6418a77edfe28d4c4ba3142a2a6f8a70de6 Mon Sep 17 00:00:00 2001 From: BakerNet Date: Tue, 16 Apr 2024 14:23:17 -0700 Subject: [PATCH 06/15] switch to --get-mode instead of separate operation --- keyring/cli.py | 40 +++++++++++++++++++++++++++------------- 1 file changed, 27 insertions(+), 13 deletions(-) diff --git a/keyring/cli.py b/keyring/cli.py index 9a733d4d..8532e27b 100644 --- a/keyring/cli.py +++ b/keyring/cli.py @@ -42,7 +42,21 @@ def __init__(self): self.parser.add_argument( "--disable", action="store_true", help="Disable keyring and exit" ) - self.parser._operations = ["get", "set", "del", "getcreds", "diagnose"] + self.parser._get_modes = ["password", "creds"] + self.parser.add_argument( + "--get-mode", + choices=self.parser._get_modes, + dest="get_mode", + default="password", + help=""" + Mode for 'get' operation. + 'password' requires a username and will return only the password. + 'creds' does not require a username and will return both the username and password separated by a newline. + + Default is 'password' + """, + ) + self.parser._operations = ["get", "set", "del", "diagnose"] self.parser.add_argument( 'operation', choices=self.parser._operations, @@ -82,24 +96,24 @@ def run(self, argv): def _check_args(self): if self.operation: - if self.operation == 'getcreds': + if self.operation == 'get' and self.get_mode == 'creds': if self.service is None: self.parser.error(f"{self.operation} requires service") elif self.service is None or self.username is None: self.parser.error(f"{self.operation} requires service and username") def do_get(self): - password = get_password(self.service, self.username) - if password is None: - raise SystemExit(1) - print(password) - - def do_getcreds(self): - creds = get_credential(self.service, self.username) - if creds is None: - raise SystemExit(1) - print(f"username: {creds.username}") - print(f"password: {creds.password}") + if self.get_mode == 'creds': + creds = get_credential(self.service, self.username) + if creds is None: + raise SystemExit(1) + print(f"{creds.username}") + print(f"{creds.password}") + else: + password = get_password(self.service, self.username) + if password is None: + raise SystemExit(1) + print(password) def do_set(self): password = self.input_password( From bd1a7cabc1cb0b9f55e5d7c919d85ceb9a1c6eb9 Mon Sep 17 00:00:00 2001 From: BakerNet Date: Tue, 16 Apr 2024 14:55:28 -0700 Subject: [PATCH 07/15] Add output format argument for get operation --- keyring/cli.py | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/keyring/cli.py b/keyring/cli.py index 8532e27b..310e6e7a 100644 --- a/keyring/cli.py +++ b/keyring/cli.py @@ -2,6 +2,7 @@ import argparse import getpass +import json import sys from . import ( @@ -56,6 +57,18 @@ def __init__(self): Default is 'password' """, ) + self.parser._output_formats = ["plain", "json"] + self.parser.add_argument( + "--output", + choices=self.parser._output_formats, + dest="output_format", + default="plain", + help=""" + Outpup format for 'get' operation. + + Default is 'plain' + """, + ) self.parser._operations = ["get", "set", "del", "diagnose"] self.parser.add_argument( 'operation', @@ -103,17 +116,24 @@ def _check_args(self): self.parser.error(f"{self.operation} requires service and username") def do_get(self): + credential_dict = {} if self.get_mode == 'creds': creds = get_credential(self.service, self.username) if creds is None: raise SystemExit(1) - print(f"{creds.username}") - print(f"{creds.password}") + credential_dict['username'] = creds.username + credential_dict['password'] = creds.password else: password = get_password(self.service, self.username) if password is None: raise SystemExit(1) - print(password) + crediential_dict['password'] = password + if self.output_format == 'json': + print(json.dumps(credential_dict)) + else: + if 'username' in credential_dict.keys(): + print(credential_dict['username']) + print(credential_dict['password']) def do_set(self): password = self.input_password( From 55432e97adad27843a7731c010852ae17d1074f6 Mon Sep 17 00:00:00 2001 From: BakerNet Date: Tue, 16 Apr 2024 14:58:28 -0700 Subject: [PATCH 08/15] typos --- keyring/cli.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/keyring/cli.py b/keyring/cli.py index 310e6e7a..e777798b 100644 --- a/keyring/cli.py +++ b/keyring/cli.py @@ -64,7 +64,7 @@ def __init__(self): dest="output_format", default="plain", help=""" - Outpup format for 'get' operation. + Output format for 'get' operation. Default is 'plain' """, @@ -127,7 +127,7 @@ def do_get(self): password = get_password(self.service, self.username) if password is None: raise SystemExit(1) - crediential_dict['password'] = password + credential_dict['password'] = password if self.output_format == 'json': print(json.dumps(credential_dict)) else: From 1c6450a9ec13f1f8bb9d9819afa88b5e5bba925d Mon Sep 17 00:00:00 2001 From: BakerNet Date: Wed, 17 Apr 2024 09:53:32 -0700 Subject: [PATCH 09/15] change --get-mode to --mode --- keyring/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/keyring/cli.py b/keyring/cli.py index e777798b..6e2dc03d 100644 --- a/keyring/cli.py +++ b/keyring/cli.py @@ -45,7 +45,7 @@ def __init__(self): ) self.parser._get_modes = ["password", "creds"] self.parser.add_argument( - "--get-mode", + "--mode", choices=self.parser._get_modes, dest="get_mode", default="password", From 98ce17af441e4f4dbd821f32ef3df1beb9dcbadc Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Fri, 26 Apr 2024 11:06:21 -0400 Subject: [PATCH 10/15] Extract methods for getting the credential or password. --- keyring/cli.py | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/keyring/cli.py b/keyring/cli.py index 6e2dc03d..b6808e18 100644 --- a/keyring/cli.py +++ b/keyring/cli.py @@ -116,18 +116,7 @@ def _check_args(self): self.parser.error(f"{self.operation} requires service and username") def do_get(self): - credential_dict = {} - if self.get_mode == 'creds': - creds = get_credential(self.service, self.username) - if creds is None: - raise SystemExit(1) - credential_dict['username'] = creds.username - credential_dict['password'] = creds.password - else: - password = get_password(self.service, self.username) - if password is None: - raise SystemExit(1) - credential_dict['password'] = password + credential_dict = getattr(self, f'_get_{self.get_mode}')() if self.output_format == 'json': print(json.dumps(credential_dict)) else: @@ -135,6 +124,23 @@ def do_get(self): print(credential_dict['username']) print(credential_dict['password']) + def _get_cred(self): + creds = get_credential(self.service, self.username) + if creds is None: + raise SystemExit(1) + credential_dict = {} + credential_dict['username'] = creds.username + credential_dict['password'] = creds.password + return credential_dict + + def _get_password(self): + password = get_password(self.service, self.username) + if password is None: + raise SystemExit(1) + credential_dict = {} + credential_dict['password'] = password + return credential_dict + def do_set(self): password = self.input_password( f"Password for '{self.username}' in '{self.service}': " From 03801d78c43aee74ceaced5d1bbbb8fdcfbaf8f9 Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Fri, 26 Apr 2024 11:09:06 -0400 Subject: [PATCH 11/15] Extract methods for emitting a credential. --- keyring/cli.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/keyring/cli.py b/keyring/cli.py index b6808e18..5bb52286 100644 --- a/keyring/cli.py +++ b/keyring/cli.py @@ -117,12 +117,15 @@ def _check_args(self): def do_get(self): credential_dict = getattr(self, f'_get_{self.get_mode}')() - if self.output_format == 'json': - print(json.dumps(credential_dict)) - else: - if 'username' in credential_dict.keys(): - print(credential_dict['username']) - print(credential_dict['password']) + getattr(self, f'_emit_{self.output_format}')(credential_dict) + + def _emit_json(self, credential_dict): + print(json.dumps(credential_dict)) + + def _emit_plain(self, credential_dict): + if 'username' in credential_dict.keys(): + print(credential_dict['username']) + print(credential_dict['password']) def _get_cred(self): creds = get_credential(self.service, self.username) From 734a17d8d8f96e911976d55d9ec99b82e2240dfb Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Fri, 26 Apr 2024 11:19:27 -0400 Subject: [PATCH 12/15] Re-write do_get to re-use credential objects. --- keyring/cli.py | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/keyring/cli.py b/keyring/cli.py index 5bb52286..5e59ec1b 100644 --- a/keyring/cli.py +++ b/keyring/cli.py @@ -9,6 +9,7 @@ backend, completion, core, + credentials, delete_password, get_password, set_keyring, @@ -116,33 +117,30 @@ def _check_args(self): self.parser.error(f"{self.operation} requires service and username") def do_get(self): - credential_dict = getattr(self, f'_get_{self.get_mode}')() - getattr(self, f'_emit_{self.output_format}')(credential_dict) + credential = getattr(self, f'_get_{self.get_mode}')() + getattr(self, f'_emit_{self.output_format}')(credential) - def _emit_json(self, credential_dict): - print(json.dumps(credential_dict)) + def _emit_json(self, credential: credentials.Credential): + print( + json.dumps(dict(username=credential.username, password=credential.password)) + ) - def _emit_plain(self, credential_dict): - if 'username' in credential_dict.keys(): - print(credential_dict['username']) - print(credential_dict['password']) + def _emit_plain(self, credential: credentials.Credential): + if credential.username: + print(credential.username) + print(credential.password) - def _get_cred(self): - creds = get_credential(self.service, self.username) + def _get_cred(self) -> credentials.Credential: + creds = get_credential(self.service, self.username) # type: ignore if creds is None: raise SystemExit(1) - credential_dict = {} - credential_dict['username'] = creds.username - credential_dict['password'] = creds.password - return credential_dict + return creds - def _get_password(self): - password = get_password(self.service, self.username) + def _get_password(self) -> credentials.Credential: + password = get_password(self.service, self.username) # type: ignore if password is None: raise SystemExit(1) - credential_dict = {} - credential_dict['password'] = password - return credential_dict + return credentials.SimpleCredential(None, password) def do_set(self): password = self.input_password( From f922be67f9ba62e5386535e3fa27043e93d5eaf6 Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Fri, 26 Apr 2024 11:21:49 -0400 Subject: [PATCH 13/15] Move checks for 'no result' back into do_get. --- keyring/cli.py | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/keyring/cli.py b/keyring/cli.py index 5e59ec1b..c7300938 100644 --- a/keyring/cli.py +++ b/keyring/cli.py @@ -1,5 +1,7 @@ """Simple command line interface to get/set password from a keyring""" +from __future__ import annotations + import argparse import getpass import json @@ -118,6 +120,8 @@ def _check_args(self): def do_get(self): credential = getattr(self, f'_get_{self.get_mode}')() + if credential is None: + raise SystemExit(1) getattr(self, f'_emit_{self.output_format}')(credential) def _emit_json(self, credential: credentials.Credential): @@ -130,17 +134,16 @@ def _emit_plain(self, credential: credentials.Credential): print(credential.username) print(credential.password) - def _get_cred(self) -> credentials.Credential: - creds = get_credential(self.service, self.username) # type: ignore - if creds is None: - raise SystemExit(1) - return creds + def _get_cred(self) -> credentials.Credential | None: + return get_credential(self.service, self.username) # type: ignore - def _get_password(self) -> credentials.Credential: + def _get_password(self) -> credentials.Credential | None: password = get_password(self.service, self.username) # type: ignore - if password is None: - raise SystemExit(1) - return credentials.SimpleCredential(None, password) + return ( + credentials.SimpleCredential(None, password) + if password is not None + else None + ) def do_set(self): password = self.input_password( From 33bae95e328dec0ad1348fb058cb6f0e070648fd Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Fri, 26 Apr 2024 11:54:36 -0400 Subject: [PATCH 14/15] Rewrite _check_args using rules for required params. Consolidates logic and reduces cyclomatic complexity to 2. --- keyring/cli.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/keyring/cli.py b/keyring/cli.py index c7300938..afca4419 100644 --- a/keyring/cli.py +++ b/keyring/cli.py @@ -111,12 +111,10 @@ def run(self, argv): return method() def _check_args(self): - if self.operation: - if self.operation == 'get' and self.get_mode == 'creds': - if self.service is None: - self.parser.error(f"{self.operation} requires service") - elif self.service is None or self.username is None: - self.parser.error(f"{self.operation} requires service and username") + needs_username = self.operation != 'get' or self.get_mode != 'creds' + required = (['service'] + ['username'] * needs_username) * bool(self.operation) + if any(getattr(self, param) is None for param in required): + self.parser.error(f"{self.operation} requires {' and '.join(required)}") def do_get(self): credential = getattr(self, f'_get_{self.get_mode}')() From 49e7cf8f41159d2b9a4868970a4780eda2d181e7 Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Fri, 26 Apr 2024 11:58:16 -0400 Subject: [PATCH 15/15] Add news fragment. --- newsfragments/678.feature.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 newsfragments/678.feature.rst diff --git a/newsfragments/678.feature.rst b/newsfragments/678.feature.rst new file mode 100644 index 00000000..d0b57db5 --- /dev/null +++ b/newsfragments/678.feature.rst @@ -0,0 +1 @@ +Added options for 'keyring get' command to support credential retrieval and emit as JSON. \ No newline at end of file