diff --git a/src/Microsoft.AspNet.Identity/UserManager.cs b/src/Microsoft.AspNet.Identity/UserManager.cs index a40f1213ed..5456ad98a9 100644 --- a/src/Microsoft.AspNet.Identity/UserManager.cs +++ b/src/Microsoft.AspNet.Identity/UserManager.cs @@ -205,7 +205,6 @@ namespace Microsoft.AspNet.Identity /// true if the backing user store supports user emails, otherwise false. /// public virtual bool SupportsUserEmail - { get { @@ -1032,8 +1031,7 @@ namespace Microsoft.AspNet.Identity throw new ArgumentNullException("user"); } - var userRoles = await userRoleStore.GetRolesAsync(user, CancellationToken); - if (userRoles.Contains(role)) + if (await userRoleStore.IsInRoleAsync(user, role, CancellationToken)) { return await UserAlreadyInRoleError(user, role); } @@ -1063,10 +1061,9 @@ namespace Microsoft.AspNet.Identity throw new ArgumentNullException("roles"); } - var userRoles = await userRoleStore.GetRolesAsync(user, CancellationToken); - foreach (var role in roles) + foreach (var role in roles.Distinct()) { - if (userRoles.Contains(role)) + if (await userRoleStore.IsInRoleAsync(user, role, CancellationToken)) { return await UserAlreadyInRoleError(user, role); } diff --git a/test/Microsoft.AspNet.Identity.Test/UserManagerTest.cs b/test/Microsoft.AspNet.Identity.Test/UserManagerTest.cs index 07bddee229..c63f1ac7a4 100644 --- a/test/Microsoft.AspNet.Identity.Test/UserManagerTest.cs +++ b/test/Microsoft.AspNet.Identity.Test/UserManagerTest.cs @@ -270,7 +270,7 @@ namespace Microsoft.AspNet.Identity.Test // Setup var store = new Mock>(); var user = new TestUser { UserName = "Foo" }; - var roles = new string[] { "A", "B", "C" }; + var roles = new string[] { "A", "B", "C", "C" }; store.Setup(s => s.AddToRoleAsync(user, "A", CancellationToken.None)) .Returns(Task.FromResult(0)) .Verifiable(); @@ -280,8 +280,17 @@ namespace Microsoft.AspNet.Identity.Test store.Setup(s => s.AddToRoleAsync(user, "C", CancellationToken.None)) .Returns(Task.FromResult(0)) .Verifiable(); + store.Setup(s => s.UpdateAsync(user, CancellationToken.None)).ReturnsAsync(IdentityResult.Success).Verifiable(); - store.Setup(s => s.GetRolesAsync(user, CancellationToken.None)).ReturnsAsync(new List()).Verifiable(); + store.Setup(s => s.IsInRoleAsync(user, "A", CancellationToken.None)) + .Returns(Task.FromResult(false)) + .Verifiable(); + store.Setup(s => s.IsInRoleAsync(user, "B", CancellationToken.None)) + .Returns(Task.FromResult(false)) + .Verifiable(); + store.Setup(s => s.IsInRoleAsync(user, "C", CancellationToken.None)) + .Returns(Task.FromResult(false)) + .Verifiable(); var userManager = MockHelpers.TestUserManager(store.Object); // Act @@ -290,6 +299,7 @@ namespace Microsoft.AspNet.Identity.Test // Assert Assert.True(result.Succeeded); store.VerifyAll(); + store.Verify(s => s.AddToRoleAsync(user, "C", CancellationToken.None), Times.Once()); } [Fact] @@ -302,8 +312,10 @@ namespace Microsoft.AspNet.Identity.Test store.Setup(s => s.AddToRoleAsync(user, "A", CancellationToken.None)) .Returns(Task.FromResult(0)) .Verifiable(); - store.Setup(s => s.GetRolesAsync(user, CancellationToken.None)).ReturnsAsync(new List { "B" }).Verifiable(); - var userManager = MockHelpers.TestUserManager(store.Object); + store.Setup(s => s.IsInRoleAsync(user, "B", CancellationToken.None)) + .Returns(Task.FromResult(true)) + .Verifiable(); + var userManager = MockHelpers.TestUserManager(store.Object); // Act var result = await userManager.AddToRolesAsync(user, roles); diff --git a/test/Shared/UserManagerTestBase.cs b/test/Shared/UserManagerTestBase.cs index c864feec6d..fad54c739f 100644 --- a/test/Shared/UserManagerTestBase.cs +++ b/test/Shared/UserManagerTestBase.cs @@ -1405,6 +1405,23 @@ namespace Microsoft.AspNet.Identity.Test IdentityResultAssert.VerifyLogMessage(userMgr.Logger, $"User {await userMgr.GetUserIdAsync(user)} is already in role {roleName}."); } + [ConditionalTheory] + [FrameworkSkipCondition(RuntimeFrameworks.Mono)] + public async Task AddUserToRolesIgnoresDuplicates() + { + var context = CreateTestContext(); + var userMgr = CreateManager(context); + var roleMgr = CreateRoleManager(context); + var roleName = "addUserDupeTest" + Guid.NewGuid().ToString(); + var role = CreateTestRole(roleName, useRoleNamePrefixAsRoleName: true); + var user = CreateTestUser(); + IdentityResultAssert.IsSuccess(await userMgr.CreateAsync(user)); + IdentityResultAssert.IsSuccess(await roleMgr.CreateAsync(role)); + Assert.False(await userMgr.IsInRoleAsync(user, roleName)); + IdentityResultAssert.IsSuccess(await userMgr.AddToRolesAsync(user, new[] { roleName, roleName })); + Assert.True(await userMgr.IsInRoleAsync(user, roleName)); + } + [ConditionalTheory] [FrameworkSkipCondition(RuntimeFrameworks.Mono)] public async Task CanFindRoleByNameWithManager()