yv01p/Umbraco-CMS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

More tests fixes up a few things with external login service, adds notes changes column to ntext

Browse Source
This commit is contained in:
Shannon
2020-09-11 14:37:31 +10:00
parent dd6e51aab5
commit 26d8126497
6 changed files with 180 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
src/Umbraco.Core/Persistence/Dtos/ExternalLoginDto.cs
View File

@@ -17,6 +17,8 @@ namespace Umbraco.Core.Persistence.Dtos
[Column("userId")]
public int UserId { get; set; }
// TODO: There should be an index on both LoginProvider and ProviderKey
[Column("loginProvider")]
[Length(4000)]
[NullSetting(NullSetting = NullSettings.NotNull)]
@@ -35,8 +37,8 @@ namespace Umbraco.Core.Persistence.Dtos
/// Used to store any arbitrary data for the user and external provider - like user tokens returned from the provider
/// </summary>
[Column("userData")]
[Length(4000)]
[NullSetting(NullSetting = NullSettings.Null)]
[SpecialDbType(SpecialDbTypes.NTEXT)]
public string UserData { get; set; }
}
}

18
src/Umbraco.Core/Persistence/Factories/ExternalLoginFactory.cs
View File

@@ -1,4 +1,5 @@
using Umbraco.Core.Models.Identity;
using System;
using Umbraco.Core.Models.Identity;
using Umbraco.Core.Persistence.Dtos;
namespace Umbraco.Core.Persistence.Factories
@@ -32,5 +33,20 @@ namespace Umbraco.Core.Persistence.Factories
return dto;
}
public static ExternalLoginDto BuildDto(int userId, IExternalLogin entity, int? id = null)
{
var dto = new ExternalLoginDto
{
Id = id ?? default,
UserId = userId,
LoginProvider = entity.LoginProvider,
ProviderKey = entity.ProviderKey,
UserData = entity.UserData,
CreateDate = DateTime.Now
};
return dto;
}
}
}

62
src/Umbraco.Core/Persistence/Repositories/Implement/ExternalLoginRepository.cs
View File

@@ -33,51 +33,49 @@ namespace Umbraco.Core.Persistence.Repositories.Implement
.Where<ExternalLoginDto>(x => x.UserId == userId)
.ForUpdate();
// deduplicate the logins
logins = logins.DistinctBy(x => x.ProviderKey + x.LoginProvider).ToList();
var toUpdate = new Dictionary<int, IExternalLogin>();
var toDelete = new List<int>();
var toInsert = new List<IExternalLogin>(logins);
var existingLogins = Database.Fetch<ExternalLoginDto>(sql);
var existingLogins = Database.Query<ExternalLoginDto>(sql).OrderByDescending(x => x.CreateDate).ToList();
// used to track duplicates so they can be removed
var keys = new HashSet<(string, string)>();
foreach (var existing in existingLogins)
{
var found = logins.FirstOrDefault(x =>
x.LoginProvider.Equals(existing.LoginProvider, StringComparison.InvariantCultureIgnoreCase)
&& x.ProviderKey.Equals(existing.ProviderKey, StringComparison.InvariantCultureIgnoreCase));
if (found != null)
if (!keys.Add((existing.ProviderKey, existing.LoginProvider)))
{
toUpdate.Add(existing.Id, found);
// if it's an update then it's not an insert
toInsert.RemoveAll(x => x.ProviderKey == found.ProviderKey && x.LoginProvider == found.LoginProvider);
// if it already exists we need to remove this one
toDelete.Add(existing.Id);
}
else
{
toDelete.Add(existing.Id);
var found = logins.FirstOrDefault(x =>
x.LoginProvider.Equals(existing.LoginProvider, StringComparison.InvariantCultureIgnoreCase)
&& x.ProviderKey.Equals(existing.ProviderKey, StringComparison.InvariantCultureIgnoreCase));
if (found != null)
{
toUpdate.Add(existing.Id, found);
// if it's an update then it's not an insert
toInsert.RemoveAll(x => x.ProviderKey == found.ProviderKey && x.LoginProvider == found.LoginProvider);
}
else
{
toDelete.Add(existing.Id);
}
}
}
// do the deletes (does this syntax work?)
Database.DeleteMany<ExternalLoginDto>().Where(x => toDelete.Contains(x.Id)).Execute();
// do the deletes, updates and inserts
if (toDelete.Count > 0)
Database.DeleteMany<ExternalLoginDto>().Where(x => toDelete.Contains(x.Id)).Execute();
foreach (var u in toUpdate)
{
Database.Update(new ExternalLoginDto
{
Id = u.Key,
LoginProvider = u.Value.LoginProvider,
ProviderKey = u.Value.ProviderKey,
UserId = userId,
CreateDate = DateTime.Now
});
}
// add the inserts
Database.InsertBulk(toInsert.Select(i => new ExternalLoginDto
{
LoginProvider = i.LoginProvider,
ProviderKey = i.ProviderKey,
UserId = userId,
UserData = i.UserData,
CreateDate = DateTime.Now
}));
Database.Update(ExternalLoginFactory.BuildDto(userId, u.Value, u.Key));
Database.InsertBulk(toInsert.Select(i => ExternalLoginFactory.BuildDto(userId, i)));
}
public void SaveUserLogins(int memberId, IEnumerable<UserLoginInfo> logins)
@@ -145,7 +143,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement
foreach (var dto in dtos)
{
yield return Get(dto.Id);
yield return ExternalLoginFactory.BuildEntity(dto);
}
}

