From 484977f7289d8de9c0c25246751ebcf76dddcf99 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Mon, 30 Jan 2017 15:34:59 -0500 Subject: [PATCH 01/23] Refactor security scanner validation from single sleep to polling --- util/config/validator.py | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/util/config/validator.py b/util/config/validator.py index 3c31ca056..40b4393e2 100644 --- a/util/config/validator.py +++ b/util/config/validator.py @@ -458,21 +458,26 @@ def _validate_signer(config, user_obj, _): def _validate_security_scanner(config, user_obj, _): """ Validates the configuration for talking to a Quay Security Scanner. """ + client = app.config['HTTPCLIENT'] + api = SecurityScannerAPI(app, config, None, client=client, skip_validation=True) if not config.get('TESTING', False): # Generate a temporary Quay key to use for signing the outgoing requests. setup_jwt_proxy() - # Wait a few seconds for the JWT proxy to startup. - time.sleep(2) + # We have to wait for JWT proxy to restart with the newly generated key. + max_tries = 5 + response = None + while max_tries > 0: + response = api.ping() + if response.status_code == 200: + return - # Make a ping request to the security service. - client = app.config['HTTPCLIENT'] - api = SecurityScannerAPI(app, config, None, client=client, skip_validation=True) - response = api.ping() - if response.status_code != 200: - message = 'Expected 200 status code, got %s: %s' % (response.status_code, response.text) - raise ConfigValidationException('Could not ping security scanner: %s' % message) + time.sleep(1) + max_tries = max_tries - 1 + + message = 'Expected 200 status code, got %s: %s' % (response.status_code, response.text) + raise ConfigValidationException('Could not ping security scanner: %s' % message) def _validate_bittorrent(config, user_obj, _): From f933b3e295a72a9d8746ed907066204ae22545a4 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Mon, 30 Jan 2017 16:24:58 -0500 Subject: [PATCH 02/23] Pull out database validation into validator class --- test/test_validate_config.py | 6 ------ util/config/validator.py | 3 ++- util/config/validators/__init__.py | 20 +++++++++++++++++++ util/config/validators/database.py | 18 +++++++++++++++++ util/config/validators/test/test_database.py | 21 ++++++++++++++++++++ 5 files changed, 61 insertions(+), 7 deletions(-) create mode 100644 util/config/validators/__init__.py create mode 100644 util/config/validators/database.py create mode 100644 util/config/validators/test/test_database.py diff --git a/test/test_validate_config.py b/test/test_validate_config.py index 2ad5cf2a8..20d8b3d28 100644 --- a/test/test_validate_config.py +++ b/test/test_validate_config.py @@ -46,12 +46,6 @@ class TestValidateConfig(unittest.TestCase): # Skip mail. self.validated.add('mail') - def test_validate_database(self): - with self.assertRaisesRegexp(Exception, 'database not properly initialized'): - self.validate('database', { - 'DB_URI': 'mysql://somehost', - }) - def test_validate_jwt(self): with self.assertRaisesRegexp(ConfigValidationException, 'Missing JWT Verification endpoint'): self.validate('jwt', { diff --git a/util/config/validator.py b/util/config/validator.py index 40b4393e2..e37575400 100644 --- a/util/config/validator.py +++ b/util/config/validator.py @@ -29,6 +29,7 @@ from util.secscan.api import SecurityScannerAPI from util.registry.torrent import torrent_jwt from util.security.signing import SIGNING_ENGINES from util.security.ssl import load_certificate, CertInvalidException, KeyInvalidException +from util.config.validators.database import DatabaseValidator logger = logging.getLogger(__name__) @@ -522,7 +523,7 @@ def _validate_bittorrent(config, user_obj, _): VALIDATORS = { - 'database': _validate_database, + DatabaseValidator.name: DatabaseValidator.validate, 'redis': _validate_redis, 'registry-storage': _validate_registry_storage, 'mail': _validate_mailing, diff --git a/util/config/validators/__init__.py b/util/config/validators/__init__.py new file mode 100644 index 000000000..a3edeeb12 --- /dev/null +++ b/util/config/validators/__init__.py @@ -0,0 +1,20 @@ +from abc import ABCMeta, abstractmethod, abstractproperty +from six import add_metaclass + +class ConfigValidationException(Exception): + """ Exception raised when the configuration fails to validate for a known reason. """ + pass + + +@add_metaclass(ABCMeta) +class BaseValidator(object): + @abstractproperty + def name(self): + """ The key for the validation API. """ + pass + + @classmethod + @abstractmethod + def validate(cls, config, user, user_password): + """ Raises Exception if failure to validate. """ + pass diff --git a/util/config/validators/database.py b/util/config/validators/database.py new file mode 100644 index 000000000..5fb27fa80 --- /dev/null +++ b/util/config/validators/database.py @@ -0,0 +1,18 @@ +from peewee import OperationalError + +from data.database import validate_database_url +from util.config.validators import BaseValidator, ConfigValidationException + +class DatabaseValidator(BaseValidator): + name = "database" + + @classmethod + def validate(cls, config, user, user_password): + """ Validates connecting to the database. """ + try: + validate_database_url(config['DB_URI'], config.get('DB_CONNECTION_ARGS', {})) + except OperationalError as ex: + if ex.args and len(ex.args) > 1: + raise ConfigValidationException(ex.args[1]) + else: + raise ex diff --git a/util/config/validators/test/test_database.py b/util/config/validators/test/test_database.py new file mode 100644 index 000000000..74612641e --- /dev/null +++ b/util/config/validators/test/test_database.py @@ -0,0 +1,21 @@ +import pytest + +from util.config.validators import ConfigValidationException +from util.config.validators.database import DatabaseValidator + +@pytest.mark.parametrize('unvalidated_config,user,user_password,expected', [ + (None, None, None, TypeError), + ({}, None, None, KeyError), + ({'DB_URI': 'sqlite:///:memory:'}, None, None, None), + ({'DB_URI': 'invalid:///:memory:'}, None, None, KeyError), + ({'DB_NOTURI': 'sqlite:///:memory:'}, None, None, KeyError), + ({'DB_URI': 'mysql:///someinvalid'}, None, None, ConfigValidationException), +]) +def test_validate_database(unvalidated_config, user, user_password, expected): + validator = DatabaseValidator() + + if expected is not None: + with pytest.raises(expected): + validator.validate(unvalidated_config, user, user_password) + else: + validator.validate(unvalidated_config, user, user_password) From b2afe686320be2c0df13ecf4b8af7067c6c70d6f Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Tue, 31 Jan 2017 13:47:49 -0500 Subject: [PATCH 03/23] Pull out redis validation into validator class --- requirements-nover.txt | 1 + requirements.txt | 1 + test/test_validate_config.py | 11 --------- util/config/validator.py | 14 +++-------- util/config/validators/buildlogredis.py | 16 +++++++++++++ .../validators/test/test_buildlogredis.py | 24 +++++++++++++++++++ 6 files changed, 45 insertions(+), 22 deletions(-) create mode 100644 util/config/validators/buildlogredis.py create mode 100644 util/config/validators/test/test_buildlogredis.py diff --git a/requirements-nover.txt b/requirements-nover.txt index 1d28e929a..7fdef7c77 100644 --- a/requirements-nover.txt +++ b/requirements-nover.txt @@ -68,3 +68,4 @@ trollius tzlocal xhtml2pdf recaptcha2 +mockredispy diff --git a/requirements.txt b/requirements.txt index 0076672fa..eaf37efea 100644 --- a/requirements.txt +++ b/requirements.txt @@ -52,6 +52,7 @@ marisa-trie==0.7.2 MarkupSafe==0.23 mixpanel==4.3.1 mock==2.0.0 +mockredispy==2.9.3 -e git+https://github.com/coreos/mockldap.git@59a46efbe8c7cd8146a87a7c4f2b09746b953e11#egg=mockldap monotonic==1.2 moto==0.4.25 diff --git a/test/test_validate_config.py b/test/test_validate_config.py index 20d8b3d28..1c4dbdc89 100644 --- a/test/test_validate_config.py +++ b/test/test_validate_config.py @@ -31,17 +31,6 @@ class TestValidateConfig(unittest.TestCase): config['TESTING'] = True VALIDATORS[service](config, user, password) - def test_validate_redis(self): - with self.assertRaisesRegexp(ConfigValidationException, 'Missing redis hostname'): - self.validate('redis', {}) - - with self.assertRaises(redis.ConnectionError): - self.validate('redis', { - 'BUILDLOGS_REDIS': { - 'host': 'somehost', - }, - }) - def test_validate_mail(self): # Skip mail. self.validated.add('mail') diff --git a/util/config/validator.py b/util/config/validator.py index e37575400..860e67254 100644 --- a/util/config/validator.py +++ b/util/config/validator.py @@ -29,7 +29,9 @@ from util.secscan.api import SecurityScannerAPI from util.registry.torrent import torrent_jwt from util.security.signing import SIGNING_ENGINES from util.security.ssl import load_certificate, CertInvalidException, KeyInvalidException + from util.config.validators.database import DatabaseValidator +from util.config.validators.buildlogredis import RedisValidator logger = logging.getLogger(__name__) @@ -93,16 +95,6 @@ def _validate_database(config, user_obj, _): raise ex -def _validate_redis(config, user_obj, _): - """ Validates connecting to redis. """ - redis_config = config.get('BUILDLOGS_REDIS', {}) - if not 'host' in redis_config: - raise ConfigValidationException('Missing redis hostname') - - client = redis.StrictRedis(socket_connect_timeout=5, **redis_config) - client.ping() - - def _validate_registry_storage(config, user_obj, _): """ Validates registry storage. """ replication_enabled = config.get('FEATURE_STORAGE_REPLICATION', False) @@ -524,7 +516,7 @@ def _validate_bittorrent(config, user_obj, _): VALIDATORS = { DatabaseValidator.name: DatabaseValidator.validate, - 'redis': _validate_redis, + RedisValidator.name: RedisValidator.validate, 'registry-storage': _validate_registry_storage, 'mail': _validate_mailing, 'github-login': _validate_github('GITHUB_LOGIN_CONFIG'), diff --git a/util/config/validators/buildlogredis.py b/util/config/validators/buildlogredis.py new file mode 100644 index 000000000..92909dbf4 --- /dev/null +++ b/util/config/validators/buildlogredis.py @@ -0,0 +1,16 @@ +import redis + +from util.config.validators import BaseValidator, ConfigValidationException + +class RedisValidator(BaseValidator): + name = "redis" + + @classmethod + def validate(cls, config, user, user_password): + """ Validates connecting to redis. """ + redis_config = config.get('BUILDLOGS_REDIS', {}) + if not 'host' in redis_config: + raise ConfigValidationException('Missing redis hostname') + + client = redis.StrictRedis(socket_connect_timeout=5, **redis_config) + client.ping() diff --git a/util/config/validators/test/test_buildlogredis.py b/util/config/validators/test/test_buildlogredis.py new file mode 100644 index 000000000..9936527dc --- /dev/null +++ b/util/config/validators/test/test_buildlogredis.py @@ -0,0 +1,24 @@ +import pytest +import redis + +from mock import patch + +from mockredis import mock_strict_redis_client + +from util.config.validators import ConfigValidationException +from util.config.validators.buildlogredis import RedisValidator + +@pytest.mark.parametrize('unvalidated_config,user,user_password,use_mock,expected', [ + ({}, None, None, False, ConfigValidationException), + ({'BUILDLOGS_REDIS': {}}, None, None, False, ConfigValidationException), + ({'BUILDLOGS_REDIS': {'host': 'somehost'}}, None, None, False, redis.ConnectionError), + ({'BUILDLOGS_REDIS': {'host': 'localhost'}}, None, None, True, None), +]) +def test_validate_redis(unvalidated_config, user, user_password, use_mock, expected): + with patch('redis.StrictRedis' if use_mock else 'redis.None', mock_strict_redis_client): + validator = RedisValidator() + if expected is not None: + with pytest.raises(expected): + validator.validate(unvalidated_config, user, user_password) + else: + validator.validate(unvalidated_config, user, user_password) From ee4f5ed5d68be977a72833abe3eb6d26e4476c7f Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Tue, 31 Jan 2017 15:56:25 -0500 Subject: [PATCH 04/23] Move registry storage validator to new location --- util/config/validator.py | 40 +----------------- util/config/validators/registrystorage.py | 42 +++++++++++++++++++ .../validators/test/test_registrystorage.py | 18 ++++++++ 3 files changed, 62 insertions(+), 38 deletions(-) create mode 100644 util/config/validators/registrystorage.py create mode 100644 util/config/validators/test/test_registrystorage.py diff --git a/util/config/validator.py b/util/config/validator.py index 860e67254..4395221af 100644 --- a/util/config/validator.py +++ b/util/config/validator.py @@ -7,7 +7,6 @@ from hashlib import sha1 import ldap import peewee -import redis from flask import Flask from flask_mail import Mail, Message @@ -32,6 +31,7 @@ from util.security.ssl import load_certificate, CertInvalidException, KeyInvalid from util.config.validators.database import DatabaseValidator from util.config.validators.buildlogredis import RedisValidator +from util.config.validators.registrystorage import StorageValidator logger = logging.getLogger(__name__) @@ -50,19 +50,6 @@ CONFIG_FILENAMES = (SSL_FILENAMES + DB_SSL_FILENAMES + JWT_FILENAMES + ACI_CERT_ LDAP_FILENAMES) EXTRA_CA_DIRECTORY = 'extra_ca_certs' -def get_storage_providers(config): - storage_config = config.get('DISTRIBUTED_STORAGE_CONFIG', {}) - - drivers = {} - - try: - for name, parameters in storage_config.items(): - drivers[name] = (parameters[0], get_storage_driver(None, None, None, parameters)) - except TypeError: - logger.exception('Missing required storage configuration provider') - raise ConfigValidationException('Missing required parameter(s) for storage %s' % name) - - return drivers def validate_service_for_config(service, config, password=None): """ Attempts to validate the configuration for the given service. """ @@ -95,29 +82,6 @@ def _validate_database(config, user_obj, _): raise ex -def _validate_registry_storage(config, user_obj, _): - """ Validates registry storage. """ - replication_enabled = config.get('FEATURE_STORAGE_REPLICATION', False) - - providers = get_storage_providers(config).items() - if not providers: - raise ConfigValidationException('Storage configuration required') - - for name, (storage_type, driver) in providers: - try: - if replication_enabled and storage_type == 'LocalStorage': - raise ConfigValidationException('Locally mounted directory not supported ' + - 'with storage replication') - - # Run validation on the driver. - driver.validate(app.config['HTTPCLIENT']) - - # Run setup on the driver if the read/write succeeded. - driver.setup() - except Exception as ex: - raise ConfigValidationException('Invalid storage configuration: %s: %s' % (name, str(ex))) - - def _validate_mailing(config, user_obj, _): """ Validates sending email. """ test_app = Flask("mail-test-app") @@ -517,7 +481,7 @@ def _validate_bittorrent(config, user_obj, _): VALIDATORS = { DatabaseValidator.name: DatabaseValidator.validate, RedisValidator.name: RedisValidator.validate, - 'registry-storage': _validate_registry_storage, + StorageValidator.name: StorageValidator.validate, 'mail': _validate_mailing, 'github-login': _validate_github('GITHUB_LOGIN_CONFIG'), 'github-trigger': _validate_github('GITHUB_TRIGGER_CONFIG'), diff --git a/util/config/validators/registrystorage.py b/util/config/validators/registrystorage.py new file mode 100644 index 000000000..ca2f79ed5 --- /dev/null +++ b/util/config/validators/registrystorage.py @@ -0,0 +1,42 @@ +from app import app +from storage import get_storage_driver +from util.config.validators import BaseValidator, ConfigValidationException + +class StorageValidator(BaseValidator): + name = "registry-storage" + + @classmethod + def validate(cls, config, user, user_password): + """ Validates registry storage. """ + replication_enabled = config.get('FEATURE_STORAGE_REPLICATION', False) + + providers = _get_storage_providers(config).items() + if not providers: + raise ConfigValidationException('Storage configuration required') + + for name, (storage_type, driver) in providers: + try: + if replication_enabled and storage_type == 'LocalStorage': + raise ConfigValidationException('Locally mounted directory not supported ' + + 'with storage replication') + + # Run validation on the driver. + driver.validate(app.config['HTTPCLIENT']) + + # Run setup on the driver if the read/write succeeded. + driver.setup() + except Exception as ex: + raise ConfigValidationException('Invalid storage configuration: %s: %s' % (name, str(ex))) + + +def _get_storage_providers(config): + storage_config = config.get('DISTRIBUTED_STORAGE_CONFIG', {}) + drivers = {} + + try: + for name, parameters in storage_config.items(): + drivers[name] = (parameters[0], get_storage_driver(None, None, None, parameters)) + except TypeError: + raise ConfigValidationException('Missing required parameter(s) for storage %s' % name) + + return drivers diff --git a/util/config/validators/test/test_registrystorage.py b/util/config/validators/test/test_registrystorage.py new file mode 100644 index 000000000..0d5406500 --- /dev/null +++ b/util/config/validators/test/test_registrystorage.py @@ -0,0 +1,18 @@ +import pytest + +from util.config.validators import ConfigValidationException +from util.config.validators.registrystorage import StorageValidator + +@pytest.mark.parametrize('unvalidated_config, expected', [ + ({}, ConfigValidationException), + ({'DISTRIBUTED_STORAGE_CONFIG': {}}, ConfigValidationException), + ({'DISTRIBUTED_STORAGE_CONFIG': {'local': None}}, ConfigValidationException), + ({'DISTRIBUTED_STORAGE_CONFIG': {'local': ['FakeStorage', {}]}}, None), +]) +def test_validate_storage(unvalidated_config, expected): + validator = StorageValidator() + if expected is not None: + with pytest.raises(expected): + validator.validate(unvalidated_config, None, None) + else: + validator.validate(unvalidated_config, None, None) From 00eceb7ed5df962f3a8db02389d60912ac6bf5f3 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Thu, 9 Feb 2017 11:04:57 -0800 Subject: [PATCH 05/23] Pull out email validation into validator class --- util/config/validator.py | 20 ++------------------ util/config/validators/email.py | 25 +++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 18 deletions(-) create mode 100644 util/config/validators/email.py diff --git a/util/config/validator.py b/util/config/validator.py index 4395221af..b727bbbd3 100644 --- a/util/config/validator.py +++ b/util/config/validator.py @@ -9,7 +9,6 @@ import ldap import peewee from flask import Flask -from flask_mail import Mail, Message from app import app, config_provider, get_app_url, OVERRIDE_CONFIG_DIRECTORY from auth.auth_context import get_authenticated_user @@ -32,6 +31,7 @@ from util.security.ssl import load_certificate, CertInvalidException, KeyInvalid from util.config.validators.database import DatabaseValidator from util.config.validators.buildlogredis import RedisValidator from util.config.validators.registrystorage import StorageValidator +from util.config.validators.email import EmailValidator logger = logging.getLogger(__name__) @@ -82,22 +82,6 @@ def _validate_database(config, user_obj, _): raise ex -def _validate_mailing(config, user_obj, _): - """ Validates sending email. """ - test_app = Flask("mail-test-app") - test_app.config.update(config) - test_app.config.update({ - 'MAIL_FAIL_SILENTLY': False, - 'TESTING': False - }) - - test_mail = Mail(test_app) - test_msg = Message("Test e-mail from %s" % app.config['REGISTRY_TITLE'], - sender=config.get('MAIL_DEFAULT_SENDER')) - test_msg.add_recipient(user_obj.email) - test_mail.send(test_msg) - - def _validate_gitlab(config, user_obj, _): """ Validates the OAuth credentials and API endpoint for a GitLab service. """ github_config = config.get('GITLAB_TRIGGER_CONFIG') @@ -482,7 +466,7 @@ VALIDATORS = { DatabaseValidator.name: DatabaseValidator.validate, RedisValidator.name: RedisValidator.validate, StorageValidator.name: StorageValidator.validate, - 'mail': _validate_mailing, + EmailValidator.name: EmailValidator.validate, 'github-login': _validate_github('GITHUB_LOGIN_CONFIG'), 'github-trigger': _validate_github('GITHUB_TRIGGER_CONFIG'), 'gitlab-trigger': _validate_gitlab, diff --git a/util/config/validators/email.py b/util/config/validators/email.py new file mode 100644 index 000000000..b394fdad4 --- /dev/null +++ b/util/config/validators/email.py @@ -0,0 +1,25 @@ +from flask import Flask +from flask_mail import Mail, Message + +from app import app +from util.config.validators import BaseValidator + +class EmailValidator(BaseValidator): + name = "mail" + + @classmethod + def validate(cls, config, user, user_password): + """ Validates sending email. """ + with app.app_context(): + test_app = Flask("mail-test-app") + test_app.config.update(config) + test_app.config.update({ + 'MAIL_FAIL_SILENTLY': False, + 'TESTING': False + }) + + test_mail = Mail(test_app) + test_msg = Message("Test e-mail from %s" % app.config['REGISTRY_TITLE'], + sender=config.get('MAIL_DEFAULT_SENDER')) + test_msg.add_recipient(user.email) + test_mail.send(test_msg) From 2d64cf300052a5b310f3f84a454ce3dce1303d8d Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Thu, 9 Feb 2017 15:51:28 -0800 Subject: [PATCH 06/23] Rename config validation source files --- util/config/validator.py | 8 +-- ..._database.py => test_validate_database.py} | 0 ...uildlogredis.py => test_validate_redis.py} | 0 ...trystorage.py => test_validate_storage.py} | 0 .../{database.py => validate_database.py} | 0 .../{email.py => validate_email.py} | 0 util/config/validators/validate_ldap.py | 59 +++++++++++++++++++ .../{buildlogredis.py => validate_redis.py} | 0 ...registrystorage.py => validate_storage.py} | 0 9 files changed, 63 insertions(+), 4 deletions(-) rename util/config/validators/test/{test_database.py => test_validate_database.py} (100%) rename util/config/validators/test/{test_buildlogredis.py => test_validate_redis.py} (100%) rename util/config/validators/test/{test_registrystorage.py => test_validate_storage.py} (100%) rename util/config/validators/{database.py => validate_database.py} (100%) rename util/config/validators/{email.py => validate_email.py} (100%) create mode 100644 util/config/validators/validate_ldap.py rename util/config/validators/{buildlogredis.py => validate_redis.py} (100%) rename util/config/validators/{registrystorage.py => validate_storage.py} (100%) diff --git a/util/config/validator.py b/util/config/validator.py index b727bbbd3..5656fcef7 100644 --- a/util/config/validator.py +++ b/util/config/validator.py @@ -28,10 +28,10 @@ from util.registry.torrent import torrent_jwt from util.security.signing import SIGNING_ENGINES from util.security.ssl import load_certificate, CertInvalidException, KeyInvalidException -from util.config.validators.database import DatabaseValidator -from util.config.validators.buildlogredis import RedisValidator -from util.config.validators.registrystorage import StorageValidator -from util.config.validators.email import EmailValidator +from util.config.validators.validate_database import DatabaseValidator +from util.config.validators.validate_redis import RedisValidator +from util.config.validators.validate_storage import StorageValidator +from util.config.validators.validate_email import EmailValidator logger = logging.getLogger(__name__) diff --git a/util/config/validators/test/test_database.py b/util/config/validators/test/test_validate_database.py similarity index 100% rename from util/config/validators/test/test_database.py rename to util/config/validators/test/test_validate_database.py diff --git a/util/config/validators/test/test_buildlogredis.py b/util/config/validators/test/test_validate_redis.py similarity index 100% rename from util/config/validators/test/test_buildlogredis.py rename to util/config/validators/test/test_validate_redis.py diff --git a/util/config/validators/test/test_registrystorage.py b/util/config/validators/test/test_validate_storage.py similarity index 100% rename from util/config/validators/test/test_registrystorage.py rename to util/config/validators/test/test_validate_storage.py diff --git a/util/config/validators/database.py b/util/config/validators/validate_database.py similarity index 100% rename from util/config/validators/database.py rename to util/config/validators/validate_database.py diff --git a/util/config/validators/email.py b/util/config/validators/validate_email.py similarity index 100% rename from util/config/validators/email.py rename to util/config/validators/validate_email.py diff --git a/util/config/validators/validate_ldap.py b/util/config/validators/validate_ldap.py new file mode 100644 index 000000000..b80d5827f --- /dev/null +++ b/util/config/validators/validate_ldap.py @@ -0,0 +1,59 @@ +from app import app +from util.config.validators import BaseValidator + +class LDAPValidator(BaseValidator): + name = "ldap" + + @classmethod + def validate(cls, config, user, user_password): + """ Validates the LDAP connection. """ + if config.get('AUTHENTICATION_TYPE', 'Database') != 'LDAP': + return + + # If there is a custom LDAP certificate, then reinstall the certificates for the container. + if config_provider.volume_file_exists(LDAP_CERT_FILENAME): + subprocess.check_call(['/conf/init/certs_install.sh']) + + # Note: raises ldap.INVALID_CREDENTIALS on failure + admin_dn = config.get('LDAP_ADMIN_DN') + admin_passwd = config.get('LDAP_ADMIN_PASSWD') + + if not admin_dn: + raise ConfigValidationException('Missing Admin DN for LDAP configuration') + + if not admin_passwd: + raise ConfigValidationException('Missing Admin Password for LDAP configuration') + + ldap_uri = config.get('LDAP_URI', 'ldap://localhost') + if not ldap_uri.startswith('ldap://') and not ldap_uri.startswith('ldaps://'): + raise ConfigValidationException('LDAP URI must start with ldap:// or ldaps://') + + allow_tls_fallback = config.get('LDAP_ALLOW_INSECURE_FALLBACK', False) + + try: + with LDAPConnection(ldap_uri, admin_dn, admin_passwd, allow_tls_fallback): + pass + except ldap.LDAPError as ex: + values = ex.args[0] if ex.args else {} + if not isinstance(values, dict): + raise ConfigValidationException(str(ex.args)) + + raise ConfigValidationException(values.get('desc', 'Unknown error')) + + # Verify that the superuser exists. If not, raise an exception. + base_dn = config.get('LDAP_BASE_DN') + user_rdn = config.get('LDAP_USER_RDN', []) + uid_attr = config.get('LDAP_UID_ATTR', 'uid') + email_attr = config.get('LDAP_EMAIL_ATTR', 'mail') + requires_email = config.get('FEATURE_MAILING', True) + + users = LDAPUsers(ldap_uri, base_dn, admin_dn, admin_passwd, user_rdn, uid_attr, email_attr, + allow_tls_fallback, requires_email=requires_email) + + username = user_obj.username + (result, err_msg) = users.verify_credentials(username, password) + if not result: + msg = ('Verification of superuser %s failed: %s. \n\nThe user either does not exist ' + + 'in the remote authentication system ' + + 'OR LDAP auth is misconfigured.') % (username, err_msg) + raise ConfigValidationException(msg) diff --git a/util/config/validators/buildlogredis.py b/util/config/validators/validate_redis.py similarity index 100% rename from util/config/validators/buildlogredis.py rename to util/config/validators/validate_redis.py diff --git a/util/config/validators/registrystorage.py b/util/config/validators/validate_storage.py similarity index 100% rename from util/config/validators/registrystorage.py rename to util/config/validators/validate_storage.py From c55ddf73412c2bf5d91e8a416e1deb4696f2a955 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Thu, 9 Feb 2017 16:09:57 -0800 Subject: [PATCH 07/23] Pull out ldap validation into validator class --- util/config/validator.py | 59 +---------------- .../validators/test/test_validate_ldap.py | 64 +++++++++++++++++++ util/config/validators/validate_ldap.py | 13 ++-- 3 files changed, 75 insertions(+), 61 deletions(-) create mode 100644 util/config/validators/test/test_validate_ldap.py diff --git a/util/config/validator.py b/util/config/validator.py index 5656fcef7..848a8a8e5 100644 --- a/util/config/validator.py +++ b/util/config/validator.py @@ -1,11 +1,9 @@ import logging -import subprocess import time from StringIO import StringIO from hashlib import sha1 -import ldap import peewee from flask import Flask @@ -32,6 +30,7 @@ from util.config.validators.validate_database import DatabaseValidator from util.config.validators.validate_redis import RedisValidator from util.config.validators.validate_storage import StorageValidator from util.config.validators.validate_email import EmailValidator +from util.config.validators.validate_ldap import LDAPValidator logger = logging.getLogger(__name__) @@ -239,60 +238,6 @@ def _validate_ssl(config, user_obj, _): raise ConfigValidationException('SSL private key failed to validate: %s' % kie.message) -def _validate_ldap(config, user_obj, password): - """ Validates the LDAP connection. """ - if config.get('AUTHENTICATION_TYPE', 'Database') != 'LDAP': - return - - # If there is a custom LDAP certificate, then reinstall the certificates for the container. - if config_provider.volume_file_exists(LDAP_CERT_FILENAME): - subprocess.check_call(['/conf/init/certs_install.sh']) - - # Note: raises ldap.INVALID_CREDENTIALS on failure - admin_dn = config.get('LDAP_ADMIN_DN') - admin_passwd = config.get('LDAP_ADMIN_PASSWD') - - if not admin_dn: - raise ConfigValidationException('Missing Admin DN for LDAP configuration') - - if not admin_passwd: - raise ConfigValidationException('Missing Admin Password for LDAP configuration') - - ldap_uri = config.get('LDAP_URI', 'ldap://localhost') - if not ldap_uri.startswith('ldap://') and not ldap_uri.startswith('ldaps://'): - raise ConfigValidationException('LDAP URI must start with ldap:// or ldaps://') - - allow_tls_fallback = config.get('LDAP_ALLOW_INSECURE_FALLBACK', False) - - try: - with LDAPConnection(ldap_uri, admin_dn, admin_passwd, allow_tls_fallback): - pass - except ldap.LDAPError as ex: - values = ex.args[0] if ex.args else {} - if not isinstance(values, dict): - raise ConfigValidationException(str(ex.args)) - - raise ConfigValidationException(values.get('desc', 'Unknown error')) - - # Verify that the superuser exists. If not, raise an exception. - base_dn = config.get('LDAP_BASE_DN') - user_rdn = config.get('LDAP_USER_RDN', []) - uid_attr = config.get('LDAP_UID_ATTR', 'uid') - email_attr = config.get('LDAP_EMAIL_ATTR', 'mail') - requires_email = config.get('FEATURE_MAILING', True) - - users = LDAPUsers(ldap_uri, base_dn, admin_dn, admin_passwd, user_rdn, uid_attr, email_attr, - allow_tls_fallback, requires_email=requires_email) - - username = user_obj.username - (result, err_msg) = users.verify_credentials(username, password) - if not result: - msg = ('Verification of superuser %s failed: %s. \n\nThe user either does not exist ' + - 'in the remote authentication system ' + - 'OR LDAP auth is misconfigured.') % (username, err_msg) - raise ConfigValidationException(msg) - - def _validate_jwt(config, user_obj, password): """ Validates the JWT authentication system. """ if config.get('AUTHENTICATION_TYPE', 'Database') != 'JWT': @@ -473,7 +418,7 @@ VALIDATORS = { 'bitbucket-trigger': _validate_bitbucket, 'google-login': _validate_google_login, 'ssl': _validate_ssl, - 'ldap': _validate_ldap, + LDAPValidator.name: LDAPValidator.validate, 'jwt': _validate_jwt, 'keystone': _validate_keystone, 'signer': _validate_signer, diff --git a/util/config/validators/test/test_validate_ldap.py b/util/config/validators/test/test_validate_ldap.py new file mode 100644 index 000000000..4a2a8d761 --- /dev/null +++ b/util/config/validators/test/test_validate_ldap.py @@ -0,0 +1,64 @@ +import pytest + +from util.config.validators import ConfigValidationException +from util.config.validators.validate_ldap import LDAPValidator +from util.morecollections import AttrDict + +from test.test_ldap import mock_ldap + + +@pytest.mark.parametrize('unvalidated_config', [ + ({}), + ({'AUTHENTICATION_TYPE': 'Database'}), +]) +def test_validate_noop(unvalidated_config): + LDAPValidator.validate(unvalidated_config, None, None) + +@pytest.mark.parametrize('unvalidated_config', [ + ({'AUTHENTICATION_TYPE': 'LDAP'}), + ({'AUTHENTICATION_TYPE': 'LDAP', 'LDAP_ADMIN_DN': 'foo'}), +]) +def test_invalid_config(unvalidated_config): + with pytest.raises(ConfigValidationException): + LDAPValidator.validate(unvalidated_config, None, None) + + +@pytest.mark.parametrize('uri', [ + 'foo', + 'http://foo', + 'ldap:foo', +]) +def test_invalid_uri(uri): + config = {} + config['AUTHENTICATION_TYPE'] = 'LDAP' + config['LDAP_BASE_DN'] = ['dc=quay', 'dc=io'] + config['LDAP_ADMIN_DN'] = 'uid=testy,ou=employees,dc=quay,dc=io' + config['LDAP_ADMIN_PASSWD'] = 'password' + config['LDAP_USER_RDN'] = ['ou=employees'] + config['LDAP_URI'] = uri + + with pytest.raises(ConfigValidationException): + LDAPValidator.validate(config, None, None) + + +@pytest.mark.parametrize('username, password, expected_exception', [ + ('invaliduser', 'invalidpass', ConfigValidationException), + ('someuser', 'invalidpass', ConfigValidationException), + ('invaliduser', 'somepass', ConfigValidationException), + ('someuser', 'somepass', None), +]) +def test_validated_ldap(username, password, expected_exception): + config = {} + config['AUTHENTICATION_TYPE'] = 'LDAP' + config['LDAP_BASE_DN'] = ['dc=quay', 'dc=io'] + config['LDAP_ADMIN_DN'] = 'uid=testy,ou=employees,dc=quay,dc=io' + config['LDAP_ADMIN_PASSWD'] = 'password' + config['LDAP_USER_RDN'] = ['ou=employees'] + + if expected_exception is not None: + with pytest.raises(ConfigValidationException): + with mock_ldap(): + LDAPValidator.validate(config, AttrDict(dict(username=username)), password) + else: + with mock_ldap(): + LDAPValidator.validate(config, AttrDict(dict(username=username)), password) diff --git a/util/config/validators/validate_ldap.py b/util/config/validators/validate_ldap.py index b80d5827f..93e34453d 100644 --- a/util/config/validators/validate_ldap.py +++ b/util/config/validators/validate_ldap.py @@ -1,5 +1,10 @@ -from app import app -from util.config.validators import BaseValidator +import ldap +import subprocess + +from app import app, config_provider +from data.users import LDAP_CERT_FILENAME +from data.users.externalldap import LDAPConnection, LDAPUsers +from util.config.validators import BaseValidator, ConfigValidationException class LDAPValidator(BaseValidator): name = "ldap" @@ -50,8 +55,8 @@ class LDAPValidator(BaseValidator): users = LDAPUsers(ldap_uri, base_dn, admin_dn, admin_passwd, user_rdn, uid_attr, email_attr, allow_tls_fallback, requires_email=requires_email) - username = user_obj.username - (result, err_msg) = users.verify_credentials(username, password) + username = user.username + (result, err_msg) = users.verify_credentials(username, user_password) if not result: msg = ('Verification of superuser %s failed: %s. \n\nThe user either does not exist ' + 'in the remote authentication system ' + From 678f868bc4cd47ec105f2747e231b2c11ec8b88f Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Thu, 9 Feb 2017 16:30:07 -0800 Subject: [PATCH 08/23] Pull out keystone validation into validator class --- util/config/validator.py | 43 +--------------- .../validators/test/test_validate_keystone.py | 51 +++++++++++++++++++ util/config/validators/validate_keystone.py | 42 +++++++++++++++ 3 files changed, 95 insertions(+), 41 deletions(-) create mode 100644 util/config/validators/test/test_validate_keystone.py create mode 100644 util/config/validators/validate_keystone.py diff --git a/util/config/validator.py b/util/config/validator.py index 848a8a8e5..f09f9192a 100644 --- a/util/config/validator.py +++ b/util/config/validator.py @@ -15,9 +15,6 @@ from boot import setup_jwt_proxy from data.database import validate_database_url from data.users import LDAP_CERT_FILENAME from data.users.externaljwt import ExternalJWTAuthN -from data.users.externalldap import LDAPConnection, LDAPUsers -from data.users.keystone import get_keystone_users -from storage import get_storage_driver from oauth.services.github import GithubOAuthService from oauth.services.google import GoogleOAuthService from oauth.services.gitlab import GitLabOAuthService @@ -31,6 +28,7 @@ from util.config.validators.validate_redis import RedisValidator from util.config.validators.validate_storage import StorageValidator from util.config.validators.validate_email import EmailValidator from util.config.validators.validate_ldap import LDAPValidator +from util.config.validators.validate_keystone import KeystoneValidator logger = logging.getLogger(__name__) @@ -293,43 +291,6 @@ def _validate_jwt(config, user_obj, password): 'properly: %s' % err_msg) -def _validate_keystone(config, user_obj, password): - """ Validates the Keystone authentication system. """ - if config.get('AUTHENTICATION_TYPE', 'Database') != 'Keystone': - return - - auth_url = config.get('KEYSTONE_AUTH_URL') - auth_version = int(config.get('KEYSTONE_AUTH_VERSION', 2)) - admin_username = config.get('KEYSTONE_ADMIN_USERNAME') - admin_password = config.get('KEYSTONE_ADMIN_PASSWORD') - admin_tenant = config.get('KEYSTONE_ADMIN_TENANT') - - if not auth_url: - raise ConfigValidationException('Missing authentication URL') - - if not admin_username: - raise ConfigValidationException('Missing admin username') - - if not admin_password: - raise ConfigValidationException('Missing admin password') - - if not admin_tenant: - raise ConfigValidationException('Missing admin tenant') - - requires_email = config.get('FEATURE_MAILING', True) - users = get_keystone_users(auth_version, auth_url, admin_username, admin_password, admin_tenant, - requires_email) - - # Verify that the superuser exists. If not, raise an exception. - username = user_obj.username - (result, err_msg) = users.verify_credentials(username, password) - if not result: - msg = ('Verification of superuser %s failed: %s \n\nThe user either does not ' + - 'exist in the remote authentication system ' + - 'OR Keystone auth is misconfigured.') % (username, err_msg) - raise ConfigValidationException(msg) - - def _validate_signer(config, user_obj, _): """ Validates the GPG public+private key pair used for signing converted ACIs. """ if config.get('SIGNING_ENGINE') is None: @@ -420,7 +381,7 @@ VALIDATORS = { 'ssl': _validate_ssl, LDAPValidator.name: LDAPValidator.validate, 'jwt': _validate_jwt, - 'keystone': _validate_keystone, + KeystoneValidator.name: KeystoneValidator.validate, 'signer': _validate_signer, 'security-scanner': _validate_security_scanner, 'bittorrent': _validate_bittorrent, diff --git a/util/config/validators/test/test_validate_keystone.py b/util/config/validators/test/test_validate_keystone.py new file mode 100644 index 000000000..77a9e35b0 --- /dev/null +++ b/util/config/validators/test/test_validate_keystone.py @@ -0,0 +1,51 @@ +import pytest + +from util.config.validators import ConfigValidationException +from util.config.validators.validate_keystone import KeystoneValidator +from util.morecollections import AttrDict + +from test.test_keystone_auth import fake_keystone + + +@pytest.mark.parametrize('unvalidated_config', [ + ({}), + ({'AUTHENTICATION_TYPE': 'Database'}), +]) +def test_validate_noop(unvalidated_config): + KeystoneValidator.validate(unvalidated_config, None, None) + +@pytest.mark.parametrize('unvalidated_config', [ + ({'AUTHENTICATION_TYPE': 'Keystone'}), + ({'AUTHENTICATION_TYPE': 'Keystone', 'KEYSTONE_AUTH_URL': 'foo'}), + ({'AUTHENTICATION_TYPE': 'Keystone', 'KEYSTONE_AUTH_URL': 'foo', + 'KEYSTONE_ADMIN_USERNAME': 'bar'}), + ({'AUTHENTICATION_TYPE': 'Keystone', 'KEYSTONE_AUTH_URL': 'foo', + 'KEYSTONE_ADMIN_USERNAME': 'bar', 'KEYSTONE_ADMIN_PASSWORD': 'baz'}), +]) +def test_invalid_config(unvalidated_config): + with pytest.raises(ConfigValidationException): + KeystoneValidator.validate(unvalidated_config, None, None) + + +@pytest.mark.parametrize('username, password, expected_exception', [ + ('invaliduser', 'invalidpass', ConfigValidationException), + ('cool.user', 'invalidpass', ConfigValidationException), + ('invaliduser', 'somepass', ConfigValidationException), + ('cool.user', 'password', None), +]) +def test_validated_keystone(username, password, expected_exception): + with fake_keystone(2) as keystone_auth: + auth_url = keystone_auth.auth_url + + config = {} + config['AUTHENTICATION_TYPE'] = 'Keystone' + config['KEYSTONE_AUTH_URL'] = auth_url + config['KEYSTONE_ADMIN_USERNAME'] = 'adminuser' + config['KEYSTONE_ADMIN_PASSWORD'] = 'adminpass' + config['KEYSTONE_ADMIN_TENANT'] = 'admintenant' + + if expected_exception is not None: + with pytest.raises(ConfigValidationException): + KeystoneValidator.validate(config, AttrDict(dict(username=username)), password) + else: + KeystoneValidator.validate(config, AttrDict(dict(username=username)), password) diff --git a/util/config/validators/validate_keystone.py b/util/config/validators/validate_keystone.py new file mode 100644 index 000000000..415f7958b --- /dev/null +++ b/util/config/validators/validate_keystone.py @@ -0,0 +1,42 @@ +from util.config.validators import BaseValidator, ConfigValidationException +from data.users.keystone import get_keystone_users + +class KeystoneValidator(BaseValidator): + name = "keystone" + + @classmethod + def validate(cls, config, user, user_password): + """ Validates the Keystone authentication system. """ + if config.get('AUTHENTICATION_TYPE', 'Database') != 'Keystone': + return + + auth_url = config.get('KEYSTONE_AUTH_URL') + auth_version = int(config.get('KEYSTONE_AUTH_VERSION', 2)) + admin_username = config.get('KEYSTONE_ADMIN_USERNAME') + admin_password = config.get('KEYSTONE_ADMIN_PASSWORD') + admin_tenant = config.get('KEYSTONE_ADMIN_TENANT') + + if not auth_url: + raise ConfigValidationException('Missing authentication URL') + + if not admin_username: + raise ConfigValidationException('Missing admin username') + + if not admin_password: + raise ConfigValidationException('Missing admin password') + + if not admin_tenant: + raise ConfigValidationException('Missing admin tenant') + + requires_email = config.get('FEATURE_MAILING', True) + users = get_keystone_users(auth_version, auth_url, admin_username, admin_password, admin_tenant, + requires_email) + + # Verify that the superuser exists. If not, raise an exception. + username = user.username + (result, err_msg) = users.verify_credentials(username, user_password) + if not result: + msg = ('Verification of superuser %s failed: %s \n\nThe user either does not ' + + 'exist in the remote authentication system ' + + 'OR Keystone auth is misconfigured.') % (username, err_msg) + raise ConfigValidationException(msg) From c0f7530b295380cec55052835230d660ce71d466 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Thu, 9 Feb 2017 17:07:14 -0800 Subject: [PATCH 09/23] Pull out JWT auth validation into validator class Also fixes a small bug in validation (yay tests!) --- data/users/externaljwt.py | 5 +- test/test_external_jwt_authn.py | 3 +- util/config/validator.py | 59 +----------------- .../validators/test/test_validate_jwt.py | 49 +++++++++++++++ util/config/validators/validate_jwt.py | 62 +++++++++++++++++++ 5 files changed, 118 insertions(+), 60 deletions(-) create mode 100644 util/config/validators/test/test_validate_jwt.py create mode 100644 util/config/validators/validate_jwt.py diff --git a/data/users/externaljwt.py b/data/users/externaljwt.py index 444ada43a..33e8f506b 100644 --- a/data/users/externaljwt.py +++ b/data/users/externaljwt.py @@ -28,11 +28,12 @@ class ExternalJWTAuthN(FederatedUsers): default_key_path = os.path.join(override_config_dir, ExternalJWTAuthN.PUBLIC_KEY_FILENAME) public_key_path = public_key_path or default_key_path if not os.path.exists(public_key_path): - error_message = ('JWT Authentication public key file "%s" not found in directory %s' % - (ExternalJWTAuthN.PUBLIC_KEY_FILENAME, override_config_dir)) + error_message = ('JWT Authentication public key file "%s" not found' % public_key_path) raise Exception(error_message) + self.public_key_path = public_key_path + with open(public_key_path) as public_key_file: self.public_key = public_key_file.read() diff --git a/test/test_external_jwt_authn.py b/test/test_external_jwt_authn.py index 47b1fa4bf..dac442505 100644 --- a/test/test_external_jwt_authn.py +++ b/test/test_external_jwt_authn.py @@ -36,7 +36,8 @@ def fake_jwt(requires_email=True): getuser_url = server_url + '/user/get' jwt_auth = ExternalJWTAuthN(verify_url, query_url, getuser_url, 'authy', '', - app.config['HTTPCLIENT'], 300, public_key.name, + app.config['HTTPCLIENT'], 300, + public_key_path=public_key.name, requires_email=requires_email) with liveserver_app(jwt_app, port): diff --git a/util/config/validator.py b/util/config/validator.py index f09f9192a..f184a9264 100644 --- a/util/config/validator.py +++ b/util/config/validator.py @@ -14,7 +14,6 @@ from bitbucket import BitBucket from boot import setup_jwt_proxy from data.database import validate_database_url from data.users import LDAP_CERT_FILENAME -from data.users.externaljwt import ExternalJWTAuthN from oauth.services.github import GithubOAuthService from oauth.services.google import GoogleOAuthService from oauth.services.gitlab import GitLabOAuthService @@ -29,6 +28,7 @@ from util.config.validators.validate_storage import StorageValidator from util.config.validators.validate_email import EmailValidator from util.config.validators.validate_ldap import LDAPValidator from util.config.validators.validate_keystone import KeystoneValidator +from util.config.validators.validate_jwt import JWTAuthValidator logger = logging.getLogger(__name__) @@ -236,61 +236,6 @@ def _validate_ssl(config, user_obj, _): raise ConfigValidationException('SSL private key failed to validate: %s' % kie.message) -def _validate_jwt(config, user_obj, password): - """ Validates the JWT authentication system. """ - if config.get('AUTHENTICATION_TYPE', 'Database') != 'JWT': - return - - verify_endpoint = config.get('JWT_VERIFY_ENDPOINT') - query_endpoint = config.get('JWT_QUERY_ENDPOINT', None) - getuser_endpoint = config.get('JWT_GETUSER_ENDPOINT', None) - - issuer = config.get('JWT_AUTH_ISSUER') - - if not verify_endpoint: - raise ConfigValidationException('Missing JWT Verification endpoint') - - if not issuer: - raise ConfigValidationException('Missing JWT Issuer ID') - - # Try to instatiate the JWT authentication mechanism. This will raise an exception if - # the key cannot be found. - users = ExternalJWTAuthN(verify_endpoint, query_endpoint, getuser_endpoint, issuer, - OVERRIDE_CONFIG_DIRECTORY, - app.config['HTTPCLIENT'], - app.config.get('JWT_AUTH_MAX_FRESH_S', 300), - requires_email=config.get('FEATURE_MAILING', True)) - - # Verify that the superuser exists. If not, raise an exception. - username = user_obj.username - (result, err_msg) = users.verify_credentials(username, password) - if not result: - msg = ('Verification of superuser %s failed: %s. \n\nThe user either does not ' + - 'exist in the remote authentication system ' + - 'OR JWT auth is misconfigured') % (username, err_msg) - raise ConfigValidationException(msg) - - # If the query endpoint exists, ensure we can query to find the current user and that we can - # look up users directly. - if query_endpoint: - (results, err_msg) = users.query_users(username) - if not results: - err_msg = err_msg or ('Could not find users matching query: %s' % username) - raise ConfigValidationException('Query endpoint is misconfigured or not returning ' + - 'proper users: %s' % err_msg) - - # Make sure the get user endpoint is also configured. - if not getuser_endpoint: - raise ConfigValidationException('The lookup user endpoint must be configured if the ' + - 'query endpoint is set') - - (result, err_msg) = users.get_user(username) - if not result: - err_msg = err_msg or ('Could not find user %s' % username) - raise ConfigValidationException('Lookup endpoint is misconfigured or not returning ' + - 'properly: %s' % err_msg) - - def _validate_signer(config, user_obj, _): """ Validates the GPG public+private key pair used for signing converted ACIs. """ if config.get('SIGNING_ENGINE') is None: @@ -380,7 +325,7 @@ VALIDATORS = { 'google-login': _validate_google_login, 'ssl': _validate_ssl, LDAPValidator.name: LDAPValidator.validate, - 'jwt': _validate_jwt, + JWTAuthValidator.name: JWTAuthValidator.validate, KeystoneValidator.name: KeystoneValidator.validate, 'signer': _validate_signer, 'security-scanner': _validate_security_scanner, diff --git a/util/config/validators/test/test_validate_jwt.py b/util/config/validators/test/test_validate_jwt.py new file mode 100644 index 000000000..81114bb8a --- /dev/null +++ b/util/config/validators/test/test_validate_jwt.py @@ -0,0 +1,49 @@ +import pytest + +from util.config.validators import ConfigValidationException +from util.config.validators.validate_jwt import JWTAuthValidator +from util.morecollections import AttrDict + +from test.test_external_jwt_authn import fake_jwt + + +@pytest.mark.parametrize('unvalidated_config', [ + ({}), + ({'AUTHENTICATION_TYPE': 'Database'}), +]) +def test_validate_noop(unvalidated_config): + JWTAuthValidator.validate(unvalidated_config, None, None) + + +@pytest.mark.parametrize('unvalidated_config', [ + ({'AUTHENTICATION_TYPE': 'JWT'}), + ({'AUTHENTICATION_TYPE': 'JWT', 'JWT_AUTH_ISSUER': 'foo'}), + ({'AUTHENTICATION_TYPE': 'JWT', 'JWT_VERIFY_ENDPOINT': 'foo'}), +]) +def test_invalid_config(unvalidated_config): + with pytest.raises(ConfigValidationException): + JWTAuthValidator.validate(unvalidated_config, None, None) + + +@pytest.mark.parametrize('username, password, expected_exception', [ + ('invaliduser', 'invalidpass', ConfigValidationException), + ('cool.user', 'invalidpass', ConfigValidationException), + ('invaliduser', 'somepass', ConfigValidationException), + ('cool.user', 'password', None), +]) +def test_validated_jwt(username, password, expected_exception): + with fake_jwt() as jwt_auth: + config = {} + config['AUTHENTICATION_TYPE'] = 'JWT' + config['JWT_AUTH_ISSUER'] = jwt_auth.issuer + config['JWT_VERIFY_ENDPOINT'] = jwt_auth.verify_url + config['JWT_QUERY_ENDPOINT'] = jwt_auth.query_url + config['JWT_GETUSER_ENDPOINT'] = jwt_auth.getuser_url + + if expected_exception is not None: + with pytest.raises(ConfigValidationException): + JWTAuthValidator.validate(config, AttrDict(dict(username=username)), password, + public_key_path=jwt_auth.public_key_path) + else: + JWTAuthValidator.validate(config, AttrDict(dict(username=username)), password, + public_key_path=jwt_auth.public_key_path) diff --git a/util/config/validators/validate_jwt.py b/util/config/validators/validate_jwt.py new file mode 100644 index 000000000..808e74152 --- /dev/null +++ b/util/config/validators/validate_jwt.py @@ -0,0 +1,62 @@ +from app import app, OVERRIDE_CONFIG_DIRECTORY +from data.users.externaljwt import ExternalJWTAuthN +from util.config.validators import BaseValidator, ConfigValidationException + +class JWTAuthValidator(BaseValidator): + name = "jwt" + + @classmethod + def validate(cls, config, user, user_password, public_key_path=None): + """ Validates the JWT authentication system. """ + if config.get('AUTHENTICATION_TYPE', 'Database') != 'JWT': + return + + verify_endpoint = config.get('JWT_VERIFY_ENDPOINT') + query_endpoint = config.get('JWT_QUERY_ENDPOINT', None) + getuser_endpoint = config.get('JWT_GETUSER_ENDPOINT', None) + + issuer = config.get('JWT_AUTH_ISSUER') + + if not verify_endpoint: + raise ConfigValidationException('Missing JWT Verification endpoint') + + if not issuer: + raise ConfigValidationException('Missing JWT Issuer ID') + + # Try to instatiate the JWT authentication mechanism. This will raise an exception if + # the key cannot be found. + users = ExternalJWTAuthN(verify_endpoint, query_endpoint, getuser_endpoint, issuer, + OVERRIDE_CONFIG_DIRECTORY, + app.config['HTTPCLIENT'], + app.config.get('JWT_AUTH_MAX_FRESH_S', 300), + public_key_path=public_key_path, + requires_email=config.get('FEATURE_MAILING', True)) + + # Verify that the superuser exists. If not, raise an exception. + username = user.username + (result, err_msg) = users.verify_credentials(username, user_password) + if not result: + msg = ('Verification of superuser %s failed: %s. \n\nThe user either does not ' + + 'exist in the remote authentication system ' + + 'OR JWT auth is misconfigured') % (username, err_msg) + raise ConfigValidationException(msg) + + # If the query endpoint exists, ensure we can query to find the current user and that we can + # look up users directly. + if query_endpoint: + (results, _, err_msg) = users.query_users(username) + if not results: + err_msg = err_msg or ('Could not find users matching query: %s' % username) + raise ConfigValidationException('Query endpoint is misconfigured or not returning ' + + 'proper users: %s' % err_msg) + + # Make sure the get user endpoint is also configured. + if not getuser_endpoint: + raise ConfigValidationException('The lookup user endpoint must be configured if the ' + + 'query endpoint is set') + + (result, err_msg) = users.get_user(username) + if not result: + err_msg = err_msg or ('Could not find user %s' % username) + raise ConfigValidationException('Lookup endpoint is misconfigured or not returning ' + + 'properly: %s' % err_msg) From 3db4c15459e75e408f632422e0ca63c2c210fbab Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Thu, 9 Feb 2017 17:28:39 -0800 Subject: [PATCH 10/23] Pull out security scanner validation into validator class --- util/config/validator.py | 30 +------ util/config/validators/test/conftest.py | 86 +++++++++++++++++++ .../validators/test/test_validate_secscan.py | 36 ++++++++ util/config/validators/validate_secscan.py | 36 ++++++++ util/secscan/fake.py | 9 +- 5 files changed, 168 insertions(+), 29 deletions(-) create mode 100644 util/config/validators/test/conftest.py create mode 100644 util/config/validators/test/test_validate_secscan.py create mode 100644 util/config/validators/validate_secscan.py diff --git a/util/config/validator.py b/util/config/validator.py index f184a9264..9724a8cf2 100644 --- a/util/config/validator.py +++ b/util/config/validator.py @@ -1,5 +1,4 @@ import logging -import time from StringIO import StringIO from hashlib import sha1 @@ -11,13 +10,11 @@ from flask import Flask from app import app, config_provider, get_app_url, OVERRIDE_CONFIG_DIRECTORY from auth.auth_context import get_authenticated_user from bitbucket import BitBucket -from boot import setup_jwt_proxy from data.database import validate_database_url from data.users import LDAP_CERT_FILENAME from oauth.services.github import GithubOAuthService from oauth.services.google import GoogleOAuthService from oauth.services.gitlab import GitLabOAuthService -from util.secscan.api import SecurityScannerAPI from util.registry.torrent import torrent_jwt from util.security.signing import SIGNING_ENGINES from util.security.ssl import load_certificate, CertInvalidException, KeyInvalidException @@ -29,6 +26,7 @@ from util.config.validators.validate_email import EmailValidator from util.config.validators.validate_ldap import LDAPValidator from util.config.validators.validate_keystone import KeystoneValidator from util.config.validators.validate_jwt import JWTAuthValidator +from util.config.validators.validate_secscan import SecurityScannerValidator logger = logging.getLogger(__name__) @@ -248,30 +246,6 @@ def _validate_signer(config, user_obj, _): engine.detached_sign(StringIO('test string')) -def _validate_security_scanner(config, user_obj, _): - """ Validates the configuration for talking to a Quay Security Scanner. """ - client = app.config['HTTPCLIENT'] - api = SecurityScannerAPI(app, config, None, client=client, skip_validation=True) - - if not config.get('TESTING', False): - # Generate a temporary Quay key to use for signing the outgoing requests. - setup_jwt_proxy() - - # We have to wait for JWT proxy to restart with the newly generated key. - max_tries = 5 - response = None - while max_tries > 0: - response = api.ping() - if response.status_code == 200: - return - - time.sleep(1) - max_tries = max_tries - 1 - - message = 'Expected 200 status code, got %s: %s' % (response.status_code, response.text) - raise ConfigValidationException('Could not ping security scanner: %s' % message) - - def _validate_bittorrent(config, user_obj, _): """ Validates the configuration for using BitTorrent for downloads. """ announce_url = config.get('BITTORRENT_ANNOUNCE_URL') @@ -328,6 +302,6 @@ VALIDATORS = { JWTAuthValidator.name: JWTAuthValidator.validate, KeystoneValidator.name: KeystoneValidator.validate, 'signer': _validate_signer, - 'security-scanner': _validate_security_scanner, + SecurityScannerValidator.name: SecurityScannerValidator.validate, 'bittorrent': _validate_bittorrent, } diff --git a/util/config/validators/test/conftest.py b/util/config/validators/test/conftest.py new file mode 100644 index 000000000..707fc24ca --- /dev/null +++ b/util/config/validators/test/conftest.py @@ -0,0 +1,86 @@ +import os + +import pytest +import shutil +from flask import Flask, jsonify +from flask.ext.login import LoginManager +from peewee import SqliteDatabase + +from app import app as application +from data import model +from data.database import (close_db_filter, db) +from data.model.user import LoginWrappedDBUser +from endpoints.api import api_bp +from initdb import initialize_database, populate_database +from path_converters import APIRepositoryPathConverter, RegexConverter + + +@pytest.fixture() +def app(appconfig): + """ Used by pytest-flask plugin to inject app by test for client See test_security by name injection of client. """ + app = Flask(__name__) + login_manager = LoginManager(app) + + @app.errorhandler(model.DataModelException) + def handle_dme(ex): + response = jsonify({'message': ex.message}) + response.status_code = 400 + return response + + @login_manager.user_loader + def load_user(user_uuid): + return LoginWrappedDBUser(user_uuid) + + app.url_map.converters['regex'] = RegexConverter + app.url_map.converters['apirepopath'] = APIRepositoryPathConverter + app.register_blueprint(api_bp, url_prefix='/api') + app.config.update(appconfig) + return app + + +@pytest.fixture(scope="session") +def init_db_path(tmpdir_factory): + """ Creates a new db and appropriate configuration. Used for parameter by name injection. """ + sqlitedb_file = str(tmpdir_factory.mktemp("data").join("test.db")) + sqlitedb = 'sqlite:///{0}'.format(sqlitedb_file) + conf = {"TESTING": True, + "DEBUG": True, + "DB_URI": sqlitedb} + os.environ['TEST_DATABASE_URI'] = str(sqlitedb) + os.environ['DB_URI'] = str(sqlitedb) + db.initialize(SqliteDatabase(sqlitedb_file)) + application.config.update(conf) + application.config.update({"DB_URI": sqlitedb}) + initialize_database() + populate_database() + close_db_filter(None) + return str(sqlitedb_file) + + +@pytest.fixture() +def database_uri(monkeypatch, init_db_path, sqlitedb_file): + """ Creates the db uri. Used for parameter by name injection. """ + shutil.copy2(init_db_path, sqlitedb_file) + db.initialize(SqliteDatabase(sqlitedb_file)) + db_path = 'sqlite:///{0}'.format(sqlitedb_file) + monkeypatch.setenv("DB_URI", db_path) + return db_path + + +@pytest.fixture() +def sqlitedb_file(tmpdir): + """ Makes file for db. Used for parameter by name injection. """ + test_db_file = tmpdir.mkdir("quaydb").join("test.db") + return str(test_db_file) + + +@pytest.fixture() +def appconfig(database_uri): + """ Makes conf with database_uri. Used for parameter by name injection """ + conf = { + "TESTING": True, + "DEBUG": True, + "DB_URI": database_uri, + "SECRET_KEY": 'superdupersecret!!!1', + } + return conf diff --git a/util/config/validators/test/test_validate_secscan.py b/util/config/validators/test/test_validate_secscan.py new file mode 100644 index 000000000..e47aa9bf5 --- /dev/null +++ b/util/config/validators/test/test_validate_secscan.py @@ -0,0 +1,36 @@ +import pytest + +from util.config.validators import ConfigValidationException +from util.config.validators.validate_secscan import SecurityScannerValidator +from util.secscan.fake import fake_security_scanner + +@pytest.mark.parametrize('unvalidated_config', [ + ({'DISTRIBUTED_STORAGE_PREFERENCE': []}), +]) +def test_validate_noop(unvalidated_config, app): + SecurityScannerValidator.validate(unvalidated_config, None, None) + + +@pytest.mark.parametrize('unvalidated_config, expected_error, error_message', [ + ({ + 'TESTING': True, + 'DISTRIBUTED_STORAGE_PREFERENCE': [], + 'FEATURE_SECURITY_SCANNER': True, + 'SECURITY_SCANNER_ENDPOINT': 'http://invalidhost', + }, Exception, 'Connection error when trying to connect to security scanner endpoint'), + + ({ + 'TESTING': True, + 'DISTRIBUTED_STORAGE_PREFERENCE': [], + 'FEATURE_SECURITY_SCANNER': True, + 'SECURITY_SCANNER_ENDPOINT': 'http://fakesecurityscanner', + }, None, None), +]) +def test_validate(unvalidated_config, expected_error, error_message, app): + with fake_security_scanner(hostname='fakesecurityscanner'): + if expected_error is not None: + with pytest.raises(expected_error) as ipe: + SecurityScannerValidator.validate(unvalidated_config, None, None) + assert ipe.value.message == error_message + else: + SecurityScannerValidator.validate(unvalidated_config, None, None) diff --git a/util/config/validators/validate_secscan.py b/util/config/validators/validate_secscan.py new file mode 100644 index 000000000..14cf0e81b --- /dev/null +++ b/util/config/validators/validate_secscan.py @@ -0,0 +1,36 @@ +import time + +from app import app +from boot import setup_jwt_proxy +from util.secscan.api import SecurityScannerAPI +from util.config.validators import BaseValidator, ConfigValidationException + +class SecurityScannerValidator(BaseValidator): + name = "security-scanner" + + @classmethod + def validate(cls, config, user, user_password): + """ Validates the configuration for talking to a Quay Security Scanner. """ + if not config.get('FEATURE_SECURITY_SCANNER', False): + return + + client = app.config['HTTPCLIENT'] + api = SecurityScannerAPI(app, config, None, client=client, skip_validation=True) + + if not config.get('TESTING', False): + # Generate a temporary Quay key to use for signing the outgoing requests. + setup_jwt_proxy() + + # We have to wait for JWT proxy to restart with the newly generated key. + max_tries = 5 + response = None + while max_tries > 0: + response = api.ping() + if response.status_code == 200: + return + + time.sleep(1) + max_tries = max_tries - 1 + + message = 'Expected 200 status code, got %s: %s' % (response.status_code, response.text) + raise ConfigValidationException('Could not ping security scanner: %s' % message) diff --git a/util/secscan/fake.py b/util/secscan/fake.py index 0f39f3ff9..849d0c2ca 100644 --- a/util/secscan/fake.py +++ b/util/secscan/fake.py @@ -316,6 +316,13 @@ class FakeSecurityScanner(object): 'content': json.dumps(response), } + @urlmatch(netloc=r'(.*\.)?' + self.hostname, path=r'/v1/metrics$', method='GET') + def metrics(url, _): + return { + 'status_code': 200, + 'content': json.dumps({'fake': True}), + } + @all_requests def response_content(url, _): return { @@ -324,4 +331,4 @@ class FakeSecurityScanner(object): } return [get_layer_mock, post_layer_mock, remove_layer_mock, get_notification, - delete_notification, response_content] + delete_notification, metrics, response_content] From dcabb36ac7c57b0ef4d6295265964b69a3c4515e Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Wed, 15 Feb 2017 11:52:01 -0500 Subject: [PATCH 11/23] Add TODO --- util/config/validators/test/conftest.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/util/config/validators/test/conftest.py b/util/config/validators/test/conftest.py index 707fc24ca..3e7d37fa5 100644 --- a/util/config/validators/test/conftest.py +++ b/util/config/validators/test/conftest.py @@ -15,6 +15,8 @@ from initdb import initialize_database, populate_database from path_converters import APIRepositoryPathConverter, RegexConverter +# TODO(jschorr): Unify with the API conftest once the other PR gets in. + @pytest.fixture() def app(appconfig): """ Used by pytest-flask plugin to inject app by test for client See test_security by name injection of client. """ From 8844ecbb7c07d88e38c86209fd0e173285ff7dee Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Wed, 15 Feb 2017 11:52:08 -0500 Subject: [PATCH 12/23] Fix imports --- util/config/validators/test/test_validate_database.py | 2 +- util/config/validators/test/test_validate_redis.py | 2 +- util/config/validators/test/test_validate_storage.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/util/config/validators/test/test_validate_database.py b/util/config/validators/test/test_validate_database.py index 74612641e..8314f17d3 100644 --- a/util/config/validators/test/test_validate_database.py +++ b/util/config/validators/test/test_validate_database.py @@ -1,7 +1,7 @@ import pytest from util.config.validators import ConfigValidationException -from util.config.validators.database import DatabaseValidator +from util.config.validators.validate_database import DatabaseValidator @pytest.mark.parametrize('unvalidated_config,user,user_password,expected', [ (None, None, None, TypeError), diff --git a/util/config/validators/test/test_validate_redis.py b/util/config/validators/test/test_validate_redis.py index 9936527dc..36cb0335f 100644 --- a/util/config/validators/test/test_validate_redis.py +++ b/util/config/validators/test/test_validate_redis.py @@ -6,7 +6,7 @@ from mock import patch from mockredis import mock_strict_redis_client from util.config.validators import ConfigValidationException -from util.config.validators.buildlogredis import RedisValidator +from util.config.validators.validate_redis import RedisValidator @pytest.mark.parametrize('unvalidated_config,user,user_password,use_mock,expected', [ ({}, None, None, False, ConfigValidationException), diff --git a/util/config/validators/test/test_validate_storage.py b/util/config/validators/test/test_validate_storage.py index 0d5406500..2e271edf0 100644 --- a/util/config/validators/test/test_validate_storage.py +++ b/util/config/validators/test/test_validate_storage.py @@ -1,7 +1,7 @@ import pytest from util.config.validators import ConfigValidationException -from util.config.validators.registrystorage import StorageValidator +from util.config.validators.validate_storage import StorageValidator @pytest.mark.parametrize('unvalidated_config, expected', [ ({}, ConfigValidationException), From 2944a4e13dae405c169302fe6cd1f67333537107 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Wed, 15 Feb 2017 11:56:19 -0500 Subject: [PATCH 13/23] Pull out signing validation into validator class --- util/config/validator.py | 17 ++-------------- .../validators/test/test_validate_signer.py | 17 ++++++++++++++++ util/config/validators/validate_signer.py | 20 +++++++++++++++++++ 3 files changed, 39 insertions(+), 15 deletions(-) create mode 100644 util/config/validators/test/test_validate_signer.py create mode 100644 util/config/validators/validate_signer.py diff --git a/util/config/validator.py b/util/config/validator.py index 9724a8cf2..a492be3d6 100644 --- a/util/config/validator.py +++ b/util/config/validator.py @@ -1,6 +1,5 @@ import logging -from StringIO import StringIO from hashlib import sha1 import peewee @@ -16,7 +15,6 @@ from oauth.services.github import GithubOAuthService from oauth.services.google import GoogleOAuthService from oauth.services.gitlab import GitLabOAuthService from util.registry.torrent import torrent_jwt -from util.security.signing import SIGNING_ENGINES from util.security.ssl import load_certificate, CertInvalidException, KeyInvalidException from util.config.validators.validate_database import DatabaseValidator @@ -27,6 +25,7 @@ from util.config.validators.validate_ldap import LDAPValidator from util.config.validators.validate_keystone import KeystoneValidator from util.config.validators.validate_jwt import JWTAuthValidator from util.config.validators.validate_secscan import SecurityScannerValidator +from util.config.validators.validate_signer import SignerValidator logger = logging.getLogger(__name__) @@ -234,18 +233,6 @@ def _validate_ssl(config, user_obj, _): raise ConfigValidationException('SSL private key failed to validate: %s' % kie.message) -def _validate_signer(config, user_obj, _): - """ Validates the GPG public+private key pair used for signing converted ACIs. """ - if config.get('SIGNING_ENGINE') is None: - return - - if config['SIGNING_ENGINE'] not in SIGNING_ENGINES: - raise ConfigValidationException('Unknown signing engine: %s' % config['SIGNING_ENGINE']) - - engine = SIGNING_ENGINES[config['SIGNING_ENGINE']](config, config_provider) - engine.detached_sign(StringIO('test string')) - - def _validate_bittorrent(config, user_obj, _): """ Validates the configuration for using BitTorrent for downloads. """ announce_url = config.get('BITTORRENT_ANNOUNCE_URL') @@ -301,7 +288,7 @@ VALIDATORS = { LDAPValidator.name: LDAPValidator.validate, JWTAuthValidator.name: JWTAuthValidator.validate, KeystoneValidator.name: KeystoneValidator.validate, - 'signer': _validate_signer, + SignerValidator.name: SignerValidator.validate, SecurityScannerValidator.name: SecurityScannerValidator.validate, 'bittorrent': _validate_bittorrent, } diff --git a/util/config/validators/test/test_validate_signer.py b/util/config/validators/test/test_validate_signer.py new file mode 100644 index 000000000..e7501723f --- /dev/null +++ b/util/config/validators/test/test_validate_signer.py @@ -0,0 +1,17 @@ +import pytest + +from util.config.validators import ConfigValidationException +from util.config.validators.validate_signer import SignerValidator + +@pytest.mark.parametrize('unvalidated_config,expected', [ + ({}, None), + ({'SIGNING_ENGINE': 'foobar'}, ConfigValidationException), + ({'SIGNING_ENGINE': 'gpg2'}, Exception), +]) +def test_validate_signer(unvalidated_config,expected): + validator = SignerValidator() + if expected is not None: + with pytest.raises(expected): + validator.validate(unvalidated_config, None, None) + else: + validator.validate(unvalidated_config, None, None) diff --git a/util/config/validators/validate_signer.py b/util/config/validators/validate_signer.py new file mode 100644 index 000000000..b44cb3c3d --- /dev/null +++ b/util/config/validators/validate_signer.py @@ -0,0 +1,20 @@ +from StringIO import StringIO + +from app import config_provider +from util.config.validators import BaseValidator, ConfigValidationException +from util.security.signing import SIGNING_ENGINES + +class SignerValidator(BaseValidator): + name = "signer" + + @classmethod + def validate(cls, config, user, user_password): + """ Validates the GPG public+private key pair used for signing converted ACIs. """ + if config.get('SIGNING_ENGINE') is None: + return + + if config['SIGNING_ENGINE'] not in SIGNING_ENGINES: + raise ConfigValidationException('Unknown signing engine: %s' % config['SIGNING_ENGINE']) + + engine = SIGNING_ENGINES[config['SIGNING_ENGINE']](config, config_provider) + engine.detached_sign(StringIO('test string')) From 09b3cfd5493e3a81efb6c1b6ef11d54972d97815 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Wed, 15 Feb 2017 12:12:19 -0500 Subject: [PATCH 14/23] Pull out torrent validation into validator class --- util/config/validator.py | 47 +--------------- .../validators/test/test_validate_torrent.py | 28 ++++++++++ util/config/validators/validate_torrent.py | 53 +++++++++++++++++++ 3 files changed, 83 insertions(+), 45 deletions(-) create mode 100644 util/config/validators/test/test_validate_torrent.py create mode 100644 util/config/validators/validate_torrent.py diff --git a/util/config/validator.py b/util/config/validator.py index a492be3d6..8d7c36374 100644 --- a/util/config/validator.py +++ b/util/config/validator.py @@ -1,7 +1,5 @@ import logging -from hashlib import sha1 - import peewee from flask import Flask @@ -14,7 +12,6 @@ from data.users import LDAP_CERT_FILENAME from oauth.services.github import GithubOAuthService from oauth.services.google import GoogleOAuthService from oauth.services.gitlab import GitLabOAuthService -from util.registry.torrent import torrent_jwt from util.security.ssl import load_certificate, CertInvalidException, KeyInvalidException from util.config.validators.validate_database import DatabaseValidator @@ -26,6 +23,7 @@ from util.config.validators.validate_keystone import KeystoneValidator from util.config.validators.validate_jwt import JWTAuthValidator from util.config.validators.validate_secscan import SecurityScannerValidator from util.config.validators.validate_signer import SignerValidator +from util.config.validators.validate_torrent import BittorrentValidator logger = logging.getLogger(__name__) @@ -233,47 +231,6 @@ def _validate_ssl(config, user_obj, _): raise ConfigValidationException('SSL private key failed to validate: %s' % kie.message) -def _validate_bittorrent(config, user_obj, _): - """ Validates the configuration for using BitTorrent for downloads. """ - announce_url = config.get('BITTORRENT_ANNOUNCE_URL') - if not announce_url: - raise ConfigValidationException('Missing announce URL') - - # Ensure that the tracker is reachable and accepts requests signed with a registry key. - client = app.config['HTTPCLIENT'] - - params = { - 'info_hash': sha1('somedata').digest(), - 'peer_id': '-QUAY00-6wfG2wk6wWLc', - 'uploaded': 0, - 'downloaded': 0, - 'left': 0, - 'numwant': 0, - 'port': 80, - } - - encoded_jwt = torrent_jwt(params) - params['jwt'] = encoded_jwt - - resp = client.get(announce_url, timeout=5, params=params) - logger.debug('Got tracker response: %s: %s', resp.status_code, resp.text) - - if resp.status_code == 404: - raise ConfigValidationException('Announce path not found; did you forget `/announce`?') - - if resp.status_code == 500: - raise ConfigValidationException('Did not get expected response from Tracker; ' + - 'please check your settings') - - if resp.status_code == 200: - if 'invalid jwt' in resp.text: - raise ConfigValidationException('Could not authorize to Tracker; is your Tracker ' + - 'properly configured?') - - if 'failure reason' in resp.text: - raise ConfigValidationException('Could not validate signed announce request: ' + resp.text) - - VALIDATORS = { DatabaseValidator.name: DatabaseValidator.validate, RedisValidator.name: RedisValidator.validate, @@ -290,5 +247,5 @@ VALIDATORS = { KeystoneValidator.name: KeystoneValidator.validate, SignerValidator.name: SignerValidator.validate, SecurityScannerValidator.name: SecurityScannerValidator.validate, - 'bittorrent': _validate_bittorrent, + BittorrentValidator.name: BittorrentValidator.validate, } diff --git a/util/config/validators/test/test_validate_torrent.py b/util/config/validators/test/test_validate_torrent.py new file mode 100644 index 000000000..69d29232d --- /dev/null +++ b/util/config/validators/test/test_validate_torrent.py @@ -0,0 +1,28 @@ +import pytest + +from httmock import urlmatch, HTTMock + +from util.config.validators import ConfigValidationException +from util.config.validators.validate_torrent import BittorrentValidator + +@pytest.mark.parametrize('unvalidated_config,expected', [ + ({}, ConfigValidationException), + ({'BITTORRENT_ANNOUNCE_URL': 'http://faketorrent/announce'}, None), +]) +def test_validate_torrent(unvalidated_config,expected): + announcer_hit = [False] + + @urlmatch(netloc=r'faketorrent', path='/announce') + def handler(url, request): + announcer_hit[0] = True + return {'status_code': 200, 'content': ''} + + with HTTMock(handler): + validator = BittorrentValidator() + if expected is not None: + with pytest.raises(expected): + validator.validate(unvalidated_config, None, None) + assert not announcer_hit[0] + else: + validator.validate(unvalidated_config, None, None) + assert announcer_hit[0] diff --git a/util/config/validators/validate_torrent.py b/util/config/validators/validate_torrent.py new file mode 100644 index 000000000..2cfc376b6 --- /dev/null +++ b/util/config/validators/validate_torrent.py @@ -0,0 +1,53 @@ +import logging + +from hashlib import sha1 + +from app import app +from util.config.validators import BaseValidator, ConfigValidationException +from util.registry.torrent import torrent_jwt + +logger = logging.getLogger(__name__) + +class BittorrentValidator(BaseValidator): + name = "bittorrent" + + @classmethod + def validate(cls, config, user, user_password): + """ Validates the configuration for using BitTorrent for downloads. """ + announce_url = config.get('BITTORRENT_ANNOUNCE_URL') + if not announce_url: + raise ConfigValidationException('Missing announce URL') + + # Ensure that the tracker is reachable and accepts requests signed with a registry key. + client = app.config['HTTPCLIENT'] + + params = { + 'info_hash': sha1('somedata').digest(), + 'peer_id': '-QUAY00-6wfG2wk6wWLc', + 'uploaded': 0, + 'downloaded': 0, + 'left': 0, + 'numwant': 0, + 'port': 80, + } + + encoded_jwt = torrent_jwt(params) + params['jwt'] = encoded_jwt + + resp = client.get(announce_url, timeout=5, params=params) + logger.debug('Got tracker response: %s: %s', resp.status_code, resp.text) + + if resp.status_code == 404: + raise ConfigValidationException('Announce path not found; did you forget `/announce`?') + + if resp.status_code == 500: + raise ConfigValidationException('Did not get expected response from Tracker; ' + + 'please check your settings') + + if resp.status_code == 200: + if 'invalid jwt' in resp.text: + raise ConfigValidationException('Could not authorize to Tracker; is your Tracker ' + + 'properly configured?') + + if 'failure reason' in resp.text: + raise ConfigValidationException('Could not validate signed announce request: ' + resp.text) From e76b95f0e637a3f15ed7948e4d1cf32431fbe4ee Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Wed, 15 Feb 2017 13:11:16 -0500 Subject: [PATCH 15/23] Add S3 storage test to validator tests --- .../validators/test/test_validate_storage.py | 18 ++++++++++++++++++ util/config/validators/validate_storage.py | 3 ++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/util/config/validators/test/test_validate_storage.py b/util/config/validators/test/test_validate_storage.py index 2e271edf0..6ce1e4483 100644 --- a/util/config/validators/test/test_validate_storage.py +++ b/util/config/validators/test/test_validate_storage.py @@ -1,3 +1,4 @@ +import moto import pytest from util.config.validators import ConfigValidationException @@ -16,3 +17,20 @@ def test_validate_storage(unvalidated_config, expected): validator.validate(unvalidated_config, None, None) else: validator.validate(unvalidated_config, None, None) + +def test_validate_s3_storage(): + validator = StorageValidator() + with moto.mock_s3(): + with pytest.raises(ConfigValidationException) as ipe: + validator.validate({ + 'DISTRIBUTED_STORAGE_CONFIG': { + 'default': ('S3Storage', { + 's3_access_key': 'invalid', + 's3_secret_key': 'invalid', + 's3_bucket': 'somebucket', + 'storage_path': '' + }), + } + }, None, None) + + assert ipe.value.message == 'Invalid storage configuration: default: S3ResponseError: 404 Not Found' \ No newline at end of file diff --git a/util/config/validators/validate_storage.py b/util/config/validators/validate_storage.py index ca2f79ed5..b240d6094 100644 --- a/util/config/validators/validate_storage.py +++ b/util/config/validators/validate_storage.py @@ -26,7 +26,8 @@ class StorageValidator(BaseValidator): # Run setup on the driver if the read/write succeeded. driver.setup() except Exception as ex: - raise ConfigValidationException('Invalid storage configuration: %s: %s' % (name, str(ex))) + msg = str(ex).strip() + raise ConfigValidationException('Invalid storage configuration: %s: %s' % (name, msg)) def _get_storage_providers(config): From 620e377fafc9c0925f4d4ee0c2dfcfdd2c3a70f6 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Wed, 15 Feb 2017 15:17:07 -0500 Subject: [PATCH 16/23] Pull out ssl validation into validator class --- util/config/validator.py | 56 +---------------- .../validators/test/test_validate_ssl.py | 62 +++++++++++++++++++ util/config/validators/validate_ssl.py | 59 ++++++++++++++++++ 3 files changed, 123 insertions(+), 54 deletions(-) create mode 100644 util/config/validators/test/test_validate_ssl.py create mode 100644 util/config/validators/validate_ssl.py diff --git a/util/config/validator.py b/util/config/validator.py index 8d7c36374..c2ad42da8 100644 --- a/util/config/validator.py +++ b/util/config/validator.py @@ -12,7 +12,6 @@ from data.users import LDAP_CERT_FILENAME from oauth.services.github import GithubOAuthService from oauth.services.google import GoogleOAuthService from oauth.services.gitlab import GitLabOAuthService -from util.security.ssl import load_certificate, CertInvalidException, KeyInvalidException from util.config.validators.validate_database import DatabaseValidator from util.config.validators.validate_redis import RedisValidator @@ -24,6 +23,7 @@ from util.config.validators.validate_jwt import JWTAuthValidator from util.config.validators.validate_secscan import SecurityScannerValidator from util.config.validators.validate_signer import SignerValidator from util.config.validators.validate_torrent import BittorrentValidator +from util.config.validators.validate_ssl import SSLValidator, SSL_FILENAMES logger = logging.getLogger(__name__) @@ -33,7 +33,6 @@ class ConfigValidationException(Exception): # Note: Only add files required for HTTPS to the SSL_FILESNAMES list. -SSL_FILENAMES = ['ssl.cert', 'ssl.key'] DB_SSL_FILENAMES = ['database.pem'] JWT_FILENAMES = ['jwt-authn.cert'] ACI_CERT_FILENAMES = ['signing-public.gpg', 'signing-private.gpg'] @@ -180,57 +179,6 @@ def _validate_google_login(config, user_obj, _): raise ConfigValidationException('Invalid client id or client secret') -def _validate_ssl(config, user_obj, _): - """ Validates the SSL configuration (if enabled). """ - - # Skip if non-SSL. - if config.get('PREFERRED_URL_SCHEME', 'http') != 'https': - return - - # Skip if externally terminated. - if config.get('EXTERNAL_TLS_TERMINATION', False) is True: - return - - # Verify that we have all the required SSL files. - for filename in SSL_FILENAMES: - if not config_provider.volume_file_exists(filename): - raise ConfigValidationException('Missing required SSL file: %s' % filename) - - # Read the contents of the SSL certificate. - with config_provider.get_volume_file(SSL_FILENAMES[0]) as f: - cert_contents = f.read() - - # Validate the certificate. - try: - certificate = load_certificate(cert_contents) - except CertInvalidException as cie: - raise ConfigValidationException('Could not load SSL certificate: %s' % cie.message) - - # Verify the certificate has not expired. - if certificate.expired: - raise ConfigValidationException('The specified SSL certificate has expired.') - - # Verify the hostname matches the name in the certificate. - if not certificate.matches_name(config['SERVER_HOSTNAME']): - msg = ('Supported names "%s" in SSL cert do not match server hostname "%s"' % - (', '.join(list(certificate.names)), config['SERVER_HOSTNAME'])) - raise ConfigValidationException(msg) - - # Verify the private key against the certificate. - private_key_path = None - with config_provider.get_volume_file(SSL_FILENAMES[1]) as f: - private_key_path = f.name - - if not private_key_path: - # Only in testing. - return - - try: - certificate.validate_private_key(private_key_path) - except KeyInvalidException as kie: - raise ConfigValidationException('SSL private key failed to validate: %s' % kie.message) - - VALIDATORS = { DatabaseValidator.name: DatabaseValidator.validate, RedisValidator.name: RedisValidator.validate, @@ -241,7 +189,7 @@ VALIDATORS = { 'gitlab-trigger': _validate_gitlab, 'bitbucket-trigger': _validate_bitbucket, 'google-login': _validate_google_login, - 'ssl': _validate_ssl, + SSLValidator.name: SSLValidator.validate, LDAPValidator.name: LDAPValidator.validate, JWTAuthValidator.name: JWTAuthValidator.validate, KeystoneValidator.name: KeystoneValidator.validate, diff --git a/util/config/validators/test/test_validate_ssl.py b/util/config/validators/test/test_validate_ssl.py new file mode 100644 index 000000000..ee5a4aa22 --- /dev/null +++ b/util/config/validators/test/test_validate_ssl.py @@ -0,0 +1,62 @@ +import pytest + +from mock import patch +from tempfile import NamedTemporaryFile + +from util.config.validators import ConfigValidationException +from util.config.validators.validate_ssl import SSLValidator, SSL_FILENAMES +from test.test_ssl_util import generate_test_cert + +@pytest.mark.parametrize('unvalidated_config', [ + ({}), + ({'PREFERRED_URL_SCHEME': 'http'}), + ({'PREFERRED_URL_SCHEME': 'https', 'EXTERNAL_TLS_TERMINATION': True}), +]) +def test_skip_validate_ssl(unvalidated_config): + validator = SSLValidator() + validator.validate(unvalidated_config, None, None) + + +@pytest.mark.parametrize('cert, expected_error, error_message', [ + ('invalidcert', ConfigValidationException, 'Could not load SSL certificate: no start line'), + (generate_test_cert(hostname='someserver'), None, None), + (generate_test_cert(hostname='invalidserver'), ConfigValidationException, + 'Supported names "invalidserver" in SSL cert do not match server hostname "someserver"'), +]) +def test_validate_ssl(cert, expected_error, error_message): + with NamedTemporaryFile(delete=False) as cert_file: + cert_file.write(cert[0]) + cert_file.seek(0) + + with NamedTemporaryFile(delete=False) as key_file: + key_file.write(cert[1]) + key_file.seek(0) + + def return_true(filename): + return True + + def get_volume_file(filename): + if filename == SSL_FILENAMES[0]: + return open(cert_file.name) + + if filename == SSL_FILENAMES[1]: + return open(key_file.name) + + return None + + config = { + 'PREFERRED_URL_SCHEME': 'https', + 'SERVER_HOSTNAME': 'someserver', + } + + with patch('app.config_provider.volume_file_exists', return_true): + with patch('app.config_provider.get_volume_file', get_volume_file): + validator = SSLValidator() + + if expected_error is not None: + with pytest.raises(expected_error) as ipe: + validator.validate(config, None, None) + + assert ipe.value.message == error_message + else: + validator.validate(config, None, None) diff --git a/util/config/validators/validate_ssl.py b/util/config/validators/validate_ssl.py new file mode 100644 index 000000000..ea1ae3188 --- /dev/null +++ b/util/config/validators/validate_ssl.py @@ -0,0 +1,59 @@ +from app import config_provider +from util.config.validators import BaseValidator, ConfigValidationException +from util.security.ssl import load_certificate, CertInvalidException, KeyInvalidException + +SSL_FILENAMES = ['ssl.cert', 'ssl.key'] + +class SSLValidator(BaseValidator): + name = "ssl" + + @classmethod + def validate(cls, config, user, user_password): + """ Validates the SSL configuration (if enabled). """ + + # Skip if non-SSL. + if config.get('PREFERRED_URL_SCHEME', 'http') != 'https': + return + + # Skip if externally terminated. + if config.get('EXTERNAL_TLS_TERMINATION', False) is True: + return + + # Verify that we have all the required SSL files. + for filename in SSL_FILENAMES: + if not config_provider.volume_file_exists(filename): + raise ConfigValidationException('Missing required SSL file: %s' % filename) + + # Read the contents of the SSL certificate. + with config_provider.get_volume_file(SSL_FILENAMES[0]) as f: + cert_contents = f.read() + + # Validate the certificate. + try: + certificate = load_certificate(cert_contents) + except CertInvalidException as cie: + raise ConfigValidationException('Could not load SSL certificate: %s' % cie.message) + + # Verify the certificate has not expired. + if certificate.expired: + raise ConfigValidationException('The specified SSL certificate has expired.') + + # Verify the hostname matches the name in the certificate. + if not certificate.matches_name(config['SERVER_HOSTNAME']): + msg = ('Supported names "%s" in SSL cert do not match server hostname "%s"' % + (', '.join(list(certificate.names)), config['SERVER_HOSTNAME'])) + raise ConfigValidationException(msg) + + # Verify the private key against the certificate. + private_key_path = None + with config_provider.get_volume_file(SSL_FILENAMES[1]) as f: + private_key_path = f.name + + if not private_key_path: + # Only in testing. + return + + try: + certificate.validate_private_key(private_key_path) + except KeyInvalidException as kie: + raise ConfigValidationException('SSL private key failed to validate: %s' % kie.message) From 49638b081b490f61d5851f45eeb24d7f537b289b Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Wed, 15 Feb 2017 15:26:15 -0500 Subject: [PATCH 17/23] Pull out google login validation into validator class --- util/config/validator.py | 27 ++------------ .../test/test_validate_google_login.py | 37 +++++++++++++++++++ .../validators/validate_google_login.py | 25 +++++++++++++ 3 files changed, 65 insertions(+), 24 deletions(-) create mode 100644 util/config/validators/test/test_validate_google_login.py create mode 100644 util/config/validators/validate_google_login.py diff --git a/util/config/validator.py b/util/config/validator.py index c2ad42da8..1be17b614 100644 --- a/util/config/validator.py +++ b/util/config/validator.py @@ -2,15 +2,12 @@ import logging import peewee -from flask import Flask - -from app import app, config_provider, get_app_url, OVERRIDE_CONFIG_DIRECTORY +from app import app, get_app_url from auth.auth_context import get_authenticated_user from bitbucket import BitBucket from data.database import validate_database_url from data.users import LDAP_CERT_FILENAME from oauth.services.github import GithubOAuthService -from oauth.services.google import GoogleOAuthService from oauth.services.gitlab import GitLabOAuthService from util.config.validators.validate_database import DatabaseValidator @@ -24,6 +21,7 @@ from util.config.validators.validate_secscan import SecurityScannerValidator from util.config.validators.validate_signer import SignerValidator from util.config.validators.validate_torrent import BittorrentValidator from util.config.validators.validate_ssl import SSLValidator, SSL_FILENAMES +from util.config.validators.validate_google_login import GoogleLoginValidator logger = logging.getLogger(__name__) @@ -160,25 +158,6 @@ def _validate_bitbucket(config, user_obj, _): raise ConfigValidationException('Invalid consumer key or secret') -def _validate_google_login(config, user_obj, _): - """ Validates the Google Login client ID and secret. """ - google_login_config = config.get('GOOGLE_LOGIN_CONFIG') - if not google_login_config: - raise ConfigValidationException('Missing client ID and client secret') - - if not google_login_config.get('CLIENT_ID'): - raise ConfigValidationException('Missing Client ID') - - if not google_login_config.get('CLIENT_SECRET'): - raise ConfigValidationException('Missing Client Secret') - - client = app.config['HTTPCLIENT'] - oauth = GoogleOAuthService(config, 'GOOGLE_LOGIN_CONFIG') - result = oauth.validate_client_id_and_secret(client, app.config) - if not result: - raise ConfigValidationException('Invalid client id or client secret') - - VALIDATORS = { DatabaseValidator.name: DatabaseValidator.validate, RedisValidator.name: RedisValidator.validate, @@ -188,7 +167,7 @@ VALIDATORS = { 'github-trigger': _validate_github('GITHUB_TRIGGER_CONFIG'), 'gitlab-trigger': _validate_gitlab, 'bitbucket-trigger': _validate_bitbucket, - 'google-login': _validate_google_login, + GoogleLoginValidator.name: GoogleLoginValidator.validate, SSLValidator.name: SSLValidator.validate, LDAPValidator.name: LDAPValidator.validate, JWTAuthValidator.name: JWTAuthValidator.validate, diff --git a/util/config/validators/test/test_validate_google_login.py b/util/config/validators/test/test_validate_google_login.py new file mode 100644 index 000000000..8f41668c5 --- /dev/null +++ b/util/config/validators/test/test_validate_google_login.py @@ -0,0 +1,37 @@ +import pytest + +from httmock import urlmatch, HTTMock + +from util.config.validators import ConfigValidationException +from util.config.validators.validate_google_login import GoogleLoginValidator + +@pytest.mark.parametrize('unvalidated_config', [ + ({}), + ({'GOOGLE_LOGIN_CONFIG': {}}), + ({'GOOGLE_LOGIN_CONFIG': {'CLIENT_ID': 'foo'}}), + ({'GOOGLE_LOGIN_CONFIG': {'CLIENT_SECRET': 'foo'}}), +]) +def test_validate_invalid_google_login_config(unvalidated_config): + validator = GoogleLoginValidator() + + with pytest.raises(ConfigValidationException): + validator.validate(unvalidated_config, None, None) + +def test_validate_google_login(): + url_hit = [False] + @urlmatch(netloc=r'www.googleapis.com', path='/oauth2/v3/token') + def handler(_, __): + url_hit[0] = True + return {'status_code': 200, 'content': ''} + + validator = GoogleLoginValidator() + + with HTTMock(handler): + validator.validate({ + 'GOOGLE_LOGIN_CONFIG': { + 'CLIENT_ID': 'foo', + 'CLIENT_SECRET': 'bar', + }, + }, None, None) + + assert url_hit[0] diff --git a/util/config/validators/validate_google_login.py b/util/config/validators/validate_google_login.py new file mode 100644 index 000000000..80e1537f0 --- /dev/null +++ b/util/config/validators/validate_google_login.py @@ -0,0 +1,25 @@ +from app import app +from oauth.services.google import GoogleOAuthService +from util.config.validators import BaseValidator, ConfigValidationException + +class GoogleLoginValidator(BaseValidator): + name = "google-login" + + @classmethod + def validate(cls, config, user, user_password): + """ Validates the Google Login client ID and secret. """ + google_login_config = config.get('GOOGLE_LOGIN_CONFIG') + if not google_login_config: + raise ConfigValidationException('Missing client ID and client secret') + + if not google_login_config.get('CLIENT_ID'): + raise ConfigValidationException('Missing Client ID') + + if not google_login_config.get('CLIENT_SECRET'): + raise ConfigValidationException('Missing Client Secret') + + client = app.config['HTTPCLIENT'] + oauth = GoogleOAuthService(config, 'GOOGLE_LOGIN_CONFIG') + result = oauth.validate_client_id_and_secret(client, app.config) + if not result: + raise ConfigValidationException('Invalid client id or client secret') From 7a260d81d32376207919a7f11af1dad54843704e Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Wed, 15 Feb 2017 15:30:46 -0500 Subject: [PATCH 18/23] Pull out bitbucket trigger validation into validator class --- util/config/validator.py | 26 +----------- .../test/test_validate_bitbucket_trigger.py | 40 +++++++++++++++++++ .../validators/validate_bitbucket_trigger.py | 29 ++++++++++++++ 3 files changed, 71 insertions(+), 24 deletions(-) create mode 100644 util/config/validators/test/test_validate_bitbucket_trigger.py create mode 100644 util/config/validators/validate_bitbucket_trigger.py diff --git a/util/config/validator.py b/util/config/validator.py index 1be17b614..5835d1267 100644 --- a/util/config/validator.py +++ b/util/config/validator.py @@ -4,7 +4,6 @@ import peewee from app import app, get_app_url from auth.auth_context import get_authenticated_user -from bitbucket import BitBucket from data.database import validate_database_url from data.users import LDAP_CERT_FILENAME from oauth.services.github import GithubOAuthService @@ -22,6 +21,7 @@ from util.config.validators.validate_signer import SignerValidator from util.config.validators.validate_torrent import BittorrentValidator from util.config.validators.validate_ssl import SSLValidator, SSL_FILENAMES from util.config.validators.validate_google_login import GoogleLoginValidator +from util.config.validators.validate_bitbucket_trigger import BitbucketTriggerValidator logger = logging.getLogger(__name__) @@ -136,28 +136,6 @@ def _validate_github_with_key(config_key, config): raise ConfigValidationException('Invalid organization: %s' % org_id) -def _validate_bitbucket(config, user_obj, _): - """ Validates the config for BitBucket. """ - trigger_config = config.get('BITBUCKET_TRIGGER_CONFIG') - if not trigger_config: - raise ConfigValidationException('Missing client ID and client secret') - - if not trigger_config.get('CONSUMER_KEY'): - raise ConfigValidationException('Missing Consumer Key') - - if not trigger_config.get('CONSUMER_SECRET'): - raise ConfigValidationException('Missing Consumer Secret') - - key = trigger_config['CONSUMER_KEY'] - secret = trigger_config['CONSUMER_SECRET'] - callback_url = '%s/oauth1/bitbucket/callback/trigger/' % (get_app_url()) - - bitbucket_client = BitBucket(key, secret, callback_url) - (result, _, _) = bitbucket_client.get_authorization_url() - if not result: - raise ConfigValidationException('Invalid consumer key or secret') - - VALIDATORS = { DatabaseValidator.name: DatabaseValidator.validate, RedisValidator.name: RedisValidator.validate, @@ -166,7 +144,7 @@ VALIDATORS = { 'github-login': _validate_github('GITHUB_LOGIN_CONFIG'), 'github-trigger': _validate_github('GITHUB_TRIGGER_CONFIG'), 'gitlab-trigger': _validate_gitlab, - 'bitbucket-trigger': _validate_bitbucket, + BitbucketTriggerValidator.name: BittorrentValidator.validate, GoogleLoginValidator.name: GoogleLoginValidator.validate, SSLValidator.name: SSLValidator.validate, LDAPValidator.name: LDAPValidator.validate, diff --git a/util/config/validators/test/test_validate_bitbucket_trigger.py b/util/config/validators/test/test_validate_bitbucket_trigger.py new file mode 100644 index 000000000..234843561 --- /dev/null +++ b/util/config/validators/test/test_validate_bitbucket_trigger.py @@ -0,0 +1,40 @@ +import pytest + +from httmock import urlmatch, HTTMock + +from util.config.validators import ConfigValidationException +from util.config.validators.validate_bitbucket_trigger import BitbucketTriggerValidator + +@pytest.mark.parametrize('unvalidated_config', [ + ({}), + ({'BITBUCKET_TRIGGER_CONFIG': {}}), + ({'BITBUCKET_TRIGGER_CONFIG': {'CONSUMER_KEY': 'foo'}}), + ({'BITBUCKET_TRIGGER_CONFIG': {'CONSUMER_SECRET': 'foo'}}), +]) +def test_validate_invalid_bitbucket_trigger_config(unvalidated_config): + validator = BitbucketTriggerValidator() + + with pytest.raises(ConfigValidationException): + validator.validate(unvalidated_config, None, None) + +def test_validate_bitbucket_trigger(): + url_hit = [False] + + @urlmatch(netloc=r'bitbucket.org') + def handler(url, request): + url_hit[0] = True + return { + 'status_code': 200, + 'content': 'oauth_token=foo&oauth_token_secret=bar', + } + + with HTTMock(handler): + validator = BitbucketTriggerValidator() + validator.validate({ + 'BITBUCKET_TRIGGER_CONFIG': { + 'CONSUMER_KEY': 'foo', + 'CONSUMER_SECRET': 'bar', + }, + }, None, None) + + assert url_hit[0] diff --git a/util/config/validators/validate_bitbucket_trigger.py b/util/config/validators/validate_bitbucket_trigger.py new file mode 100644 index 000000000..15378c1b4 --- /dev/null +++ b/util/config/validators/validate_bitbucket_trigger.py @@ -0,0 +1,29 @@ +from bitbucket import BitBucket + +from app import get_app_url +from util.config.validators import BaseValidator, ConfigValidationException + +class BitbucketTriggerValidator(BaseValidator): + name = "bitbucket-trigger" + + @classmethod + def validate(cls, config, user, user_password): + """ Validates the config for BitBucket. """ + trigger_config = config.get('BITBUCKET_TRIGGER_CONFIG') + if not trigger_config: + raise ConfigValidationException('Missing client ID and client secret') + + if not trigger_config.get('CONSUMER_KEY'): + raise ConfigValidationException('Missing Consumer Key') + + if not trigger_config.get('CONSUMER_SECRET'): + raise ConfigValidationException('Missing Consumer Secret') + + key = trigger_config['CONSUMER_KEY'] + secret = trigger_config['CONSUMER_SECRET'] + callback_url = '%s/oauth1/bitbucket/callback/trigger/' % (get_app_url()) + + bitbucket_client = BitBucket(key, secret, callback_url) + (result, _, _) = bitbucket_client.get_authorization_url() + if not result: + raise ConfigValidationException('Invalid consumer key or secret') From a31f2267e86bcca7833fef753783bb32ad7e6b4a Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Wed, 15 Feb 2017 15:39:03 -0500 Subject: [PATCH 19/23] Pull out gitlab trigger validation into validator class --- util/config/validator.py | 29 +------------ .../test/test_validate_gitlab_trigger.py | 41 +++++++++++++++++++ .../validators/validate_gitlab_trigger.py | 32 +++++++++++++++ 3 files changed, 75 insertions(+), 27 deletions(-) create mode 100644 util/config/validators/test/test_validate_gitlab_trigger.py create mode 100644 util/config/validators/validate_gitlab_trigger.py diff --git a/util/config/validator.py b/util/config/validator.py index 5835d1267..a263d6d89 100644 --- a/util/config/validator.py +++ b/util/config/validator.py @@ -22,6 +22,7 @@ from util.config.validators.validate_torrent import BittorrentValidator from util.config.validators.validate_ssl import SSLValidator, SSL_FILENAMES from util.config.validators.validate_google_login import GoogleLoginValidator from util.config.validators.validate_bitbucket_trigger import BitbucketTriggerValidator +from util.config.validators.validate_gitlab_trigger import GitLabTriggerValidator logger = logging.getLogger(__name__) @@ -71,32 +72,6 @@ def _validate_database(config, user_obj, _): raise ex -def _validate_gitlab(config, user_obj, _): - """ Validates the OAuth credentials and API endpoint for a GitLab service. """ - github_config = config.get('GITLAB_TRIGGER_CONFIG') - if not github_config: - raise ConfigValidationException('Missing GitLab client id and client secret') - - endpoint = github_config.get('GITLAB_ENDPOINT') - if not endpoint: - raise ConfigValidationException('Missing GitLab Endpoint') - - if endpoint.find('http://') != 0 and endpoint.find('https://') != 0: - raise ConfigValidationException('GitLab Endpoint must start with http:// or https://') - - if not github_config.get('CLIENT_ID'): - raise ConfigValidationException('Missing Client ID') - - if not github_config.get('CLIENT_SECRET'): - raise ConfigValidationException('Missing Client Secret') - - client = app.config['HTTPCLIENT'] - oauth = GitLabOAuthService(config, 'GITLAB_TRIGGER_CONFIG') - result = oauth.validate_client_id_and_secret(client, app.config) - if not result: - raise ConfigValidationException('Invalid client id or client secret') - - def _validate_github(config_key): return lambda config, user_obj, _: _validate_github_with_key(config_key, config) @@ -143,7 +118,7 @@ VALIDATORS = { EmailValidator.name: EmailValidator.validate, 'github-login': _validate_github('GITHUB_LOGIN_CONFIG'), 'github-trigger': _validate_github('GITHUB_TRIGGER_CONFIG'), - 'gitlab-trigger': _validate_gitlab, + GitLabTriggerValidator.name: GitLabTriggerValidator.validate, BitbucketTriggerValidator.name: BittorrentValidator.validate, GoogleLoginValidator.name: GoogleLoginValidator.validate, SSLValidator.name: SSLValidator.validate, diff --git a/util/config/validators/test/test_validate_gitlab_trigger.py b/util/config/validators/test/test_validate_gitlab_trigger.py new file mode 100644 index 000000000..eaa855db8 --- /dev/null +++ b/util/config/validators/test/test_validate_gitlab_trigger.py @@ -0,0 +1,41 @@ +import json +import pytest + +from httmock import urlmatch, HTTMock + +from util.config.validators import ConfigValidationException +from util.config.validators.validate_gitlab_trigger import GitLabTriggerValidator + +@pytest.mark.parametrize('unvalidated_config', [ + ({}), + ({'GITLAB_TRIGGER_CONFIG': {}}), + ({'GITLAB_TRIGGER_CONFIG': {'GITLAB_ENDPOINT': 'foo'}}), + ({'GITLAB_TRIGGER_CONFIG': {'GITLAB_ENDPOINT': 'http://someendpoint', 'CLIENT_ID': 'foo'}}), + ({'GITLAB_TRIGGER_CONFIG': {'GITLAB_ENDPOINT': 'http://someendpoint', 'CLIENT_SECRET': 'foo'}}), +]) +def test_validate_invalid_gitlab_trigger_config(unvalidated_config): + validator = GitLabTriggerValidator() + + with pytest.raises(ConfigValidationException): + validator.validate(unvalidated_config, None, None) + +def test_validate_gitlab_trigger(): + url_hit = [False] + + @urlmatch(netloc=r'somegitlab', path='/oauth/token') + def handler(_, __): + url_hit[0] = True + return {'status_code': 400, 'content': json.dumps({'error': 'invalid code'})} + + with HTTMock(handler): + validator = GitLabTriggerValidator() + validator.validate({ + 'GITLAB_TRIGGER_CONFIG': { + 'GITLAB_ENDPOINT': 'http://somegitlab', + 'CLIENT_ID': 'foo', + 'CLIENT_SECRET': 'bar', + }, + }, None, None) + + assert url_hit[0] + diff --git a/util/config/validators/validate_gitlab_trigger.py b/util/config/validators/validate_gitlab_trigger.py new file mode 100644 index 000000000..d507d8ad6 --- /dev/null +++ b/util/config/validators/validate_gitlab_trigger.py @@ -0,0 +1,32 @@ +from app import app +from oauth.services.gitlab import GitLabOAuthService +from util.config.validators import BaseValidator, ConfigValidationException + +class GitLabTriggerValidator(BaseValidator): + name = "gitlab-trigger" + + @classmethod + def validate(cls, config, user, user_password): + """ Validates the OAuth credentials and API endpoint for a GitLab service. """ + github_config = config.get('GITLAB_TRIGGER_CONFIG') + if not github_config: + raise ConfigValidationException('Missing GitLab client id and client secret') + + endpoint = github_config.get('GITLAB_ENDPOINT') + if not endpoint: + raise ConfigValidationException('Missing GitLab Endpoint') + + if endpoint.find('http://') != 0 and endpoint.find('https://') != 0: + raise ConfigValidationException('GitLab Endpoint must start with http:// or https://') + + if not github_config.get('CLIENT_ID'): + raise ConfigValidationException('Missing Client ID') + + if not github_config.get('CLIENT_SECRET'): + raise ConfigValidationException('Missing Client Secret') + + client = app.config['HTTPCLIENT'] + oauth = GitLabOAuthService(config, 'GITLAB_TRIGGER_CONFIG') + result = oauth.validate_client_id_and_secret(client, app.config) + if not result: + raise ConfigValidationException('Invalid client id or client secret') From d4eb4f7f3c9095953edbc408c9ac16eeadcefd85 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Wed, 15 Feb 2017 16:07:25 -0500 Subject: [PATCH 20/23] Pull out github trigger and login validation into validator class --- util/config/validator.py | 96 ++++--------------- .../validators/test/test_validate_github.py | 62 ++++++++++++ util/config/validators/validate_github.py | 51 ++++++++++ 3 files changed, 132 insertions(+), 77 deletions(-) create mode 100644 util/config/validators/test/test_validate_github.py create mode 100644 util/config/validators/validate_github.py diff --git a/util/config/validator.py b/util/config/validator.py index a263d6d89..705b48217 100644 --- a/util/config/validator.py +++ b/util/config/validator.py @@ -1,13 +1,7 @@ import logging -import peewee - -from app import app, get_app_url from auth.auth_context import get_authenticated_user -from data.database import validate_database_url from data.users import LDAP_CERT_FILENAME -from oauth.services.github import GithubOAuthService -from oauth.services.gitlab import GitLabOAuthService from util.config.validators.validate_database import DatabaseValidator from util.config.validators.validate_redis import RedisValidator @@ -23,6 +17,7 @@ from util.config.validators.validate_ssl import SSLValidator, SSL_FILENAMES from util.config.validators.validate_google_login import GoogleLoginValidator from util.config.validators.validate_bitbucket_trigger import BitbucketTriggerValidator from util.config.validators.validate_gitlab_trigger import GitLabTriggerValidator +from util.config.validators.validate_github import GitHubLoginValidator, GitHubTriggerValidator logger = logging.getLogger(__name__) @@ -30,7 +25,6 @@ class ConfigValidationException(Exception): """ Exception raised when the configuration fails to validate for a known reason. """ pass - # Note: Only add files required for HTTPS to the SSL_FILESNAMES list. DB_SSL_FILENAMES = ['database.pem'] JWT_FILENAMES = ['jwt-authn.cert'] @@ -40,6 +34,24 @@ CONFIG_FILENAMES = (SSL_FILENAMES + DB_SSL_FILENAMES + JWT_FILENAMES + ACI_CERT_ LDAP_FILENAMES) EXTRA_CA_DIRECTORY = 'extra_ca_certs' +VALIDATORS = { + DatabaseValidator.name: DatabaseValidator.validate, + RedisValidator.name: RedisValidator.validate, + StorageValidator.name: StorageValidator.validate, + EmailValidator.name: EmailValidator.validate, + GitHubLoginValidator.name: GitHubLoginValidator.validate, + GitHubTriggerValidator.name: GitHubTriggerValidator.validate, + GitLabTriggerValidator.name: GitLabTriggerValidator.validate, + BitbucketTriggerValidator.name: BittorrentValidator.validate, + GoogleLoginValidator.name: GoogleLoginValidator.validate, + SSLValidator.name: SSLValidator.validate, + LDAPValidator.name: LDAPValidator.validate, + JWTAuthValidator.name: JWTAuthValidator.validate, + KeystoneValidator.name: KeystoneValidator.validate, + SignerValidator.name: SignerValidator.validate, + SecurityScannerValidator.name: SecurityScannerValidator.validate, + BittorrentValidator.name: BittorrentValidator.validate, +} def validate_service_for_config(service, config, password=None): """ Attempts to validate the configuration for the given service. """ @@ -59,73 +71,3 @@ def validate_service_for_config(service, config, password=None): 'status': False, 'reason': str(ex) } - - -def _validate_database(config, user_obj, _): - """ Validates connecting to the database. """ - try: - validate_database_url(config['DB_URI'], config.get('DB_CONNECTION_ARGS', {})) - except peewee.OperationalError as ex: - if ex.args and len(ex.args) > 1: - raise ConfigValidationException(ex.args[1]) - else: - raise ex - - -def _validate_github(config_key): - return lambda config, user_obj, _: _validate_github_with_key(config_key, config) - - -def _validate_github_with_key(config_key, config): - """ Validates the OAuth credentials and API endpoint for a Github service. """ - github_config = config.get(config_key) - if not github_config: - raise ConfigValidationException('Missing GitHub client id and client secret') - - endpoint = github_config.get('GITHUB_ENDPOINT') - if not endpoint: - raise ConfigValidationException('Missing GitHub Endpoint') - - if endpoint.find('http://') != 0 and endpoint.find('https://') != 0: - raise ConfigValidationException('Github Endpoint must start with http:// or https://') - - if not github_config.get('CLIENT_ID'): - raise ConfigValidationException('Missing Client ID') - - if not github_config.get('CLIENT_SECRET'): - raise ConfigValidationException('Missing Client Secret') - - if github_config.get('ORG_RESTRICT') and not github_config.get('ALLOWED_ORGANIZATIONS'): - raise ConfigValidationException('Organization restriction must have at least one allowed ' + - 'organization') - - client = app.config['HTTPCLIENT'] - oauth = GithubOAuthService(config, config_key) - result = oauth.validate_client_id_and_secret(client, app.config) - if not result: - raise ConfigValidationException('Invalid client id or client secret') - - if github_config.get('ALLOWED_ORGANIZATIONS'): - for org_id in github_config.get('ALLOWED_ORGANIZATIONS'): - if not oauth.validate_organization(org_id, client): - raise ConfigValidationException('Invalid organization: %s' % org_id) - - -VALIDATORS = { - DatabaseValidator.name: DatabaseValidator.validate, - RedisValidator.name: RedisValidator.validate, - StorageValidator.name: StorageValidator.validate, - EmailValidator.name: EmailValidator.validate, - 'github-login': _validate_github('GITHUB_LOGIN_CONFIG'), - 'github-trigger': _validate_github('GITHUB_TRIGGER_CONFIG'), - GitLabTriggerValidator.name: GitLabTriggerValidator.validate, - BitbucketTriggerValidator.name: BittorrentValidator.validate, - GoogleLoginValidator.name: GoogleLoginValidator.validate, - SSLValidator.name: SSLValidator.validate, - LDAPValidator.name: LDAPValidator.validate, - JWTAuthValidator.name: JWTAuthValidator.validate, - KeystoneValidator.name: KeystoneValidator.validate, - SignerValidator.name: SignerValidator.validate, - SecurityScannerValidator.name: SecurityScannerValidator.validate, - BittorrentValidator.name: BittorrentValidator.validate, -} diff --git a/util/config/validators/test/test_validate_github.py b/util/config/validators/test/test_validate_github.py new file mode 100644 index 000000000..14582e99c --- /dev/null +++ b/util/config/validators/test/test_validate_github.py @@ -0,0 +1,62 @@ +import pytest + +from httmock import urlmatch, HTTMock + +from util.config.validators import ConfigValidationException +from util.config.validators.validate_github import GitHubLoginValidator, GitHubTriggerValidator + +@pytest.fixture(params=[GitHubLoginValidator, GitHubTriggerValidator]) +def github_validator(request): + return request.param + + +@pytest.mark.parametrize('github_config', [ + ({}), + ({'GITHUB_ENDPOINT': 'foo'}), + ({'GITHUB_ENDPOINT': 'http://github.com'}), + ({'GITHUB_ENDPOINT': 'http://github.com', 'CLIENT_ID': 'foo'}), + ({'GITHUB_ENDPOINT': 'http://github.com', 'CLIENT_SECRET': 'foo'}), + ({ + 'GITHUB_ENDPOINT': 'http://github.com', + 'CLIENT_ID': 'foo', + 'CLIENT_SECRET': 'foo', + 'ORG_RESTRICT': True + }), + ({ + 'GITHUB_ENDPOINT': 'http://github.com', + 'CLIENT_ID': 'foo', + 'CLIENT_SECRET': 'foo', + 'ORG_RESTRICT': True, + 'ALLOWED_ORGANIZATIONS': [], + }), +]) +def test_validate_invalid_github_config(github_config, github_validator): + with pytest.raises(ConfigValidationException): + unvalidated_config = {} + unvalidated_config[github_validator.config_key] = github_config + github_validator.validate(unvalidated_config, None, None) + +def test_validate_github(github_validator): + url_hit = [False, False] + + @urlmatch(netloc=r'somehost') + def handler(url, request): + url_hit[0] = True + return {'status_code': 200, 'content': '', 'headers': {'X-GitHub-Request-Id': 'foo'}} + + @urlmatch(netloc=r'somehost', path=r'/api/v3/applications/foo/tokens/foo') + def app_handler(url, request): + url_hit[1] = True + return {'status_code': 404, 'content': '', 'headers': {'X-GitHub-Request-Id': 'foo'}} + + with HTTMock(app_handler, handler): + github_validator.validate({ + github_validator.config_key: { + 'GITHUB_ENDPOINT': 'http://somehost', + 'CLIENT_ID': 'foo', + 'CLIENT_SECRET': 'bar', + }, + }, None, None) + + assert url_hit[0] + assert url_hit[1] diff --git a/util/config/validators/validate_github.py b/util/config/validators/validate_github.py new file mode 100644 index 000000000..39293a11d --- /dev/null +++ b/util/config/validators/validate_github.py @@ -0,0 +1,51 @@ +from app import app +from oauth.services.github import GithubOAuthService +from util.config.validators import BaseValidator, ConfigValidationException + +class BaseGitHubValidator(BaseValidator): + name = None + config_key = None + + @classmethod + def validate(cls, config, user, user_password): + """ Validates the OAuth credentials and API endpoint for a Github service. """ + github_config = config.get(cls.config_key) + if not github_config: + raise ConfigValidationException('Missing GitHub client id and client secret') + + endpoint = github_config.get('GITHUB_ENDPOINT') + if not endpoint: + raise ConfigValidationException('Missing GitHub Endpoint') + + if endpoint.find('http://') != 0 and endpoint.find('https://') != 0: + raise ConfigValidationException('Github Endpoint must start with http:// or https://') + + if not github_config.get('CLIENT_ID'): + raise ConfigValidationException('Missing Client ID') + + if not github_config.get('CLIENT_SECRET'): + raise ConfigValidationException('Missing Client Secret') + + if github_config.get('ORG_RESTRICT') and not github_config.get('ALLOWED_ORGANIZATIONS'): + raise ConfigValidationException('Organization restriction must have at least one allowed ' + + 'organization') + + client = app.config['HTTPCLIENT'] + oauth = GithubOAuthService(config, cls.config_key) + result = oauth.validate_client_id_and_secret(client, app.config) + if not result: + raise ConfigValidationException('Invalid client id or client secret') + + if github_config.get('ALLOWED_ORGANIZATIONS'): + for org_id in github_config.get('ALLOWED_ORGANIZATIONS'): + if not oauth.validate_organization(org_id, client): + raise ConfigValidationException('Invalid organization: %s' % org_id) + + +class GitHubLoginValidator(BaseGitHubValidator): + name = "github-login" + config_key = "GITHUB_LOGIN_CONFIG" + +class GitHubTriggerValidator(BaseGitHubValidator): + name = "github-trigger" + config_key = "GITHUB_TRIGGER_CONFIG" From f8d74305e150d9294e455fe0cc7c74bb4e1c3573 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Wed, 15 Feb 2017 16:08:04 -0500 Subject: [PATCH 21/23] Remove old validator tests --- test/test_validate_config.py | 303 ----------------------------------- 1 file changed, 303 deletions(-) delete mode 100644 test/test_validate_config.py diff --git a/test/test_validate_config.py b/test/test_validate_config.py deleted file mode 100644 index 1c4dbdc89..000000000 --- a/test/test_validate_config.py +++ /dev/null @@ -1,303 +0,0 @@ -import unittest -import redis -import moto -import json - -from httmock import urlmatch, HTTMock - -from initdb import setup_database_for_testing, finished_database_for_testing - -from util.config.validator import VALIDATORS, ConfigValidationException -from util.morecollections import AttrDict - -from app import app - -class TestValidateConfig(unittest.TestCase): - validated = set([]) - - def setUp(self): - setup_database_for_testing(self) - - self.app = app.test_client() - self.ctx = app.test_request_context() - self.ctx.__enter__() - - def tearDown(self): - finished_database_for_testing(self) - self.ctx.__exit__(True, None, None) - - def validate(self, service, config, user=None, password=None): - self.validated.add(service) - config['TESTING'] = True - VALIDATORS[service](config, user, password) - - def test_validate_mail(self): - # Skip mail. - self.validated.add('mail') - - def test_validate_jwt(self): - with self.assertRaisesRegexp(ConfigValidationException, 'Missing JWT Verification endpoint'): - self.validate('jwt', { - 'AUTHENTICATION_TYPE': 'JWT', - }) - - with self.assertRaisesRegexp(ConfigValidationException, 'Missing JWT Issuer ID'): - self.validate('jwt', { - 'AUTHENTICATION_TYPE': 'JWT', - 'JWT_VERIFY_ENDPOINT': 'somehost', - }) - - with self.assertRaisesRegexp(Exception, 'JWT Authentication public key file'): - self.validate('jwt', { - 'AUTHENTICATION_TYPE': 'JWT', - 'JWT_VERIFY_ENDPOINT': 'somehost', - 'JWT_AUTH_ISSUER': 'someissuer', - }) - - # TODO(jschorr): Add another test once we switch JWT auth to use the config provider to - # find the file - - def test_validate_registry_storage(self): - with self.assertRaisesRegexp(ConfigValidationException, 'Storage configuration required'): - self.validate('registry-storage', {}) - - with self.assertRaisesRegexp(ConfigValidationException, 'Locally mounted directory not'): - self.validate('registry-storage', { - 'FEATURE_STORAGE_REPLICATION': True, - 'DISTRIBUTED_STORAGE_CONFIG': { - 'default': ('LocalStorage', { - 'storage_path': '', - }), - } - }) - - with self.assertRaisesRegexp(ConfigValidationException, 'No such file or directory'): - self.validate('registry-storage', { - 'DISTRIBUTED_STORAGE_CONFIG': { - 'default': ('LocalStorage', { - 'storage_path': '', - }), - } - }) - - with moto.mock_s3(): - with self.assertRaisesRegexp(ConfigValidationException, 'S3ResponseError: 404 Not Found'): - self.validate('registry-storage', { - 'DISTRIBUTED_STORAGE_CONFIG': { - 'default': ('S3Storage', { - 's3_access_key': 'invalid', - 's3_secret_key': 'invalid', - 's3_bucket': 'somebucket', - 'storage_path': '' - }), - } - }) - - def test_validate_bittorrent(self): - with self.assertRaisesRegexp(ConfigValidationException, 'Missing announce URL'): - self.validate('bittorrent', {}) - - announcer_hit = [False] - - @urlmatch(netloc=r'somehost', path='/announce') - def handler(url, request): - announcer_hit[0] = True - return {'status_code': 200, 'content': ''} - - with HTTMock(handler): - self.validate('bittorrent', { - 'BITTORRENT_ANNOUNCE_URL': 'http://somehost/announce', - }) - - self.assertTrue(announcer_hit[0]) - - def test_validate_ssl(self): - self.validate('ssl', { - 'PREFERRED_URL_SCHEME': 'http', - }) - - self.validate('ssl', { - 'PREFERRED_URL_SCHEME': 'https', - 'EXTERNAL_TLS_TERMINATION': True, - }) - - with self.assertRaisesRegexp(ConfigValidationException, 'Missing required SSL file'): - self.validate('ssl', { - 'PREFERRED_URL_SCHEME': 'https', - }) - - def test_validate_keystone(self): - with self.assertRaisesRegexp(ConfigValidationException, - 'Verification of superuser someuser failed'): - self.validate('keystone', { - 'AUTHENTICATION_TYPE': 'Keystone', - 'KEYSTONE_AUTH_URL': 'somehost', - 'KEYSTONE_AUTH_VERSION': 2, - 'KEYSTONE_ADMIN_USERNAME': 'someusername', - 'KEYSTONE_ADMIN_PASSWORD': 'somepassword', - 'KEYSTONE_ADMIN_TENANT': 'sometenant', - }, user=AttrDict(dict(username='someuser'))) - - def test_validate_ldap(self): - with self.assertRaisesRegexp(ConfigValidationException, 'Missing Admin DN for LDAP'): - self.validate('ldap', { - 'AUTHENTICATION_TYPE': 'LDAP', - }) - - with self.assertRaisesRegexp(ConfigValidationException, 'Missing Admin Password for LDAP'): - self.validate('ldap', { - 'AUTHENTICATION_TYPE': 'LDAP', - 'LDAP_ADMIN_DN': 'somedn', - }) - - with self.assertRaisesRegexp(ConfigValidationException, 'Can\'t contact LDAP server'): - self.validate('ldap', { - 'AUTHENTICATION_TYPE': 'LDAP', - 'LDAP_ADMIN_DN': 'somedn', - 'LDAP_ADMIN_PASSWD': 'somepass', - 'LDAP_URI': 'ldap://localhost', - }) - - def test_validate_signer(self): - with self.assertRaisesRegexp(ConfigValidationException, 'Unknown signing engine'): - self.validate('signer', { - 'SIGNING_ENGINE': 'foobar', - }) - - def test_validate_security_scanner(self): - url_hit = [False] - @urlmatch(netloc=r'somehost') - def handler(url, request): - url_hit[0] = True - return {'status_code': 200, 'content': ''} - - with HTTMock(handler): - self.validate('security-scanner', { - 'DISTRIBUTED_STORAGE_PREFERENCE': ['local'], - 'DISTRIBUTED_STORAGE_CONFIG': { - 'default': ('LocalStorage', { - 'storage_path': '', - }), - }, - 'SECURITY_SCANNER_ENDPOINT': 'http://somehost', - }) - - - def test_validate_github_trigger(self): - with self.assertRaisesRegexp(ConfigValidationException, 'Missing GitHub client id'): - self.validate('github-trigger', {}) - - url_hit = [False] - @urlmatch(netloc=r'somehost') - def handler(url, request): - url_hit[0] = True - return {'status_code': 200, 'content': ''} - - with HTTMock(handler): - with self.assertRaisesRegexp(Exception, 'Endpoint is not a Github'): - self.validate('github-trigger', { - 'GITHUB_TRIGGER_CONFIG': { - 'GITHUB_ENDPOINT': 'http://somehost', - 'CLIENT_ID': 'foo', - 'CLIENT_SECRET': 'bar', - }, - }) - - self.assertTrue(url_hit[0]) - - def test_validate_github_login(self): - with self.assertRaisesRegexp(ConfigValidationException, 'Missing GitHub client id'): - self.validate('github-login', {}) - - url_hit = [False] - @urlmatch(netloc=r'somehost') - def handler(url, request): - url_hit[0] = True - return {'status_code': 200, 'content': ''} - - with HTTMock(handler): - with self.assertRaisesRegexp(Exception, 'Endpoint is not a Github'): - self.validate('github-login', { - 'GITHUB_LOGIN_CONFIG': { - 'GITHUB_ENDPOINT': 'http://somehost', - 'CLIENT_ID': 'foo', - 'CLIENT_SECRET': 'bar', - }, - }) - - self.assertTrue(url_hit[0]) - - def test_validate_bitbucket_trigger(self): - with self.assertRaisesRegexp(ConfigValidationException, 'Missing client ID and client secret'): - self.validate('bitbucket-trigger', {}) - - url_hit = [False] - @urlmatch(netloc=r'bitbucket.org') - def handler(url, request): - url_hit[0] = True - return { - 'status_code': 200, - 'content': 'oauth_token=foo&oauth_token_secret=bar', - } - - with HTTMock(handler): - self.validate('bitbucket-trigger', { - 'BITBUCKET_TRIGGER_CONFIG': { - 'CONSUMER_KEY': 'foo', - 'CONSUMER_SECRET': 'bar', - }, - }) - - self.assertTrue(url_hit[0]) - - def test_validate_google_login(self): - with self.assertRaisesRegexp(ConfigValidationException, 'Missing client ID and client secret'): - self.validate('google-login', {}) - - url_hit = [False] - @urlmatch(netloc=r'www.googleapis.com', path='/oauth2/v3/token') - def handler(url, request): - url_hit[0] = True - return {'status_code': 200, 'content': ''} - - with HTTMock(handler): - self.validate('google-login', { - 'GOOGLE_LOGIN_CONFIG': { - 'CLIENT_ID': 'foo', - 'CLIENT_SECRET': 'bar', - }, - }) - - self.assertTrue(url_hit[0]) - - def test_validate_gitlab_trigger(self): - with self.assertRaisesRegexp(ConfigValidationException, 'Missing GitLab client id'): - self.validate('gitlab-trigger', {}) - - url_hit = [False] - @urlmatch(netloc=r'somegitlab', path='/oauth/token') - def handler(url, request): - url_hit[0] = True - return {'status_code': 200, 'content': '{}'} - - with HTTMock(handler): - with self.assertRaisesRegexp(ConfigValidationException, "Invalid client id or client secret"): - self.validate('gitlab-trigger', { - 'GITLAB_TRIGGER_CONFIG': { - 'GITLAB_ENDPOINT': 'http://somegitlab', - 'CLIENT_ID': 'foo', - 'CLIENT_SECRET': 'bar', - }, - }) - - self.assertTrue(url_hit[0]) - - - @classmethod - def tearDownClass(cls): - not_run = set(VALIDATORS.keys()) - cls.validated - assert not not_run, not_run - - -if __name__ == '__main__': - unittest.main() \ No newline at end of file From 8ec6221ca2f88e4ec993e4036879ff18291d492b Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Wed, 15 Feb 2017 16:20:05 -0500 Subject: [PATCH 22/23] Fix health check --- data/model/health.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/data/model/health.py b/data/model/health.py index dcef9022b..0d177b400 100644 --- a/data/model/health.py +++ b/data/model/health.py @@ -1,12 +1,9 @@ import logging -from data.database import TeamRole -from util.config.validator import validate_database_url - +from data.database import TeamRole, validate_database_url logger = logging.getLogger(__name__) - def check_health(app_config): # Attempt to connect to the database first. If the DB is not responding, # using the validate_database_url will timeout quickly, as opposed to From 88b808f468c37ec8ad06fb7bade41fd2e36ae92d Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Wed, 15 Feb 2017 16:21:45 -0500 Subject: [PATCH 23/23] Fix typo --- util/config/validator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/config/validator.py b/util/config/validator.py index 705b48217..9dd9246ae 100644 --- a/util/config/validator.py +++ b/util/config/validator.py @@ -42,7 +42,7 @@ VALIDATORS = { GitHubLoginValidator.name: GitHubLoginValidator.validate, GitHubTriggerValidator.name: GitHubTriggerValidator.validate, GitLabTriggerValidator.name: GitLabTriggerValidator.validate, - BitbucketTriggerValidator.name: BittorrentValidator.validate, + BitbucketTriggerValidator.name: BitbucketTriggerValidator.validate, GoogleLoginValidator.name: GoogleLoginValidator.validate, SSLValidator.name: SSLValidator.validate, LDAPValidator.name: LDAPValidator.validate,