yv01p/Umbraco-CMS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

V14: Move towards get guid (#15889)

Browse Source 
* Implement using keymap for member

* Remove current usages of GetUserById

* User userId resolver to resolve user key

* Refactor user repository to use GUID not int

* Add happy path test

* Remove user in cache when user gets updated

* Use await in async method

* Fix up according to review

* Update IMetricsConsentService.cs to have async method

* Fix according to review

* Fix more according to comments

* Revert "Fix up according to review"

This reverts commit a75acaaa

* Get current backoffice user from method

* Update user repository delete functionality

* Fix up more test

* Try to get user by id if key fails

* Add user key as required claim

* Fix tests

* Don't set claim in BackofficeController

* Create constant for the Sub claim

---------

Co-authored-by: kjac <kja@umbraco.dk>
This commit is contained in:
Nikolaj Geisle
2024-04-11 13:53:34 +02:00
committed by GitHub
parent 0b62df2bb4
commit d5809da665
31 changed files with 244 additions and 169 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
src/Umbraco.Cms.Api.Management/Controllers/Security/BackOfficeController.cs
View File

@@ -253,7 +253,6 @@ public class BackOfficeController : SecurityControllerBase
private async Task<IActionResult> SignInBackOfficeUser(BackOfficeIdentityUser backOfficeUser, OpenIddictRequest request)
{
ClaimsPrincipal backOfficePrincipal = await _backOfficeSignInManager.CreateUserPrincipalAsync(backOfficeUser);
backOfficePrincipal.SetClaim(OpenIddictConstants.Claims.Subject, backOfficeUser.Key.ToString());
Claim[] backOfficeClaims = backOfficePrincipal.Claims.ToArray();
foreach (Claim backOfficeClaim in backOfficeClaims)

2
src/Umbraco.Cms.Api.Management/Controllers/Telemetry/SetTelemetryController.cs
View File

@@ -33,7 +33,7 @@ public class SetTelemetryController : TelemetryControllerBase
return BadRequest(invalidModelProblem);
}
_metricsConsentService.SetConsentLevel(telemetryRepresentationBase.TelemetryLevel);
await _metricsConsentService.SetConsentLevelAsync(telemetryRepresentationBase.TelemetryLevel);
return await Task.FromResult(Ok());
}
}

7
src/Umbraco.Cms.Api.Management/Factories/AuditLogPresentationFactory.cs
View File

@@ -17,14 +17,16 @@ public class AuditLogPresentationFactory : IAuditLogPresentationFactory
private readonly MediaFileManager _mediaFileManager;
private readonly IImageUrlGenerator _imageUrlGenerator;
private readonly IEntityService _entityService;
private readonly IUserIdKeyResolver _userIdKeyResolver;
public AuditLogPresentationFactory(IUserService userService, AppCaches appCaches, MediaFileManager mediaFileManager, IImageUrlGenerator imageUrlGenerator, IEntityService entityService)
public AuditLogPresentationFactory(IUserService userService, AppCaches appCaches, MediaFileManager mediaFileManager, IImageUrlGenerator imageUrlGenerator, IEntityService entityService, IUserIdKeyResolver userIdKeyResolver)
{
_userService = userService;
_appCaches = appCaches;
_mediaFileManager = mediaFileManager;
_imageUrlGenerator = imageUrlGenerator;
_entityService = entityService;
_userIdKeyResolver = userIdKeyResolver;
}
public IEnumerable<AuditLogResponseModel> CreateAuditLogViewModel(IEnumerable<IAuditItem> auditItems) => auditItems.Select(CreateAuditLogViewModel);
@@ -46,7 +48,8 @@ public class AuditLogPresentationFactory : IAuditLogPresentationFactory
private T CreateResponseModel<T>(IAuditItem auditItem, out IUser user)
where T : AuditLogBaseModel, new()
{
user = _userService.GetUserById(auditItem.UserId)
Guid userKey = _userIdKeyResolver.GetAsync(auditItem.UserId).GetAwaiter().GetResult();
user = _userService.GetAsync(userKey).GetAwaiter().GetResult()
?? throw new ArgumentException($"Could not find user with id {auditItem.UserId}");
IEntitySlim? entitySlim = _entityService.Get(auditItem.Id);

2
src/Umbraco.Cms.Api.Management/Install/CreateUnattendedUserNotificationHandler.cs
View File

@@ -52,7 +52,7 @@ public class CreateUnattendedUserNotificationHandler : INotificationAsyncHandler
return;
}
IUser? admin = _userService.GetUserById(Constants.Security.SuperUserId);
IUser? admin = await _userService.GetAsync(Constants.Security.SuperUserKey);
if (admin == null)
{
throw new InvalidOperationException("Could not find the super user!");

8
src/Umbraco.Core/Cache/DistributedCacheExtensions.cs
View File

@@ -32,7 +32,13 @@ public static class DistributedCacheExtensions
=> dc.Refresh(UserCacheRefresher.UniqueId, userId);
public static void RefreshUserCache(this DistributedCache dc, IEnumerable<IUser> users)
=> dc.Refresh(UserCacheRefresher.UniqueId, users.Select(x => x.Id).Distinct().ToArray());
{
foreach (IUser user in users)
{
dc.Refresh(UserCacheRefresher.UniqueId, user.Key);
dc.Refresh(UserCacheRefresher.UniqueId, user.Id);
}
}
public static void RefreshAllUserCache(this DistributedCache dc)
=> dc.RefreshAll(UserCacheRefresher.UniqueId);

12
src/Umbraco.Core/Cache/Refreshers/Implement/UserCacheRefresher.cs
View File

@@ -30,25 +30,19 @@ public sealed class UserCacheRefresher : CacheRefresherBase<UserCacheRefresherNo
base.RefreshAll();
}
public override void Refresh(int id)
{
Remove(id);
base.Refresh(id);
}
public override void Remove(int id)
public override void Refresh(Guid id)
{
Attempt<IAppPolicyCache?> userCache = AppCaches.IsolatedCaches.Get<IUser>();
if (userCache.Success)
{
userCache.Result?.Clear(RepositoryCacheKeys.GetKey<IUser, int>(id));
userCache.Result?.Clear(RepositoryCacheKeys.GetKey<IUser, Guid>(id));
userCache.Result?.ClearByKey(CacheKeys.UserContentStartNodePathsPrefix + id);
userCache.Result?.ClearByKey(CacheKeys.UserMediaStartNodePathsPrefix + id);
userCache.Result?.ClearByKey(CacheKeys.UserAllContentStartNodesPrefix + id);
userCache.Result?.ClearByKey(CacheKeys.UserAllMediaStartNodesPrefix + id);
}
base.Remove(id);
base.Refresh(id);
}
#endregion

