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/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 ea118d3b5..a1e25304a 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 = '/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 9e17701ab..d83ad88eb 100644 --- a/config_app/config_endpoints/api/suconfig.py +++ b/config_app/config_endpoints/api/suconfig.py @@ -4,10 +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 +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 @@ -74,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 @@ -275,7 +253,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 34bb9fba9..7eceac565 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 @@ -10,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__) @@ -48,14 +49,10 @@ 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 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([os.path.join(INIT_SCRIPTS_LOCATION, 'certs_install.sh')], + env={ 'QUAYCONF': config_provider.get_config_dir_path() }) != 0: + raise Exception('Could not install certificates') return '', 204 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/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/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() 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/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 331cd87d3..7870353d4 100644 --- a/util/config/validators/validate_ldap.py +++ b/util/config/validators/validate_ldap.py @@ -13,16 +13,16 @@ 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 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')], + env={ 'QUAYCONF': config_provider.get_config_dir_path() }) # Note: raises ldap.INVALID_CREDENTIALS on failure admin_dn = config.get('LDAP_ADMIN_DN') @@ -60,10 +60,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)