_userManager.IsInRoleAsync is always false about aspnetcore.identity.mongodbcore

[alexandre-spieser]

Hi,
Thanks for your message.
I have run some tests and it seems like adding a claim is different than adding a role in the specs tested by Microsoft.
I have the following integrations tests that pass:

        /// <summary>
        /// Test.
        /// </summary>
        /// <returns>Task</returns>
        [Fact]
        public async Task AddUserToRoleFailsIfAlreadyInRole()
        {
            if (ShouldSkipDbTests())
            {
                return;
            }
            var userMgr = CreateManager();
            var roleMgr = CreateRoleManager();
            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));
            IdentityResultAssert.IsSuccess(await userMgr.AddToRoleAsync(user, roleName));
            Assert.True(await userMgr.IsInRoleAsync(user, roleName)); // user is indeed in Role
            IdentityResultAssert.IsFailure(await userMgr.AddToRoleAsync(user, roleName), _errorDescriber.UserAlreadyInRole(roleName));
            IdentityResultAssert.VerifyLogMessage(userMgr.Logger, $"User {await userMgr.GetUserIdAsync(user)} is already in role {roleName}.");
        }

        /// <summary>
        /// Test.
        /// </summary>
        /// <returns>Task</returns>
        [Fact]
        public async Task RemoveUserNotInRoleFails()
        {
            if (ShouldSkipDbTests())
            {
                return;
            }
            var userMgr = CreateManager();
            var roleMgr = CreateRoleManager();
            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));
            var result = await userMgr.RemoveFromRoleAsync(user, roleName);
            IdentityResultAssert.IsFailure(result, _errorDescriber.UserNotInRole(roleName));
            IdentityResultAssert.VerifyLogMessage(userMgr.Logger, $"User {await userMgr.GetUserIdAsync(user)} is not in role {roleName}.");
        }

The code for those test is in this file:
https://github.com/alexandre-spieser/AspNetCore.Identity.MongoDbCore/blob/master/test/AspNetCore.Identity.MongoDbCore.IntegrationTests/Specification/IdentitySpecificationTestBase.cs

Adding a claim of type role will add it to the claims lists, instead of the roles list. This case was not tested by Microsoft and indeed it fails.
The can however validate that the following code works:

           await userMgr.AddToRoleAsync(user, roleName);
            // pull the user from the DB again
            var newUser = await userMgr.FindByNameAsync(user.UserName);
            Assert.True(await userMgr.IsInRoleAsync(newUser, roleName));

[alexandre-spieser]

The UserManager implementation of AddClaimAsync does this:

        /// <summary>
        /// Adds the specified <paramref name="claim"/> to the <paramref name="user"/>.
        /// </summary>
        /// <param name="user">The user to add the claim to.</param>
        /// <param name="claim">The claim to add.</param>
        /// <returns>
        /// The <see cref="Task"/> that represents the asynchronous operation, containing the <see cref="IdentityResult"/>
        /// of the operation.
        /// </returns>
        public virtual Task<IdentityResult> AddClaimAsync(TUser user, Claim claim)
        {
            ThrowIfDisposed();
            var claimStore = GetClaimStore();
            if (claim == null)
            {
                throw new ArgumentNullException(nameof(claim));
            }
            if (user == null)
            {
                throw new ArgumentNullException(nameof(user));
            }
            return AddClaimsAsync(user, new Claim[] { claim });
        }

