Allow OAuth and OIDC login engines to bind to fields in internal auth

This feature is subtle but very important: Currently, when a user logs in via an "external" auth system (such as Github), they are either logged into an existing bound account or a new account is created for them in the database. While this normally works jut fine, it hits a roadblock when the *internal* auth system configured is not the database, but instead something like LDAP. In that case, *most* Enterprise customers will prefer that logging in via external auth (like OIDC) will also *automatically* bind the newly created account to the backing *internal* auth account. For example, login via PingFederate OIDC (backed by LDAP) should also bind the new QE account to the associated LDAP account, via either username or email. This change allows for this binding field to be specified, and thereafter will perform the proper lookups and bindings.

endpoints/api/test/conftest.py

@@ -11,8 +11,10 @@ from data import model
 from data.database import (close_db_filter, db)
 from data.model.user import LoginWrappedDBUser
 from endpoints.api import api_bp
+from endpoints.web import web
+
 from initdb import initialize_database, populate_database
-from path_converters import APIRepositoryPathConverter, RegexConverter
+from path_converters import APIRepositoryPathConverter, RegexConverter, RepositoryPathConverter
 
 
 @pytest.fixture()
@@ -33,7 +35,9 @@ def app(appconfig):
 
   app.url_map.converters['regex'] = RegexConverter
   app.url_map.converters['apirepopath'] = APIRepositoryPathConverter
+  app.url_map.converters['repopath'] = RepositoryPathConverter
   app.register_blueprint(api_bp, url_prefix='/api')
+  app.register_blueprint(web, url_prefix='/')
   app.config.update(appconfig)
   return app
 

endpoints/oauthlogin.py

@@ -1,11 +1,12 @@
 import logging
 
+from collections import namedtuple
 from flask import request, redirect, url_for, Blueprint
 from peewee import IntegrityError
 
 import features
 
-from app import app, analytics, get_app_url, oauth_login
+from app import app, analytics, get_app_url, oauth_login, authentication
 from auth.auth_context import get_authenticated_user
 from auth.process import require_session_login
 from data import model
@@ -21,6 +22,100 @@ oauthlogin = Blueprint('oauthlogin', __name__)
 
 oauthlogin_csrf_protect = csrf_protect(OAUTH_CSRF_TOKEN_NAME, 'state', all_methods=True)
 
+
+OAuthResult = namedtuple('oauthresult', ['to_login', 'service_name', 'error_message',
+                                         'register_redirect'])
+
+def _oauthresult(to_login=None, service_name=None, error_message=None, register_redirect=False):
+  return OAuthResult(to_login, service_name, error_message, register_redirect)
+
+def _get_response(result):
+  if result.error_message is not None:
+    return _render_ologin_error(result.service_name, result.error_message, result.register_redirect)
+
+  return _perform_login(result.to_login, result.service_name)
+
+def _conduct_oauth_login(auth_system, login_service, lid, lusername, lemail, metadata=None):
+  """ Conducts login from the result of an OAuth service's login flow and returns
+      the status of the login, as well as the followup step. """
+  service_id = login_service.service_id()
+  service_name = login_service.service_name()
+
+  # Check for an existing account *bound to this service*. If found, conduct login of that account
+  # and redirect.
+  to_login = model.user.verify_federated_login(service_id, lid)
+  if to_login is not None:
+    return _oauthresult(to_login=to_login, service_name=service_name)
+
+  # If the login service has a bound field name, and we have a defined internal auth type that is
+  # not the database, then search for an existing account with that matching field. This allows
+  # users to setup SSO while also being backed by something like LDAP.
+  bound_field_name = login_service.login_binding_field()
+  if auth_system.federated_service is not None and bound_field_name is not None:
+    # Perform lookup.
+    logger.debug('Got oauth bind field name of "%s"', bound_field_name)
+    lookup_value = None
+    if bound_field_name == 'username':
+      lookup_value = lusername
+    elif bound_field_name == 'email':
+      lookup_value = lemail
+
+    if lookup_value is None:
+      logger.error('Missing lookup value for OAuth login')
+      return _oauthresult(service_name=service_name,
+                          error_message='Configuration error in this provider')
+
+    (user_obj, err) = auth_system.link_user(lookup_value)
+    if err is not None:
+      logger.debug('%s %s not found: %s', bound_field_name, lookup_value, err)
+      msg = '%s %s not found in backing auth system' % (bound_field_name, lookup_value)
+      return _oauthresult(service_name=service_name, error_message=msg)
+
+    # Found an existing user. Bind their internal auth account to this service as well.
+    result = _attach_service(login_service, user_obj, lid, lusername)
+    if result.error_message is not None:
+      return result
+
+    return _oauthresult(to_login=user_obj, service_name=service_name)
+
+  # Otherwise, we need to create a new user account.
+  if not features.USER_CREATION:
+    error_message = 'User creation is disabled. Please contact your administrator'
+    return _oauthresult(service_name=service_name, error_message=error_message)
+
+  # Try to create the user
+  try:
+    # Generate a valid username.
+    new_username = None
+    for valid in generate_valid_usernames(lusername):
+      if model.user.get_user_or_org(valid):
+        continue
+
+      new_username = valid
+      break
+
+    prompts = model.user.get_default_user_prompts(features)
+    to_login = model.user.create_federated_user(new_username, lemail, service_id, lid,
+                                                set_password_notification=True,
+                                                metadata=metadata or {},
+                                                prompts=prompts,
+                                                email_required=features.MAILING)
+
+    # Success, tell analytics
+    analytics.track(to_login.username, 'register', {'service': service_name.lower()})
+    return _oauthresult(to_login=to_login, service_name=service_name)
+
+  except model.InvalidEmailAddressException:
+    message = "The e-mail address %s is already associated " % (lemail, )
+    message = message + "with an existing %s account." % (app.config['REGISTRY_TITLE_SHORT'], )
+    message = message + "\nPlease log in with your username and password and "
+    message = message + "associate your %s account to use it in the future." % (service_name, )
+    return _oauthresult(service_name=service_name, error_message=message,
+                        register_redirect=True)
+
+  except model.DataModelException as ex:
+    return _oauthresult(service_name=service_name, error_message=ex.message)
+
 def _render_ologin_error(service_name, error_message=None, register_redirect=False):
   """ Returns a Flask response indicating an OAuth error. """
 
