Fix setup tool when binding to external auth
We now query the external auth provider for the external service's identifier before adding the linking row into the database. This fixes the case where the external service resolves a different identifier for the same username. Fixes #1477
This commit is contained in:
Joseph Schorr
parent
d6b73a41de
commit
60bbca2185
6 changed files with 151 additions and 62 deletions
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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', {})
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -1022,7 +1022,12 @@
|
|||
<button class="btn btn-warning" ng-click="checkValidateAndSave()" ng-show="!configform.$valid"
|
||||
ng-click="checkValidateAndSave()">
|
||||
<i class="fa fa-lg fa-sort"></i>
|
||||
{{ configform.$error['required'].length }} configuration field<span ng-show="configform.$error['required'].length != 1">s</span> remaining
|
||||
<span ng-if="configform.$error['required'].length">
|
||||
{{ configform.$error['required'].length }} configuration field<span ng-show="configform.$error['required'].length != 1">s</span> remaining
|
||||
</span>
|
||||
<span ng-if="!configform.$error['required'].length">
|
||||
Invalid configuration field
|
||||
</span>
|
||||
</button>
|
||||
</div>
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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/(.+)')
|
||||
|
|
|
|||
Reference in a new issue