Extended and updated unit tests. Put store methods into try and catch where accessing a service. Added more identity results and try/catches where needed.

Updated memberuserstore. Ignore temporary test cached files.

.gitignore

@@ -201,3 +201,4 @@ src/Umbraco.Tests/TEMP/
 
 /src/Umbraco.Web.UI/config/umbracoSettings.config
 /src/Umbraco.Web.UI.NetCore/Umbraco/models/*
+src/Umbraco.Tests.UnitTests/umbraco/Data/TEMP/

src/Umbraco.Infrastructure/Security/MemberRoleStore.cs

@@ -15,6 +15,11 @@ namespace Umbraco.Cms.Core.Security
         private readonly IMemberGroupService _memberGroupService;
         private bool _disposed;
 
+        //TODO: Move into custom error describer.
+        //TODO: How revealing can the error messages be?
+        private readonly IdentityError _intParseError = new IdentityError { Code = "IdentityIdParseError", Description = "Cannot parse ID to int" };
+        private readonly IdentityError _memberGroupNotFoundError = new IdentityError { Code = "IdentityMemberGroupNotFound", Description = "Member group not found" };
+
         public MemberRoleStore(IMemberGroupService memberGroupService, IdentityErrorDescriber errorDescriber)
         {
             _memberGroupService = memberGroupService ?? throw new ArgumentNullException(nameof(memberGroupService));
@@ -27,10 +32,13 @@ namespace Umbraco.Cms.Core.Security
         public IdentityErrorDescriber ErrorDescriber { get; set; }
 
         /// <inheritdoc />
-        public Task<IdentityResult> CreateAsync(TRole role, CancellationToken cancellationToken = new CancellationToken())
+        public Task<IdentityResult> CreateAsync(TRole role, CancellationToken cancellationToken = default)
+        {
+            try
             {
                 cancellationToken.ThrowIfCancellationRequested();
                 ThrowIfDisposed();
+
                 if (role == null)
                 {
                     throw new ArgumentNullException(nameof(role));
@@ -47,22 +55,29 @@ namespace Umbraco.Cms.Core.Security
 
                 return Task.FromResult(IdentityResult.Success);
             }
+            catch (Exception ex)
+            {
+                return Task.FromResult(IdentityResult.Failed(new IdentityError { Code = ex.Message, Description = ex.Message }));
+            }
+        }
 
 
         /// <inheritdoc />
-        public Task<IdentityResult> UpdateAsync(TRole role, CancellationToken cancellationToken = new CancellationToken())
+        public Task<IdentityResult> UpdateAsync(TRole role, CancellationToken cancellationToken = default)
+        {
+            try
             {
                 cancellationToken.ThrowIfCancellationRequested();
-            ThrowIfDisposed();
                 if (role == null)
                 {
                     throw new ArgumentNullException(nameof(role));
                 }
 
+                ThrowIfDisposed();
+
                 if (!int.TryParse(role.Id, out int roleId))
                 {
-                //TODO: what identity error should we return in this case?
-                return Task.FromResult(IdentityResult.Failed(ErrorDescriber.DefaultError()));
+                    return Task.FromResult(IdentityResult.Failed(_intParseError));
                 }
 
                 IMemberGroup memberGroup = _memberGroupService.GetById(roleId);
@@ -72,21 +87,94 @@ namespace Umbraco.Cms.Core.Security
                     {
                         _memberGroupService.Save(memberGroup);
                     }
+                    //TODO: if nothing changed, do we need to report this?
+                    return Task.FromResult(IdentityResult.Success);
                 }
                 else
                 {
-                //TODO: throw exception when not found, or return failure? And is this the correct message
-                return Task.FromResult(IdentityResult.Failed(ErrorDescriber.InvalidRoleName(role.Name)));
+                    //TODO: throw exception when not found, or return failure?
+                    return Task.FromResult(IdentityResult.Failed(_memberGroupNotFoundError));
+                }
+
+            }
+            catch (Exception ex)
+            {
+                return Task.FromResult(IdentityResult.Failed(new IdentityError { Code = ex.Message, Description = ex.Message }));
+            }
+        }
+
+        /// <inheritdoc />
+        public Task<IdentityResult> DeleteAsync(TRole role, CancellationToken cancellationToken = default)
+        {
+            try
+            {
+                cancellationToken.ThrowIfCancellationRequested();
+                ThrowIfDisposed();
+
+                if (role == null)
+                {
+                    throw new ArgumentNullException(nameof(role));
+                }
+
+                if (!int.TryParse(role.Id, out int roleId))
+                {
+                    //TODO: what identity error should we return in this case?
+                    return Task.FromResult(IdentityResult.Failed(_intParseError));
+                }
+
+                IMemberGroup memberGroup = _memberGroupService.GetById(roleId);
+                if (memberGroup != null)
+                {
+                    _memberGroupService.Delete(memberGroup);
+                }
+                else
+                {
+                    return Task.FromResult(IdentityResult.Failed(_memberGroupNotFoundError));
                 }
 
                 return Task.FromResult(IdentityResult.Success);
             }
+            catch (Exception ex)
+            {
+                return Task.FromResult(IdentityResult.Failed(new IdentityError { Code = ex.Message, Description = ex.Message }));
+            }
+        }
 
         /// <inheritdoc />
-        public Task<IdentityResult> DeleteAsync(TRole role, CancellationToken cancellationToken = new CancellationToken())
+
+        public Task<string> GetRoleIdAsync(TRole role, CancellationToken cancellationToken = default)
         {
             cancellationToken.ThrowIfCancellationRequested();
             ThrowIfDisposed();
+
+            if (role == null)
+            {
+                throw new ArgumentNullException(nameof(role));
+            }
+
+            return Task.FromResult(role.Id);
+        }
+
+        /// <inheritdoc />
+        public Task<string> GetRoleNameAsync(TRole role, CancellationToken cancellationToken = default)
+        {
+            cancellationToken.ThrowIfCancellationRequested();
+            ThrowIfDisposed();
+
+            if (role == null)
+            {
+                throw new ArgumentNullException(nameof(role));
+            }
+
+            return Task.FromResult(role.Name);
+        }
+
+        /// <inheritdoc />
+        public Task SetRoleNameAsync(TRole role, string roleName, CancellationToken cancellationToken = default)
+        {
+            cancellationToken.ThrowIfCancellationRequested();
+            ThrowIfDisposed();
+
             if (role == null)
             {
                 throw new ArgumentNullException(nameof(role));
@@ -99,94 +187,86 @@ namespace Umbraco.Cms.Core.Security
             }
 
             IMemberGroup memberGroup = _memberGroupService.GetById(roleId);
+
             if (memberGroup != null)
             {
-                _memberGroupService.Delete(memberGroup);
+                //TODO: confirm logic
+                memberGroup.Name = roleName;
+                _memberGroupService.Save(memberGroup);
+                role.Name = roleName;
             }
             else
             {
-                //TODO: throw exception when not found, or return failure? And is this the correct message
-                return Task.FromResult(IdentityResult.Failed(ErrorDescriber.InvalidRoleName(role.Name)));
+                return Task.FromResult(IdentityResult.Failed(_memberGroupNotFoundError));
             }
 
-            return Task.FromResult(IdentityResult.Success);
+            return Task.CompletedTask;
         }
 
         /// <inheritdoc />
-
-        public Task<string> GetRoleIdAsync(TRole role, CancellationToken cancellationToken)
+        public Task<string> GetNormalizedRoleNameAsync(TRole role, CancellationToken cancellationToken = default)
         {
             cancellationToken.ThrowIfCancellationRequested();
             ThrowIfDisposed();
+
             if (role == null)
             {
                 throw new ArgumentNullException(nameof(role));
             }
 
-            return Task.FromResult(role.Id);
-        }
-
-        public Task<string> GetRoleNameAsync(TRole role, CancellationToken cancellationToken)
-        {
-            cancellationToken.ThrowIfCancellationRequested();
-            ThrowIfDisposed();
-
-            if (!int.TryParse(role.Id, out int roleId))
-            {
-                return null;
-            }
-
-            IMemberGroup memberGroup = _memberGroupService.GetById(roleId);
-
-            return Task.FromResult(memberGroup?.Name);
-        }
-
-        public Task SetRoleNameAsync(TRole role, string roleName, CancellationToken cancellationToken)
-        {
-            cancellationToken.ThrowIfCancellationRequested();
-            ThrowIfDisposed();
-            return null;
-        }
-
-        public Task<string> GetNormalizedRoleNameAsync(TRole role, CancellationToken cancellationToken)
-        {
-            cancellationToken.ThrowIfCancellationRequested();
-            ThrowIfDisposed();
-            return null;
-        }
-
-        public Task SetNormalizedRoleNameAsync(TRole role, string normalizedName, CancellationToken cancellationToken)
-        {
-            cancellationToken.ThrowIfCancellationRequested();
-            ThrowIfDisposed();
-            return null;
+            //TODO: are we utilising NormalizedRoleName?
+            return Task.FromResult(role.NormalizedName);
         }
 
         /// <inheritdoc />
-        public Task<TRole> FindByIdAsync(string id, CancellationToken cancellationToken = new CancellationToken())
+        public Task SetNormalizedRoleNameAsync(TRole role, string normalizedName, CancellationToken cancellationToken = default)
         {
             cancellationToken.ThrowIfCancellationRequested();
             ThrowIfDisposed();
-            if (!int.TryParse(id, out int roleId))
+
+            if (role == null)
             {
-                return null;
+                throw new ArgumentNullException(nameof(role));
             }
 
-            IMemberGroup memberGroup = _memberGroupService.GetById(roleId);
+            //TODO: are we utilising NormalizedRoleName and do we need to set it in the memberGroupService?
+            role.NormalizedName = normalizedName;
 
+            return Task.CompletedTask;
+        }
+
+        /// <inheritdoc />
+        public Task<TRole> FindByIdAsync(string roleId, CancellationToken cancellationToken = default)
+        {
+            cancellationToken.ThrowIfCancellationRequested();
+            ThrowIfDisposed();
+
+            if (string.IsNullOrWhiteSpace(roleId))
+            {
+                throw new ArgumentNullException(nameof(roleId));
+            }
+
+            if (!int.TryParse(roleId, out int id))
+            {
+                throw new ArgumentOutOfRangeException(nameof(roleId), $"{nameof(roleId)} is not a valid Int");
+            }
+
+            IMemberGroup memberGroup = _memberGroupService.GetById(id);
             return Task.FromResult(memberGroup == null ? null : MapFromMemberGroup(memberGroup));
         }
 
         /// <inheritdoc />
-        public Task<TRole> FindByNameAsync(string name, CancellationToken cancellationToken = new CancellationToken())
+        public Task<TRole> FindByNameAsync(string name, CancellationToken cancellationToken = default)
         {
             cancellationToken.ThrowIfCancellationRequested();
             ThrowIfDisposed();
-            if (name == null)
+
+            if (string.IsNullOrWhiteSpace(name))
             {
                 throw new ArgumentNullException(nameof(name));
             }
             IMemberGroup memberGroup = _memberGroupService.GetByName(name);
+            //TODO: throw exception when not found?
 
             return Task.FromResult(memberGroup == null ? null : MapFromMemberGroup(memberGroup));
         }
@@ -227,10 +307,12 @@ namespace Umbraco.Cms.Core.Security
             return anythingChanged;
         }
 
-        public void Dispose()
-        {
-            //TODO: is any dispose action necessary here or is this all managed by the IOC container?
-        }
+        //TODO: is any dispose action necessary here?
+
+        /// <summary>
+        /// Dispose the store
+        /// </summary>
+        public void Dispose() => _disposed = true;
 
         /// <summary>
         /// Throws if this class has been disposed.

src/Umbraco.Infrastructure/Security/MemberUserStore.cs

@@ -19,7 +19,7 @@ namespace Umbraco.Cms.Core.Security
     /// <summary>
     /// A custom user store that uses Umbraco member data
     /// </summary>
-    public class MemberUserStore : UserStoreBase<MemberIdentityUser, IdentityRole<string>, string, IdentityUserClaim<string>, IdentityUserRole<string>, IdentityUserLogin<string>, IdentityUserToken<string>, IdentityRoleClaim<string>>
+    public class MemberUserStore : UserStoreBase<MemberIdentityUser, IdentityRole, string, IdentityUserClaim<string>, IdentityUserRole<string>, IdentityUserLogin<string>, IdentityUserToken<string>, IdentityRoleClaim<string>>
     {
         private readonly IMemberService _memberService;
         private readonly UmbracoMapper _mapper;
@@ -40,6 +40,7 @@ namespace Umbraco.Cms.Core.Security
             _scopeProvider = scopeProvider ?? throw new ArgumentNullException(nameof(scopeProvider));
         }
 
+        //TODO: why is this not supported?
         /// <summary>
         /// Not supported in Umbraco
         /// </summary>
@@ -55,6 +56,8 @@ namespace Umbraco.Cms.Core.Security
 
         /// <inheritdoc />
         public override Task<IdentityResult> CreateAsync(MemberIdentityUser user, CancellationToken cancellationToken = default)
+        {
+            try
             {
                 cancellationToken.ThrowIfCancellationRequested();
                 ThrowIfDisposed();
@@ -96,11 +99,19 @@ namespace Umbraco.Cms.Core.Security
                 //            x.UserData)));
                 //}
 
+
                 return Task.FromResult(IdentityResult.Success);
             }
+            catch (Exception ex)
+            {
+                return Task.FromResult(IdentityResult.Failed(new IdentityError { Code = ex.Message, Description = ex.Message }));
+            }
+        }
 
         /// <inheritdoc />
         public override Task<IdentityResult> UpdateAsync(MemberIdentityUser user, CancellationToken cancellationToken = default)
+        {
+            try
             {
                 cancellationToken.ThrowIfCancellationRequested();
                 ThrowIfDisposed();
@@ -112,7 +123,8 @@ namespace Umbraco.Cms.Core.Security
                 Attempt<int> asInt = user.Id.TryConvertTo<int>();
                 if (asInt == false)
                 {
-                throw new InvalidOperationException("The user id must be an integer to work with the Umbraco");
+                    //TODO: should this be thrown, or an identity result?
+                    throw new InvalidOperationException("The user id must be an integer to work with Umbraco");
                 }
 
                 using (IScope scope = _scopeProvider.CreateScope())
@@ -142,13 +154,20 @@ namespace Umbraco.Cms.Core.Security
                     }
 
                     scope.Complete();
-            }
 
                     return Task.FromResult(IdentityResult.Success);
                 }
+            }
+            catch (Exception ex)
+            {
+                return Task.FromResult(IdentityResult.Failed(new IdentityError { Code = ex.Message, Description = ex.Message }));
+            }
+        }
 
         /// <inheritdoc />
         public override Task<IdentityResult> DeleteAsync(MemberIdentityUser user, CancellationToken cancellationToken = default)
+        {
+            try
             {
                 cancellationToken.ThrowIfCancellationRequested();
                 ThrowIfDisposed();
@@ -168,6 +187,11 @@ namespace Umbraco.Cms.Core.Security
 
                 return Task.FromResult(IdentityResult.Success);
             }
+            catch (Exception ex)
+            {
+                return Task.FromResult(IdentityResult.Failed(new IdentityError { Code = ex.Message, Description = ex.Message }));
+            }
+        }
 
         /// <inheritdoc />
         public override Task<MemberIdentityUser> FindByIdAsync(string userId, CancellationToken cancellationToken = default) => FindUserAsync(userId, cancellationToken);
@@ -178,6 +202,16 @@ namespace Umbraco.Cms.Core.Security
             cancellationToken.ThrowIfCancellationRequested();
             ThrowIfDisposed();
 
+            if (string.IsNullOrWhiteSpace(userId))
+            {
+                throw new ArgumentNullException(nameof(userId));
+            }
+
+            if (!Guid.TryParse(userId, out Guid id))
+            {
+                throw new ArgumentOutOfRangeException(nameof(userId), $"{nameof(userId)} is not a valid x");
+            }
+
             IMember user = _memberService.GetById(UserIdToInt(userId));
             if (user == null)
             {
@@ -208,7 +242,8 @@ namespace Umbraco.Cms.Core.Security
         {
             await base.SetPasswordHashAsync(user, passwordHash, cancellationToken);
 
-            user.PasswordConfig = null; // Clear this so that it's reset at the repository level
+            // Clear this so that it's reset at the repository level
+            user.PasswordConfig = null;
             user.LastPasswordChangeDateUtc = DateTime.UtcNow;
         }
 
@@ -216,7 +251,7 @@ namespace Umbraco.Cms.Core.Security
         public override async Task<bool> HasPasswordAsync(MemberIdentityUser user, CancellationToken cancellationToken = default)
         {
             // This checks if it's null
-            var result = await base.HasPasswordAsync(user, cancellationToken);
+            bool result = await base.HasPasswordAsync(user, cancellationToken);
             if (result)
             {
                 // we also want to check empty
@@ -262,8 +297,22 @@ namespace Umbraco.Cms.Core.Security
                 throw new ArgumentNullException(nameof(login));
             }
 
+            if (string.IsNullOrWhiteSpace(login.LoginProvider))
+            {
+                throw new ArgumentNullException(nameof(login.LoginProvider));
+            }
+
+            if (string.IsNullOrWhiteSpace(login.ProviderKey))
+            {
+                throw new ArgumentNullException(nameof(login.ProviderKey));
+            }
+
             ICollection<IIdentityUserLogin> logins = user.Logins;
-            var instance = new IdentityUserLogin(login.LoginProvider, login.ProviderKey, user.Id.ToString());
+            var instance = new IdentityUserLogin(
+                login.LoginProvider,
+                login.ProviderKey,
+                user.Id.ToString());
+
             IdentityUserLogin userLogin = instance;
             logins.Add(userLogin);
 
@@ -280,6 +329,16 @@ namespace Umbraco.Cms.Core.Security
                 throw new ArgumentNullException(nameof(user));
             }
 
+            if (string.IsNullOrWhiteSpace(loginProvider))
+            {
+                throw new ArgumentNullException(nameof(loginProvider));
+            }
+
+            if (string.IsNullOrWhiteSpace(providerKey))
+            {
+                throw new ArgumentNullException(nameof(providerKey));
+            }
+
             IIdentityUserLogin userLogin = user.Logins.SingleOrDefault(l => l.LoginProvider == loginProvider && l.ProviderKey == providerKey);
             if (userLogin != null)
             {
@@ -311,21 +370,34 @@ namespace Umbraco.Cms.Core.Security
             MemberIdentityUser user = await FindUserAsync(userId, cancellationToken);
             if (user == null)
             {
-                return null;
+                //TODO: error throw or null result?
+                return await Task.FromResult((IdentityUserLogin<string>)null);
+            }
+
+            if (string.IsNullOrWhiteSpace(loginProvider))
+            {
+                throw new ArgumentNullException(nameof(loginProvider));
+            }
+
+            if (string.IsNullOrWhiteSpace(providerKey))
+            {
+                throw new ArgumentNullException(nameof(providerKey));
             }
 
             IList<UserLoginInfo> logins = await GetLoginsAsync(user, cancellationToken);
             UserLoginInfo found = logins.FirstOrDefault(x => x.ProviderKey == providerKey && x.LoginProvider == loginProvider);
             if (found == null)
             {
-                return null;
+                //TODO: error throw or null result?
+                return await Task.FromResult((IdentityUserLogin<string>)null);
             }
 
             return new IdentityUserLogin<string>
             {
                 LoginProvider = found.LoginProvider,
                 ProviderKey = found.ProviderKey,
-                ProviderDisplayName = found.ProviderDisplayName, // TODO: We don't store this value so it will be null
+                // TODO: We don't store this value so it will be null
+                ProviderDisplayName = found.ProviderDisplayName,
                 UserId = user.Id
             };
         }
@@ -336,9 +408,19 @@ namespace Umbraco.Cms.Core.Security
             cancellationToken.ThrowIfCancellationRequested();
             ThrowIfDisposed();
 
+            if (string.IsNullOrWhiteSpace(loginProvider))
+            {
+                throw new ArgumentNullException(nameof(loginProvider));
+            }
+
+            if (string.IsNullOrWhiteSpace(providerKey))
+            {
+                throw new ArgumentNullException(nameof(providerKey));
+            }
+
             var logins = new List<IIdentityUserLogin>();
 
-            // TODO: external login needed?
+            // TODO: external login needed
             //_externalLoginService.Find(loginProvider, providerKey).ToList();
             if (logins.Count == 0)
             {
@@ -350,15 +432,20 @@ namespace Umbraco.Cms.Core.Security
             {
                 LoginProvider = found.LoginProvider,
                 ProviderKey = found.ProviderKey,
-                ProviderDisplayName = null, // TODO: We don't store this value so it will be null
+                // TODO: We don't store this value so it will be null
+                ProviderDisplayName = null,
                 UserId = found.UserId
             });
         }
 
         /// <inheritdoc />
         public override Task AddToRoleAsync(MemberIdentityUser user, string role, CancellationToken cancellationToken = default)
+        {
+            if (cancellationToken != null)
             {
                 cancellationToken.ThrowIfCancellationRequested();
+            }
+
             ThrowIfDisposed();
             if (user == null)
             {
@@ -443,7 +530,7 @@ namespace Umbraco.Cms.Core.Security
         /// <summary>
         /// Returns true if a user is in the role
         /// </summary>
-        public override Task<bool> IsInRoleAsync(MemberIdentityUser user, string normalizedRoleName, CancellationToken cancellationToken = default)
+        public override Task<bool> IsInRoleAsync(MemberIdentityUser user, string roleName, CancellationToken cancellationToken = default)
         {
             cancellationToken.ThrowIfCancellationRequested();
             ThrowIfDisposed();
@@ -452,22 +539,28 @@ namespace Umbraco.Cms.Core.Security
                 throw new ArgumentNullException(nameof(user));
             }
 
-            return Task.FromResult(user.Roles.Select(x => x.RoleId).InvariantContains(normalizedRoleName));
+            if (string.IsNullOrWhiteSpace(roleName))
+            {
+                throw new ArgumentNullException(nameof(roleName));
+            }
+
+            return Task.FromResult(user.Roles.Select(x => x.RoleId).InvariantContains(roleName));
         }
 
         /// <summary>
         /// Lists all users of a given role.
         /// </summary>
-        public override Task<IList<MemberIdentityUser>> GetUsersInRoleAsync(string normalizedRoleName, CancellationToken cancellationToken = default)
+        public override Task<IList<MemberIdentityUser>> GetUsersInRoleAsync(string roleName, CancellationToken cancellationToken = default)
         {
             cancellationToken.ThrowIfCancellationRequested();
             ThrowIfDisposed();
-            if (normalizedRoleName == null)
+
+            if (string.IsNullOrWhiteSpace(roleName))
             {
-                throw new ArgumentNullException(nameof(normalizedRoleName));
+                throw new ArgumentNullException(nameof(roleName));
             }
 
-            IEnumerable<IMember> members = _memberService.GetMembersByMemberType(normalizedRoleName);
+            IEnumerable<IMember> members = _memberService.GetMembersByMemberType(roleName);
 
             IList<MemberIdentityUser> membersIdentityUsers = members.Select(x => _mapper.Map<MemberIdentityUser>(x)).ToList();
 
@@ -475,18 +568,23 @@ namespace Umbraco.Cms.Core.Security
         }
 
         /// <inheritdoc/>
-        protected override Task<IdentityRole<string>> FindRoleAsync(string normalizedRoleName, CancellationToken cancellationToken)
+        protected override Task<IdentityRole> FindRoleAsync(string roleName, CancellationToken cancellationToken)
         {
-            IMemberGroup group = _memberService.GetAllRoles().SingleOrDefault(x => x.Name == normalizedRoleName);
-            if (group == null)
+            if (string.IsNullOrWhiteSpace(roleName))
             {
-                return Task.FromResult((IdentityRole<string>)null);
+                throw new ArgumentNullException(nameof(roleName));
             }
 
-            return Task.FromResult(new IdentityRole<string>(group.Name)
+            IMemberGroup group = _memberService.GetAllRoles().SingleOrDefault(x => x.Name == roleName);
+            if (group == null)
+            {
+                return Task.FromResult((IdentityRole)null);
+            }
+
+            return Task.FromResult(new IdentityRole(group.Name)
             {
                 //TODO: what should the alias be?
-                Id = @group.Id.ToString()
+                Id = group.Id.ToString()
             });
         }
 
@@ -523,7 +621,7 @@ namespace Umbraco.Cms.Core.Security
         {
             if (user != null)
             {
-                //TODO: when to
+                //TODO: implement
                 //user.SetLoginsCallback(new Lazy<IEnumerable<IIdentityUserLogin>>(() => _externalLoginService.GetAll(UserIdToInt(user.Id))));
             }
 

src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberRoleStoreTests.cs

@@ -247,7 +247,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Security
 
             // assert
             Assert.IsTrue(identityResult.Succeeded == false);
-            Assert.IsTrue(identityResult.Errors.Any(x=>x.Code == "InvalidRoleName" && x.Description == "Role name 'testname' is invalid."));
+            Assert.IsTrue(identityResult.Errors.Any(x => x.Code == "InvalidRoleName" && x.Description == "Role name 'testname' is invalid."));
             _mockMemberGroupService.Verify(x => x.GetById(777));
             _mockMemberGroupService.VerifyNoOtherCalls();
         }
@@ -257,23 +257,23 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Security
         {
             // arrange
             MemberRoleStore<IdentityRole> sut = CreateSut();
-            var fakeRole = new IdentityRole<string>("fakeGroupName")
+            var fakeRole = new IdentityRole("fakeGroupName")
             {
                 Id = "777"
             };
             int fakeRoleId = 777;
 
-            IMemberGroup mockMemberGroup = Mock.Of<IMemberGroup>(m =>
-                m.Name == "fakeGroupName" &&
-                m.CreatorId == 123 &&
-                m.Id == 777);
+            IMemberGroup fakeMemberGroup = new MemberGroup()
+            {
+                Name = "fakeGroupName",
+                CreatorId = 123,
+                Id = 777
+            };
 
-            var fakeCancellationToken = new CancellationToken() { };
-
-            _mockMemberGroupService.Setup(x => x.GetById(fakeRoleId)).Returns(mockMemberGroup);
+            _mockMemberGroupService.Setup(x => x.GetById(fakeRoleId)).Returns(fakeMemberGroup);
 
             // act
-            IdentityRole actual = await sut.FindByIdAsync(fakeRole.Id, fakeCancellationToken);
+            IdentityRole actual = await sut.FindByIdAsync(fakeRole.Id);
 
             // assert
             Assert.AreEqual(fakeRole.Name, actual.Name);
@@ -379,10 +379,6 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Security
             // act
             Task<string> actual = sut.GetRoleIdAsync(fakeRole, fakeCancellationToken);
 
-            // assert
-            Assert.That(actual, Throws.ArgumentNullException);
-
-
             // assert
             Assert.AreEqual(fakeRoleId, actual.Result);
         }