@@ -38,56 +133,33 @@ def _render_ologin_error(service_name, error_message=None, register_redirect=Fal
   resp.status_code = 400
   return resp
 
-def _conduct_oauth_login(service_id, service_name, user_id, username, email, metadata=None):
+def _perform_login(to_login, service_name):
-  """ Conducts login from the result of an OAuth service's login flow. """
+  """ Attempts to login the given user, returning the Flask result of whether the login succeeded.
-
+  """
-  to_login = model.user.verify_federated_login(service_id, user_id)
-  if not to_login:
-    # See if we can create a new user.
-    if not features.USER_CREATION:
-      error_message = 'User creation is disabled. Please contact your administrator'
-      return _render_ologin_error(service_name, error_message)
-
-    # Try to create the user
-    try:
-      # Generate a valid username.
-      new_username = None
-      for valid in generate_valid_usernames(username):
-        if model.user.get_user_or_org(valid):
-          continue
-
-        new_username = valid
-        break
-
-      prompts = model.user.get_default_user_prompts(features)
-      to_login = model.user.create_federated_user(new_username, email, service_id,
-                                                  user_id, set_password_notification=True,
-                                                  metadata=metadata or {},
-                                                  prompts=prompts,
-                                                  email_required=features.MAILING)
-
-      # Success, tell analytics
-      analytics.track(to_login.username, 'register', {'service': service_name.lower()})
-
-    except model.InvalidEmailAddressException:
-      message = "The e-mail address %s is already associated " % (email, )
-      message = message + "with an existing %s account." % (app.config['REGISTRY_TITLE_SHORT'], )
-      message = message + "\nPlease log in with your username and password and "
-      message = message + "associate your %s account to use it in the future." % (service_name, )
-
-      return _render_ologin_error(service_name, message, register_redirect=True)
-
-    except model.DataModelException as ex:
-      return _render_ologin_error(service_name, ex.message)
-
   if common_login(to_login):
     if model.user.has_user_prompts(to_login):
       return redirect(url_for('web.updateuser'))
     else:
       return redirect(url_for('web.index'))
+  else:
+    return _render_ologin_error(service_name, 'Could not login. Account may be disabled')
 
-  return _render_ologin_error(service_name)
+def _attach_service(login_service, user_obj, lid, lusername):
+  """ Attaches the given user account to the given service, with the given service user ID and
+      service username.
+  """
+  metadata = {
+    'service_username': lusername
+  }
 
+  try:
+    model.user.attach_federated_login(user_obj, login_service.service_id(), lid,
+                                      metadata=metadata)
+    return _oauthresult(to_login=user_obj)
+  except IntegrityError:
+    err = '%s account %s is already attached to a %s account' % (
+      login_service.service_name(), lusername, app.config['REGISTRY_TITLE_SHORT'])
+    return _oauthresult(service_name=login_service.service_name(), error_message=err)
 
 def _register_service(login_service):
   """ Registers the given login service, adding its callback and attach routes to the blueprint. """
@@ -112,8 +184,9 @@ def _register_service(login_service):
       'service_username': lusername
     }
 