5
src/Umbraco.Core/Constants-Security.cs
View File

@@ -112,6 +112,11 @@ public static partial class Constants
/// </summary>
public const string SecurityStampClaimType = "AspNet.Identity.SecurityStamp";
/// <summary>
/// The claim type for the mandatory OpenIdDict sub claim
/// </summary>
public const string OpenIdDictSubClaimType = "sub";
public const string AspNetCoreV3PasswordHashAlgorithmName = "PBKDF2.ASPNETCORE.V3";
public const string AspNetCoreV2PasswordHashAlgorithmName = "PBKDF2.ASPNETCORE.V2";
public const string AspNetUmbraco8PasswordHashAlgorithmName = "HMACSHA256";

2
src/Umbraco.Core/Events/UserNotificationsHandler.cs
View File

@@ -197,7 +197,7 @@ public sealed class UserNotificationsHandler :
_logger.LogDebug(
"There is no current Umbraco user logged in, the notifications will be sent from the administrator");
}
user = _userService.GetUserById(Constants.Security.SuperUserId);
user = _userService.GetAsync(Constants.Security.SuperUserKey).GetAwaiter().GetResult();
if (user == null)
{
_logger.LogWarning(

15
src/Umbraco.Core/Extensions/ClaimsIdentityExtensions.cs
View File

@@ -193,6 +193,7 @@ public static class ClaimsIdentityExtensions
/// </summary>
/// <param name="identity">this</param>
/// <param name="userId">The users Id</param>
/// <param name="userKey">The users key</param>
/// <param name="username">Username</param>
/// <param name="realName">Real name</param>
/// <param name="startContentNodes">Start content nodes</param>
@@ -201,7 +202,7 @@ public static class ClaimsIdentityExtensions
/// <param name="securityStamp">Security stamp</param>
/// <param name="allowedApps">Allowed apps</param>
/// <param name="roles">Roles</param>
public static void AddRequiredClaims(this ClaimsIdentity identity, string userId, string username, string realName, IEnumerable<int>? startContentNodes, IEnumerable<int>? startMediaNodes, string culture, string securityStamp, IEnumerable<string> allowedApps, IEnumerable<string> roles)
public static void AddRequiredClaims(this ClaimsIdentity identity, string userId, Guid userKey, string username, string realName, IEnumerable<int>? startContentNodes, IEnumerable<int>? startMediaNodes, string culture, string securityStamp, IEnumerable<string> allowedApps, IEnumerable<string> roles)
{
// This is the id that 'identity' uses to check for the user id
if (identity.HasClaim(x => x.Type == ClaimTypes.NameIdentifier) == false)
@@ -215,6 +216,18 @@ public static class ClaimsIdentityExtensions
identity));
}
// This is the id that 'identity' uses to check for the user id
if (identity.HasClaim(x => x.Type == Constants.Security.OpenIdDictSubClaimType) == false)
{
identity.AddClaim(new Claim(
Constants.Security.OpenIdDictSubClaimType,
userKey.ToString(),
ClaimValueTypes.String,
AuthenticationType,
AuthenticationType,
identity));
}
if (identity.HasClaim(x => x.Type == ClaimTypes.Name) == false)
{
identity.AddClaim(new Claim(

2
src/Umbraco.Core/Handlers/AuditNotificationsHandler.cs
View File

@@ -82,7 +82,7 @@ public sealed class AuditNotificationsHandler :
get
{
IUser? identity = _backOfficeSecurityAccessor.BackOfficeSecurity?.CurrentUser;
IUser? user = identity == null ? null : _userService.GetUserById(Convert.ToInt32(identity.Id));
IUser? user = identity == null ? null : _userService.GetAsync(identity.Key).GetAwaiter().GetResult();
return user ?? UnknownUser(_globalSettings);
}
}

4
src/Umbraco.Core/Persistence/Repositories/IUserRepository.cs
View File

@@ -4,7 +4,7 @@ using Umbraco.Cms.Core.Persistence.Querying;
namespace Umbraco.Cms.Core.Persistence.Repositories;
public interface IUserRepository : IReadWriteQueryRepository<int, IUser>
public interface IUserRepository : IReadWriteQueryRepository<Guid, IUser>
{
/// <summary>
/// Gets the count of items based on a complex query
@@ -141,6 +141,4 @@ public interface IUserRepository : IReadWriteQueryRepository<int, IUser>
int ClearLoginSessions(TimeSpan timespan);
void ClearLoginSession(Guid sessionId);
IEnumerable<IUser> GetNextUsers(int id, int count);
}

1
src/Umbraco.Core/Security/IBackofficeSecurity.cs
View File

@@ -21,6 +21,7 @@ public interface IBackOfficeSecurity
/// <returns>The current user's Id that has been authenticated for the request.</returns>
/// <remarks>If authentication hasn't taken place this will be unsuccessful.</remarks>
// TODO: This should just be an extension method on ClaimsIdentity
[Obsolete("Scheduled for removal in V15")]
Attempt<int> GetUserId();
/// <summary>

3
src/Umbraco.Core/Services/IMetricsConsentService.cs
View File

@@ -6,5 +6,8 @@ public interface IMetricsConsentService
{
TelemetryLevel GetConsentLevel();
[Obsolete("Please use SetConsentLevelAsync instead, scheduled for removal in V15")]
void SetConsentLevel(TelemetryLevel telemetryLevel);
Task SetConsentLevelAsync(TelemetryLevel telemetryLevel);
}

2
src/Umbraco.Core/Services/IUserService.cs
View File

@@ -322,8 +322,6 @@ public interface IUserService : IMembershipUserService
/// </returns>
IEnumerable<IUser> GetAllNotInGroup(int groupId);
IEnumerable<IUser> GetNextUsers(int id, int count);
#region User groups
/// <summary>

15
src/Umbraco.Core/Services/MemberService.cs
View File

@@ -20,8 +20,8 @@ namespace Umbraco.Cms.Core.Services
private readonly IMemberTypeRepository _memberTypeRepository;
private readonly IMemberGroupRepository _memberGroupRepository;
private readonly IAuditRepository _auditRepository;
private readonly IMemberGroupService _memberGroupService;
private readonly Lazy<IIdKeyMap> _idKeyMap;
#region Constructor
@@ -33,13 +33,15 @@ namespace Umbraco.Cms.Core.Services
IMemberRepository memberRepository,
IMemberTypeRepository memberTypeRepository,
IMemberGroupRepository memberGroupRepository,
IAuditRepository auditRepository)
IAuditRepository auditRepository,
Lazy<IIdKeyMap> idKeyMap)
: base(provider, loggerFactory, eventMessagesFactory)
{
_memberRepository = memberRepository;
_memberTypeRepository = memberTypeRepository;
_memberGroupRepository = memberGroupRepository;
_auditRepository = auditRepository;
_idKeyMap = idKeyMap;
_memberGroupService = memberGroupService ?? throw new ArgumentNullException(nameof(memberGroupService));
}
@@ -333,8 +335,7 @@ namespace Umbraco.Cms.Core.Services
{
using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true);
scope.ReadLock(Constants.Locks.MemberTree);
IQuery<IMember> query = Query<IMember>().Where(x => x.Key == id);
return _memberRepository.Get(query)?.FirstOrDefault();
return GetMemberFromRepository(id);
}
[Obsolete($"Use {nameof(GetById)}. Will be removed in V15.")]
@@ -1069,6 +1070,12 @@ namespace Umbraco.Cms.Core.Services
private void Audit(AuditType type, int userId, int objectId, string? message = null) => _auditRepository.Save(new AuditItem(objectId, type, userId, ObjectTypes.GetName(UmbracoObjectTypes.Member), message));
private IMember? GetMemberFromRepository(Guid id)
=> _idKeyMap.Value.GetIdForKey(id, UmbracoObjectTypes.Member) switch
{
{ Success: false } => null,
{ Result: var intId } => _memberRepository.Get(intId),
};
#endregion
#region Membership

