Merge pull request #1483 from coreos-inc/superuser-external-user

Fix setup tool when binding to external auth

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):
+def create_federated_user(username, email, service_name, service_ident,
-  """ 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,
                           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:

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']
-
+    users = get_users_handler(app.config, config_provider, override_config_dir)
-    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)
 
     # register extension with app
     app.extensions = getattr(app, 'extensions', {})

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)

static/directives/config/config-setup-tool.html

@@ -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>
 

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

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/(.+)')