14
src/Umbraco.Core/Security/BackOfficeUserStore.cs
View File

@@ -115,7 +115,7 @@ namespace Umbraco.Core.Security
/// </summary>
/// <param name="user"/>
/// <returns/>
public async Task UpdateAsync(BackOfficeIdentityUser user)
public Task UpdateAsync(BackOfficeIdentityUser user)
{
ThrowIfDisposed();
if (user == null) throw new ArgumentNullException(nameof(user));
@@ -126,6 +126,8 @@ namespace Umbraco.Core.Security
throw new InvalidOperationException("The user id must be an integer to work with the Umbraco");
}
// TODO: Wrap this in a scope!
var found = _userService.GetUserById(asInt.Result);
if (found != null)
{
@@ -139,10 +141,16 @@ namespace Umbraco.Core.Security
if (isLoginsPropertyDirty)
{
var logins = await GetLoginsAsync(user);
_externalLoginService.Save(found.Id, logins.Select(x => new ExternalLogin(x.LoginProvider, x.ProviderKey)));
_externalLoginService.Save(
found.Id,
user.Logins.Select(x => new ExternalLogin(
x.LoginProvider,
x.ProviderKey,
(x is IIdentityUserLoginExtended extended) ? extended.UserData : null)));
}
}
return Task.CompletedTask;
}
/// <summary>

118
src/Umbraco.Tests/Services/ExternalLoginServiceTests.cs
View File

