From f5a8116f5a870327dad1afdae387ecdbc2cb3824 Mon Sep 17 00:00:00 2001
From: Sam Chow <schow@redhat.com>
Date: Tue, 17 Jul 2018 10:57:56 -0400
Subject: [PATCH] Remove password confirmation in config app
Small fix to manually clean up temp dir when creating new temp dir,
small fix to font awesome icons, change the jwt/keystone
validators to not use username/password
---
.../config/TransientDirectoryProvider.py | 1 +
.../js/core-config-setup/core-config-setup.js | 51 ++-----------------
.../static/css/config-setup-app-component.css | 2 +
data/users/externaljwt.py | 1 +
data/users/keystone.py | 31 +++++++++++
test/test_keystone_auth.py | 5 ++
.../validators/test/test_validate_keystone.py | 17 +++----
util/config/validators/validate_jwt.py | 37 ++------------
util/config/validators/validate_keystone.py | 12 ++---
9 files changed, 61 insertions(+), 96 deletions(-)
diff --git a/config_app/config_util/config/TransientDirectoryProvider.py b/config_app/config_util/config/TransientDirectoryProvider.py
index 3737ec732..9c96bedf6 100644
--- a/config_app/config_util/config/TransientDirectoryProvider.py
+++ b/config_app/config_util/config/TransientDirectoryProvider.py
@@ -24,6 +24,7 @@ class TransientDirectoryProvider(FileConfigProvider):
"""
Update the path with a new temporary directory, deleting the old one in the process
"""
+ self.temp_dir.cleanup()
temp_dir = TemporaryDirectory()
self.config_volume = temp_dir.name
diff --git a/config_app/js/core-config-setup/core-config-setup.js b/config_app/js/core-config-setup/core-config-setup.js
index 73841712a..425c2a407 100644
--- a/config_app/js/core-config-setup/core-config-setup.js
+++ b/config_app/js/core-config-setup/core-config-setup.js
@@ -51,15 +51,15 @@ angular.module("quay-config")
{'id': 'ldap', 'title': 'LDAP Authentication', 'condition': function(config) {
return config.AUTHENTICATION_TYPE == 'LDAP';
- }, 'password': true},
+ }},
{'id': 'jwt', 'title': 'JWT Authentication', 'condition': function(config) {
return config.AUTHENTICATION_TYPE == 'JWT';
- }, 'password': true},
+ }},
{'id': 'keystone', 'title': 'Keystone Authentication', 'condition': function(config) {
return config.AUTHENTICATION_TYPE == 'Keystone';
- }, 'password': true},
+ }},
{'id': 'apptoken-auth', 'title': 'App Token Authentication', 'condition': function(config) {
return config.AUTHENTICATION_TYPE == 'AppToken';
@@ -345,50 +345,7 @@ angular.module("quay-config")
$scope.validateAndSave = function() {
$scope.validating = $scope.getServices($scope.config);
- var requirePassword = false;
- for (var i = 0; i < $scope.validating.length; ++i) {
- var serviceInfo = $scope.validating[i];
- if (serviceInfo.service.password) {
- requirePassword = true;
- break;
- }
- }
-
- if (!requirePassword) {
- $scope.performValidateAndSave();
- return;
- }
-
- var box = bootbox.dialog({
- "message": 'Please enter your superuser password to validate your auth configuration:' +
- '<form style="margin-top: 10px" action="javascript:$(\'.btn-continue\').click()">' +
- '<input id="validatePassword" class="form-control" type="password" placeholder="Password">' +
- '</form>',
- "title": 'Enter Password',
- "buttons": {
- "success": {
- "label": "Validate Config",
- "className": "btn-success btn-continue",
- "callback": function() {
- $scope.performValidateAndSave($('#validatePassword').val());
- }
- },
- "close": {
- "label": "Cancel",
- "className": "btn-default",
- "callback": function() {
- }
- }
- }
- });
-
- box.bind('shown.bs.modal', function(){
- box.find("input").focus();
- box.find("form").submit(function() {
- if (!$('#validatePassword').val()) { return; }
- box.modal('hide');
- });
- });
+ $scope.performValidateAndSave();
};
$scope.performValidateAndSave = function(opt_password) {
diff --git a/config_app/static/css/config-setup-app-component.css b/config_app/static/css/config-setup-app-component.css
index 453a08019..a70bc0ab1 100644
--- a/config_app/static/css/config-setup-app-component.css
+++ b/config_app/static/css/config-setup-app-component.css
@@ -29,8 +29,10 @@
/* Fixes the transition to font awesome 5 */
.quay-config-app .co-alert.co-alert-warning::before {
font-family: Font Awesome\ 5 Free;
+ font-weight: 900;
}
.quay-config-app .co-alert.co-alert-info::before {
font-family: Font Awesome\ 5 Free;
+ font-weight: 900;
}
diff --git a/data/users/externaljwt.py b/data/users/externaljwt.py
index 64960a08b..da4d003db 100644
--- a/data/users/externaljwt.py
+++ b/data/users/externaljwt.py
@@ -39,6 +39,7 @@ class ExternalJWTAuthN(FederatedUsers):
def ping(self):
result = self.client.get(self.getuser_url, timeout=2)
+ # We expect a 401 or 403 of some kind, since we explicitly don't send an auth header
if result.status_code // 100 != 4:
return (False, result.text or 'Could not reach JWT authn endpoint')
diff --git a/data/users/keystone.py b/data/users/keystone.py
index d6fa3c628..21a7e6ec5 100644
--- a/data/users/keystone.py
+++ b/data/users/keystone.py
@@ -51,6 +51,24 @@ class KeystoneV2Users(FederatedUsers):
return (True, None)
+ def at_least_one_user_exists(self):
+ logger.debug('Checking if any users exist in Keystone')
+ try:
+ keystone_client = kclient.Client(username=self.admin_username, password=self.admin_password,
+ tenant_name=self.admin_tenant, auth_url=self.auth_url,
+ timeout=self.timeout, debug=self.debug)
+
+ user_list = keystone_client.users.list(tenant_id=self.admin_tenant, limit=1)
+ if len(user_list) < 1:
+ return (False, None)
+
+ return (True, None)
+ except Exception as e:
+ # Catch exceptions to give the user our custom error message
+ logger.exception('Unable to list users in Keystone')
+ return (False, e.message)
+
+
def verify_credentials(self, username_or_email, password):
try:
keystone_client = kclient.Client(username=username_or_email, password=password,
@@ -116,6 +134,19 @@ class KeystoneV3Users(FederatedUsers):
return (True, None)
+ def at_least_one_user_exists(self):
+ logger.debug('Checking if any users exist in admin tenant in Keystone')
+ try:
+ user_list = self._get_admin_client().users.list(self.admin_tenant, limit=1)
+ if len(user_list) < 1:
+ return (False, 'No users found in tenant: %s' % self.admin_tenant)
+
+ return (True, None)
+ except Exception as e:
+ # Catch exceptions to give the user our custom error message
+ logger.exception('Unable to list users in Keystone')
+ return (False, e.message)
+
def verify_credentials(self, username_or_email, password):
try:
keystone_client = kv3client.Client(username=username_or_email, password=password,
diff --git a/test/test_keystone_auth.py b/test/test_keystone_auth.py
index 4d70a4c1b..099d0e64b 100644
--- a/test/test_keystone_auth.py
+++ b/test/test_keystone_auth.py
@@ -83,6 +83,11 @@ def _create_app(requires_email=True):
abort(404)
+ # v2 referred to all groups as tenants, so replace occurrences of 'group' with 'tenant'
+ @ks_app.route('/v2.0/admin/tenants/<tenant>/users', methods=['GET'])
+ def getv2_tenant_members(tenant):
+ return getv3groupmembers(tenant)
+
@ks_app.route('/v3/identity/groups/<groupid>/users', methods=['GET'])
def getv3groupmembers(groupid):
for group in groups:
diff --git a/util/config/validators/test/test_validate_keystone.py b/util/config/validators/test/test_validate_keystone.py
index f6d8cf0c2..f3776a0b7 100644
--- a/util/config/validators/test/test_validate_keystone.py
+++ b/util/config/validators/test/test_validate_keystone.py
@@ -3,7 +3,6 @@ import pytest
from util.config.validator import ValidatorContext
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
@@ -29,13 +28,13 @@ def test_invalid_config(unvalidated_config, app):
KeystoneValidator.validate(ValidatorContext(unvalidated_config))
-@pytest.mark.parametrize('username, password, expected_exception', [
- ('invaliduser', 'invalidpass', ConfigValidationException),
- ('cool.user', 'invalidpass', ConfigValidationException),
- ('invaliduser', 'somepass', ConfigValidationException),
- ('cool.user', 'password', None),
+@pytest.mark.parametrize('admin_tenant_id, expected_exception', [
+ ('somegroupid', None),
+ ('groupwithnousers', ConfigValidationException),
+ ('somegroupid', None),
+ ('groupwithnousers', ConfigValidationException),
])
-def test_validated_keystone(username, password, expected_exception, app):
+def test_validated_keystone(admin_tenant_id, expected_exception, app):
with fake_keystone(2) as keystone_auth:
auth_url = keystone_auth.auth_url
@@ -44,11 +43,9 @@ def test_validated_keystone(username, password, expected_exception, app):
config['KEYSTONE_AUTH_URL'] = auth_url
config['KEYSTONE_ADMIN_USERNAME'] = 'adminuser'
config['KEYSTONE_ADMIN_PASSWORD'] = 'adminpass'
- config['KEYSTONE_ADMIN_TENANT'] = 'admintenant'
+ config['KEYSTONE_ADMIN_TENANT'] = admin_tenant_id
unvalidated_config = ValidatorContext(config)
- unvalidated_config.user = AttrDict(dict(username=username))
- unvalidated_config.user_password = password
if expected_exception is not None:
with pytest.raises(ConfigValidationException):
diff --git a/util/config/validators/validate_jwt.py b/util/config/validators/validate_jwt.py
index c56f3630b..d7b5e36f5 100644
--- a/util/config/validators/validate_jwt.py
+++ b/util/config/validators/validate_jwt.py
@@ -9,8 +9,6 @@ class JWTAuthValidator(BaseValidator):
def validate(cls, validator_context, public_key_path=None):
""" Validates the JWT authentication system. """
config = validator_context.config
- user = validator_context.user
- user_password = validator_context.user_password
http_client = validator_context.http_client
jwt_auth_max = validator_context.jwt_auth_max
config_provider = validator_context.config_provider
@@ -31,10 +29,7 @@ class JWTAuthValidator(BaseValidator):
raise ConfigValidationException('Missing JWT Issuer ID')
- # TODO(jschorr): fix this
- return
-
- override_config_directory = os.path.join(config_provider.get_config_root(), '../stack/')
+ override_config_directory = config_provider.get_config_dir_path()
# Try to instatiate the JWT authentication mechanism. This will raise an exception if
# the key cannot be found.
@@ -45,31 +40,9 @@ class JWTAuthValidator(BaseValidator):
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)
+ # Verify that we can reach the jwt server
+ (result, err_msg) = users.ping()
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)
+ msg = ('Verification of JWT failed: %s. \n\nWe cannot reach the JWT server' +
+ 'OR JWT auth is misconfigured') % 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)
diff --git a/util/config/validators/validate_keystone.py b/util/config/validators/validate_keystone.py
index 2d4f7bbe0..c9dcc22c8 100644
--- a/util/config/validators/validate_keystone.py
+++ b/util/config/validators/validate_keystone.py
@@ -8,8 +8,6 @@ class KeystoneValidator(BaseValidator):
def validate(cls, validator_context):
""" Validates the Keystone authentication system. """
config = validator_context.config
- user = validator_context.user
- user_password = validator_context.user_password
if config.get('AUTHENTICATION_TYPE', 'Database') != 'Keystone':
return
@@ -37,10 +35,10 @@ class KeystoneValidator(BaseValidator):
requires_email)
# Verify that the superuser exists. If not, raise an exception.
- username = user.username
- (result, err_msg) = users.verify_credentials(username, user_password)
+ (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 in the remote authentication system ' +
- 'OR Keystone auth is misconfigured.') % (username, err_msg)
+ msg = ('Verification that users exist failed: %s. \n\nNo users exist ' +
+ 'in the admin tenant/project ' +
+ 'in the remote authentication system ' +
+ 'OR Keystone auth is misconfigured.') % err_msg
raise ConfigValidationException(msg)