(https://github.com/aspnet/Identity/blob/master/src/Core/UserManager.cs)

The AddClaimsAsync async in the MongoUserStore does this:

        /// <summary>
        /// Adds the <paramref name="claims"/> given to the specified <paramref name="user"/>.
        /// </summary>
        /// <param name="user">The user to add the claim to.</param>
        /// <param name="claims">The claim to add to the user.</param>
        /// <param name="cancellationToken">The <see cref="CancellationToken"/> used to propagate notifications that the operation should be canceled.</param>
        /// <returns>The <see cref="Task"/> that represents the asynchronous operation.</returns>
        public override Task AddClaimsAsync(TUser user, IEnumerable<Claim> claims, CancellationToken cancellationToken = default(CancellationToken))
        {
            ThrowIfDisposed();
            if (user == null)
            {
                throw new ArgumentNullException(nameof(user));
            }
            if (claims == null)
            {
                throw new ArgumentNullException(nameof(claims));
            }
            var addedSome = false;
            foreach (var claim in claims)
            {
                if (user.AddClaim(claim))
                {
                    addedSome |= true;
                }
            }
            if (addedSome)
            {
                var success = MongoRepository.UpdateOne<TUser, TKey, List<MongoClaim>>(user, p => p.Claims, user.Claims);
                if (!success)
                {
                    throw new Exception($"Failed to add claims to user {user.Id.ToString()}");
                }
            }
            return Task.FromResult(false);
        }

The EF UserStore written by Microsoft does this:

        /// <summary>
        /// Adds the <paramref name="claims"/> given to the specified <paramref name="user"/>.
        /// </summary>
        /// <param name="user">The user to add the claim to.</param>
        /// <param name="claims">The claim to add to the user.</param>
        /// <param name="cancellationToken">The <see cref="CancellationToken"/> used to propagate notifications that the operation should be canceled.</param>
        /// <returns>The <see cref="Task"/> that represents the asynchronous operation.</returns>
        public override Task AddClaimsAsync(TUser user, IEnumerable<Claim> claims, CancellationToken cancellationToken = default(CancellationToken))
        {
            ThrowIfDisposed();
            if (user == null)
            {
                throw new ArgumentNullException(nameof(user));
            }
            if (claims == null)
            {
                throw new ArgumentNullException(nameof(claims));
            }
            foreach (var claim in claims)
            {
                UserClaims.Add(CreateUserClaim(user, claim));
            }
            return Task.FromResult(false);
        }

(https://github.com/aspnet/Identity/blob/master/src/EF/UserStore.cs).

It seems like this is not a bug and that in Identity, UserClaims and UserRoles are different data sets.

[alexandre-spieser]

Also worth noting that in your example you pass the RoleId to check the role.

        /// <summary>
        /// Returns a flag indicating whether the specified <paramref name="user"/> is a member of the give named role.
        /// </summary>
        /// <param name="user">The user whose role membership should be checked.</param>
        /// <param name="role">The name of the role to be checked.</param>
        /// <returns>
        /// The <see cref="Task"/> that represents the asynchronous operation, containing a flag indicating whether the specified <paramref name="user"/> is
        /// a member of the named role.
        /// </returns>
        public virtual async Task<bool> IsInRoleAsync(TUser user, string role)

The param should be the Name of the Role, not the Role Id.

I hope this helps!

[DamienDoumer]

Thanks for your answers, I changed the way I used the library based on what you said.

So, I first added an existing user to a role this way:

var res = await _userManager.AddToRoleAsync(user, roleAddModel.RoleName);

so, I tested if a user is in a role this way: And it still returned false. though the user has that role in the database.

var r = await _userManager.IsInRoleAsync(user, roleAddModel.RoleName);

And after that, I tried removing the user from the role this way:

var res = await _userManager.RemoveFromRoleAsync(user, roleAddModel.RoleName);

But I got this error:

{ "code": "UserNotInRole", "description": "User is not in role 'AdminUserRole'." }

[alexandre-spieser]

That's weird, can you validate that you are using the default key normalizer ?

https://github.com/aspnet/Identity/search?q=%3A+ILookupNormalizer&unscoped_q=%3A+ILookupNormalizer

it should be: UpperInvariantLookupNormalizer, in your example AdminUserRole might not be the normalized value stored.
Can you check that the values are normalized before being saved?

You are definitely using Identity 2.0 right ? Identity 3.0 is not supported by this package.

[alexandre-spieser]

If you still have this problem, can you give me some code for me to reproduce the issue?
Also worth noting: If you are on a replica set, you might have syncing issues where data written to the primary might not have synced to the secondary if you read from it very quickly after writing.

[DamienDoumer]

Hi, appologies for this issue. I went through and discovered the package I was using was not the one from this repo, but another one with a very similar name. https://github.com/matteofabbri/AspNetCore.Identity.Mongo I got confused by the name and opened a PR on your repo instead.

[alexandre-spieser]

👍 Merci pour l'info!