11
src/Umbraco.Core/Services/MetricsConsentService.cs
View File

@@ -39,13 +39,12 @@ public class MetricsConsentService : IMetricsConsentService
return analyticsLevel;
}
public void SetConsentLevel(TelemetryLevel telemetryLevel)
[Obsolete("Please use SetConsentLevelAsync instead, scheduled for removal in V15")]
public void SetConsentLevel(TelemetryLevel telemetryLevel) => SetConsentLevelAsync(telemetryLevel).GetAwaiter().GetResult();
public async Task SetConsentLevelAsync(TelemetryLevel telemetryLevel)
{
IUser? currentUser = _backOfficeSecurityAccessor.BackOfficeSecurity?.CurrentUser;
if (currentUser is null)
{
currentUser = _userService.GetUserById(Constants.Security.SuperUserId);
}
IUser? currentUser = _backOfficeSecurityAccessor.BackOfficeSecurity?.CurrentUser ?? await _userService.GetAsync(Constants.Security.SuperUserKey);
_logger.LogInformation("Telemetry level set to {telemetryLevel} by {username}", telemetryLevel, currentUser?.Username);
_keyValueService.SetValue(Key, telemetryLevel.ToString());

71
src/Umbraco.Core/Services/NotificationService.cs
View File

@@ -94,52 +94,41 @@ public class NotificationService : INotificationService
// lazily get versions
var prevVersionDictionary = new Dictionary<int, IContentBase?>();
// see notes above
var id = Constants.Security.SuperUserId;
const int pagesz = 400; // load batches of 400 users
do
var notifications = GetUsersNotifications(new List<int>(), action, Enumerable.Empty<int>(), Constants.ObjectTypes.Document)?.ToList();
if (notifications is null || notifications.Count == 0)
{
var notifications = GetUsersNotifications(new List<int>(), action, Enumerable.Empty<int>(), Constants.ObjectTypes.Document)?.ToList();
if (notifications is null || notifications.Count == 0)
return;
}
IUser[] users = _userService.GetAll(0, int.MaxValue, out _).ToArray();
foreach (IUser user in users)
{
Notification[] userNotifications = notifications.Where(n => n.UserId == user.Id).ToArray();
foreach (Notification notification in userNotifications)
{
// notifications are inherited down the tree - find the topmost entity
// relevant to this notification (entity list is sorted by path)
IContent? entityForNotification = entitiesL
.FirstOrDefault(entity =>
pathsByEntityId.TryGetValue(entity.Id, out var path) &&
path.Contains(notification.EntityId));
if (entityForNotification == null)
{
continue;
}
if (prevVersionDictionary.ContainsKey(entityForNotification.Id) == false)
{
prevVersionDictionary[entityForNotification.Id] = GetPreviousVersion(entityForNotification.Id);
}
// queue notification
NotificationRequest req = CreateNotificationRequest(operatingUser, user, entityForNotification, prevVersionDictionary[entityForNotification.Id], actionName, siteUri, createSubject, createBody);
Enqueue(req);
break;
}
// users are returned ordered by id, notifications are returned ordered by user id
var users = _userService.GetNextUsers(id, pagesz).Where(x => x.IsApproved).ToList();
foreach (IUser user in users)
{
Notification[] userNotifications = notifications.Where(n => n.UserId == user.Id).ToArray();
foreach (Notification notification in userNotifications)
{
// notifications are inherited down the tree - find the topmost entity
// relevant to this notification (entity list is sorted by path)
IContent? entityForNotification = entitiesL
.FirstOrDefault(entity =>
pathsByEntityId.TryGetValue(entity.Id, out var path) &&
path.Contains(notification.EntityId));
if (entityForNotification == null)
{
continue;
}
if (prevVersionDictionary.ContainsKey(entityForNotification.Id) == false)
{
prevVersionDictionary[entityForNotification.Id] = GetPreviousVersion(entityForNotification.Id);
}
// queue notification
NotificationRequest req = CreateNotificationRequest(operatingUser, user, entityForNotification, prevVersionDictionary[entityForNotification.Id], actionName, siteUri, createSubject, createBody);
Enqueue(req);
break;
}
}
// load more users if any
id = users.Count == pagesz ? users.Last().Id + 1 : -1;
}
while (id > 0);
}
/// <summary>

