From 01c23be9d629a55ab04600ccd0312dbfc09d32ca Mon Sep 17 00:00:00 2001 From: Sam Chow Date: Mon, 9 Jul 2018 16:32:41 -0400 Subject: [PATCH 1/5] Install certs locally in config app to validate --- config_app/config_endpoints/api/superuser.py | 12 +++--- config_app/init/certs_install.sh | 43 ++++++++++++++++++++ 2 files changed, 48 insertions(+), 7 deletions(-) create mode 100755 config_app/init/certs_install.sh diff --git a/config_app/config_endpoints/api/superuser.py b/config_app/config_endpoints/api/superuser.py index 34bb9fba9..71cf33e47 100644 --- a/config_app/config_endpoints/api/superuser.py +++ b/config_app/config_endpoints/api/superuser.py @@ -1,6 +1,7 @@ import logging import pathvalidate import os +import subprocess from flask import request, jsonify @@ -49,13 +50,10 @@ class SuperUserCustomCertificate(ApiResource): return '', 204 # TODO(QUAY-991): properly install the custom certs provided by user - # Call the update script to install the certificate immediately. - # if not app.config['TESTING']: - # logger.debug('Calling certs_install.sh') - # if os.system('/conf/init/certs_install.sh') != 0: - # raise Exception('Could not install certificates') - # - # logger.debug('certs_install.sh completed') + # Call the update script with config dir location to install the certificate immediately. + if subprocess.call(['/quay-registry/config_app/init/certs_install.sh'], + env={ 'QUAYCONF': config_provider.get_config_dir_path() }) != 0: + raise Exception('Could not install certificates') return '', 204 diff --git a/config_app/init/certs_install.sh b/config_app/init/certs_install.sh new file mode 100755 index 000000000..45a08f5ce --- /dev/null +++ b/config_app/init/certs_install.sh @@ -0,0 +1,43 @@ +#! /bin/bash +set -e +QUAYPATH=${QUAYPATH:-"."} +QUAYCONF=${QUAYCONF:-"$QUAYPATH/conf/stack"} + +cd ${QUAYDIR:-"/quay-registry"} +pwd + +# Add the custom LDAP certificate +if [ -e $QUAYCONF/ldap.crt ] +then + cp $QUAYCONF/ldap.crt /usr/local/share/ca-certificates/ldap.crt +fi + +# Add extra trusted certificates (as a directory) +if [ -d $QUAYCONF/extra_ca_certs ]; then + if test "$(ls -A "$QUAYCONF/extra_ca_certs")"; then + echo "Installing extra certificates found in $QUAYCONF/extra_ca_certs directory" + cp $QUAYCONF/extra_ca_certs/* /usr/local/share/ca-certificates/ + cat $QUAYCONF/extra_ca_certs/* >> venv/lib/python2.7/site-packages/requests/cacert.pem + cat $QUAYCONF/extra_ca_certs/* >> venv/lib/python2.7/site-packages/certifi/cacert.pem + fi +fi + +# Add extra trusted certificates (as a file) +if [ -f $QUAYCONF/extra_ca_certs ]; then + echo "Installing extra certificates found in $QUAYCONF/extra_ca_certs file" + csplit -z -f /usr/local/share/ca-certificates/extra-ca- $QUAYCONF/extra_ca_certs '/-----BEGIN CERTIFICATE-----/' '{*}' + cat $QUAYCONF/extra_ca_certs >> venv/lib/python2.7/site-packages/requests/cacert.pem + cat $QUAYCONF/extra_ca_certs >> venv/lib/python2.7/site-packages/certifi/cacert.pem +fi + +# Add extra trusted certificates (prefixed) +for f in $(find $QUAYCONF/ -maxdepth 1 -type f -name "extra_ca*") +do + echo "Installing extra cert $f" + cp "$f" /usr/local/share/ca-certificates/ + cat "$f" >> venv/lib/python2.7/site-packages/requests/cacert.pem + cat "$f" >> venv/lib/python2.7/site-packages/certifi/cacert.pem +done + +# Update all CA certificates. +update-ca-certificates From bd54eacbad2e767f1f9882542ece50fadc0c6e85 Mon Sep 17 00:00:00 2001 From: Sam Chow Date: Tue, 10 Jul 2018 11:43:34 -0400 Subject: [PATCH 2/5] Add app var for init scripts location to access certs install --- app.py | 1 + config_app/c_app.py | 1 + config_app/config_endpoints/api/suconfig.py | 6 ++++-- config_app/config_endpoints/api/superuser.py | 4 ++-- endpoints/api/suconfig.py | 5 +++-- endpoints/api/superuser.py | 4 ++-- util/config/validator.py | 16 ++++++++++------ util/config/validators/validate_ldap.py | 3 ++- 8 files changed, 25 insertions(+), 15 deletions(-) diff --git a/app.py b/app.py index 7845b43fd..bf4ae4fe0 100644 --- a/app.py +++ b/app.py @@ -62,6 +62,7 @@ OVERRIDE_CONFIG_PY_FILENAME = os.path.join(CONF_DIR, 'stack/config.py') OVERRIDE_CONFIG_KEY = 'QUAY_OVERRIDE_CONFIG' DOCKER_V2_SIGNINGKEY_FILENAME = 'docker_v2.pem' +INIT_SCRIPTS_LOCATION = '/conf/init/' app = Flask(__name__) logger = logging.getLogger(__name__) diff --git a/config_app/c_app.py b/config_app/c_app.py index ea118d3b5..8e116a3cb 100644 --- a/config_app/c_app.py +++ b/config_app/c_app.py @@ -16,6 +16,7 @@ app = Flask(__name__) logger = logging.getLogger(__name__) OVERRIDE_CONFIG_DIRECTORY = os.path.join(ROOT_DIR, 'config_app/conf/stack') +INIT_SCRIPTS_LOCATION = '/quay-registry/config_app/init/' is_testing = 'TEST' in os.environ diff --git a/config_app/config_endpoints/api/suconfig.py b/config_app/config_endpoints/api/suconfig.py index 9e17701ab..6bae4b89b 100644 --- a/config_app/config_endpoints/api/suconfig.py +++ b/config_app/config_endpoints/api/suconfig.py @@ -4,7 +4,8 @@ from flask import abort, request from config_app.config_endpoints.api.suconfig_models_pre_oci import pre_oci_model as model from config_app.config_endpoints.api import resource, ApiResource, nickname, validate_json_request -from config_app.c_app import app, config_provider, superusers, OVERRIDE_CONFIG_DIRECTORY, ip_resolver, instance_keys +from config_app.c_app import (app, config_provider, superusers, OVERRIDE_CONFIG_DIRECTORY, + ip_resolver, instance_keys, INIT_SCRIPTS_LOCATION) from auth.auth_context import get_authenticated_user from data.users import get_federated_service_name, get_users_handler @@ -275,7 +276,8 @@ class SuperUserConfigValidate(ApiResource): validator_context = ValidatorContext.from_app(app, config, request.get_json().get('password', ''), instance_keys=instance_keys, ip_resolver=ip_resolver, - config_provider=config_provider) + config_provider=config_provider, + init_scripts_location=INIT_SCRIPTS_LOCATION) return validate_service_for_config(service, validator_context) diff --git a/config_app/config_endpoints/api/superuser.py b/config_app/config_endpoints/api/superuser.py index 71cf33e47..a0848fb93 100644 --- a/config_app/config_endpoints/api/superuser.py +++ b/config_app/config_endpoints/api/superuser.py @@ -11,7 +11,7 @@ from config_app.config_endpoints.exception import InvalidRequest from config_app.config_endpoints.api import resource, ApiResource, nickname from config_app.config_endpoints.api.superuser_models_pre_oci import pre_oci_model from config_app.config_util.ssl import load_certificate, CertInvalidException -from config_app.c_app import app, config_provider +from config_app.c_app import config_provider, INIT_SCRIPTS_LOCATION logger = logging.getLogger(__name__) @@ -51,7 +51,7 @@ class SuperUserCustomCertificate(ApiResource): # TODO(QUAY-991): properly install the custom certs provided by user # Call the update script with config dir location to install the certificate immediately. - if subprocess.call(['/quay-registry/config_app/init/certs_install.sh'], + if subprocess.call([os.path.join(INIT_SCRIPTS_LOCATION, 'certs_install.sh')], env={ 'QUAYCONF': config_provider.get_config_dir_path() }) != 0: raise Exception('Could not install certificates') diff --git a/endpoints/api/suconfig.py b/endpoints/api/suconfig.py index 77954c6ae..bceeb3e43 100644 --- a/endpoints/api/suconfig.py +++ b/endpoints/api/suconfig.py @@ -8,7 +8,7 @@ import subprocess from flask import abort from app import (app, config_provider, superusers, OVERRIDE_CONFIG_DIRECTORY, ip_resolver, - instance_keys) + instance_keys, INIT_SCRIPTS_LOCATION) from auth.permissions import SuperUserPermission from auth.auth_context import get_authenticated_user from data.database import configure @@ -410,7 +410,8 @@ class SuperUserConfigValidate(ApiResource): request.get_json().get('password', ''), instance_keys=instance_keys, ip_resolver=ip_resolver, - config_provider=config_provider) + config_provider=config_provider, + init_scripts_location=INIT_SCRIPTS_LOCATION) return validate_service_for_config(service, validator_context) diff --git a/endpoints/api/superuser.py b/endpoints/api/superuser.py index ee0b039d8..1efb49547 100644 --- a/endpoints/api/superuser.py +++ b/endpoints/api/superuser.py @@ -13,7 +13,7 @@ from flask import request, make_response, jsonify import features -from app import app, avatar, superusers, authentication, config_provider +from app import app, avatar, superusers, authentication, config_provider, INIT_SCRIPTS_LOCATION from auth import scopes from auth.auth_context import get_authenticated_user from auth.permissions import SuperUserPermission @@ -950,7 +950,7 @@ class SuperUserCustomCertificate(ApiResource): # Call the update script to install the certificate immediately. if not app.config['TESTING']: logger.debug('Calling certs_install.sh') - if os.system('/conf/init/certs_install.sh') != 0: + if os.system(os.path.join(INIT_SCRIPTS_LOCATION, 'certs_install.sh')) != 0: raise Exception('Could not install certificates') logger.debug('certs_install.sh completed') diff --git a/util/config/validator.py b/util/config/validator.py index a0924c9e2..54f23938d 100644 --- a/util/config/validator.py +++ b/util/config/validator.py @@ -102,7 +102,8 @@ class ValidatorContext(object): def __init__(self, config, user_password=None, http_client=None, context=None, url_scheme_and_hostname=None, jwt_auth_max=None, registry_title=None, ip_resolver=None, feature_sec_scanner=False, is_testing=False, - uri_creator=None, config_provider=None, instance_keys=None): + uri_creator=None, config_provider=None, instance_keys=None, + init_scripts_location=None): self.config = config self.user = get_authenticated_user() self.user_password = user_password @@ -117,10 +118,11 @@ class ValidatorContext(object): self.uri_creator = uri_creator self.config_provider = config_provider self.instance_keys = instance_keys + self.init_scripts_location = init_scripts_location @classmethod def from_app(cls, app, config, user_password, ip_resolver, instance_keys, client=None, - config_provider=None): + config_provider=None, init_scripts_location=None): """ Creates a ValidatorContext from an app config, with a given config to validate :param app: the Flask app to pull configuration information from @@ -128,9 +130,10 @@ class ValidatorContext(object): :param user_password: request password :param instance_keys: The instance keys handler :param ip_resolver: an App - :param client: - :param config_provider: - :return: + :param client: http client used to connect to services + :param config_provider: config provider used to access config volume(s) + :param init_scripts_location: location where initial load scripts are stored + :return: ValidatorContext """ url_scheme_and_hostname = URLSchemeAndHostname.from_app_config(app.config) @@ -146,4 +149,5 @@ class ValidatorContext(object): is_testing=app.config.get('TESTING', False), uri_creator=get_blob_download_uri_getter(app.test_request_context('/'), url_scheme_and_hostname), config_provider=config_provider, - instance_keys=instance_keys) + instance_keys=instance_keys, + init_scripts_location=init_scripts_location) diff --git a/util/config/validators/validate_ldap.py b/util/config/validators/validate_ldap.py index 331cd87d3..ec1dafe8d 100644 --- a/util/config/validators/validate_ldap.py +++ b/util/config/validators/validate_ldap.py @@ -16,13 +16,14 @@ class LDAPValidator(BaseValidator): user = validator_context.user user_password = validator_context.user_password config_provider = validator_context.config_provider + init_scripts_location = validator_context.init_scripts_location 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([os.path.join(config_provider.get_config_root(), '../init/certs_install.sh')]) + subprocess.check_call([os.path.join(init_scripts_location, 'certs_install.sh')]) # Note: raises ldap.INVALID_CREDENTIALS on failure admin_dn = config.get('LDAP_ADMIN_DN') From 9024419896367265167a7a77884e8eea8e0766a1 Mon Sep 17 00:00:00 2001 From: Sam Chow Date: Wed, 11 Jul 2018 16:03:36 -0400 Subject: [PATCH 3/5] Modify ldap validator to just check user existence Remove auth user check from updating config app config remove duplicate certs install script --- conf/init/certs_install.sh | 32 +++++++-------- config_app/c_app.py | 2 +- config_app/config_endpoints/api/suconfig.py | 27 +----------- config_app/config_endpoints/api/superuser.py | 3 +- config_app/init/certs_install.sh | 43 -------------------- data/users/externalldap.py | 26 ++++++++++++ util/config/validators/validate_ldap.py | 11 ++--- 7 files changed, 52 insertions(+), 92 deletions(-) delete mode 100755 config_app/init/certs_install.sh diff --git a/conf/init/certs_install.sh b/conf/init/certs_install.sh index 34f0d4ac3..981064f70 100755 --- a/conf/init/certs_install.sh +++ b/conf/init/certs_install.sh @@ -1,36 +1,36 @@ #! /bin/bash set -e QUAYPATH=${QUAYPATH:-"."} -QUAYCONF=${QUAYCONF:-"$QUAYPATH/conf"} +QUAYCONF=${QUAYCONF:-"$QUAYPATH/conf/stack"} -cd ${QUAYDIR:-"/"} +cd ${QUAYDIR:-"/quay-registry"} # Add the custom LDAP certificate -if [ -e $QUAYCONF/stack/ldap.crt ] +if [ -e $QUAYCONF/ldap.crt ] then - cp $QUAYCONF/stack/ldap.crt /usr/local/share/ca-certificates/ldap.crt + cp $QUAYCONF/ldap.crt /usr/local/share/ca-certificates/ldap.crt fi # Add extra trusted certificates (as a directory) -if [ -d $QUAYCONF/stack/extra_ca_certs ]; then - if test "$(ls -A "$QUAYCONF/stack/extra_ca_certs")"; then - echo "Installing extra certificates found in $QUAYCONF/stack/extra_ca_certs directory" - cp $QUAYCONF/stack/extra_ca_certs/* /usr/local/share/ca-certificates/ - cat $QUAYCONF/stack/extra_ca_certs/* >> venv/lib/python2.7/site-packages/requests/cacert.pem - cat $QUAYCONF/stack/extra_ca_certs/* >> venv/lib/python2.7/site-packages/certifi/cacert.pem +if [ -d $QUAYCONF/extra_ca_certs ]; then + if test "$(ls -A "$QUAYCONF/extra_ca_certs")"; then + echo "Installing extra certificates found in $QUAYCONF/extra_ca_certs directory" + cp $QUAYCONF/extra_ca_certs/* /usr/local/share/ca-certificates/ + cat $QUAYCONF/extra_ca_certs/* >> venv/lib/python2.7/site-packages/requests/cacert.pem + cat $QUAYCONF/extra_ca_certs/* >> venv/lib/python2.7/site-packages/certifi/cacert.pem fi fi # Add extra trusted certificates (as a file) -if [ -f $QUAYCONF/stack/extra_ca_certs ]; then - echo "Installing extra certificates found in $QUAYCONF/stack/extra_ca_certs file" - csplit -z -f /usr/local/share/ca-certificates/extra-ca- $QUAYCONF/stack/extra_ca_certs '/-----BEGIN CERTIFICATE-----/' '{*}' - cat $QUAYCONF/stack/extra_ca_certs >> venv/lib/python2.7/site-packages/requests/cacert.pem - cat $QUAYCONF/stack/extra_ca_certs >> venv/lib/python2.7/site-packages/certifi/cacert.pem +if [ -f $QUAYCONF/extra_ca_certs ]; then + echo "Installing extra certificates found in $QUAYCONF/extra_ca_certs file" + csplit -z -f /usr/local/share/ca-certificates/extra-ca- $QUAYCONF/extra_ca_certs '/-----BEGIN CERTIFICATE-----/' '{*}' + cat $QUAYCONF/extra_ca_certs >> venv/lib/python2.7/site-packages/requests/cacert.pem + cat $QUAYCONF/extra_ca_certs >> venv/lib/python2.7/site-packages/certifi/cacert.pem fi # Add extra trusted certificates (prefixed) -for f in $(find $QUAYCONF/stack/ -maxdepth 1 -type f -name "extra_ca*") +for f in $(find $QUAYCONF/ -maxdepth 1 -type f -name "extra_ca*") do echo "Installing extra cert $f" cp "$f" /usr/local/share/ca-certificates/ diff --git a/config_app/c_app.py b/config_app/c_app.py index 8e116a3cb..a1e25304a 100644 --- a/config_app/c_app.py +++ b/config_app/c_app.py @@ -16,7 +16,7 @@ app = Flask(__name__) logger = logging.getLogger(__name__) OVERRIDE_CONFIG_DIRECTORY = os.path.join(ROOT_DIR, 'config_app/conf/stack') -INIT_SCRIPTS_LOCATION = '/quay-registry/config_app/init/' +INIT_SCRIPTS_LOCATION = '/conf/init/' is_testing = 'TEST' in os.environ diff --git a/config_app/config_endpoints/api/suconfig.py b/config_app/config_endpoints/api/suconfig.py index 6bae4b89b..d83ad88eb 100644 --- a/config_app/config_endpoints/api/suconfig.py +++ b/config_app/config_endpoints/api/suconfig.py @@ -4,11 +4,9 @@ from flask import abort, request from config_app.config_endpoints.api.suconfig_models_pre_oci import pre_oci_model as model from config_app.config_endpoints.api import resource, ApiResource, nickname, validate_json_request -from config_app.c_app import (app, config_provider, superusers, OVERRIDE_CONFIG_DIRECTORY, - ip_resolver, instance_keys, INIT_SCRIPTS_LOCATION) +from config_app.c_app import (app, config_provider, superusers, ip_resolver, + instance_keys, INIT_SCRIPTS_LOCATION) -from auth.auth_context import get_authenticated_user -from data.users import get_federated_service_name, get_users_handler from data.database import configure from data.runmigration import run_alembic_migration from util.config.configutil import add_enterprise_config_defaults @@ -75,27 +73,6 @@ class SuperUserConfig(ApiResource): # Write the configuration changes to the config override file. config_provider.save_config(config_object) - # If the authentication system is federated, link the superuser account to the - # the authentication system chosen. - service_name = get_federated_service_name(config_object['AUTHENTICATION_TYPE']) - if service_name is not None: - current_user = get_authenticated_user() - if current_user is None: - abort(401) - - service_name = get_federated_service_name(config_object['AUTHENTICATION_TYPE']) - if not model.has_federated_login(current_user.username, service_name): - # Verify the user's credentials and retrieve the user's external username+email. - handler = get_users_handler(config_object, config_provider, OVERRIDE_CONFIG_DIRECTORY) - (result, err_msg) = handler.verify_credentials(current_user.username, - request.get_json().get('password', '')) - if not result: - logger.error('Could not save configuration due to external auth failure: %s', err_msg) - abort(400) - - # Link the existing user to the external user. - model.attach_federated_login(current_user.username, service_name, result.username) - return { 'exists': True, 'config': config_object diff --git a/config_app/config_endpoints/api/superuser.py b/config_app/config_endpoints/api/superuser.py index a0848fb93..7eceac565 100644 --- a/config_app/config_endpoints/api/superuser.py +++ b/config_app/config_endpoints/api/superuser.py @@ -49,10 +49,9 @@ class SuperUserCustomCertificate(ApiResource): logger.exception('Got IO error for cert %s', certpath) return '', 204 - # TODO(QUAY-991): properly install the custom certs provided by user # Call the update script with config dir location to install the certificate immediately. if subprocess.call([os.path.join(INIT_SCRIPTS_LOCATION, 'certs_install.sh')], - env={ 'QUAYCONF': config_provider.get_config_dir_path() }) != 0: + env={ 'QUAYCONF': config_provider.get_config_dir_path() }) != 0: raise Exception('Could not install certificates') return '', 204 diff --git a/config_app/init/certs_install.sh b/config_app/init/certs_install.sh deleted file mode 100755 index 45a08f5ce..000000000 --- a/config_app/init/certs_install.sh +++ /dev/null @@ -1,43 +0,0 @@ -#! /bin/bash -set -e -QUAYPATH=${QUAYPATH:-"."} -QUAYCONF=${QUAYCONF:-"$QUAYPATH/conf/stack"} - -cd ${QUAYDIR:-"/quay-registry"} -pwd - -# Add the custom LDAP certificate -if [ -e $QUAYCONF/ldap.crt ] -then - cp $QUAYCONF/ldap.crt /usr/local/share/ca-certificates/ldap.crt -fi - -# Add extra trusted certificates (as a directory) -if [ -d $QUAYCONF/extra_ca_certs ]; then - if test "$(ls -A "$QUAYCONF/extra_ca_certs")"; then - echo "Installing extra certificates found in $QUAYCONF/extra_ca_certs directory" - cp $QUAYCONF/extra_ca_certs/* /usr/local/share/ca-certificates/ - cat $QUAYCONF/extra_ca_certs/* >> venv/lib/python2.7/site-packages/requests/cacert.pem - cat $QUAYCONF/extra_ca_certs/* >> venv/lib/python2.7/site-packages/certifi/cacert.pem - fi -fi - -# Add extra trusted certificates (as a file) -if [ -f $QUAYCONF/extra_ca_certs ]; then - echo "Installing extra certificates found in $QUAYCONF/extra_ca_certs file" - csplit -z -f /usr/local/share/ca-certificates/extra-ca- $QUAYCONF/extra_ca_certs '/-----BEGIN CERTIFICATE-----/' '{*}' - cat $QUAYCONF/extra_ca_certs >> venv/lib/python2.7/site-packages/requests/cacert.pem - cat $QUAYCONF/extra_ca_certs >> venv/lib/python2.7/site-packages/certifi/cacert.pem -fi - -# Add extra trusted certificates (prefixed) -for f in $(find $QUAYCONF/ -maxdepth 1 -type f -name "extra_ca*") -do - echo "Installing extra cert $f" - cp "$f" /usr/local/share/ca-certificates/ - cat "$f" >> venv/lib/python2.7/site-packages/requests/cacert.pem - cat "$f" >> venv/lib/python2.7/site-packages/certifi/cacert.pem -done - -# Update all CA certificates. -update-ca-certificates diff --git a/data/users/externalldap.py b/data/users/externalldap.py index f2c7e298f..53248be00 100644 --- a/data/users/externalldap.py +++ b/data/users/externalldap.py @@ -205,6 +205,32 @@ class LDAPUsers(FederatedUsers): return (True, None) + def at_least_one_user_exists(self): + logger.debug('Checking if any users exist in LDAP') + try: + with self._ldap.get_connection(): + pass + except ldap.INVALID_CREDENTIALS: + return (None, 'LDAP Admin dn or password is invalid') + + with self._ldap.get_connection() as conn: + for user_search_dn in self._user_dns: + try: + (pairs, err_msg) = conn.search_ext_s(user_search_dn, ldap.SCOPE_SUBTREE) + except Exception as e: + # Catch ldap exceptions to give the user our custom error message + return (False, e.message) + + # if we find any users at all the ldap is valid + if pairs is not None and len(pairs) > 0: + return (True, None) + + if err_msg is not None: + return (None, err_msg) + + return (False, None) + + def get_user(self, username_or_email): """ Looks up a username or email in LDAP. """ logger.debug('Looking up LDAP username or email %s', username_or_email) diff --git a/util/config/validators/validate_ldap.py b/util/config/validators/validate_ldap.py index ec1dafe8d..595487587 100644 --- a/util/config/validators/validate_ldap.py +++ b/util/config/validators/validate_ldap.py @@ -23,7 +23,8 @@ class LDAPValidator(BaseValidator): # 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([os.path.join(init_scripts_location, 'certs_install.sh')]) + subprocess.check_call([os.path.join(init_scripts_location, 'certs_install.sh')], + env={ 'QUAYCONF': config_provider.get_config_dir_path() }) # Note: raises ldap.INVALID_CREDENTIALS on failure admin_dn = config.get('LDAP_ADMIN_DN') @@ -61,10 +62,10 @@ 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.username - (result, err_msg) = users.verify_credentials(username, user_password) + # Ensure at least one user exists to verify the connection is setup properly + (result, err_msg) = users.at_least_one_user_exists() if not result: - msg = ('Verification of superuser %s failed: %s. \n\nThe user either does not exist ' + + msg = ('Verification that users exist failed: %s. \n\nNo users exist ' + 'in the remote authentication system ' + - 'OR LDAP auth is misconfigured.') % (username, err_msg) + 'OR LDAP auth is misconfigured.') % err_msg raise ConfigValidationException(msg) From 1add9925254d1f5d3aeef33caf339909b026cafb Mon Sep 17 00:00:00 2001 From: Sam Chow Date: Thu, 12 Jul 2018 16:53:27 -0400 Subject: [PATCH 4/5] Add ldap tests for verifying a user exists --- test/test_ldap.py | 55 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/test/test_ldap.py b/test/test_ldap.py index 2b71b58ce..9c7232fef 100644 --- a/test/test_ldap.py +++ b/test/test_ldap.py @@ -126,6 +126,14 @@ def mock_ldap(requires_email=True): obj.search_s.seed('ou=otheremployees,dc=quay,dc=io', 2, '(|(uid=unknown*)(mail=unknown*))')([]) + no_users_found_exception = Exception() + no_users_found_exception.message = { 'matched': 'dc=quay,dc=io', 'desc': 'No such object' } + + obj.search_s.seed('ou=nonexistent,dc=quay,dc=io', 2)(no_users_found_exception) + obj.search_s.seed('ou=employees,dc=quay,dc=io', 2)([ + ('uid=cool.user,ou=employees,dc=quay,dc=io', cool_block) + ]) + obj._results = {} def result3(messageid): @@ -161,8 +169,12 @@ def mock_ldap(requires_email=True): obj._results['messageid'] = (None, results, None, [page_control]) return msgid + def search_ext_s(user_search_dn, scope): + return (obj.search_s(user_search_dn, scope), None) + obj.search_ext = search_ext obj.result3 = result3 + obj.search_ext_s = search_ext_s return obj @@ -456,6 +468,49 @@ class TestLDAP(unittest.TestCase): with mock_ldap() as ldap: assert 'base_dn' in ldap.service_metadata() + + def test_at_least_one_user_exists_invalid_creds(self): + base_dn = ['dc=quay', 'dc=io'] + admin_dn = 'uid=testy,ou=employees,dc=quay,dc=io' + admin_passwd = 'INVALIDPASSWORD' + user_rdn = ['ou=employees'] + uid_attr = 'uid' + email_attr = 'mail' + + with mock_ldap(): + ldap = LDAPUsers('ldap://localhost', base_dn, admin_dn, admin_passwd, user_rdn, + uid_attr, email_attr) + + # Try to query with invalid credentials. + (response, err_msg) = ldap.at_least_one_user_exists() + self.assertFalse(response) + self.assertEquals('LDAP Admin dn or password is invalid', err_msg) + + def test_at_least_one_user_exists_no_users(self): + base_dn = ['dc=quay', 'dc=io'] + admin_dn = 'uid=testy,ou=employees,dc=quay,dc=io' + admin_passwd = 'password' + user_rdn = ['ou=nonexistent'] + uid_attr = 'uid' + email_attr = 'mail' + + with mock_ldap(): + ldap = LDAPUsers('ldap://localhost', base_dn, admin_dn, admin_passwd, user_rdn, + uid_attr, email_attr) + + # Try to find users in a nonexistent group. + (response, err_msg) = ldap.at_least_one_user_exists() + self.assertFalse(response) + self.assertDictEqual({'matched': 'dc=quay,dc=io', 'desc': 'No such object'}, err_msg) + + def test_at_least_one_user_exists_true(self): + with mock_ldap() as ldap: + # Ensure we have at least a single user in the valid group + (response, err_msg) = ldap.at_least_one_user_exists() + self.assertIsNone(err_msg) + self.assertTrue(response) + + if __name__ == '__main__': unittest.main() From ba4b10b3860e3639ba33bdcee7740539feda7c7c Mon Sep 17 00:00:00 2001 From: Sam Chow Date: Mon, 16 Jul 2018 11:18:22 -0400 Subject: [PATCH 5/5] Update ldap validation test to match expected behavior --- .../validators/test/test_validate_ldap.py | 21 +++++++++---------- util/config/validators/validate_ldap.py | 2 -- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/util/config/validators/test/test_validate_ldap.py b/util/config/validators/test/test_validate_ldap.py index f5d5a7425..4738e4c26 100644 --- a/util/config/validators/test/test_validate_ldap.py +++ b/util/config/validators/test/test_validate_ldap.py @@ -47,22 +47,21 @@ def test_invalid_uri(uri, app): LDAPValidator.validate(config) -@pytest.mark.parametrize('username, password, expected_exception', [ - ('invaliduser', 'invalidpass', ConfigValidationException), - ('someuser', 'invalidpass', ConfigValidationException), - ('invaliduser', 'somepass', ConfigValidationException), - ('someuser', 'somepass', None), +@pytest.mark.parametrize('admin_dn, admin_passwd, user_rdn, expected_exception', [ + ('uid=testy,ou=employees,dc=quay,dc=io', 'password', ['ou=employees'], None), + ('uid=invalidadmindn', 'password', ['ou=employees'], ConfigValidationException), + ('uid=testy,ou=employees,dc=quay,dc=io', 'invalid_password', ['ou=employees'], ConfigValidationException), + ('uid=testy,ou=employees,dc=quay,dc=io', 'password', ['ou=invalidgroup'], ConfigValidationException), ]) -def test_validated_ldap(username, password, expected_exception, app): +def test_validated_ldap(admin_dn, admin_passwd, user_rdn, expected_exception, app): 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_ADMIN_DN'] = admin_dn + config['LDAP_ADMIN_PASSWD'] = admin_passwd + config['LDAP_USER_RDN'] = user_rdn - unvalidated_config = ValidatorContext(config, user_password=password, config_provider=config_provider) - unvalidated_config.user = AttrDict(dict(username=username)) + unvalidated_config = ValidatorContext(config, config_provider=config_provider) if expected_exception is not None: with pytest.raises(ConfigValidationException): diff --git a/util/config/validators/validate_ldap.py b/util/config/validators/validate_ldap.py index 595487587..7870353d4 100644 --- a/util/config/validators/validate_ldap.py +++ b/util/config/validators/validate_ldap.py @@ -13,8 +13,6 @@ class LDAPValidator(BaseValidator): def validate(cls, validator_context): """ Validates the LDAP connection. """ config = validator_context.config - user = validator_context.user - user_password = validator_context.user_password config_provider = validator_context.config_provider init_scripts_location = validator_context.init_scripts_location