From 88a8a0cdd8d9c7f4624d413d8a28ef281e4f4a17 Mon Sep 17 00:00:00 2001 From: Daniel Mach Date: Thu, 24 Mar 2022 10:27:36 +0100 Subject: [PATCH 1/6] Print credentials managers as a table --- osc/conf.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/osc/conf.py b/osc/conf.py index e5a08bbd..ec342cbc 100644 --- a/osc/conf.py +++ b/osc/conf.py @@ -1109,8 +1109,18 @@ def select_credentials_manager_descr(): if not credentials.has_keyring_support(): print('To use keyrings please install python%d-keyring.' % sys.version_info.major) creds_mgr_descriptors = credentials.get_credentials_manager_descriptors() + + rows = [] for i, creds_mgr_descr in enumerate(creds_mgr_descriptors, 1): - print('%d) %s (%s)' % (i, creds_mgr_descr.name(), creds_mgr_descr.description()))# + rows += [str(i), creds_mgr_descr.name(), creds_mgr_descr.description()] + + from .core import build_table + headline = ('NUM', 'NAME', 'DESCRIPTION') + table = build_table(len(headline), rows, headline) + print() + for row in table: + print(row) + i = raw_input('Select credentials manager: ') if not i.isdigit(): sys.exit('Invalid selection') From 8e0e0a9ca8b91fcb4b8c097189fa0665eb28ab65 Mon Sep 17 00:00:00 2001 From: Daniel Mach Date: Thu, 24 Mar 2022 11:11:36 +0100 Subject: [PATCH 2/6] Cherry-pick supported python-keyring backends Also provide pretty names and descriptions. --- osc/credentials.py | 43 ++++++++++++++++++++++++++++++++++--------- 1 file changed, 34 insertions(+), 9 deletions(-) diff --git a/osc/credentials.py b/osc/credentials.py index d2ce493b..9fa80ad4 100644 --- a/osc/credentials.py +++ b/osc/credentials.py @@ -195,16 +195,21 @@ class KeyringCredentialsManager(AbstractCredentialsManager): class KeyringCredentialsDescriptor(AbstractCredentialsManagerDescriptor): - def __init__(self, keyring_backend): + def __init__(self, keyring_backend, name=None, description=None): self._keyring_backend = keyring_backend + self._name = name + self._description = description def name(self): + if self._name: + return self._name if hasattr(self._keyring_backend, 'name'): return self._keyring_backend.name - else: - return self._keyring_backend.__class__.__name__ + return self._keyring_backend.__class__.__name__ def description(self): + if self._description: + return self._description return 'Backend provided by python-keyring' def create(self, cp): @@ -280,14 +285,34 @@ class GnomeKeyringCredentialsDescriptor(AbstractCredentialsManagerDescriptor): return GnomeKeyringCredentialsManager(cp, None) +# we're supporting only selected python-keyring backends in osc +SUPPORTED_KEYRING_BACKENDS = { + "keyutils.osc.OscKernelKeyringBackend": { + "name": "Kernel keyring", + "description": "Store password in user session keyring in kernel keyring [secure, in-memory, per-session]", + }, + "keyring.backends.SecretService.Keyring": { + "name": "Secret Service", + "description": "Store password in Secret Service (GNOME Keyring backend) [secure, persistent]", + }, + "keyring.backends.kwallet.DBusKeyring": { + "name": "KWallet", + "description": "Store password in KWallet [secure, persistent]", + }, +} + + def get_credentials_manager_descriptors(): - if has_keyring_support(): - backend_list = keyring.backend.get_all_keyring() - else: - backend_list = [] descriptors = [] - for backend in backend_list: - descriptors.append(KeyringCredentialsDescriptor(backend)) + + if has_keyring_support(): + for backend in keyring.backend.get_all_keyring(): + qualified_backend_name = qualified_name(backend) + data = SUPPORTED_KEYRING_BACKENDS.get(qualified_backend_name, None) + if not data: + continue + descriptor = KeyringCredentialsDescriptor(backend, data["name"], data["description"]) + descriptors.append(descriptor) descriptors.sort() if gnomekeyring: descriptors.append(GnomeKeyringCredentialsDescriptor()) From d3f4b7a930aa4065067632a38fd2371b223180d4 Mon Sep 17 00:00:00 2001 From: Daniel Mach Date: Thu, 24 Mar 2022 11:22:21 +0100 Subject: [PATCH 3/6] Reword names and decriptions of credentials managers --- osc/credentials.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/osc/credentials.py b/osc/credentials.py index 9fa80ad4..29358a45 100644 --- a/osc/credentials.py +++ b/osc/credentials.py @@ -81,10 +81,10 @@ class PlaintextConfigFileCredentialsManager(AbstractCredentialsManager): class PlaintextConfigFileDescriptor(AbstractCredentialsManagerDescriptor): def name(self): - return 'Config file credentials manager' + return 'Config' def description(self): - return 'Store the credentials in the config file (plain text)' + return 'Store the password in plain text in the osc config file [insecure, persistent]' def create(self, cp): return PlaintextConfigFileCredentialsManager(cp, None) @@ -116,10 +116,10 @@ class ObfuscatedConfigFileCredentialsManager( class ObfuscatedConfigFileDescriptor(AbstractCredentialsManagerDescriptor): def name(self): - return 'Obfuscated Config file credentials manager' + return 'Obfuscated config' def description(self): - return 'Store the credentials in the config file (obfuscated)' + return 'Store the password in obfuscated form in the osc config file [insecure, persistent]' def create(self, cp): return ObfuscatedConfigFileCredentialsManager(cp, None) @@ -154,10 +154,10 @@ class TransientCredentialsManager(AbstractCredentialsManager): class TransientDescriptor(AbstractCredentialsManagerDescriptor): def name(self): - return 'Transient password store' + return 'Transient' def description(self): - return 'Do not store the password and always ask for the password' + return 'Do not store the password and always ask for it [secure, in-memory]' def create(self, cp): return TransientCredentialsManager(cp, None) From 853a3848e87c95bad0a4eacbd7d8654a4f837af5 Mon Sep 17 00:00:00 2001 From: Daniel Mach Date: Thu, 24 Mar 2022 11:31:01 +0100 Subject: [PATCH 4/6] Order credentials managers by priority --- osc/credentials.py | 39 +++++++++++++++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/osc/credentials.py b/osc/credentials.py index 29358a45..bbae01c4 100644 --- a/osc/credentials.py +++ b/osc/credentials.py @@ -23,11 +23,16 @@ class AbstractCredentialsManagerDescriptor(object): def description(self): raise NotImplementedError() + def priority(self): + # priority determines order in the credentials managers list + # higher number means higher priority + raise NotImplementedError() + def create(self, cp): raise NotImplementedError() def __lt__(self, other): - return self.name() < other.name() + return (-self.priority(), self.name()) < (-other.priority(), other.name()) class AbstractCredentialsManager(object): @@ -86,6 +91,9 @@ class PlaintextConfigFileDescriptor(AbstractCredentialsManagerDescriptor): def description(self): return 'Store the password in plain text in the osc config file [insecure, persistent]' + def priority(self): + return 1 + def create(self, cp): return PlaintextConfigFileCredentialsManager(cp, None) @@ -121,6 +129,9 @@ class ObfuscatedConfigFileDescriptor(AbstractCredentialsManagerDescriptor): def description(self): return 'Store the password in obfuscated form in the osc config file [insecure, persistent]' + def priority(self): + return 2 + def create(self, cp): return ObfuscatedConfigFileCredentialsManager(cp, None) @@ -159,6 +170,9 @@ class TransientDescriptor(AbstractCredentialsManagerDescriptor): def description(self): return 'Do not store the password and always ask for it [secure, in-memory]' + def priority(self): + return 3 + def create(self, cp): return TransientCredentialsManager(cp, None) @@ -195,10 +209,11 @@ class KeyringCredentialsManager(AbstractCredentialsManager): class KeyringCredentialsDescriptor(AbstractCredentialsManagerDescriptor): - def __init__(self, keyring_backend, name=None, description=None): + def __init__(self, keyring_backend, name=None, description=None, priority=None): self._keyring_backend = keyring_backend self._name = name self._description = description + self._priority = priority def name(self): if self._name: @@ -212,6 +227,11 @@ class KeyringCredentialsDescriptor(AbstractCredentialsManagerDescriptor): return self._description return 'Backend provided by python-keyring' + def priority(self): + if self._priority is not None: + return self._priority + return 0 + def create(self, cp): qualified_backend_name = qualified_name(self._keyring_backend) return KeyringCredentialsManager(cp, qualified_backend_name) @@ -281,6 +301,9 @@ class GnomeKeyringCredentialsDescriptor(AbstractCredentialsManagerDescriptor): return 'Deprecated GNOME Keyring Manager. If you use \ this we will send you a Dial-In modem' + def priority(self): + return 0 + def create(self, cp): return GnomeKeyringCredentialsManager(cp, None) @@ -290,14 +313,17 @@ SUPPORTED_KEYRING_BACKENDS = { "keyutils.osc.OscKernelKeyringBackend": { "name": "Kernel keyring", "description": "Store password in user session keyring in kernel keyring [secure, in-memory, per-session]", + "priority": 10, }, "keyring.backends.SecretService.Keyring": { "name": "Secret Service", "description": "Store password in Secret Service (GNOME Keyring backend) [secure, persistent]", + "priority": 9, }, "keyring.backends.kwallet.DBusKeyring": { "name": "KWallet", "description": "Store password in KWallet [secure, persistent]", + "priority": 8, }, } @@ -311,14 +337,19 @@ def get_credentials_manager_descriptors(): data = SUPPORTED_KEYRING_BACKENDS.get(qualified_backend_name, None) if not data: continue - descriptor = KeyringCredentialsDescriptor(backend, data["name"], data["description"]) + descriptor = KeyringCredentialsDescriptor( + backend, + data["name"], + data["description"], + data["priority"] + ) descriptors.append(descriptor) - descriptors.sort() if gnomekeyring: descriptors.append(GnomeKeyringCredentialsDescriptor()) descriptors.append(PlaintextConfigFileDescriptor()) descriptors.append(ObfuscatedConfigFileDescriptor()) descriptors.append(TransientDescriptor()) + descriptors.sort() return descriptors From 8a857895738c211a42c58c956694edfc331da89a Mon Sep 17 00:00:00 2001 From: Daniel Mach Date: Thu, 24 Mar 2022 11:33:10 +0100 Subject: [PATCH 5/6] Set the first (highest prio) credentials manager as the default --- osc/conf.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/osc/conf.py b/osc/conf.py index ec342cbc..07c6c967 100644 --- a/osc/conf.py +++ b/osc/conf.py @@ -1121,7 +1121,9 @@ def select_credentials_manager_descr(): for row in table: print(row) - i = raw_input('Select credentials manager: ') + i = raw_input('Select credentials manager [default=1]: ') + if not i: + i = "1" if not i.isdigit(): sys.exit('Invalid selection') i = int(i) - 1 From 90a1cb838b3f71463c00cf33fe89f5b653ed4e5d Mon Sep 17 00:00:00 2001 From: Daniel Mach Date: Thu, 24 Mar 2022 14:35:43 +0100 Subject: [PATCH 6/6] Report a config error when trying to load credentials_mgr_class that does't exist --- osc/babysitter.py | 2 +- osc/credentials.py | 9 ++++++++- osc/oscerr.py | 6 ++++++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/osc/babysitter.py b/osc/babysitter.py index 973580c2..43f31c17 100644 --- a/osc/babysitter.py +++ b/osc/babysitter.py @@ -151,7 +151,7 @@ def run(prg, argv=None): raise print(e, file=sys.stderr) except (oscerr.ConfigError, oscerr.NoConfigfile) as e: - print(e.msg, file=sys.stderr) + print(e, file=sys.stderr) except configparser.Error as e: print(e.message, file=sys.stderr) except oscerr.OscIOError as e: diff --git a/osc/credentials.py b/osc/credentials.py index bbae01c4..e27e5e58 100644 --- a/osc/credentials.py +++ b/osc/credentials.py @@ -15,6 +15,9 @@ try: except ImportError: gnomekeyring = None +from . import conf +from . import oscerr + class AbstractCredentialsManagerDescriptor(object): def name(self): @@ -184,7 +187,11 @@ class KeyringCredentialsManager(AbstractCredentialsManager): self._backend_cls_name = options def _load_backend(self): - keyring_backend = keyring.core.load_keyring(self._backend_cls_name) + try: + keyring_backend = keyring.core.load_keyring(self._backend_cls_name) + except ModuleNotFoundError: + msg = "Invalid credentials_mgr_class: {}".format(self._backend_cls_name) + raise oscerr.ConfigError(msg, conf.config['conffile']) keyring.set_keyring(keyring_backend) @classmethod diff --git a/osc/oscerr.py b/osc/oscerr.py index cd014839..e3ad9217 100644 --- a/osc/oscerr.py +++ b/osc/oscerr.py @@ -22,6 +22,9 @@ class ConfigError(OscBaseError): self.msg = msg self.file = fname + def __str__(self): + return "Error in config file {}\n {}".format(self.file, self.msg) + class ConfigMissingApiurl(ConfigError): """Exception raised when a apiurl does not exist in the config file""" def __init__(self, msg, fname, url): @@ -46,6 +49,9 @@ class NoConfigfile(OscBaseError): self.file = fname self.msg = msg + def __str__(self): + return "Config file cannot be found: {}\n {}".format(self.file, self.msg) + class ExtRuntimeError(OscBaseError): """Exception raised when there is a runtime error of an external tool""" def __init__(self, msg, fname):