68
src/Umbraco.Core/Services/UserService.cs
View File

@@ -7,6 +7,7 @@ using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Umbraco.Cms.Core.Configuration.Models;
using Umbraco.Cms.Core.DependencyInjection;
using Umbraco.Cms.Core.Editors;
using Umbraco.Cms.Core.Events;
using Umbraco.Cms.Core.Exceptions;
@@ -50,7 +51,9 @@ internal class UserService : RepositoryService, IUserService
private readonly IIsoCodeValidator _isoCodeValidator;
private readonly IUserRepository _userRepository;
private readonly ContentSettings _contentSettings;
private readonly IUserIdKeyResolver _userIdKeyResolver;
[Obsolete("Use the constructor that takes an IUserIdKeyResolver instead. Scheduled for removal in V15.")]
public UserService(
ICoreScopeProvider provider,
ILoggerFactory loggerFactory,
@@ -70,6 +73,49 @@ internal class UserService : RepositoryService, IUserService
IOptions<ContentSettings> contentSettings,
IIsoCodeValidator isoCodeValidator,
IUserForgotPasswordSender forgotPasswordSender)
: this(
provider,
loggerFactory,
eventMessagesFactory,
userRepository,
userGroupRepository,
globalSettings,
securitySettings,
userEditorAuthorizationHelper,
serviceScopeFactory,
entityService,
localLoginSettingProvider,
inviteSender,
mediaFileManager,
temporaryFileService,
shortStringHelper,
contentSettings,
isoCodeValidator,
forgotPasswordSender,
StaticServiceProvider.Instance.GetRequiredService<IUserIdKeyResolver>())
{
}
public UserService(
ICoreScopeProvider provider,
ILoggerFactory loggerFactory,
IEventMessagesFactory eventMessagesFactory,
IUserRepository userRepository,
IUserGroupRepository userGroupRepository,
IOptions<GlobalSettings> globalSettings,
IOptions<SecuritySettings> securitySettings,
UserEditorAuthorizationHelper userEditorAuthorizationHelper,
IServiceScopeFactory serviceScopeFactory,
IEntityService entityService,
ILocalLoginSettingProvider localLoginSettingProvider,
IUserInviteSender inviteSender,
MediaFileManager mediaFileManager,
ITemporaryFileService temporaryFileService,
IShortStringHelper shortStringHelper,
IOptions<ContentSettings> contentSettings,
IIsoCodeValidator isoCodeValidator,
IUserForgotPasswordSender forgotPasswordSender,
IUserIdKeyResolver userIdKeyResolver)
: base(provider, loggerFactory, eventMessagesFactory)
{
_userRepository = userRepository;
@@ -84,6 +130,7 @@ internal class UserService : RepositoryService, IUserService
_shortStringHelper = shortStringHelper;
_isoCodeValidator = isoCodeValidator;
_forgotPasswordSender = forgotPasswordSender;
_userIdKeyResolver = userIdKeyResolver;
_globalSettings = globalSettings.Value;
_securitySettings = securitySettings.Value;
_contentSettings = contentSettings.Value;
@@ -187,11 +234,13 @@ internal class UserService : RepositoryService, IUserService
/// <returns>
/// <see cref="IUser" />
/// </returns>
[Obsolete("Please use GetAsync instead. Scheduled for removal in V15.")]
public IUser? GetById(int id)
{
using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true))
{
return _userRepository.Get(id);
Guid userKey = _userIdKeyResolver.GetAsync(id).GetAwaiter().GetResult();
return _userRepository.Get(userKey);
}
}
@@ -1669,14 +1718,6 @@ internal class UserService : RepositoryService, IUserService
}
}
public IEnumerable<IUser> GetNextUsers(int id, int count)
{
using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true))
{
return _userRepository.GetNextUsers(id, count);
}
}
/// <summary>
/// Gets a list of <see cref="IUser" /> objects associated with a given group
/// </summary>
@@ -1969,10 +2010,17 @@ internal class UserService : RepositoryService, IUserService
userGroup.HasIdentity ? _userRepository.GetAllInGroup(userGroup.Id).ToArray() : empty;
var xGroupUsers = groupUsers.ToDictionary(x => x.Id, x => x);
var groupIds = groupUsers.Select(x => x.Id).ToArray();
var addedUserKeys = new List<Guid>();
foreach (var userId in userIds.Except(groupIds))
{
Guid userKey = _userIdKeyResolver.GetAsync(userId).GetAwaiter().GetResult();
addedUserKeys.Add(userKey);
}
IEnumerable<int> addedUserIds = userIds.Except(groupIds);
addedUsers = addedUserIds.Count() > 0
? _userRepository.GetMany(addedUserIds.ToArray()).Where(x => x.Id != 0).ToArray()
? _userRepository.GetMany(addedUserKeys.ToArray()).Where(x => x.Id != 0).ToArray()
: new IUser[] { };
removedUsers = groupIds.Except(userIds).Select(x => xGroupUsers[x]).Where(x => x.Id != 0).ToArray();
}

