diff --git a/data/model/user.py b/data/model/user.py index 29eb51f85..28620b5a1 100644 --- a/data/model/user.py +++ b/data/model/user.py @@ -305,16 +305,7 @@ def list_entity_robot_permission_teams(entity_name, include_permissions=False): return TupleSelector(query, fields) -def confirm_attached_federated_login(user, service_name): - """ Verifies that the given user has a federated service identity for the specified service. - If none found, a row is added for that service and user. - """ - with db_transaction(): - if not lookup_federated_login(user, service_name): - attach_federated_login(user, service_name, user.username) - - -def create_federated_user(username, email, service_name, service_id, +def create_federated_user(username, email, service_name, service_ident, set_password_notification, metadata={}): if not is_create_user_allowed(): raise TooManyUsersException() @@ -325,7 +316,7 @@ def create_federated_user(username, email, service_name, service_id, service = LoginService.get(LoginService.name == service_name) FederatedLogin.create(user=new_user, service=service, - service_ident=service_id, + service_ident=service_ident, metadata_json=json.dumps(metadata)) if set_password_notification: @@ -334,20 +325,20 @@ def create_federated_user(username, email, service_name, service_id, return new_user -def attach_federated_login(user, service_name, service_id, metadata={}): +def attach_federated_login(user, service_name, service_ident, metadata={}): service = LoginService.get(LoginService.name == service_name) - FederatedLogin.create(user=user, service=service, service_ident=service_id, + FederatedLogin.create(user=user, service=service, service_ident=service_ident, metadata_json=json.dumps(metadata)) return user -def verify_federated_login(service_name, service_id): +def verify_federated_login(service_name, service_ident): try: found = (FederatedLogin .select(FederatedLogin, User) .join(LoginService) .switch(FederatedLogin).join(User) - .where(FederatedLogin.service_ident == service_id, LoginService.name == service_name) + .where(FederatedLogin.service_ident == service_ident, LoginService.name == service_name) .get()) return found.user except FederatedLogin.DoesNotExist: diff --git a/data/users/__init__.py b/data/users/__init__.py index bcdd81912..4c1f2ba4c 100644 --- a/data/users/__init__.py +++ b/data/users/__init__.py @@ -29,6 +29,48 @@ def get_federated_service_name(authentication_type): LDAP_CERT_FILENAME = 'ldap.crt' +def get_users_handler(config, config_provider, override_config_dir): + """ Returns a users handler for the authentication configured in the given config object. """ + authentication_type = config.get('AUTHENTICATION_TYPE', 'Database') + + if authentication_type == 'Database': + return DatabaseUsers() + + if authentication_type == 'LDAP': + ldap_uri = config.get('LDAP_URI', 'ldap://localhost') + base_dn = config.get('LDAP_BASE_DN') + admin_dn = config.get('LDAP_ADMIN_DN') + admin_passwd = config.get('LDAP_ADMIN_PASSWD') + user_rdn = config.get('LDAP_USER_RDN', []) + uid_attr = config.get('LDAP_UID_ATTR', 'uid') + email_attr = config.get('LDAP_EMAIL_ATTR', 'mail') + + allow_tls_fallback = config.get('LDAP_ALLOW_INSECURE_FALLBACK', False) + tls_cert_path = None + if config_provider.volume_file_exists(LDAP_CERT_FILENAME): + with config_provider.get_volume_file(LDAP_CERT_FILENAME) as f: + tls_cert_path = f.name + + return LDAPUsers(ldap_uri, base_dn, admin_dn, admin_passwd, user_rdn, uid_attr, email_attr, + tls_cert_path, allow_tls_fallback) + + if authentication_type == 'JWT': + verify_url = config.get('JWT_VERIFY_ENDPOINT') + issuer = config.get('JWT_AUTH_ISSUER') + max_fresh_s = config.get('JWT_AUTH_MAX_FRESH_S', 300) + return ExternalJWTAuthN(verify_url, issuer, override_config_dir, config['HTTPCLIENT'], + max_fresh_s) + + if authentication_type == 'Keystone': + auth_url = config.get('KEYSTONE_AUTH_URL') + keystone_admin_username = config.get('KEYSTONE_ADMIN_USERNAME') + keystone_admin_password = config.get('KEYSTONE_ADMIN_PASSWORD') + keystone_admin_tenant = config.get('KEYSTONE_ADMIN_TENANT') + return KeystoneUsers(auth_url, keystone_admin_username, keystone_admin_password, + keystone_admin_tenant) + + raise RuntimeError('Unknown authentication type: %s' % authentication_type) + class UserAuthentication(object): def __init__(self, app=None, config_provider=None, override_config_dir=None): self.app_secret_key = None @@ -40,44 +82,7 @@ class UserAuthentication(object): def init_app(self, app, config_provider, override_config_dir): self.app_secret_key = app.config['SECRET_KEY'] - - authentication_type = app.config.get('AUTHENTICATION_TYPE', 'Database') - - if authentication_type == 'Database': - users = DatabaseUsers() - elif authentication_type == 'LDAP': - ldap_uri = app.config.get('LDAP_URI', 'ldap://localhost') - base_dn = app.config.get('LDAP_BASE_DN') - admin_dn = app.config.get('LDAP_ADMIN_DN') - admin_passwd = app.config.get('LDAP_ADMIN_PASSWD') - user_rdn = app.config.get('LDAP_USER_RDN', []) - uid_attr = app.config.get('LDAP_UID_ATTR', 'uid') - email_attr = app.config.get('LDAP_EMAIL_ATTR', 'mail') - - allow_tls_fallback = app.config.get('LDAP_ALLOW_INSECURE_FALLBACK', False) - tls_cert_path = None - if config_provider.volume_file_exists(LDAP_CERT_FILENAME): - with config_provider.get_volume_file(LDAP_CERT_FILENAME) as f: - tls_cert_path = f.name - - users = LDAPUsers(ldap_uri, base_dn, admin_dn, admin_passwd, user_rdn, uid_attr, email_attr, - tls_cert_path, allow_tls_fallback) - - elif authentication_type == 'JWT': - verify_url = app.config.get('JWT_VERIFY_ENDPOINT') - issuer = app.config.get('JWT_AUTH_ISSUER') - max_fresh_s = app.config.get('JWT_AUTH_MAX_FRESH_S', 300) - users = ExternalJWTAuthN(verify_url, issuer, override_config_dir, - app.config['HTTPCLIENT'], max_fresh_s) - elif authentication_type == 'Keystone': - auth_url = app.config.get('KEYSTONE_AUTH_URL') - keystone_admin_username = app.config.get('KEYSTONE_ADMIN_USERNAME') - keystone_admin_password = app.config.get('KEYSTONE_ADMIN_PASSWORD') - keystone_admin_tenant = app.config.get('KEYSTONE_ADMIN_TENANT') - users = KeystoneUsers(auth_url, keystone_admin_username, keystone_admin_password, - keystone_admin_tenant) - else: - raise RuntimeError('Unknown authentication type: %s' % authentication_type) + users = get_users_handler(app.config, config_provider, override_config_dir) # register extension with app app.extensions = getattr(app, 'extensions', {}) diff --git a/endpoints/api/suconfig.py b/endpoints/api/suconfig.py index 5a5cd9346..502c048d4 100644 --- a/endpoints/api/suconfig.py +++ b/endpoints/api/suconfig.py @@ -9,7 +9,7 @@ from endpoints.api import (ApiResource, nickname, resource, internal_only, show_ require_fresh_login, request, validate_json_request, verify_not_prod) from endpoints.common import common_login -from app import app, config_provider, superusers +from app import app, config_provider, superusers, OVERRIDE_CONFIG_DIRECTORY from data import model from data.database import configure from auth.permissions import SuperUserPermission @@ -19,7 +19,7 @@ from util.config.configutil import add_enterprise_config_defaults from util.config.database import sync_database_with_config from util.config.validator import validate_service_for_config, CONFIG_FILENAMES from data.runmigration import run_alembic_migration -from data.users import get_federated_service_name +from data.users import get_federated_service_name, get_users_handler import features @@ -175,7 +175,10 @@ class SuperUserConfig(ApiResource): }, 'hostname': { 'type': 'string' - } + }, + 'password': { + 'type': 'string' + }, }, }, } @@ -213,9 +216,22 @@ class SuperUserConfig(ApiResource): # If the authentication system is not the database, link the superuser account to the # the authentication system chosen. if config_object.get('AUTHENTICATION_TYPE', 'Database') != 'Database': - service_name = get_federated_service_name(config_object['AUTHENTICATION_TYPE']) current_user = get_authenticated_user() - model.user.confirm_attached_federated_login(current_user, service_name) + if current_user is None: + abort(401) + + service_name = get_federated_service_name(config_object['AUTHENTICATION_TYPE']) + if not model.user.lookup_federated_login(current_user, 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.user.attach_federated_login(current_user, service_name, result.username) # Ensure database is up-to-date with config sync_database_with_config(config_object) diff --git a/static/directives/config/config-setup-tool.html b/static/directives/config/config-setup-tool.html index 0f46a77eb..8e07f1521 100644 --- a/static/directives/config/config-setup-tool.html +++ b/static/directives/config/config-setup-tool.html @@ -1022,7 +1022,12 @@ diff --git a/static/js/core-config-setup.js b/static/js/core-config-setup.js index 384e8c878..51ba81a48 100644 --- a/static/js/core-config-setup.js +++ b/static/js/core-config-setup.js @@ -11,6 +11,8 @@ angular.module("core-config-setup", ['angularFileUpload']) 'configurationSaved': '&configurationSaved' }, controller: function($rootScope, $scope, $element, $timeout, ApiService) { + var authPassword = null; + $scope.HOSTNAME_REGEX = '^[a-zA-Z-0-9\.]+(:[0-9]+)?$'; $scope.GITHOST_REGEX = '^https?://([a-zA-Z0-9]+\.?\/?)+$'; @@ -188,10 +190,21 @@ angular.module("core-config-setup", ['angularFileUpload']) 'password': opt_password || '' }; + var errorDisplay = ApiService.errorDisplay( + 'Could not validate configuration. Please report this error.', + function() { + authPassword = null; + }); + ApiService.scValidateConfig(data, params).then(function(resp) { serviceInfo.status = resp.status ? 'success' : 'error'; serviceInfo.errorMessage = $.trim(resp.reason || ''); - }, ApiService.errorDisplay('Could not validate configuration. Please report this error.')); + + if (!resp.status) { + authPassword = null; + } + + }, errorDisplay); }; $scope.checkValidateAndSave = function() { @@ -262,6 +275,8 @@ angular.module("core-config-setup", ['angularFileUpload']) $scope.savingConfiguration = false; $scope.validating = $scope.getServices($scope.config); + authPassword = opt_password; + $('#validateAndSaveModal').modal({ keyboard: false, backdrop: 'static' @@ -282,15 +297,26 @@ angular.module("core-config-setup", ['angularFileUpload']) var data = { 'config': $scope.config, - 'hostname': window.location.host + 'hostname': window.location.host, + 'password': authPassword || '' }; + var errorDisplay = ApiService.errorDisplay( + 'Could not save configuration. Please report this error.', + function() { + authPassword = null; + }); + ApiService.scUpdateConfig(data).then(function(resp) { + authPassword = null; + $scope.savingConfiguration = false; $scope.mapped.$hasChanges = false; + $('#validateAndSaveModal').modal('hide'); + $scope.configurationSaved({'config': $scope.config}); - }, ApiService.errorDisplay('Could not save configuration. Please report this error.')); + }, errorDisplay); }; // Convert storage config to an array diff --git a/test/test_api_usage.py b/test/test_api_usage.py index 2aefabd02..b3c5f3a87 100644 --- a/test/test_api_usage.py +++ b/test/test_api_usage.py @@ -15,6 +15,7 @@ from playhouse.test_utils import assert_query_count, _QueryLogHandler from httmock import urlmatch, HTTMock from cryptography.hazmat.primitives import serialization from cryptography.hazmat.backends import default_backend +from mockldap import MockLdap from endpoints.api import api_bp, api from endpoints.building import PreparedBuild @@ -3523,6 +3524,51 @@ class TestSuperUserConfig(ApiTestCase): json = self.getJsonResponse(SuperUserConfigFile, params=dict(filename='ssl.cert')) self.assertTrue(json['exists']) + def test_update_with_external_auth(self): + self.login(ADMIN_ACCESS_USER) + + # Run a mock LDAP. + mockldap = MockLdap({ + 'dc=quay,dc=io': {'dc': ['quay', 'io']}, + 'ou=employees,dc=quay,dc=io': { + 'dc': ['quay', 'io'], + 'ou': 'employees' + }, + 'uid=' + ADMIN_ACCESS_USER + ',ou=employees,dc=quay,dc=io': { + 'dc': ['quay', 'io'], + 'ou': 'employees', + 'uid': [ADMIN_ACCESS_USER], + 'userPassword': ['password'], + 'mail': [ADMIN_ACCESS_EMAIL], + }, + }) + + config = { + 'AUTHENTICATION_TYPE': 'LDAP', + 'LDAP_BASE_DN': ['dc=quay', 'dc=io'], + 'LDAP_ADMIN_DN': 'uid=devtable,ou=employees,dc=quay,dc=io', + 'LDAP_ADMIN_PASSWD': 'password', + 'LDAP_USER_RDN': ['ou=employees'], + 'LDAP_UID_ATTR': 'uid', + 'LDAP_EMAIL_ATTR': 'mail', + } + + mockldap.start() + try: + # Try writing some config with an invalid password. + self.putResponse(SuperUserConfig, data={'config': config, 'hostname': 'foo'}, expected_code=400) + self.putResponse(SuperUserConfig, + data={'config': config, 'password': 'invalid', 'hostname': 'foo'}, expected_code=400) + + # Write the config with the valid password. + self.putResponse(SuperUserConfig, + data={'config': config, 'password': 'password', 'hostname': 'foo'}, expected_code=200) + + # Ensure that the user row has been linked. + self.assertEquals(ADMIN_ACCESS_USER, model.user.verify_federated_login('ldap', ADMIN_ACCESS_USER).username) + finally: + mockldap.stop() + @urlmatch(netloc=r'(.*\.)?mockclairservice', path=r'/v1/layers/(.+)')