-    return _conduct_oauth_login(login_service.service_id(), login_service.service_name(), lid,
+    result = _conduct_oauth_login(authentication, login_service, lid, lusername, lemail,
-                                lusername, lemail, metadata=metadata)
+                                  metadata=metadata)
+    return _get_response(result)
 
 
   @require_session_login
@@ -132,19 +205,10 @@ def _register_service(login_service):
       return _render_ologin_error(login_service.service_name(), ole.message)
 
     # Conduct attach.
-    metadata = {
-      'service_username': lusername
-    }
-
     user_obj = get_authenticated_user()
-
+    result = _attach_service(login_service, user_obj, lid, lusername)
-    try:
+    if result.error_message is not None:
-      model.user.attach_federated_login(user_obj, login_service.service_id(), lid,
+      return _get_response(result)
-                                        metadata=metadata)
-    except IntegrityError:
-      err = '%s account %s is already attached to a %s account' % (
-        login_service.service_name(), lusername, app.config['REGISTRY_TITLE_SHORT'])
-      return _render_ologin_error(login_service.service_name(), err)
 
     return redirect(url_for('web.user_view', path=user_obj.username, tab='external'))
 

endpoints/test/test_oauthlogin.py

@@ -0,0 +1,177 @@
+import pytest
+
+from endpoints.oauthlogin import _conduct_oauth_login
+from endpoints.api.test.conftest import app, appconfig, database_uri, init_db_path, sqlitedb_file
+
+from oauth.services.github import GithubOAuthService
+
+from data import model, database
+from data.users import get_users_handler, DatabaseUsers
+from test.test_ldap import mock_ldap
+
+
+@pytest.fixture(params=[None, 'username', 'email'])
+def login_service(request, app):
+  config = {'GITHUB': {}}
+  if request is not None:
+    config['GITHUB']['LOGIN_BINDING_FIELD'] = request.param
+
+  return GithubOAuthService(config, 'GITHUB')
+
+
+@pytest.fixture(params=['Database', 'LDAP'])
+def auth_system(request):
+  return _get_users_handler(request.param)
+
+def _get_users_handler(auth_type):
+  config = {}
+  config['AUTHENTICATION_TYPE'] = auth_type
+  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']
+
+  return get_users_handler(config, None, None)
+
+def test_existing_account(auth_system, login_service):
+  login_service_lid = 'someexternaluser'
+
+  # Create an existing bound federated user.
+  created_user = model.user.create_federated_user('someuser', 'example@example.com',
+                                                  login_service.service_id(),
+                                                  login_service_lid, False)
+  existing_user_count = database.User.select().count()
+
+  with mock_ldap():
+    result = _conduct_oauth_login(auth_system, login_service,
+                                  login_service_lid, login_service_lid,
+                                  'example@example.com')
+
+    assert result.to_login == created_user
+
+    # Ensure that no addtional users were created.
+    current_user_count = database.User.select().count()
+    assert current_user_count == existing_user_count
+
+
+def test_new_account_via_database(login_service):
+  existing_user_count = database.User.select().count()
+  login_service_lid = 'someexternaluser'
+  internal_auth = DatabaseUsers()
+
+  # Conduct login. Since the external user doesn't (yet) bind to a user in the database,
+  # a new user should be created and bound to the external service.
+  result = _conduct_oauth_login(internal_auth, login_service, login_service_lid, login_service_lid,
+                                'example@example.com')
+  assert result.to_login is not None
+
+  current_user_count = database.User.select().count()
+  assert current_user_count == existing_user_count + 1
+
+  # Find the user and ensure it is bound.
+  new_user = model.user.get_user(login_service_lid)
+  federated_login = model.user.lookup_federated_login(new_user, login_service.service_id())
+  assert federated_login is not None
+
+
+@pytest.mark.parametrize('binding_field, lusername, lemail, expected_error', [
+  # No binding field + newly seen user -> New unlinked user
+  (None, 'someunknownuser', 'someemail@example.com', None),
+
+  # username binding field + unknown username -> Error.
+  ('username', 'someunknownuser', 'foo@bar.com',
+   'username someunknownuser not found in backing auth system'),
+
+  # email binding field + unknown email address -> Error.
+  ('email', 'someuser', 'someemail@example.com',
+   'email someemail@example.com not found in backing auth system'),
+
+  # No binding field + newly seen user -> New unlinked user.
+  (None, 'someuser', 'foo@bar.com', None),
+
+  # username binding field + valid username -> fully bound user.
+  ('username', 'someuser', 'foo@bar.com', None),
+
+  # email binding field + valid email -> fully bound user.
+  ('email', 'someuser', 'foo@bar.com', None),
+
+  # username binding field + valid username + invalid email -> fully bound user.
+  ('username', 'someuser', 'another@email.com', None),
+
+  # email binding field + valid email + invalid username -> fully bound user.
+  ('email', 'someotherusername', 'foo@bar.com', None),
+])
+def test_new_account_via_ldap(binding_field, lusername, lemail, expected_error, app):
+  existing_user_count = database.User.select().count()
+
+  config = {'GITHUB': {}}
+  if binding_field is not None:
+    config['GITHUB']['LOGIN_BINDING_FIELD'] = binding_field
+
+  external_auth = GithubOAuthService(config, 'GITHUB')
+  internal_auth = _get_users_handler('LDAP')
+
+  with mock_ldap():
+    # Conduct OAuth login.
+    result = _conduct_oauth_login(internal_auth, external_auth, 'someid', lusername, lemail)
+    assert result.error_message == expected_error
+
+    current_user_count = database.User.select().count()
+    if expected_error is None:
+      # Ensure that the new user was created and that it is bound to both the
+      # external login service and to LDAP (if a binding_field was given).
+      assert current_user_count == existing_user_count + 1
+      assert result.to_login is not None
+
+      # Check the service bindings.
+      external_login = model.user.lookup_federated_login(result.to_login,
+                                                         external_auth.service_id())
+      assert external_login is not None
+
+      internal_login = model.user.lookup_federated_login(result.to_login,
+                                                         internal_auth.federated_service)
+      if binding_field is not None:
+        assert internal_login is not None
+      else:
+        assert internal_login is None
+
+    else:
+      # Ensure that no addtional users were created.
+      assert current_user_count == existing_user_count
+
+
+def test_existing_account_in_ldap(app):
+  config = {'GITHUB': {'LOGIN_BINDING_FIELD': 'username'}}
+
+  external_auth = GithubOAuthService(config, 'GITHUB')
+  internal_auth = _get_users_handler('LDAP')
+
+  # Add an existing federated user bound to the LDAP account associated with `someuser`.
+  bound_user = model.user.create_federated_user('someuser', 'foo@bar.com',
+                                                internal_auth.federated_service, 'someuser', False)
+
+  existing_user_count = database.User.select().count()
+
+  with mock_ldap():
+    # Conduct OAuth login with the same lid and bound field. This should find the existing LDAP
+    # user (via the `username` binding), and then bind Github to it as well.
+    result = _conduct_oauth_login(internal_auth, external_auth, bound_user.username,
+                                  bound_user.username, bound_user.email)
+    assert result.error_message is None
+
+    # Ensure that the same user was returned, and that it is now bound to the Github account
+    # as well.
+    assert result.to_login.id == bound_user.id
+
+    # Ensure that no additional users were created.
+    current_user_count = database.User.select().count()
+    assert current_user_count == existing_user_count
+
+    # Check the service bindings.
+    external_login = model.user.lookup_federated_login(result.to_login,
+                                                       external_auth.service_id())
+    assert external_login is not None
+
+    internal_login = model.user.lookup_federated_login(result.to_login,
+                                                       internal_auth.federated_service)
+    assert internal_login is not None

oauth/base.py

@@ -64,6 +64,17 @@ class OAuthService(object):
   def client_secret(self):
     return self.config.get('CLIENT_SECRET')
 
+  def login_binding_field(self):
+    """ Returns the name of the field (`username` or `email`) used for auto binding an external
+        login service account to an *internal* login service account. For example, if the external
+        login service is GitHub and the internal login service is LDAP, a value of `email` here
+        will cause login-with-Github to conduct a search (via email) in LDAP for a user, an auto
+        bind the external and internal users together. May return None, in which case no binding
+        is performing, and login with this external account will simply create a new account in the
+        database.
+    """
+    return self.config.get('LOGIN_BINDING_FIELD', None)
+
   def get_auth_url(self, app_config, redirect_suffix, csrf_token, scopes):
     """ Retrieves the authorization URL for this login service. """
     redirect_uri = '%s/oauth2/%s/callback%s' % (get_app_url(app_config), self.service_id(),