4
src/Umbraco.Infrastructure/Installer/Steps/CreateUserStep.cs
View File

@@ -59,7 +59,7 @@ public class CreateUserStep : StepBase, IInstallStep
public async Task<Attempt<InstallationResult>> ExecuteAsync(InstallData model)
{
IUser? admin = _userService.GetUserById(Constants.Security.SuperUserId);
IUser? admin = _userService.GetAsync(Constants.Security.SuperUserKey).GetAwaiter().GetResult();
if (admin is null)
{
return FailWithMessage("Could not find the super user");
@@ -92,7 +92,7 @@ public class CreateUserStep : StepBase, IInstallStep
return FailWithMessage("Could not reset password: " + string.Join(", ", resetResult.Errors.ToErrorMessage()));
}
_metricsConsentService.SetConsentLevel(model.TelemetryLevel);
await _metricsConsentService.SetConsentLevelAsync(model.TelemetryLevel);
if (model.User.SubscribeToNewsletter)
{

66
src/Umbraco.Infrastructure/Persistence/Repositories/Implement/UserRepository.cs
View File

@@ -25,7 +25,7 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement;
/// <summary>
/// Represents the UserRepository for doing CRUD operations for <see cref="IUser"/>
/// </summary>
internal class UserRepository : EntityRepositoryBase<int, IUser>, IUserRepository
internal class UserRepository : EntityRepositoryBase<Guid, IUser>, IUserRepository
{
private readonly IMapperCollection _mapperCollection;
private readonly GlobalSettings _globalSettings;
@@ -106,34 +106,14 @@ internal class UserRepository : EntityRepositoryBase<int, IUser>, IUserRepositor
private IEnumerable<IUser> ConvertFromDtos(IEnumerable<UserDto> dtos) =>
dtos.Select(x => UserFactory.BuildEntity(_globalSettings, x, _permissionMappers));
#region Overrides of RepositoryBase<int,IUser>
#region Overrides of RepositoryBase<Guid,IUser>
protected override IUser? PerformGet(int id)
protected override IUser? PerformGet(Guid key)
{
// This will never resolve to a user, yet this is asked
// for all of the time (especially in cases of members).
// Don't issue a SQL call for this, we know it will not exist.
if (_runtimeState.Level == RuntimeLevel.Upgrade)
{
// when upgrading people might come from version 7 where user 0 was the default,
// only in upgrade mode do we want to fetch the user of Id 0
if (id < -1)
{
return null;
}
}
else
{
if (id == default || id < -1)
{
return null;
}
}
Sql<ISqlContext> sql = SqlContext.Sql()
.Select<UserDto>()
.From<UserDto>()
.Where<UserDto>(x => x.Id == id);
.Where<UserDto>(x => x.Key == key);
List<UserDto>? dtos = Database.Fetch<UserDto>(sql);
if (dtos.Count == 0)
@@ -145,6 +125,8 @@ internal class UserRepository : EntityRepositoryBase<int, IUser>, IUserRepositor
return UserFactory.BuildEntity(_globalSettings, dtos[0], _permissionMappers);
}
protected override Guid GetEntityId(IUser entity) => entity.Key;
/// <summary>
/// Returns a user by username
/// </summary>
@@ -345,11 +327,12 @@ SELECT 4 AS [Key], COUNT(id) AS [Value] FROM umbracoUser WHERE userDisabled = 0
.Update<UserLoginDto>(u => u.Set(x => x.LoggedOutUtc, DateTime.UtcNow))
.Where<UserLoginDto>(x => x.SessionId == sessionId));
protected override IEnumerable<IUser> PerformGetAll(params int[]? ids)
protected override IEnumerable<IUser> PerformGetAll(params Guid[]? ids)
{
List<UserDto> dtos = ids?.Length == 0
? GetDtosWith(null, true)
: GetDtosWith(sql => sql.WhereIn<UserDto>(x => x.Id, ids), true);
: GetDtosWith(sql => sql.WhereIn<UserDto>(x => x.Key, ids), true);
var users = new IUser[dtos.Count];
var i = 0;
foreach (UserDto dto in dtos)
@@ -683,12 +666,13 @@ SELECT 4 AS [Key], COUNT(id) AS [Value] FROM umbracoUser WHERE userDisabled = 0
};
return list;
}
protected override void PersistDeletedItem(IUser entity)
{
IEnumerable<string> deletes = GetDeleteClauses();
foreach (var delete in deletes)
{
Database.Execute(delete, new { id = GetEntityId(entity), key = entity.Key });
Database.Execute(delete, new { id = entity.Id, key = GetEntityId(entity) });
}
entity.DeleteDate = DateTime.Now;
@@ -911,6 +895,16 @@ SELECT 4 AS [Key], COUNT(id) AS [Value] FROM umbracoUser WHERE userDisabled = 0
return Database.ExecuteScalar<int>(sql);
}
protected override bool PerformExists(Guid key)
{
Sql<ISqlContext> sql = SqlContext.Sql()
.SelectCount()
.From<UserDto>()
.Where<UserDto>(x => x.Key == key);
return Database.ExecuteScalar<int>(sql) > 0;
}
public bool Exists(string username) => ExistsByUserName(username);
public bool ExistsByUserName(string username)
@@ -1194,23 +1188,5 @@ SELECT 4 AS [Key], COUNT(id) AS [Value] FROM umbracoUser WHERE userDisabled = 0
return sql;
}
public IEnumerable<IUser> GetNextUsers(int id, int count)
{
Sql<ISqlContext> idsQuery = SqlContext.Sql()
.Select<UserDto>(x => x.Id)
.From<UserDto>()
.Where<UserDto>(x => x.Id >= id)
.OrderBy<UserDto>(x => x.Id);
// first page is index 1, not zero
var ids = Database.Page<int>(1, count, idsQuery).Items.ToArray();
// now get the actual users and ensure they are ordered properly (same clause)
return ids.Length == 0
? Enumerable.Empty<IUser>()
: GetMany(ids).OrderBy(x => x.Id) ?? Enumerable.Empty<IUser>();
}
#endregion
}

