diff --git a/backend/src/Squidex.Domain.Apps.Core.Operations/HandleRules/EventEnricher.cs b/backend/src/Squidex.Domain.Apps.Core.Operations/HandleRules/EventEnricher.cs index aa71f0c01..7241f15ee 100644 --- a/backend/src/Squidex.Domain.Apps.Core.Operations/HandleRules/EventEnricher.cs +++ b/backend/src/Squidex.Domain.Apps.Core.Operations/HandleRules/EventEnricher.cs @@ -59,7 +59,7 @@ namespace Squidex.Domain.Apps.Core.HandleRules IUser? user; try { - user = await userResolver.FindByIdOrEmailAsync(actor.Identifier); + user = await userResolver.FindByIdAsync(actor.Identifier); } catch { diff --git a/backend/src/Squidex.Domain.Apps.Entities/Apps/Guards/GuardAppContributors.cs b/backend/src/Squidex.Domain.Apps.Entities/Apps/Guards/GuardAppContributors.cs index 46bbc50ea..d6e8d7b5c 100644 --- a/backend/src/Squidex.Domain.Apps.Entities/Apps/Guards/GuardAppContributors.cs +++ b/backend/src/Squidex.Domain.Apps.Entities/Apps/Guards/GuardAppContributors.cs @@ -36,15 +36,13 @@ namespace Squidex.Domain.Apps.Entities.Apps.Guards } else { - var user = await users.FindByIdOrEmailAsync(command.ContributorId); + var user = await users.FindByIdAsync(command.ContributorId); if (user == null) { throw new DomainObjectNotFoundException(command.ContributorId, "Contributors", typeof(IAppEntity)); } - command.ContributorId = user.Id; - if (!command.Restoring) { if (string.Equals(command.ContributorId, command.Actor?.Identifier, StringComparison.OrdinalIgnoreCase)) diff --git a/backend/src/Squidex.Domain.Apps.Entities/Apps/Invitation/InvitationEventConsumer.cs b/backend/src/Squidex.Domain.Apps.Entities/Apps/Invitation/InvitationEventConsumer.cs index 106df9bd4..9e66713c2 100644 --- a/backend/src/Squidex.Domain.Apps.Entities/Apps/Invitation/InvitationEventConsumer.cs +++ b/backend/src/Squidex.Domain.Apps.Entities/Apps/Invitation/InvitationEventConsumer.cs @@ -86,7 +86,7 @@ namespace Squidex.Domain.Apps.Entities.Apps.Invitation var assignerId = appContributorAssigned.Actor.Identifier; var assigneeId = appContributorAssigned.ContributorId; - var assigner = await userResolver.FindByIdOrEmailAsync(assignerId); + var assigner = await userResolver.FindByIdAsync(assignerId); if (assigner == null) { @@ -94,7 +94,7 @@ namespace Squidex.Domain.Apps.Entities.Apps.Invitation return; } - var assignee = await userResolver.FindByIdOrEmailAsync(appContributorAssigned.ContributorId); + var assignee = await userResolver.FindByIdAsync(appContributorAssigned.ContributorId); if (assignee == null) { diff --git a/backend/src/Squidex.Domain.Apps.Entities/Apps/Invitation/InviteUserCommandMiddleware.cs b/backend/src/Squidex.Domain.Apps.Entities/Apps/Invitation/InviteUserCommandMiddleware.cs index 878c1aa74..7700bcdaa 100644 --- a/backend/src/Squidex.Domain.Apps.Entities/Apps/Invitation/InviteUserCommandMiddleware.cs +++ b/backend/src/Squidex.Domain.Apps.Entities/Apps/Invitation/InviteUserCommandMiddleware.cs @@ -28,7 +28,12 @@ namespace Squidex.Domain.Apps.Entities.Apps.Invitation { if (context.Command is AssignContributor assignContributor && ShouldInvite(assignContributor)) { - var created = await userResolver.CreateUserIfNotExistsAsync(assignContributor.ContributorId, true); + var (user, created) = await userResolver.CreateUserIfNotExistsAsync(assignContributor.ContributorId, true); + + if (user != null) + { + assignContributor.ContributorId = user.Id; + } await next(context); diff --git a/backend/src/Squidex.Domain.Apps.Entities/Backup/UserMapping.cs b/backend/src/Squidex.Domain.Apps.Entities/Backup/UserMapping.cs index 9d273dbbc..c0579e27f 100644 --- a/backend/src/Squidex.Domain.Apps.Entities/Backup/UserMapping.cs +++ b/backend/src/Squidex.Domain.Apps.Entities/Backup/UserMapping.cs @@ -74,12 +74,7 @@ namespace Squidex.Domain.Apps.Entities.Backup foreach (var (userId, email) in json) { - var user = await userResolver.FindByIdOrEmailAsync(email); - - if (user == null && await userResolver.CreateUserIfNotExistsAsync(email, false)) - { - user = await userResolver.FindByIdOrEmailAsync(email); - } + var (user, _) = await userResolver.CreateUserIfNotExistsAsync(email, false); if (user != null) { diff --git a/backend/src/Squidex.Domain.Users/DefaultUserResolver.cs b/backend/src/Squidex.Domain.Users/DefaultUserResolver.cs index 023cd57aa..f766e76fa 100644 --- a/backend/src/Squidex.Domain.Users/DefaultUserResolver.cs +++ b/backend/src/Squidex.Domain.Users/DefaultUserResolver.cs @@ -14,6 +14,8 @@ using Microsoft.Extensions.DependencyInjection; using Squidex.Infrastructure; using Squidex.Shared.Users; +#pragma warning disable RECS0022 // A catch clause that catches System.Exception and has an empty body + namespace Squidex.Domain.Users { public sealed class DefaultUserResolver : IUserResolver @@ -27,33 +29,58 @@ namespace Squidex.Domain.Users this.serviceProvider = serviceProvider; } - public async Task CreateUserIfNotExistsAsync(string email, bool invited) + public async Task<(IUser? User, bool Created)> CreateUserIfNotExistsAsync(string email, bool invited) { Guard.NotNullOrEmpty(email); + var created = false; + using (var scope = serviceProvider.CreateScope()) { var userFactory = scope.ServiceProvider.GetRequiredService(); var userManager = scope.ServiceProvider.GetRequiredService>(); - var user = userFactory.Create(email); - try { + var user = userFactory.Create(email); + var result = await userManager.CreateAsync(user); if (result.Succeeded) { + created = true; + var values = new UserValues { DisplayName = email, Invited = invited }; await userManager.UpdateAsync(user, values); } - - return result.Succeeded; } catch { - return false; + } + + var found = await userManager.FindByEmailWithClaimsAsync(email); + + return (found, created); + } + } + + public async Task FindByIdAsync(string idOrEmail) + { + Guard.NotNullOrEmpty(idOrEmail); + + using (var scope = serviceProvider.CreateScope()) + { + var userFactory = scope.ServiceProvider.GetRequiredService(); + var userManager = scope.ServiceProvider.GetRequiredService>(); + + if (userFactory.IsId(idOrEmail)) + { + return await userManager.FindByIdWithClaimsAsync(idOrEmail); + } + else + { + return await userManager.FindByEmailWithClaimsAsync(idOrEmail); } } } diff --git a/backend/src/Squidex.Shared/Users/IUserResolver.cs b/backend/src/Squidex.Shared/Users/IUserResolver.cs index c519ef44c..4de82aa2f 100644 --- a/backend/src/Squidex.Shared/Users/IUserResolver.cs +++ b/backend/src/Squidex.Shared/Users/IUserResolver.cs @@ -12,10 +12,12 @@ namespace Squidex.Shared.Users { public interface IUserResolver { - Task CreateUserIfNotExistsAsync(string email, bool invited = false); + Task<(IUser? User, bool Created)> CreateUserIfNotExistsAsync(string email, bool invited = false); Task FindByIdOrEmailAsync(string idOrEmail); + Task FindByIdAsync(string idOrEmail); + Task> QueryByEmailAsync(string email); Task> QueryManyAsync(string[] ids); diff --git a/backend/src/Squidex/Areas/Api/Controllers/Users/UsersController.cs b/backend/src/Squidex/Areas/Api/Controllers/Users/UsersController.cs index da5d84b6a..97056f31c 100644 --- a/backend/src/Squidex/Areas/Api/Controllers/Users/UsersController.cs +++ b/backend/src/Squidex/Areas/Api/Controllers/Users/UsersController.cs @@ -127,7 +127,7 @@ namespace Squidex.Areas.Api.Controllers.Users { try { - var entity = await userResolver.FindByIdOrEmailAsync(id); + var entity = await userResolver.FindByIdAsync(id); if (entity != null) { @@ -162,7 +162,7 @@ namespace Squidex.Areas.Api.Controllers.Users { try { - var entity = await userResolver.FindByIdOrEmailAsync(id); + var entity = await userResolver.FindByIdAsync(id); if (entity != null) { diff --git a/backend/tests/Squidex.Domain.Apps.Entities.Tests/Apps/Guards/GuardAppContributorsTests.cs b/backend/tests/Squidex.Domain.Apps.Entities.Tests/Apps/Guards/GuardAppContributorsTests.cs index 2429b8672..251552595 100644 --- a/backend/tests/Squidex.Domain.Apps.Entities.Tests/Apps/Guards/GuardAppContributorsTests.cs +++ b/backend/tests/Squidex.Domain.Apps.Entities.Tests/Apps/Guards/GuardAppContributorsTests.cs @@ -36,15 +36,11 @@ namespace Squidex.Domain.Apps.Entities.Apps.Guards A.CallTo(() => user2.Id).Returns("2"); A.CallTo(() => user3.Id).Returns("3"); - A.CallTo(() => users.FindByIdOrEmailAsync("1")).Returns(user1); - A.CallTo(() => users.FindByIdOrEmailAsync("2")).Returns(user2); - A.CallTo(() => users.FindByIdOrEmailAsync("3")).Returns(user3); + A.CallTo(() => users.FindByIdAsync("1")).Returns(user1); + A.CallTo(() => users.FindByIdAsync("2")).Returns(user2); + A.CallTo(() => users.FindByIdAsync("3")).Returns(user3); - A.CallTo(() => users.FindByIdOrEmailAsync("1@email.com")).Returns(user1); - A.CallTo(() => users.FindByIdOrEmailAsync("2@email.com")).Returns(user2); - A.CallTo(() => users.FindByIdOrEmailAsync("3@email.com")).Returns(user3); - - A.CallTo(() => users.FindByIdOrEmailAsync("notfound")) + A.CallTo(() => users.FindByIdAsync("notfound")) .Returns(Task.FromResult(null)); A.CallTo(() => appPlan.MaxContributors) @@ -120,16 +116,6 @@ namespace Squidex.Domain.Apps.Entities.Apps.Guards new ValidationError("You have reached the maximum number of contributors for your plan.")); } - [Fact] - public async Task CanAssign_assign_if_if_user_added_by_email() - { - var command = new AssignContributor { ContributorId = "1@email.com" }; - - await GuardAppContributors.CanAssign(contributors_0, roles, command, users, appPlan); - - Assert.Equal("1", command.ContributorId); - } - [Fact] public async Task CanAssign_should_not_throw_exception_if_user_found() { diff --git a/backend/tests/Squidex.Domain.Apps.Entities.Tests/Apps/Invitation/InvitationEventConsumerTests.cs b/backend/tests/Squidex.Domain.Apps.Entities.Tests/Apps/Invitation/InvitationEventConsumerTests.cs index 4a535fc75..32bfa92d9 100644 --- a/backend/tests/Squidex.Domain.Apps.Entities.Tests/Apps/Invitation/InvitationEventConsumerTests.cs +++ b/backend/tests/Squidex.Domain.Apps.Entities.Tests/Apps/Invitation/InvitationEventConsumerTests.cs @@ -36,10 +36,10 @@ namespace Squidex.Domain.Apps.Entities.Apps.Invitation.Notifications A.CallTo(() => notificatíonSender.IsActive) .Returns(true); - A.CallTo(() => userResolver.FindByIdOrEmailAsync(assignerId)) + A.CallTo(() => userResolver.FindByIdAsync(assignerId)) .Returns(assigner); - A.CallTo(() => userResolver.FindByIdOrEmailAsync(assigneeId)) + A.CallTo(() => userResolver.FindByIdAsync(assigneeId)) .Returns(assignee); sut = new InvitationEventConsumer(notificatíonSender, userResolver, log); @@ -69,7 +69,9 @@ namespace Squidex.Domain.Apps.Entities.Apps.Invitation.Notifications [Fact] public async Task Should_not_send_email_for_old_events() { - var @event = CreateEvent(RefTokenType.Subject, true, instant: SystemClock.Instance.GetCurrentInstant().Minus(Duration.FromHours(50))); + var created = SystemClock.Instance.GetCurrentInstant().Minus(Duration.FromHours(50)); + + var @event = CreateEvent(RefTokenType.Subject, true, instant: created); await sut.On(@event); @@ -107,7 +109,7 @@ namespace Squidex.Domain.Apps.Entities.Apps.Invitation.Notifications { var @event = CreateEvent(RefTokenType.Subject, true); - A.CallTo(() => userResolver.FindByIdOrEmailAsync(assignerId)) + A.CallTo(() => userResolver.FindByIdAsync(assignerId)) .Returns(Task.FromResult(null)); await sut.On(@event); @@ -121,7 +123,7 @@ namespace Squidex.Domain.Apps.Entities.Apps.Invitation.Notifications { var @event = CreateEvent(RefTokenType.Subject, true); - A.CallTo(() => userResolver.FindByIdOrEmailAsync(assigneeId)) + A.CallTo(() => userResolver.FindByIdAsync(assigneeId)) .Returns(Task.FromResult(null)); await sut.On(@event); @@ -160,7 +162,7 @@ namespace Squidex.Domain.Apps.Entities.Apps.Invitation.Notifications private void MustNotResolveUser() { - A.CallTo(() => userResolver.FindByIdOrEmailAsync(A._)) + A.CallTo(() => userResolver.FindByIdAsync(A._)) .MustNotHaveHappened(); } diff --git a/backend/tests/Squidex.Domain.Apps.Entities.Tests/Apps/Invitation/InviteUserCommandMiddlewareTests.cs b/backend/tests/Squidex.Domain.Apps.Entities.Tests/Apps/Invitation/InviteUserCommandMiddlewareTests.cs index 321884b9f..2915df56d 100644 --- a/backend/tests/Squidex.Domain.Apps.Entities.Tests/Apps/Invitation/InviteUserCommandMiddlewareTests.cs +++ b/backend/tests/Squidex.Domain.Apps.Entities.Tests/Apps/Invitation/InviteUserCommandMiddlewareTests.cs @@ -30,7 +30,7 @@ namespace Squidex.Domain.Apps.Entities.Apps.Invitation } [Fact] - public async Task Should_invite_user_and_change_result() + public async Task Should_invite_user_and_change_result_and_update_command() { var command = new AssignContributor { ContributorId = "me@email.com", Invite = true }; @@ -38,12 +38,15 @@ namespace Squidex.Domain.Apps.Entities.Apps.Invitation new CommandContext(command, commandBus) .Complete(app); + var user = CreateUser("123"); + A.CallTo(() => userResolver.CreateUserIfNotExistsAsync("me@email.com", true)) - .Returns(true); + .Returns((user, true)); await sut.HandleAsync(context); Assert.Same(context.Result().App, app); + Assert.Equal(user.Id, command.ContributorId); A.CallTo(() => userResolver.CreateUserIfNotExistsAsync("me@email.com", true)) .MustHaveHappened(); @@ -58,12 +61,15 @@ namespace Squidex.Domain.Apps.Entities.Apps.Invitation new CommandContext(command, commandBus) .Complete(app); + var user = CreateUser("123"); + A.CallTo(() => userResolver.CreateUserIfNotExistsAsync("me@email.com", true)) - .Returns(false); + .Returns((user, false)); await sut.HandleAsync(context); Assert.Same(context.Result(), app); + Assert.Equal(user.Id, command.ContributorId); A.CallTo(() => userResolver.CreateUserIfNotExistsAsync("me@email.com", true)) .MustHaveHappened(); @@ -98,5 +104,14 @@ namespace Squidex.Domain.Apps.Entities.Apps.Invitation A.CallTo(() => userResolver.CreateUserIfNotExistsAsync(A._, A._)) .MustNotHaveHappened(); } + + private static IUser CreateUser(string id) + { + var user = A.Fake(); + + A.CallTo(() => user.Id).Returns(id); + + return user; + } } } diff --git a/backend/tests/Squidex.Domain.Apps.Entities.Tests/Backup/UserMappingTests.cs b/backend/tests/Squidex.Domain.Apps.Entities.Tests/Backup/UserMappingTests.cs index b5c14192c..9239c3b32 100644 --- a/backend/tests/Squidex.Domain.Apps.Entities.Tests/Backup/UserMappingTests.cs +++ b/backend/tests/Squidex.Domain.Apps.Entities.Tests/Backup/UserMappingTests.cs @@ -74,37 +74,19 @@ namespace Squidex.Domain.Apps.Entities.Backup var userResolver = A.Fake(); - A.CallTo(() => userResolver.FindByIdOrEmailAsync(user1.Email)) - .Returns(user1); + A.CallTo(() => userResolver.CreateUserIfNotExistsAsync(user1.Email, false)) + .Returns((user1, false)); - A.CallTo(() => userResolver.FindByIdOrEmailAsync(user2.Email)) - .Returns(user2); + A.CallTo(() => userResolver.CreateUserIfNotExistsAsync(user2.Email, false)) + .Returns((user2, true)); await sut.RestoreAsync(reader, userResolver); Assert.True(sut.TryMap("user1_old", out var mapped1)); - Assert.Equal(Subject("user1"), mapped1); - Assert.True(sut.TryMap(Subject("user2_old"), out var mapped2)); - Assert.Equal(Subject("user2"), mapped2); - } - - [Fact] - public async Task Should_create_user_if_not_found() - { - var user = CreateUser("newId1", "mail1@squidex.io"); - - var reader = SetupReader(user); - var userResolver = A.Fake(); - - A.CallTo(() => userResolver.FindByIdOrEmailAsync(user.Email)) - .Returns(Task.FromResult(null)); - - await sut.RestoreAsync(reader, userResolver); - - A.CallTo(() => userResolver.CreateUserIfNotExistsAsync(user.Email, false)) - .MustHaveHappened(); + Assert.Equal(Subject("user1"), mapped1); + Assert.Equal(Subject("user2"), mapped2); } [Fact] diff --git a/backend/tests/Squidex.Domain.Users.Tests/DefaultUserResolverTests.cs b/backend/tests/Squidex.Domain.Users.Tests/DefaultUserResolverTests.cs index 08699296c..91dcc798a 100644 --- a/backend/tests/Squidex.Domain.Users.Tests/DefaultUserResolverTests.cs +++ b/backend/tests/Squidex.Domain.Users.Tests/DefaultUserResolverTests.cs @@ -19,13 +19,12 @@ namespace Squidex.Domain.Users { public class DefaultUserResolverTests { + private readonly IUserFactory userFactory = A.Fake(); private readonly UserManager userManager = A.Fake>(); private readonly DefaultUserResolver sut; public DefaultUserResolverTests() { - var userFactory = A.Fake(); - A.CallTo(() => userFactory.IsId(A.That.StartsWith("id"))) .Returns(true); @@ -56,10 +55,73 @@ namespace Squidex.Domain.Users sut = new DefaultUserResolver(serviceProvider); } + [Fact] + public async Task Should_create_user_and_return_true_when_created() + { + var (user, claims) = GenerateUser("id1"); + + A.CallTo(() => userFactory.Create(user.Email)) + .Returns(user); + + A.CallTo(() => userManager.CreateAsync(user)) + .Returns(IdentityResult.Success); + + SetupUser(user, claims); + + var (result, created) = await sut.CreateUserIfNotExistsAsync(user.Email, false); + + Assert.Equal(user.Id, result!.Id); + Assert.Equal(user.Email, result!.Email); + + Assert.True(created); + } + + [Fact] + public async Task Should_create_user_and_return_false_when_already_exists() + { + var (user, claims) = GenerateUser("id1"); + + A.CallTo(() => userFactory.Create(user.Email)) + .Returns(user); + + A.CallTo(() => userManager.CreateAsync(user)) + .Returns(IdentityResult.Failed()); + + SetupUser(user, claims); + + var (result, created) = await sut.CreateUserIfNotExistsAsync(user.Email, false); + + Assert.Equal(user.Id, result!.Id); + Assert.Equal(user.Email, result!.Email); + + Assert.False(created); + } + + [Fact] + public async Task Should_create_user_and_return_false_when_exception_thrown() + { + var (user, claims) = GenerateUser("id1"); + + A.CallTo(() => userFactory.Create(user.Email)) + .Throws(new InvalidOperationException()); + + A.CallTo(() => userManager.CreateAsync(user)) + .Returns(IdentityResult.Failed()); + + SetupUser(user, claims); + + var (result, created) = await sut.CreateUserIfNotExistsAsync(user.Email, false); + + Assert.Equal(user.Id, result!.Id); + Assert.Equal(user.Email, result!.Email); + + Assert.False(created); + } + [Fact] public async Task Should_resolve_user_by_email() { - var (user, claims) = GernerateUser("id1"); + var (user, claims) = GenerateUser("id1"); A.CallTo(() => userManager.FindByEmailAsync(user.Email)) .Returns(user); @@ -76,9 +138,9 @@ namespace Squidex.Domain.Users } [Fact] - public async Task Should_resolve_user_by_id1() + public async Task Should_resolve_user_by_id() { - var (user, claims) = GernerateUser("id2"); + var (user, claims) = GenerateUser("id2"); A.CallTo(() => userManager.FindByIdAsync(user.Id)) .Returns(user); @@ -94,11 +156,30 @@ namespace Squidex.Domain.Users Assert.Equal(claims, result!.Claims); } + [Fact] + public async Task Should_resolve_user_by_id_only() + { + var (user, claims) = GenerateUser("id2"); + + A.CallTo(() => userManager.FindByIdAsync(user.Id)) + .Returns(user); + + A.CallTo(() => userManager.GetClaimsAsync(user)) + .Returns(claims); + + var result = await sut.FindByIdAsync(user.Id)!; + + Assert.Equal(user.Id, result!.Id); + Assert.Equal(user.Email, result!.Email); + + Assert.Equal(claims, result!.Claims); + } + [Fact] public async Task Should_query_many_by_email_async() { - var (user1, claims1) = GernerateUser("id1"); - var (user2, claims2) = GernerateUser("id2"); + var (user1, claims1) = GenerateUser("id1"); + var (user2, claims2) = GenerateUser("id2"); var list = new List { user1, user2 }; @@ -119,7 +200,7 @@ namespace Squidex.Domain.Users .MustNotHaveHappened(); } - private static (IdentityUser, List) GernerateUser(string id) + private static (IdentityUser, List) GenerateUser(string id) { var user = new IdentityUser { Id = id, Email = $"email_{id}", NormalizedEmail = $"EMAIL_{id}" }; @@ -131,5 +212,14 @@ namespace Squidex.Domain.Users return (user, claims); } + + private void SetupUser(IdentityUser user, List claims) + { + A.CallTo(() => userManager.FindByEmailAsync(user.Email)) + .Returns(user); + + A.CallTo(() => userManager.GetClaimsAsync(user)) + .Returns(claims); + } } }