diff --git a/endpoints/api/test/__init__.py b/endpoints/api/test/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/endpoints/api/test/conftest.py b/endpoints/api/test/conftest.py index 707fc24ca..07128a0c7 100644 --- a/endpoints/api/test/conftest.py +++ b/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 diff --git a/endpoints/oauthlogin.py b/endpoints/oauthlogin.py index 538117f95..d5d4ac591 100644 --- a/endpoints/oauthlogin.py +++ b/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): - """ Conducts login from the result of an OAuth service's login flow. """ - - 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) - +def _perform_login(to_login, service_name): + """ Attempts to login the given user, returning the Flask result of whether the login succeeded. + """ 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, - lusername, lemail, metadata=metadata) + result = _conduct_oauth_login(authentication, login_service, lid, lusername, lemail, + 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() - - try: - model.user.attach_federated_login(user_obj, login_service.service_id(), lid, - 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) + result = _attach_service(login_service, user_obj, lid, lusername) + if result.error_message is not None: + return _get_response(result) return redirect(url_for('web.user_view', path=user_obj.username, tab='external')) diff --git a/endpoints/test/test_oauthlogin.py b/endpoints/test/test_oauthlogin.py new file mode 100644 index 000000000..340e33c1a --- /dev/null +++ b/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 diff --git a/oauth/base.py b/oauth/base.py index 0afa418c9..e7a6232c8 100644 --- a/oauth/base.py +++ b/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(),