diff --git a/.gitignore b/.gitignore
index 0cffac8343..f8a2ce511f 100644
--- a/.gitignore
+++ b/.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/
diff --git a/src/Umbraco.Infrastructure/Security/MemberRoleStore.cs b/src/Umbraco.Infrastructure/Security/MemberRoleStore.cs
index 0b91237af9..4477c4870c 100644
--- a/src/Umbraco.Infrastructure/Security/MemberRoleStore.cs
+++ b/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));
@@ -25,99 +30,123 @@ namespace Umbraco.Cms.Core.Security
/// Gets or sets the for any error that occurred with the current operation.
///
public IdentityErrorDescriber ErrorDescriber { get; set; }
-
- ///
- public Task CreateAsync(TRole role, CancellationToken cancellationToken = new CancellationToken())
- {
- cancellationToken.ThrowIfCancellationRequested();
- ThrowIfDisposed();
- if (role == null)
- {
- throw new ArgumentNullException(nameof(role));
- }
-
- var memberGroup = new MemberGroup
- {
- Name = role.Name
- };
-
- _memberGroupService.Save(memberGroup);
-
- role.Id = memberGroup.Id.ToString();
-
- return Task.FromResult(IdentityResult.Success);
- }
-
///
- public Task UpdateAsync(TRole role, CancellationToken cancellationToken = new CancellationToken())
+ public Task CreateAsync(TRole role, CancellationToken cancellationToken = default)
{
- cancellationToken.ThrowIfCancellationRequested();
- ThrowIfDisposed();
- if (role == null)
+ try
{
- throw new ArgumentNullException(nameof(role));
- }
+ cancellationToken.ThrowIfCancellationRequested();
+ 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()));
- }
-
- IMemberGroup memberGroup = _memberGroupService.GetById(roleId);
- if (memberGroup != null)
- {
- if (MapToMemberGroup(role, memberGroup))
+ if (role == null)
{
- _memberGroupService.Save(memberGroup);
+ throw new ArgumentNullException(nameof(role));
}
- }
- 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.Success);
+ var memberGroup = new MemberGroup
+ {
+ Name = role.Name
+ };
+
+ _memberGroupService.Save(memberGroup);
+
+ role.Id = memberGroup.Id.ToString();
+
+ return Task.FromResult(IdentityResult.Success);
+ }
+ catch (Exception ex)
+ {
+ return Task.FromResult(IdentityResult.Failed(new IdentityError { Code = ex.Message, Description = ex.Message }));
+ }
+ }
+
+
+ ///
+ public Task UpdateAsync(TRole role, CancellationToken cancellationToken = default)
+ {
+ try
+ {
+ cancellationToken.ThrowIfCancellationRequested();
+ if (role == null)
+ {
+ throw new ArgumentNullException(nameof(role));
+ }
+
+ ThrowIfDisposed();
+
+ if (!int.TryParse(role.Id, out int roleId))
+ {
+ return Task.FromResult(IdentityResult.Failed(_intParseError));
+ }
+
+ IMemberGroup memberGroup = _memberGroupService.GetById(roleId);
+ if (memberGroup != null)
+ {
+ if (MapToMemberGroup(role, memberGroup))
+ {
+ _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?
+ return Task.FromResult(IdentityResult.Failed(_memberGroupNotFoundError));
+ }
+
+ }
+ catch (Exception ex)
+ {
+ return Task.FromResult(IdentityResult.Failed(new IdentityError { Code = ex.Message, Description = ex.Message }));
+ }
}
///
- public Task DeleteAsync(TRole role, CancellationToken cancellationToken = new CancellationToken())
+ public Task DeleteAsync(TRole role, CancellationToken cancellationToken = default)
{
- cancellationToken.ThrowIfCancellationRequested();
- ThrowIfDisposed();
- if (role == null)
+ try
{
- throw new ArgumentNullException(nameof(role));
- }
+ cancellationToken.ThrowIfCancellationRequested();
+ 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()));
- }
+ if (role == null)
+ {
+ throw new ArgumentNullException(nameof(role));
+ }
- IMemberGroup memberGroup = _memberGroupService.GetById(roleId);
- if (memberGroup != null)
- {
- _memberGroupService.Delete(memberGroup);
- }
- 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)));
- }
+ if (!int.TryParse(role.Id, out int roleId))
+ {
+ //TODO: what identity error should we return in this case?
+ return Task.FromResult(IdentityResult.Failed(_intParseError));
+ }
- return Task.FromResult(IdentityResult.Success);
+ 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 }));
+ }
}
///
- public Task GetRoleIdAsync(TRole role, CancellationToken cancellationToken)
+ public Task GetRoleIdAsync(TRole role, CancellationToken cancellationToken = default)
{
cancellationToken.ThrowIfCancellationRequested();
ThrowIfDisposed();
+
if (role == null)
{
throw new ArgumentNullException(nameof(role));
@@ -126,67 +155,118 @@ namespace Umbraco.Cms.Core.Security
return Task.FromResult(role.Id);
}
- public Task GetRoleNameAsync(TRole role, CancellationToken cancellationToken)
+ ///
+ public Task GetRoleNameAsync(TRole role, CancellationToken cancellationToken = default)
{
cancellationToken.ThrowIfCancellationRequested();
ThrowIfDisposed();
- if (!int.TryParse(role.Id, out int roleId))
+ if (role == null)
{
- return null;
+ throw new ArgumentNullException(nameof(role));
}
- 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 GetNormalizedRoleNameAsync(TRole role, CancellationToken cancellationToken)
- {
- cancellationToken.ThrowIfCancellationRequested();
- ThrowIfDisposed();
- return null;
- }
-
- public Task SetNormalizedRoleNameAsync(TRole role, string normalizedName, CancellationToken cancellationToken)
- {
- cancellationToken.ThrowIfCancellationRequested();
- ThrowIfDisposed();
- return null;
+ return Task.FromResult(role.Name);
}
///
- public Task FindByIdAsync(string id, CancellationToken cancellationToken = new CancellationToken())
+ public Task SetRoleNameAsync(TRole role, string roleName, CancellationToken cancellationToken = default)
{
cancellationToken.ThrowIfCancellationRequested();
ThrowIfDisposed();
- if (!int.TryParse(id, out int roleId))
+
+ if (role == null)
{
- return 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(ErrorDescriber.DefaultError()));
}
IMemberGroup memberGroup = _memberGroupService.GetById(roleId);
+ if (memberGroup != null)
+ {
+ //TODO: confirm logic
+ memberGroup.Name = roleName;
+ _memberGroupService.Save(memberGroup);
+ role.Name = roleName;
+ }
+ else
+ {
+ return Task.FromResult(IdentityResult.Failed(_memberGroupNotFoundError));
+ }
+
+ return Task.CompletedTask;
+ }
+
+ ///
+ public Task GetNormalizedRoleNameAsync(TRole role, CancellationToken cancellationToken = default)
+ {
+ cancellationToken.ThrowIfCancellationRequested();
+ ThrowIfDisposed();
+
+ if (role == null)
+ {
+ throw new ArgumentNullException(nameof(role));
+ }
+
+ //TODO: are we utilising NormalizedRoleName?
+ return Task.FromResult(role.NormalizedName);
+ }
+
+ ///
+ public Task SetNormalizedRoleNameAsync(TRole role, string normalizedName, CancellationToken cancellationToken = default)
+ {
+ cancellationToken.ThrowIfCancellationRequested();
+ ThrowIfDisposed();
+
+ if (role == null)
+ {
+ throw new ArgumentNullException(nameof(role));
+ }
+
+ //TODO: are we utilising NormalizedRoleName and do we need to set it in the memberGroupService?
+ role.NormalizedName = normalizedName;
+
+ return Task.CompletedTask;
+ }
+
+ ///
+ public Task 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));
}
///
- public Task FindByNameAsync(string name, CancellationToken cancellationToken = new CancellationToken())
+ public Task 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?
+
+ ///
+ /// Dispose the store
+ ///
+ public void Dispose() => _disposed = true;
///
/// Throws if this class has been disposed.
diff --git a/src/Umbraco.Infrastructure/Security/MemberUserStore.cs b/src/Umbraco.Infrastructure/Security/MemberUserStore.cs
index 17a45764ad..1d871e4134 100644
--- a/src/Umbraco.Infrastructure/Security/MemberUserStore.cs
+++ b/src/Umbraco.Infrastructure/Security/MemberUserStore.cs
@@ -19,7 +19,7 @@ namespace Umbraco.Cms.Core.Security
///
/// A custom user store that uses Umbraco member data
///
- public class MemberUserStore : UserStoreBase, string, IdentityUserClaim, IdentityUserRole, IdentityUserLogin, IdentityUserToken, IdentityRoleClaim>
+ public class MemberUserStore : UserStoreBase, IdentityUserRole, IdentityUserLogin, IdentityUserToken, IdentityRoleClaim>
{
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?
///
/// Not supported in Umbraco
///
@@ -56,117 +57,140 @@ namespace Umbraco.Cms.Core.Security
///
public override Task CreateAsync(MemberIdentityUser user, CancellationToken cancellationToken = default)
{
- cancellationToken.ThrowIfCancellationRequested();
- ThrowIfDisposed();
- if (user == null)
+ try
{
- throw new ArgumentNullException(nameof(user));
+ cancellationToken.ThrowIfCancellationRequested();
+ ThrowIfDisposed();
+ if (user == null)
+ {
+ throw new ArgumentNullException(nameof(user));
+ }
+
+ // create member
+ IMember memberEntity = _memberService.CreateMember(
+ user.UserName,
+ user.Email,
+ user.Name.IsNullOrWhiteSpace() ? user.UserName : user.Name,
+ user.MemberTypeAlias.IsNullOrWhiteSpace() ? Constants.Security.DefaultMemberTypeAlias : user.MemberTypeAlias);
+
+ UpdateMemberProperties(memberEntity, user);
+
+ // create the member
+ _memberService.Save(memberEntity);
+
+ if (!memberEntity.HasIdentity)
+ {
+ throw new DataException("Could not create the member, check logs for details");
+ }
+
+ // re-assign id
+ user.Id = UserIdToString(memberEntity.Id);
+
+ // [from backofficeuser] we have to remember whether Logins property is dirty, since the UpdateMemberProperties will reset it.
+ // var isLoginsPropertyDirty = user.IsPropertyDirty(nameof(MembersIdentityUser.Logins));
+ // TODO: confirm re externallogins implementation
+ //if (isLoginsPropertyDirty)
+ //{
+ // _externalLoginService.Save(
+ // user.Id,
+ // user.Logins.Select(x => new ExternalLogin(
+ // x.LoginProvider,
+ // x.ProviderKey,
+ // x.UserData)));
+ //}
+
+
+ return Task.FromResult(IdentityResult.Success);
}
-
- // create member
- IMember memberEntity = _memberService.CreateMember(
- user.UserName,
- user.Email,
- user.Name.IsNullOrWhiteSpace() ? user.UserName : user.Name,
- user.MemberTypeAlias.IsNullOrWhiteSpace() ? Constants.Security.DefaultMemberTypeAlias : user.MemberTypeAlias);
-
- UpdateMemberProperties(memberEntity, user);
-
- // create the member
- _memberService.Save(memberEntity);
-
- if (!memberEntity.HasIdentity)
+ catch (Exception ex)
{
- throw new DataException("Could not create the member, check logs for details");
+ return Task.FromResult(IdentityResult.Failed(new IdentityError { Code = ex.Message, Description = ex.Message }));
}
-
- // re-assign id
- user.Id = UserIdToString(memberEntity.Id);
-
- // [from backofficeuser] we have to remember whether Logins property is dirty, since the UpdateMemberProperties will reset it.
- // var isLoginsPropertyDirty = user.IsPropertyDirty(nameof(MembersIdentityUser.Logins));
- // TODO: confirm re externallogins implementation
- //if (isLoginsPropertyDirty)
- //{
- // _externalLoginService.Save(
- // user.Id,
- // user.Logins.Select(x => new ExternalLogin(
- // x.LoginProvider,
- // x.ProviderKey,
- // x.UserData)));
- //}
-
- return Task.FromResult(IdentityResult.Success);
}
///
public override Task UpdateAsync(MemberIdentityUser user, CancellationToken cancellationToken = default)
{
- cancellationToken.ThrowIfCancellationRequested();
- ThrowIfDisposed();
- if (user == null)
+ try
{
- throw new ArgumentNullException(nameof(user));
- }
-
- Attempt asInt = user.Id.TryConvertTo();
- if (asInt == false)
- {
- throw new InvalidOperationException("The user id must be an integer to work with the Umbraco");
- }
-
- using (IScope scope = _scopeProvider.CreateScope())
- {
- IMember found = _memberService.GetById(asInt.Result);
- if (found != null)
+ cancellationToken.ThrowIfCancellationRequested();
+ ThrowIfDisposed();
+ if (user == null)
{
- // we have to remember whether Logins property is dirty, since the UpdateMemberProperties will reset it.
- var isLoginsPropertyDirty = user.IsPropertyDirty(nameof(MemberIdentityUser.Logins));
-
- if (UpdateMemberProperties(found, user))
- {
- _memberService.Save(found);
- }
-
- // TODO: when to implement external login service?
-
- //if (isLoginsPropertyDirty)
- //{
- // _externalLoginService.Save(
- // found.Id,
- // user.Logins.Select(x => new ExternalLogin(
- // x.LoginProvider,
- // x.ProviderKey,
- // x.UserData)));
- //}
+ throw new ArgumentNullException(nameof(user));
}
- scope.Complete();
- }
+ Attempt asInt = user.Id.TryConvertTo();
+ if (asInt == false)
+ {
+ //TODO: should this be thrown, or an identity result?
+ throw new InvalidOperationException("The user id must be an integer to work with Umbraco");
+ }
- return Task.FromResult(IdentityResult.Success);
+ using (IScope scope = _scopeProvider.CreateScope())
+ {
+ IMember found = _memberService.GetById(asInt.Result);
+ if (found != null)
+ {
+ // we have to remember whether Logins property is dirty, since the UpdateMemberProperties will reset it.
+ var isLoginsPropertyDirty = user.IsPropertyDirty(nameof(MemberIdentityUser.Logins));
+
+ if (UpdateMemberProperties(found, user))
+ {
+ _memberService.Save(found);
+ }
+
+ // TODO: when to implement external login service?
+
+ //if (isLoginsPropertyDirty)
+ //{
+ // _externalLoginService.Save(
+ // found.Id,
+ // user.Logins.Select(x => new ExternalLogin(
+ // x.LoginProvider,
+ // x.ProviderKey,
+ // x.UserData)));
+ //}
+ }
+
+ scope.Complete();
+
+ return Task.FromResult(IdentityResult.Success);
+ }
+ }
+ catch (Exception ex)
+ {
+ return Task.FromResult(IdentityResult.Failed(new IdentityError { Code = ex.Message, Description = ex.Message }));
+ }
}
///
public override Task DeleteAsync(MemberIdentityUser user, CancellationToken cancellationToken = default)
{
- cancellationToken.ThrowIfCancellationRequested();
- ThrowIfDisposed();
- if (user == null)
+ try
{
- throw new ArgumentNullException(nameof(user));
- }
+ cancellationToken.ThrowIfCancellationRequested();
+ ThrowIfDisposed();
+ if (user == null)
+ {
+ throw new ArgumentNullException(nameof(user));
+ }
- IMember found = _memberService.GetById(UserIdToInt(user.Id));
- if (found != null)
+ IMember found = _memberService.GetById(UserIdToInt(user.Id));
+ if (found != null)
+ {
+ _memberService.Delete(found);
+ }
+
+ // TODO: when to implement external login service?
+ //_externalLoginService.DeleteUserLogins(UserIdToInt(user.Id));
+
+ return Task.FromResult(IdentityResult.Success);
+ }
+ catch (Exception ex)
{
- _memberService.Delete(found);
+ return Task.FromResult(IdentityResult.Failed(new IdentityError { Code = ex.Message, Description = ex.Message }));
}
-
- // TODO: when to implement external login service?
- //_externalLoginService.DeleteUserLogins(UserIdToInt(user.Id));
-
- return Task.FromResult(IdentityResult.Success);
}
///
@@ -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 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 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)null);
+ }
+
+ if (string.IsNullOrWhiteSpace(loginProvider))
+ {
+ throw new ArgumentNullException(nameof(loginProvider));
+ }
+
+ if (string.IsNullOrWhiteSpace(providerKey))
+ {
+ throw new ArgumentNullException(nameof(providerKey));
}
IList 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)null);
}
return new IdentityUserLogin
{
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();
- // TODO: external login needed?
+ // TODO: external login needed
//_externalLoginService.Find(loginProvider, providerKey).ToList();
if (logins.Count == 0)
{
@@ -350,7 +432,8 @@ 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
});
}
@@ -358,7 +441,11 @@ namespace Umbraco.Cms.Core.Security
///
public override Task AddToRoleAsync(MemberIdentityUser user, string role, CancellationToken cancellationToken = default)
{
- cancellationToken.ThrowIfCancellationRequested();
+ if (cancellationToken != null)
+ {
+ cancellationToken.ThrowIfCancellationRequested();
+ }
+
ThrowIfDisposed();
if (user == null)
{
@@ -443,7 +530,7 @@ namespace Umbraco.Cms.Core.Security
///
/// Returns true if a user is in the role
///
- public override Task IsInRoleAsync(MemberIdentityUser user, string normalizedRoleName, CancellationToken cancellationToken = default)
+ public override Task 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));
}
///
/// Lists all users of a given role.
///
- public override Task> GetUsersInRoleAsync(string normalizedRoleName, CancellationToken cancellationToken = default)
+ public override Task> 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 members = _memberService.GetMembersByMemberType(normalizedRoleName);
+ IEnumerable members = _memberService.GetMembersByMemberType(roleName);
IList membersIdentityUsers = members.Select(x => _mapper.Map(x)).ToList();
@@ -475,18 +568,23 @@ namespace Umbraco.Cms.Core.Security
}
///
- protected override Task> FindRoleAsync(string normalizedRoleName, CancellationToken cancellationToken)
+ protected override Task 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)null);
+ throw new ArgumentNullException(nameof(roleName));
}
- return Task.FromResult(new IdentityRole(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>(() => _externalLoginService.GetAll(UserIdToInt(user.Id))));
}
diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberRoleStoreTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberRoleStoreTests.cs
index ed77c63afc..3b78bc1b39 100644
--- a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberRoleStoreTests.cs
+++ b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberRoleStoreTests.cs
@@ -162,7 +162,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Security
Name = "testname"
};
var fakeCancellationToken = new CancellationToken() { };
-
+
// act
IdentityResult identityResult = await sut.UpdateAsync(fakeRole, fakeCancellationToken);
@@ -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 sut = CreateSut();
- var fakeRole = new IdentityRole("fakeGroupName")
+ var fakeRole = new IdentityRole("fakeGroupName")
{
Id = "777"
};
int fakeRoleId = 777;
- IMemberGroup mockMemberGroup = Mock.Of(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);
@@ -293,7 +293,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Security
Name = "testname"
};
var fakeCancellationToken = new CancellationToken() { };
-
+
// act
Task actual = sut.FindByIdAsync(fakeRole.Id, fakeCancellationToken);
@@ -342,7 +342,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Security
// act
Action actual = () => sut.FindByNameAsync(fakeRole.Name, fakeCancellationToken);
-
+
// assert
Assert.That(actual, Throws.ArgumentNullException);
_mockMemberGroupService.VerifyNoOtherCalls();
@@ -379,10 +379,6 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Security
// act
Task actual = sut.GetRoleIdAsync(fakeRole, fakeCancellationToken);
- // assert
- Assert.That(actual, Throws.ArgumentNullException);
-
-
// assert
Assert.AreEqual(fakeRoleId, actual.Result);
}