1
src/Umbraco.Infrastructure/Security/BackOfficeClaimsPrincipalFactory.cs
View File

@@ -51,6 +51,7 @@ public class BackOfficeClaimsPrincipalFactory : UserClaimsPrincipalFactory<BackO
// ensure our required claims are there
id.AddRequiredClaims(
user.Id,
user.Key,
user.UserName!,
user.Name!,
user.CalculatedContentStartNodeIds,

14
src/Umbraco.Infrastructure/Security/BackOfficeUserStore.cs
View File

@@ -249,7 +249,8 @@ public class BackOfficeUserStore :
try
{
return Task.FromResult(_userRepository.Get(id));
IQuery<IUser> query = _scopeProvider.CreateQuery<IUser>().Where(x => x.Id == id);
return Task.FromResult(_userRepository.Get(query).FirstOrDefault());
}
catch (DbException)
{
@@ -269,13 +270,14 @@ public class BackOfficeUserStore :
public Task<IEnumerable<IUser>> GetUsersAsync(params int[]? ids)
{
if (ids?.Length <= 0)
if (ids is null || ids.Length <= 0)
{
return Task.FromResult(Enumerable.Empty<IUser>());
}
using ICoreScope scope = _scopeProvider.CreateCoreScope(autoComplete: true);
IEnumerable<IUser> users = _userRepository.GetMany(ids);
IQuery<IUser> query = _scopeProvider.CreateQuery<IUser>().Where(x => ids.Contains(x.Id));
IEnumerable<IUser> users = _userRepository.Get(query);
return Task.FromResult(users);
}
@@ -288,8 +290,7 @@ public class BackOfficeUserStore :
}
using ICoreScope scope = _scopeProvider.CreateCoreScope(autoComplete: true);
IQuery<IUser> query = _scopeProvider.CreateQuery<IUser>().Where(x => keys.Contains(x.Key));
IEnumerable<IUser> users = _userRepository.Get(query);
IEnumerable<IUser> users = _userRepository.GetMany(keys);
return Task.FromResult(users);
}
@@ -298,8 +299,7 @@ public class BackOfficeUserStore :
public Task<IUser?> GetAsync(Guid key)
{
using ICoreScope scope = _scopeProvider.CreateCoreScope(autoComplete: true);
IQuery<IUser> query = _scopeProvider.CreateQuery<IUser>().Where(x => x.Key == key);
return Task.FromResult(_userRepository.Get(query).FirstOrDefault());
return Task.FromResult(_userRepository.Get(key));
}
/// <inheritdoc />

15
src/Umbraco.Web.Common/Security/BackofficeSecurity.cs
View File

