Fix invites in clsutering (#520)

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
                 {

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))

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)
                 {

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);
 

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);
+                var (user, _) = await userResolver.CreateUserIfNotExistsAsync(email, false);
-
-                if (user == null && await userResolver.CreateUserIfNotExistsAsync(email, false))
-                {
-                    user = await userResolver.FindByIdOrEmailAsync(email);
-                }
 
                 if (user != null)
                 {

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<bool> 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<IUserFactory>();
                 var userManager = scope.ServiceProvider.GetRequiredService<UserManager<IdentityUser>>();
 
-                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<IUser?> FindByIdAsync(string idOrEmail)
+        {
+            Guard.NotNullOrEmpty(idOrEmail);
+
+            using (var scope = serviceProvider.CreateScope())
+            {
+                var userFactory = scope.ServiceProvider.GetRequiredService<IUserFactory>();
+                var userManager = scope.ServiceProvider.GetRequiredService<UserManager<IdentityUser>>();
+
+                if (userFactory.IsId(idOrEmail))
+                {
+                    return await userManager.FindByIdWithClaimsAsync(idOrEmail);
+                }
+                else
+                {
+                    return await userManager.FindByEmailWithClaimsAsync(idOrEmail);
                 }
             }
         }

backend/src/Squidex.Shared/Users/IUserResolver.cs

@@ -12,10 +12,12 @@ namespace Squidex.Shared.Users
 {
     public interface IUserResolver
     {
-        Task<bool> CreateUserIfNotExistsAsync(string email, bool invited = false);
+        Task<(IUser? User, bool Created)> CreateUserIfNotExistsAsync(string email, bool invited = false);
 
         Task<IUser?> FindByIdOrEmailAsync(string idOrEmail);
 
+        Task<IUser?> FindByIdAsync(string idOrEmail);
+
         Task<List<IUser>> QueryByEmailAsync(string email);
 
         Task<Dictionary<string, IUser>> QueryManyAsync(string[] ids);

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)
                 {

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.FindByIdAsync("1")).Returns(user1);
-            A.CallTo(() => users.FindByIdOrEmailAsync("2")).Returns(user2);
+            A.CallTo(() => users.FindByIdAsync("2")).Returns(user2);
-            A.CallTo(() => users.FindByIdOrEmailAsync("3")).Returns(user3);
+            A.CallTo(() => users.FindByIdAsync("3")).Returns(user3);
 
-            A.CallTo(() => users.FindByIdOrEmailAsync("1@email.com")).Returns(user1);
+            A.CallTo(() => users.FindByIdAsync("notfound"))
-            A.CallTo(() => users.FindByIdOrEmailAsync("2@email.com")).Returns(user2);
-            A.CallTo(() => users.FindByIdOrEmailAsync("3@email.com")).Returns(user3);
-
-            A.CallTo(() => users.FindByIdOrEmailAsync("notfound"))
                 .Returns(Task.FromResult<IUser?>(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()
         {

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<IUser?>(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<IUser?>(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<string>._))
+            A.CallTo(() => userResolver.FindByIdAsync(A<string>._))
                 .MustNotHaveHappened();
         }
 

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<InvitedResult>().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<IAppEntity>(), 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<string>._, A<bool>._))
                 .MustNotHaveHappened();
         }
+
+        private static IUser CreateUser(string id)
+        {
+            var user = A.Fake<IUser>();
+
+            A.CallTo(() => user.Id).Returns(id);
+
+            return user;
+        }
     }
 }

backend/tests/Squidex.Domain.Apps.Entities.Tests/Backup/UserMappingTests.cs

@@ -74,37 +74,19 @@ namespace Squidex.Domain.Apps.Entities.Backup
 
             var userResolver = A.Fake<IUserResolver>();
 
-            A.CallTo(() => userResolver.FindByIdOrEmailAsync(user1.Email))
+            A.CallTo(() => userResolver.CreateUserIfNotExistsAsync(user1.Email, false))
-                .Returns(user1);
+                .Returns((user1, false));
 
-            A.CallTo(() => userResolver.FindByIdOrEmailAsync(user2.Email))
+            A.CallTo(() => userResolver.CreateUserIfNotExistsAsync(user2.Email, false))
-                .Returns(user2);
+                .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<IUserResolver>();
+            Assert.Equal(Subject("user1"), mapped1);
-
+            Assert.Equal(Subject("user2"), mapped2);
-            A.CallTo(() => userResolver.FindByIdOrEmailAsync(user.Email))
-                .Returns(Task.FromResult<IUser?>(null));
-
-            await sut.RestoreAsync(reader, userResolver);
-
-            A.CallTo(() => userResolver.CreateUserIfNotExistsAsync(user.Email, false))
-                .MustHaveHappened();
         }
 
         [Fact]

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<IUserFactory>();
         private readonly UserManager<IdentityUser> userManager = A.Fake<UserManager<IdentityUser>>();
         private readonly DefaultUserResolver sut;
 
         public DefaultUserResolverTests()
         {
-            var userFactory = A.Fake<IUserFactory>();
-
             A.CallTo(() => userFactory.IsId(A<string>.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 (user1, claims1) = GenerateUser("id1");
-            var (user2, claims2) = GernerateUser("id2");
+            var (user2, claims2) = GenerateUser("id2");
 
             var list = new List<IdentityUser> { user1, user2 };
 
@@ -119,7 +200,7 @@ namespace Squidex.Domain.Users
                 .MustNotHaveHappened();
         }
 
-        private static (IdentityUser, List<Claim>) GernerateUser(string id)
+        private static (IdentityUser, List<Claim>) 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<Claim> claims)
+        {
+            A.CallTo(() => userManager.FindByEmailAsync(user.Email))
+                .Returns(user);
+
+            A.CallTo(() => userManager.GetClaimsAsync(user))
+                .Returns(claims);
+        }
     }
 }