diff --git a/endpoints/oauthlogin.py b/endpoints/oauthlogin.py index d5d4ac591..f887fe2ed 100644 --- a/endpoints/oauthlogin.py +++ b/endpoints/oauthlogin.py @@ -55,7 +55,9 @@ def _conduct_oauth_login(auth_system, login_service, lid, lusername, lemail, met # Perform lookup. logger.debug('Got oauth bind field name of "%s"', bound_field_name) lookup_value = None - if bound_field_name == 'username': + if bound_field_name == 'sub': + lookup_value = lid + elif bound_field_name == 'username': lookup_value = lusername elif bound_field_name == 'email': lookup_value = lemail diff --git a/endpoints/test/test_oauthlogin.py b/endpoints/test/test_oauthlogin.py index 9cb365f33..0a8ad3632 100644 --- a/endpoints/test/test_oauthlogin.py +++ b/endpoints/test/test_oauthlogin.py @@ -74,34 +74,41 @@ def test_new_account_via_database(login_service): assert federated_login is not None -@pytest.mark.parametrize('binding_field, lusername, lemail, expected_error', [ +@pytest.mark.parametrize('binding_field, lid, lusername, lemail, expected_error', [ # No binding field + newly seen user -> New unlinked user - (None, 'someunknownuser', 'someemail@example.com', None), + (None, 'someid', 'someunknownuser', 'someemail@example.com', None), + + # sub binding field + unknown sub -> Error. + ('sub', 'someid', 'someuser', 'foo@bar.com', + 'sub someid not found in backing auth system'), # username binding field + unknown username -> Error. - ('username', 'someunknownuser', 'foo@bar.com', + ('username', 'someid', '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', 'someid', '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), + (None, 'someid', 'someuser', 'foo@bar.com', None), # username binding field + valid username -> fully bound user. - ('username', 'someuser', 'foo@bar.com', None), + ('username', 'someid', 'someuser', 'foo@bar.com', None), + + # sub binding field + valid sub -> fully bound user. + ('sub', 'someuser', 'someusername', 'foo@bar.com', None), # email binding field + valid email -> fully bound user. - ('email', 'someuser', 'foo@bar.com', None), + ('email', 'someid', 'someuser', 'foo@bar.com', None), # username binding field + valid username + invalid email -> fully bound user. - ('username', 'someuser', 'another@email.com', None), + ('username', 'someid', 'someuser', 'another@email.com', None), # email binding field + valid email + invalid username -> fully bound user. - ('email', 'someotherusername', 'foo@bar.com', None), + ('email', 'someid', 'someotherusername', 'foo@bar.com', None), ]) -def test_new_account_via_ldap(binding_field, lusername, lemail, expected_error, app): +def test_new_account_via_ldap(binding_field, lid, lusername, lemail, expected_error, app): existing_user_count = database.User.select().count() config = {'GITHUB': {}} @@ -113,7 +120,7 @@ def test_new_account_via_ldap(binding_field, lusername, lemail, expected_error, with mock_ldap(): # Conduct OAuth login. - result = _conduct_oauth_login(internal_auth, external_auth, 'someid', lusername, lemail) + result = _conduct_oauth_login(internal_auth, external_auth, lid, lusername, lemail) assert result.error_message == expected_error current_user_count = database.User.select().count()