@@ -36,11 +36,8 @@ public class BackOfficeSecurity : IBackOfficeSecurity
// Check again
if (_currentUser == null)
{
Attempt<int> id = GetUserId();
if (id.Success)
{
_currentUser = id.Success ? _userService.GetUserById(id.Result) : null;
}
Attempt<Guid> keyAttempt = GetUserKey();
_currentUser = keyAttempt.Success ? _userService.GetAsync(keyAttempt.Result).GetAwaiter().GetResult() : null;
}
}
}
@@ -49,6 +46,14 @@ public class BackOfficeSecurity : IBackOfficeSecurity
}
}
private Attempt<Guid> GetUserKey()
{
ClaimsIdentity? identity = _httpContextAccessor.HttpContext?.GetCurrentIdentity();
Guid? id = identity?.GetUserKey();
return id.HasValue is false ? Attempt.Fail<Guid>() : Attempt.Succeed(id.Value);
}
/// <inheritdoc />
public Attempt<int> GetUserId()
{

8
tests/Umbraco.Tests.Integration/Umbraco.Core/Services/MetricsConsentServiceTest.cs
View File

@@ -18,9 +18,9 @@ public class MetricsConsentServiceTest : UmbracoIntegrationTest
[TestCase(TelemetryLevel.Minimal)]
[TestCase(TelemetryLevel.Basic)]
[TestCase(TelemetryLevel.Detailed)]
public void Can_Store_Consent(TelemetryLevel level)
public async Task Can_Store_Consent(TelemetryLevel level)
{
MetricsConsentService.SetConsentLevel(level);
await MetricsConsentService.SetConsentLevelAsync(level);
var actual = MetricsConsentService.GetConsentLevel();
Assert.IsNotNull(actual);
@@ -28,9 +28,9 @@ public class MetricsConsentServiceTest : UmbracoIntegrationTest
}
[Test]
public void Enum_Stored_as_string()
public async Task Enum_Stored_as_string()
{
MetricsConsentService.SetConsentLevel(TelemetryLevel.Detailed);
await MetricsConsentService.SetConsentLevelAsync(TelemetryLevel.Detailed);
var stringValue = KeyValueService.GetValue(Cms.Core.Services.MetricsConsentService.Key);

28
tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserServiceCrudTests.Update.cs
View File

@@ -126,6 +126,34 @@ public partial class UserServiceCrudTests
Assert.AreEqual(email, updatedUser.Email);
}
[Test]
public async Task Can_Update_User_Name()
{
const string userName = "UpdateUserName";
const string name = "UpdatedName";
const string email = "update@email.com";
var userService = CreateUserService(securitySettings: new SecuritySettings { UsernameIsEmail = false });
var (updateModel, createdUser) = await CreateUserForUpdate(userService);
updateModel.UserName = userName;
updateModel.Email = email;
updateModel.Name = name;
var result = await userService.UpdateAsync(Constants.Security.SuperUserKey, updateModel);
Assert.IsTrue(result.Success);
var updatedUser = await userService.GetAsync(createdUser.Key);
Assert.Multiple(() =>
{
Assert.IsNotNull(updatedUser);
Assert.AreEqual(userName, updatedUser.Username);
Assert.AreEqual(email, updatedUser.Email);
Assert.AreEqual(name, updatedUser.Name);
});
}
[Test]
public async Task Cannot_Change_Email_To_Duplicate_Email_On_Update()
{

4
tests/Umbraco.Tests.Integration/Umbraco.Core/Telemetry/TelemetryServiceTests.cs
View File

@@ -23,7 +23,7 @@ public class TelemetryServiceTests : UmbracoIntegrationTest
private IMetricsConsentService MetricsConsentService => GetRequiredService<IMetricsConsentService>();
[Test]
public void Expected_Detailed_Telemetry_Exists()
public async Task Expected_Detailed_Telemetry_Exists()
{
var expectedData = new[]
{
@@ -54,7 +54,7 @@ public class TelemetryServiceTests : UmbracoIntegrationTest
Constants.Telemetry.DeliveryApiPublicAccess
};
MetricsConsentService.SetConsentLevel(TelemetryLevel.Detailed);
await MetricsConsentService.SetConsentLevelAsync(TelemetryLevel.Detailed);
var success = TelemetryService.TryGetTelemetryReportData(out var telemetryReportData);
var detailed = telemetryReportData.Detailed.ToArray();

5
tests/Umbraco.Tests.Integration/Umbraco.Examine.Lucene/UmbracoExamine/BackOfficeExamineSearcherTests.cs
View File

@@ -85,8 +85,7 @@ public class BackOfficeExamineSearcherTests : ExamineBaseTest
private async Task SetupUserIdentity(string userId)
{
var identity =
await BackOfficeUserStore.FindByIdAsync(userId, CancellationToken.None);
var identity = await BackOfficeUserStore.FindByIdAsync(userId, CancellationToken.None);
await BackOfficeSignInManager.SignInAsync(identity, false);
var principal = await BackOfficeSignInManager.CreateUserPrincipalAsync(identity);
HttpContextAccessor.HttpContext.SetPrincipalForRequest(principal);
@@ -595,7 +594,7 @@ public class BackOfficeExamineSearcherTests : ExamineBaseTest
public async Task Check_All_Indexed_Values_For_Published_Content_With_No_Properties()
{
// Arrange
await SetupUserIdentity(Constants.Security.SuperUserIdAsString);
await SetupUserIdentity(Constants.Security.SuperUserKey.ToString());
const string contentName = "TestContent";

16
tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/UserRepositoryTest.cs
View File

@@ -126,7 +126,7 @@ public class UserRepositoryTest : UmbracoIntegrationTest
repository.Save(user);
// Act
var resolved = repository.Get(user.Id);
var resolved = repository.Get(user.Key);
var dirty = ((User)resolved).IsDirty();
// Assert
@@ -148,7 +148,7 @@ public class UserRepositoryTest : UmbracoIntegrationTest
// Act
repository.Save(user);
var id = user.Id;
var id = user.Key;
var mockRuntimeState = CreateMockRuntimeState(RuntimeLevel.Run);
@@ -185,7 +185,7 @@ public class UserRepositoryTest : UmbracoIntegrationTest
var user = CreateAndCommitUserWithGroup(repository, userGroupRepository);
// Act
var updatedItem = repository.Get(user.Id);
var updatedItem = repository.Get(user.Key);
// TODO: this test cannot work, user has 2 sections but the way it's created,
// they don't show, so the comparison with updatedItem fails - fix!
@@ -227,7 +227,7 @@ public class UserRepositoryTest : UmbracoIntegrationTest
var users = CreateAndCommitMultipleUsers(repository);
// Act
var result = repository.GetMany(users[0].Id, users[1].Id).ToArray();
var result = repository.GetMany(users[0].Key, users[1].Key).ToArray();
// Assert
Assert.That(result, Is.Not.Null);
@@ -269,7 +269,7 @@ public class UserRepositoryTest : UmbracoIntegrationTest
var users = CreateAndCommitMultipleUsers(repository);
// Act
var exists = repository.Exists(users[0].Id);
var exists = repository.Exists(users[0].Key);
// Assert
Assert.That(exists, Is.True);
@@ -396,7 +396,7 @@ public class UserRepositoryTest : UmbracoIntegrationTest
repository.Save(user);
// Get the user
var updatedUser = repository.Get(user.Id);
var updatedUser = repository.Get(user.Key);
// Ensure the Security Stamp is invalidated & no longer the same
Assert.AreNotEqual(originalSecurityStamp, updatedUser.SecurityStamp);
@@ -460,7 +460,7 @@ public class UserRepositoryTest : UmbracoIntegrationTest
var user = CreateAndCommitUserWithGroup(userRepository, userGroupRepository);
// Act
var resolved = (User)userRepository.Get(user.Id);
var resolved = (User)userRepository.Get(user.Key);
resolved.Name = "New Name";
@@ -478,7 +478,7 @@ public class UserRepositoryTest : UmbracoIntegrationTest
userRepository.Save(resolved);
var updatedItem = (User)userRepository.Get(user.Id);
var updatedItem = (User)userRepository.Get(user.Key);
// Assert
Assert.That(updatedItem.Id, Is.EqualTo(resolved.Id));

16
tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Scoping/ScopedRepositoryTests.cs
View File

@@ -63,7 +63,7 @@ public class ScopedRepositoryTests : UmbracoIntegrationTest
[TestCase(true)]
[TestCase(false)]
public void DefaultRepositoryCachePolicy(bool complete)
public async Task DefaultRepositoryCachePolicy(bool complete)
{
var scopeProvider = (ScopeProvider)ScopeProvider;
var service = (UserService)UserService;
@@ -72,13 +72,13 @@ public class ScopedRepositoryTests : UmbracoIntegrationTest
service.Save(user);
// User has been saved so the cache has been cleared of it
var globalCached = (IUser)globalCache.Get(GetCacheIdKey<IUser>(user.Id), () => null);
var globalCached = (IUser)globalCache.Get(GetCacheIdKey<IUser>(user.Key), () => null);
Assert.IsNull(globalCached);
// Get user again to load it into the cache again, this also ensure we don't modify the one that's in the cache.
user = service.GetUserById(user.Id);
user = await service.GetAsync(user.Key);
// global cache contains the entity
globalCached = (IUser)globalCache.Get(GetCacheIdKey<IUser>(user.Id), () => null);
globalCached = (IUser)globalCache.Get(GetCacheIdKey<IUser>(user.Key), () => null);
Assert.IsNotNull(globalCached);
Assert.AreEqual(user.Id, globalCached.Id);
Assert.AreEqual("name", globalCached.Name);
@@ -104,7 +104,7 @@ public class ScopedRepositoryTests : UmbracoIntegrationTest
Assert.AreEqual("changed", scopeCached.Name);
// global cache is unchanged
globalCached = (IUser)globalCache.Get(GetCacheIdKey<IUser>(user.Id), () => null);
globalCached = (IUser)globalCache.Get(GetCacheIdKey<IUser>(user.Key), () => null);
Assert.IsNotNull(globalCached);
Assert.AreEqual(user.Id, globalCached.Id);
Assert.AreEqual("name", globalCached.Name);
@@ -117,7 +117,7 @@ public class ScopedRepositoryTests : UmbracoIntegrationTest
Assert.IsNull(scopeProvider.AmbientScope);
globalCached = (IUser)globalCache.Get(GetCacheIdKey<IUser>(user.Id), () => null);
globalCached = (IUser)globalCache.Get(GetCacheIdKey<IUser>(user.Key), () => null);
if (complete)
{
// global cache has been cleared
@@ -130,11 +130,11 @@ public class ScopedRepositoryTests : UmbracoIntegrationTest
}
// get again, updated if completed
user = service.GetUserById(user.Id);
user = await service.GetAsync(user.Key);
Assert.AreEqual(complete ? "changed" : "name", user.Name);
// global cache contains the entity again
globalCached = (IUser)globalCache.Get(GetCacheIdKey<IUser>(user.Id), () => null);
globalCached = (IUser)globalCache.Get(GetCacheIdKey<IUser>(user.Key), () => null);
Assert.IsNotNull(globalCached);
Assert.AreEqual(user.Id, globalCached.Id);
Assert.AreEqual(complete ? "changed" : "name", globalCached.Name);

3
tests/Umbraco.Tests.UnitTests/Umbraco.Core/BackOffice/UmbracoBackOfficeIdentityTests.cs
View File

@@ -111,6 +111,7 @@ public class UmbracoBackOfficeIdentityTests
claimsIdentity.AddRequiredClaims(
"1234",
Guid.NewGuid(),
"testing",
"hello world",
new[] { 654 },
@@ -120,7 +121,7 @@ public class UmbracoBackOfficeIdentityTests
new[] { "content", "media" },
new[] { "admin" });
Assert.AreEqual(12, claimsIdentity.Claims.Count());
Assert.AreEqual(13, claimsIdentity.Claims.Count());
Assert.IsNull(claimsIdentity.Actor);
}
}

2
tests/Umbraco.Tests.UnitTests/Umbraco.Core/Extensions/ClaimsPrincipalExtensionsTests.cs
View File

@@ -18,6 +18,7 @@ public class ClaimsPrincipalExtensionsTests
var backOfficeIdentity = new ClaimsIdentity();
backOfficeIdentity.AddRequiredClaims(
Constants.Security.SuperUserIdAsString,
Constants.Security.SuperUserKey,
"test",
"test",
Enumerable.Empty<int>(),
@@ -55,6 +56,7 @@ public class ClaimsPrincipalExtensionsTests
var backOfficeIdentity = new ClaimsIdentity();
backOfficeIdentity.AddRequiredClaims(
Constants.Security.SuperUserIdAsString,
Constants.Security.SuperUserKey,
"test",
"test",
Enumerable.Empty<int>(),