@@ -5,6 +5,7 @@ using Microsoft.AspNet.Identity;
using NUnit.Framework;
using Umbraco.Core.Models.Identity;
using Umbraco.Core.Models.Membership;
using Umbraco.Core.Persistence.Dtos;
using Umbraco.Tests.TestHelpers;
using Umbraco.Tests.Testing;
@@ -15,6 +16,73 @@ namespace Umbraco.Tests.Services
[UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerFixture)]
public class ExternalLoginServiceTests : TestWithDatabaseBase
{
[Test]
public void Removes_Existing_Duplicates_On_Save()
{
var user = new User("Test", "test@test.com", "test", "helloworldtest");
ServiceContext.UserService.Save(user);
var providerKey = Guid.NewGuid().ToString("N");
var latest = DateTime.Now.AddDays(-1);
var oldest = DateTime.Now.AddDays(-10);
using (var scope = ScopeProvider.CreateScope())
{
// insert duplicates manuall
scope.Database.Insert(new ExternalLoginDto
{
UserId = user.Id,
LoginProvider = "test1",
ProviderKey = providerKey,
CreateDate = latest
});
scope.Database.Insert(new ExternalLoginDto
{
UserId = user.Id,
LoginProvider = "test1",
ProviderKey = providerKey,
CreateDate = oldest
});
}
// try to save 2 other duplicates
var externalLogins = new[]
{
new ExternalLogin("test2", providerKey),
new ExternalLogin("test2", providerKey),
new ExternalLogin("test1", providerKey)
};
ServiceContext.ExternalLoginService.Save(user.Id, externalLogins);
var logins = ServiceContext.ExternalLoginService.GetAll(user.Id).ToList();
// duplicates will be removed, keeping the latest entries
Assert.AreEqual(2, logins.Count);
var test1 = logins.Single(x => x.LoginProvider == "test1");
Assert.Greater(test1.CreateDate, latest);
}
[Test]
public void Does_Not_Persist_Duplicates()
{
var user = new User("Test", "test@test.com", "test", "helloworldtest");
ServiceContext.UserService.Save(user);
var providerKey = Guid.NewGuid().ToString("N");
var externalLogins = new[]
{
new ExternalLogin("test1", providerKey),
new ExternalLogin("test1", providerKey)
};
ServiceContext.ExternalLoginService.Save(user.Id, externalLogins);
var logins = ServiceContext.ExternalLoginService.GetAll(user.Id).ToList();
Assert.AreEqual(1, logins.Count);
}
[Test]
public void Single_Create()
{
@@ -54,6 +122,56 @@ namespace Umbraco.Tests.Services
Assert.AreEqual("world", found[0].UserData);
}
[Test]
public void Multiple_Update()
{
var user = new User("Test", "test@test.com", "test", "helloworldtest");
ServiceContext.UserService.Save(user);
var providerKey1 = Guid.NewGuid().ToString("N");
var providerKey2 = Guid.NewGuid().ToString("N");
var extLogins = new[]
{
new ExternalLogin("test1", providerKey1, "hello"),
new ExternalLogin("test2", providerKey2, "world")
};
ServiceContext.ExternalLoginService.Save(user.Id, extLogins);
extLogins = new[]
{
new ExternalLogin("test1", providerKey1, "123456"),
new ExternalLogin("test2", providerKey2, "987654")
};
ServiceContext.ExternalLoginService.Save(user.Id, extLogins);
var found = ServiceContext.ExternalLoginService.GetAll(user.Id).Cast<IIdentityUserLoginExtended>().OrderBy(x => x.LoginProvider).ToList();
Assert.AreEqual(2, found.Count);
Assert.AreEqual("123456", found[0].UserData);
Assert.AreEqual("987654", found[1].UserData);
}
[Test]
public void Can_Find_As_Extended_Type()
{
var user = new User("Test", "test@test.com", "test", "helloworldtest");
ServiceContext.UserService.Save(user);
var providerKey1 = Guid.NewGuid().ToString("N");
var providerKey2 = Guid.NewGuid().ToString("N");
var extLogins = new[]
{
new ExternalLogin("test1", providerKey1, "hello"),
new ExternalLogin("test2", providerKey2, "world")
};
ServiceContext.ExternalLoginService.Save(user.Id, extLogins);
var found = ServiceContext.ExternalLoginService.Find("test2", providerKey2).ToList();
Assert.AreEqual(1, found.Count);
var asExtended = found.Cast<IIdentityUserLoginExtended>().ToList();
Assert.AreEqual(1, found.Count);
}
[Test]
public void Add_Logins()
{

8
src/Umbraco.Web/Editors/BackOfficeController.cs
View File

@@ -390,7 +390,7 @@ namespace Umbraco.Web.Editors
if (response == null) throw new ArgumentNullException("response");
ExternalSignInAutoLinkOptions autoLinkOptions = null;
//Here we can check if the provider associated with the request has been configured to allow
// Here we can check if the provider associated with the request has been configured to allow
// new users (auto-linked external accounts). This would never be used with public providers such as
// Google, unless you for some reason wanted anybody to be able to access the backend if they have a Google account
// .... not likely!
@@ -408,12 +408,6 @@ namespace Umbraco.Web.Editors
var user = await UserManager.FindAsync(loginInfo.Login);
if (user != null)
{
// TODO: It might be worth keeping some of the claims associated with the ExternalLoginInfo, in which case we
// wouldn't necessarily sign the user in here with the standard login, instead we'd update the
// UseUmbracoBackOfficeExternalCookieAuthentication extension method to have the correct provider and claims factory,
// ticket format, etc.. to create our back office user including the claims assigned and in this method we'd just ensure
// that the ticket is created and stored and that the user is logged in.
var shouldSignIn = true;
if (autoLinkOptions != null && autoLinkOptions.OnExternalLogin != null)
{