From 6dd69b180bef2075e4de3065ca708cfb217df7c3 Mon Sep 17 00:00:00 2001 From: bladams Date: Mon, 16 Dec 2019 10:40:27 -0500 Subject: [PATCH] Remove keyring support Ref GH-132 --- .circleci/config.yml | 2 +- .gitignore | 2 + keg/app.py | 20 ------ keg/cli.py | 103 --------------------------- keg/config.py | 3 - keg/keyring.py | 145 -------------------------------------- keg/tests/test_cli.py | 2 +- keg/tests/test_keyring.py | 86 ---------------------- setup.py | 1 - tox.ini | 4 +- 10 files changed, 6 insertions(+), 362 deletions(-) delete mode 100644 keg/keyring.py delete mode 100644 keg/tests/test_keyring.py diff --git a/.circleci/config.yml b/.circleci/config.yml index 6b9e803..1d33944 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -16,7 +16,7 @@ jobs: - run: name: install tox and upgrade pip - command: pip install -U pip tox pipenv + command: pip install --progress-bar off -U pip tox pipenv - run: name: version checks diff --git a/.gitignore b/.gitignore index 010fd8a..8eda5bf 100644 --- a/.gitignore +++ b/.gitignore @@ -14,3 +14,5 @@ tags .vscode .pytest_cache keg_apps/keg_apps.db-config.py +.idea +.python-version \ No newline at end of file diff --git a/keg/app.py b/keg/app.py index d974bb8..8f97153 100644 --- a/keg/app.py +++ b/keg/app.py @@ -1,10 +1,8 @@ from __future__ import absolute_import import importlib -import warnings import flask -from flask.config import ConfigAttribute from six.moves import range from werkzeug.datastructures import ImmutableDict @@ -28,10 +26,8 @@ class Keg(flask.Flask): import_name = None use_blueprints = () oauth_providers = () - keyring_enabled = ConfigAttribute('KEG_KEYRING_ENABLE') config_class = keg.config.Config logging_class = keg.logging.Logging - keyring_manager_class = None _cli = None cli_loader_class = keg.cli.CLILoader @@ -61,7 +57,6 @@ def __init__(self, import_name=None, *args, **kwargs): # passed in value takes precedence import_name = import_name or self.import_name - self.keyring_manager = None self._init_config = kwargs.pop('config', {}) flask.Flask.__init__(self, import_name, *args, **kwargs) @@ -83,7 +78,6 @@ def init(self, config_profile=None, use_test_profile=False, config=None): self.init_config(config_profile, use_test_profile, config) self.init_logging() - self.init_keyring() self.init_oath() self.init_error_handling() self.init_extensions() @@ -119,20 +113,6 @@ def on_config_complete(self): """ For subclasses to override """ pass - def init_keyring(self): - # do keyring substitution - if self.keyring_enabled: - from keg.keyring import Manager, keyring - if keyring is None: - warnings.warn(_('Keyring substitution is enabled, but the keyring package is not' - ' installed. Please install the keyring package (pip install' - ' keyring) or disable keyring support by setting' - ' `KEG_KEYRING_ENABLE = False` in your configuration profile.')) - return - - self.keyring_manager = Manager(self) - self.keyring_manager.substitute(self.config) - def init_extensions(self): self.init_db() diff --git a/keg/cli.py b/keg/cli.py index 7636445..cabc096 100644 --- a/keg/cli.py +++ b/keg/cli.py @@ -2,7 +2,6 @@ from collections import defaultdict from contextlib import contextmanager -import platform import click import flask @@ -11,7 +10,6 @@ from keg import current_app from keg.extensions import gettext as _ -from keg.keyring import keyring as keg_keyring class KegAppGroup(flask.cli.AppGroup): @@ -186,107 +184,6 @@ def database_clear(): click.echo(_('Database cleared')) -class KeyringGroup(click.MultiCommand): - - def list_commands(self, ctx): - if keg_keyring: - return ['delete', 'list-keys', 'setup', 'status'] - else: - return ['status'] - - def get_command(self, ctx, name): - if name == 'status': - return keyring_status - if name == 'list-keys': - return keyring_list_keys - if name == 'delete': - return keyring_delete - - -def keyring_notify_no_module(): - click.echo(_('Keyring module not installed. Keyring functionality disabled.\n\nYou can' - ' enable keyring functionality by installing the package:' - ' `pip install keyring`.')) - - -@dev_command.command('keyring', cls=KeyringGroup, invoke_without_command=True, - help=_('Lists keyring related sub-commands.')) -@click.pass_context -def keyring_group(ctx): - # only take action if no subcommand is involved. - if ctx.invoked_subcommand is None: - if keg_keyring is None: - keyring_notify_no_module() - else: - # keyring is available, but no subcommand was given, therefore we want to just show - # the help message, which would be the default behavior if we had not used the - # invoke_Without_command option. - click.echo(ctx.get_help()) - ctx.exit() - - -@click.command('status', short_help=_('Show keyring related status info.')) -@click.option('--unavailable', default=False, is_flag=True, - help=_('Show unavailable backends with reasons.')) -@flask.cli.with_appcontext -def keyring_status(unavailable): - if keg_keyring is None: - keyring_notify_no_module() - return - import keyring - import keyring.backend as kb - viable = kb.get_all_keyring() - - # call get_all_keyring() before this so we are sure all keyrings are loaded - # on KeyringBackend - if unavailable: - click.echo(_('Unavailable backends')) - for cls in kb.KeyringBackend._classes: - try: - cls.priority - except Exception as e: - click.echo(_(' {_class.__module__}:{_class.__name__} - {exception}', - _class=cls, exception=e)) - - click.echo(_('\nAvailable backends (backends with priority < 1 are not' - ' recommended and may be insecure)')) - for backend in viable: - click.echo(_(' {_class.__module__}:{_class.__name__} (priority: {priority})', - _class=backend.__class__, priority=backend.priority)) - - click.echo(_('\nDefault backend')) - backend = keyring.get_keyring() - click.echo(_(' {_class.__module__}:{_class.__name__}', _class=backend.__class__)) - if hasattr(backend, 'file_path'): - click.echo(_(' file path: {file_path}', file_path=backend.file_path)) - - if not flask.current_app.keyring_enabled: - click.echo(_('\nKeyring functionality for this app has been DISABLED through the config' - ' setting KEG_KEYRING_ENABLE.')) - elif not flask.current_app.keyring_manager.verify_backend(): - click.echo(_('\nWARNING: the current backend is insecure,' - ' keyring substitution unavailable.')) - if platform.system() == 'Linux': - click.echo(_('\nTRY THIS: use the SecretStorage Setup utility to get a more secure' - ' keyring backend.')) - click.echo('https://pypi.python.org/pypi/SecretStorage-Setup\n') - - -@click.command('list-keys', short_help=_('Show all keys used in config value substitution.')) -@flask.cli.with_appcontext -def keyring_list_keys(): - km = flask.current_app.keyring_manager - for key in sorted(km.sub_keys_seen): - click.echo(key) - - -@click.command('delete', short_help=_('Delete an entry from the keyring.')) -@click.argument('key') -@flask.cli.with_appcontext -def keyring_delete(key): - flask.current_app.keyring_manager.delete(key) - - class CLILoader(object): """ This loader takes care of the complexity of click object setup and instantiation in the diff --git a/keg/config.py b/keg/config.py index 13f33a2..2a1a9c6 100644 --- a/keg/config.py +++ b/keg/config.py @@ -171,8 +171,6 @@ class DefaultProfile(object): after_logout='public.home', ) - KEG_KEYRING_ENABLE = True - KEG_SMTP_HOST = 'localhost' KEG_DB_DIALECT_OPTIONS = {} @@ -186,7 +184,6 @@ class DevProfile(object): class TestProfile(object): DEBUG = True TESTING = True - KEG_KEYRING_ENABLE = False KEG_LOG_SYSLOG_ENABLED = False # set this to allow generation of URLs without a request context diff --git a/keg/keyring.py b/keg/keyring.py deleted file mode 100644 index 6afc69d..0000000 --- a/keg/keyring.py +++ /dev/null @@ -1,145 +0,0 @@ -from __future__ import absolute_import - -import getpass -import re -import sys -import itertools - -import six - -try: - import keyring -except ImportError: - keyring = None - -from keg.extensions import lazy_gettext as _ - - -class KeyringError(Exception): - pass - - -class MissingValueError(Exception): - - def __init__(self, key): - self.key = key - - super(Exception, self).__init__(_("No value for {key} found in key " - "ring", key=key)) - - -class Manager(object): - - # regex that matches "${*}$" where * = any printable ASCII character - # that isn't "}" - # see: http://www.catonmat.net/blog/my-favorite-regex/ - sub_pattern = r'(\$\{([ -|~]+?)\}\$)' - - backend_min_priority = 1 - - def __init__(self, app, service_name=None, secure_entry=getpass.getpass): - self.sub_re = re.compile(self.sub_pattern) - self.log = app.logger - self.app = app - self.service_name = service_name or self.app.name - self.sub_keys_seen = set() - self.secure_entry = secure_entry - - if not self.verify_backend(): - self.log.warning(_( - 'Insecure keyring backend detected, keyring substitution unavailable. ' - 'Run this app\'s keyring command for more info.' - )) - - def verify_backend(self): - backend = keyring.get_keyring() - if backend.priority < self.backend_min_priority: - return False - return True - - def pattern_to_password(self, value, service_name=None): - """ - Substitutes a configuration value which matches self.sub_pattern with - the value from the keyring. - - :param value: the substitutable value '${value}$' - :param service_name: a configured keyring service name - """ - if not isinstance(value, (six.binary_type, six.text_type)): - return value - - # preserve the original type so that we can convert at the end - is_bytes = isinstance(value, six.binary_type) - service = service_name or self.service_name - - # Convert to a regular string if it is bytes else use as is - value = value.decode() if is_bytes else value - - # Find all the possible keys we need to look up. Some values need - # multiple replacements, for example '${user}$:${pass}$'. Therefore - # this loop will run once for each match i.e. user and pass - for match in self.sub_re.finditer(value): - raw = match.group(1) # '${user}$' - clean = match.group(2) # user - - # TODO (NZ): I am not really wild about this being here, but I don't - # have a good way of keeping the data without it. - self.sub_keys_seen.add(clean) - - if self.log: - msg = _('Keyring Substitute: replacing {raw} for key {clean}', - raw=raw, clean=clean) - self.log.debug(msg) - - stored_value = keyring.get_password(service, clean) - if not stored_value: - raise MissingValueError(clean) - - value = value.replace(raw, stored_value, 1) - - return value.encode() if is_bytes else value - - def substitute(self, data): - if not self.verify_backend(): - return - - def key_and_value(key, value): - try: - password = self.pattern_to_password(value) - except MissingValueError as err: - # Backwards comp... Ask for the value if we can't find it. - # - # TODO: Remove the need to ask for a password in this method - # instead the substitute should work on known values and setting - # values should be a different function - password = self.ask_and_create(err.key) - - return key, password - - # Rifle through the dictionary, substitute where necessary - subbed = {key: value for key, value in - itertools.starmap(key_and_value, six.iteritems(data))} - - # Mutate the data passed in for backwards comp. - # TODO: Remove this mutation and just return the data allowing the - # caller to decide what to do with it. - data.update(subbed) - return subbed - - def delete(self, key): - keyring.delete_password(self.app.name, key) - - def create(self, key, value, service_name=None): - keyring.set_password(service_name or self.service_name, key, value) - - def ask_and_create(self, key, service_name=None): - msg = _('Missing secret for "{key}": ', key=key) - - # Windows getpass implementation breaks in Python2 when passing it Unicode - if sys.version_info < (3,) and sys.platform.startswith('win'): - msg = str(msg) - - value = self.secure_entry(msg) - self.create(key, value, service_name) - - return value diff --git a/keg/tests/test_cli.py b/keg/tests/test_cli.py index c5d8875..4845cbe 100644 --- a/keg/tests/test_cli.py +++ b/keg/tests/test_cli.py @@ -73,7 +73,7 @@ def test_help_all(self): expected_lines = [ 'Usage', '', 'Options', '--profile', '--quiet', '--help-all', '--help', 'Commands', 'develop', 'Commands', 'config', 'db', 'Commands', 'clear', - 'init', 'keyring', 'Commands', 'delete', 'list-keys', 'status', 'routes', + 'init', 'routes', 'run', 'shell', 'templates', 'hello1', 'is-not-quiet', 'is-quiet', 'reverse', '' ] diff --git a/keg/tests/test_keyring.py b/keg/tests/test_keyring.py deleted file mode 100644 index 92e9fc5..0000000 --- a/keg/tests/test_keyring.py +++ /dev/null @@ -1,86 +0,0 @@ -import keyring -import keg - - -try: - import unittest.mock as mock -except ImportError: - import mock - - -class MockKeyring(keyring.backend.KeyringBackend): - priority = 1 - - def __init__(self): - self.passwords = dict() - - def set_password(self, servicename, username, password): - key = '{}:{}'.format(servicename, username) - self.passwords[key] = password - - def get_password(self, servicename, username): - key = '{}:{}'.format(servicename, username) - return self.passwords.get(key, username) - - def delete_password(self, servicename, username, password): - pass - - -class TestKeyringManager: - - class MockApp: - logger = mock.MagicMock() - name = 'fake' - - def setup_method(self, _): - self.manager = self.create_manager() - - def create_manager(self, app=MockApp, backend=MockKeyring): - keyring.set_keyring(backend()) - return keg.keyring.Manager(app()) - - def test_pattern_to_password(self): - func = self.manager.pattern_to_password - - assert func('value') == 'value' - assert func('${value}$') == 'value' - assert func(b'${value}$') == b'value' - assert func(b'${value}$:${other}$') == b'value:other' - - def test_substitute_finds_all_values_for_sub(self): - data = { - 'no_ring': 'foo', - 'int_val': 1, - 'bool_val': True, - 'string_ring': '${string}$', - 'byte_ring': b'${bytes}$', - 'multi_string': '${multi_s_one}$:${multi_s_two}$', - 'multi_byte': b'${multi_b_one}$:${multi_b_two}$', - } - returned = self.manager.substitute(data) - - assert self.manager.sub_keys_seen == {'string', 'bytes', 'multi_s_one', - 'multi_s_two', 'multi_b_one', - 'multi_b_two'} - assert returned['multi_byte'] == b'multi_b_one:multi_b_two' - assert data == returned - - def test_ask_and_create(self): - self.manager.secure_entry = lambda msg: 'password' - - self.manager.ask_and_create('key') - assert keyring.get_password('fake', 'key') == 'password' - - def test_no_value_in_key_ring(self): - class NoValueMockKeyring(MockKeyring): - def get_password(self, servicename, username): - return None - - manager = self.create_manager(backend=NoValueMockKeyring) - manager.secure_entry = lambda msg: 'thing' - - data = manager.substitute( - {'unknown': '${unknown}$'} - ) - - assert data['unknown'] == 'thing' diff --git a/setup.py b/setup.py index 3ac46bf..505028f 100644 --- a/setup.py +++ b/setup.py @@ -50,7 +50,6 @@ 'tests': [ 'flask-webtest', 'flask-wtf', - 'keyring', "sqlalchemy_pyodbc_mssql; sys_platform == 'win32'", 'pytest', 'pytest-cov', diff --git a/tox.ini b/tox.ini index 77b0f58..e62b8f1 100644 --- a/tox.ini +++ b/tox.ini @@ -11,8 +11,8 @@ skip_install = true commands = pip --version lowest: pip install flask<1.0 - pip install .[tests] - i18n: pip install .[i18n] + pip install --progress-bar off .[tests] + i18n: pip install --progress-bar off .[i18n] # Output installed versions to compare with previous test runs in case a dependency's change # breaks things for our build. pip freeze