Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Amend user start node handling #16094

Merged
merged 10 commits into from
May 3, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using Umbraco.Cms.Api.Management.Security;
using Umbraco.Cms.Api.Management.ViewModels.User;
using Umbraco.Cms.Api.Management.ViewModels.User.Current;
using Umbraco.Cms.Core;
using Umbraco.Cms.Api.Management.ViewModels.User.Item;
using Umbraco.Cms.Core.Cache;
using Umbraco.Cms.Core.Configuration.Models;
Expand Down Expand Up @@ -70,7 +71,9 @@ public UserResponseModel CreateResponseModel(IUser user)
State = user.UserState,
UserGroupIds = new HashSet<Guid>(user.Groups.Select(x => x.Key)),
DocumentStartNodeIds = GetKeysFromIds(user.StartContentIds, UmbracoObjectTypes.Document),
HasDocumentRootAccess = HasRootAccess(user.StartContentIds),
MediaStartNodeIds = GetKeysFromIds(user.StartMediaIds, UmbracoObjectTypes.Media),
HasMediaRootAccess = HasRootAccess(user.StartMediaIds),
FailedLoginAttempts = user.FailedPasswordAttempts,
LastLoginDate = user.LastLoginDate,
LastLockoutDate = user.LastLockoutDate,
Expand Down Expand Up @@ -159,7 +162,9 @@ public async Task<UserUpdateModel> CreateUpdateModelAsync(Guid existingUserKey,
UserName = updateModel.UserName,
LanguageIsoCode = updateModel.LanguageIsoCode,
ContentStartNodeKeys = updateModel.DocumentStartNodeIds,
HasContentRootAccess = updateModel.HasDocumentRootAccess,
MediaStartNodeKeys = updateModel.MediaStartNodeIds,
HasMediaRootAccess = updateModel.HasMediaRootAccess
};

model.UserGroupKeys = updateModel.UserGroupIds;
Expand All @@ -172,8 +177,10 @@ public async Task<CurrentUserResponseModel> CreateCurrentUserResponseModelAsync(
var presentationUser = CreateResponseModel(user);
var presentationGroups = await _userGroupPresentationFactory.CreateMultipleAsync(user.Groups);
var languages = presentationGroups.SelectMany(x => x.Languages).Distinct().ToArray();
var mediaStartNodeKeys = GetKeysFromIds(user.CalculateMediaStartNodeIds(_entityService, _appCaches), UmbracoObjectTypes.Media);
var documentStartNodeKeys = GetKeysFromIds(user.CalculateContentStartNodeIds(_entityService, _appCaches), UmbracoObjectTypes.Document);
var mediaStartNodeIds = user.CalculateMediaStartNodeIds(_entityService, _appCaches);
var mediaStartNodeKeys = GetKeysFromIds(mediaStartNodeIds, UmbracoObjectTypes.Media);
var contentStartNodeIds = user.CalculateContentStartNodeIds(_entityService, _appCaches);
var documentStartNodeKeys = GetKeysFromIds(contentStartNodeIds, UmbracoObjectTypes.Document);

var permissions = presentationGroups.SelectMany(x => x.Permissions).ToHashSet();
var fallbackPermissions = presentationGroups.SelectMany(x => x.FallbackPermissions).ToHashSet();
Expand All @@ -192,7 +199,9 @@ public async Task<CurrentUserResponseModel> CreateCurrentUserResponseModelAsync(
AvatarUrls = presentationUser.AvatarUrls,
LanguageIsoCode = presentationUser.LanguageIsoCode,
MediaStartNodeIds = mediaStartNodeKeys,
HasMediaRootAccess = HasRootAccess(mediaStartNodeIds),
DocumentStartNodeIds = documentStartNodeKeys,
HasDocumentRootAccess = HasRootAccess(contentStartNodeIds),
Permissions = permissions,
FallbackPermissions = fallbackPermissions,
HasAccessToAllLanguages = hasAccessToAllLanguages,
Expand All @@ -214,5 +223,6 @@ private ISet<Guid> GetKeysFromIds(IEnumerable<int>? ids, UmbracoObjectTypes type
: new HashSet<Guid>(keys);
}


private bool HasRootAccess(IEnumerable<int>? startNodeIds)
=> startNodeIds?.Contains(Constants.System.Root) is true;
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,6 @@ public override void OnActionExecuting(ActionExecutingContext context)
IUser? user = backOfficeSecurityAccessor.BackOfficeSecurity?.CurrentUser;

var startNodeIds = user != null ? GetUserStartNodeIds(user, context) : Array.Empty<int>();

// TODO: remove this once we have backoffice auth in place
startNodeIds = new[] { Constants.System.Root };

if (startNodeIds.Contains(Constants.System.Root))
{
return;
Expand Down
24 changes: 24 additions & 0 deletions src/Umbraco.Cms.Api.Management/OpenApi.json
Original file line number Diff line number Diff line change
Expand Up @@ -34357,6 +34357,8 @@
"fallbackPermissions",
"hasAccessToAllLanguages",
"hasAccessToSensitiveData",
"hasDocumentRootAccess",
"hasMediaRootAccess",
"id",
"isAdmin",
"languages",
Expand Down Expand Up @@ -34392,6 +34394,9 @@
"format": "uuid"
}
},
"hasDocumentRootAccess": {
"type": "boolean"
},
"mediaStartNodeIds": {
"uniqueItems": true,
"type": "array",
Expand All @@ -34400,6 +34405,9 @@
"format": "uuid"
}
},
"hasMediaRootAccess": {
"type": "boolean"
},
"avatarUrls": {
"type": "array",
"items": {
Expand Down Expand Up @@ -43206,6 +43214,8 @@
"required": [
"documentStartNodeIds",
"email",
"hasDocumentRootAccess",
"hasMediaRootAccess",
"languageIsoCode",
"mediaStartNodeIds",
"name",
Expand Down Expand Up @@ -43242,13 +43252,19 @@
"format": "uuid"
}
},
"hasDocumentRootAccess": {
"type": "boolean"
},
"mediaStartNodeIds": {
"uniqueItems": true,
"type": "array",
"items": {
"type": "string",
"format": "uuid"
}
},
"hasMediaRootAccess": {
"type": "boolean"
}
},
"additionalProperties": false
Expand Down Expand Up @@ -43626,6 +43642,8 @@
"documentStartNodeIds",
"email",
"failedLoginAttempts",
"hasDocumentRootAccess",
"hasMediaRootAccess",
"id",
"isAdmin",
"mediaStartNodeIds",
Expand Down Expand Up @@ -43670,6 +43688,9 @@
"format": "uuid"
}
},
"hasDocumentRootAccess": {
"type": "boolean"
},
"mediaStartNodeIds": {
"uniqueItems": true,
"type": "array",
Expand All @@ -43678,6 +43699,9 @@
"format": "uuid"
}
},
"hasMediaRootAccess": {
"type": "boolean"
},
"avatarUrls": {
"type": "array",
"items": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ public IEnumerable<UserAccessEntity> RootUserAccessEntities(UmbracoObjectTypes u
// root entities for users without root access should include:
// - the start nodes that are actual root entities (level == 1)
// - the root level ancestors to the rest of the start nodes (required for browsing to the actual start nodes - will be marked as "no access")
IEntitySlim[] userStartEntities = _entityService.GetAll(umbracoObjectType, userStartNodeIds).ToArray();
IEntitySlim[] userStartEntities = userStartNodeIds.Any()
? _entityService.GetAll(umbracoObjectType, userStartNodeIds).ToArray()
: Array.Empty<IEntitySlim>();
Comment on lines +22 to +24
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥰


// find the start nodes that are at root level (level == 1)
IEntitySlim[] allowedTopmostEntities = userStartEntities.Where(entity => entity.Level == 1).ToArray();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
using Umbraco.Cms.Api.Management.ViewModels.UserGroup;
using Umbraco.Cms.Api.Management.ViewModels.UserGroup.Permissions;
using Umbraco.Cms.Core.Models.Membership;

namespace Umbraco.Cms.Api.Management.ViewModels.User.Current;

Expand All @@ -18,8 +16,12 @@ public class CurrentUserResponseModel

public required ISet<Guid> DocumentStartNodeIds { get; init; } = new HashSet<Guid>();

public required bool HasDocumentRootAccess { get; init; }

public required ISet<Guid> MediaStartNodeIds { get; init; } = new HashSet<Guid>();

public required bool HasMediaRootAccess { get; init; }

public required IEnumerable<string> AvatarUrls { get; init; } = Enumerable.Empty<string>();

public required IEnumerable<string> Languages { get; init; } = Enumerable.Empty<string>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,9 @@ public class UpdateUserRequestModel : UserPresentationBase

public ISet<Guid> DocumentStartNodeIds { get; set; } = new HashSet<Guid>();

public bool HasDocumentRootAccess { get; init; }

public ISet<Guid> MediaStartNodeIds { get; set; } = new HashSet<Guid>();

public bool HasMediaRootAccess { get; init; }
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,12 @@ public class UserResponseModel : UserPresentationBase

public ISet<Guid> DocumentStartNodeIds { get; set; } = new HashSet<Guid>();

public bool HasDocumentRootAccess { get; set; }

public ISet<Guid> MediaStartNodeIds { get; set; } = new HashSet<Guid>();

public bool HasMediaRootAccess { get; set; }

public IEnumerable<string> AvatarUrls { get; set; } = Enumerable.Empty<string>();

public UserState State { get; set; }
Expand Down
4 changes: 4 additions & 0 deletions src/Umbraco.Core/Models/UserUpdateModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@ public class UserUpdateModel

public ISet<Guid> ContentStartNodeKeys { get; set; } = new HashSet<Guid>();

public bool HasContentRootAccess { get; set; }

public ISet<Guid> MediaStartNodeKeys { get; set; } = new HashSet<Guid>();

public bool HasMediaRootAccess { get; set; }

public ISet<Guid> UserGroupKeys { get; set; } = new HashSet<Guid>();
}
32 changes: 21 additions & 11 deletions src/Umbraco.Core/Services/UserService.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.ComponentModel.DataAnnotations;

Check notice on line 1 in src/Umbraco.Core/Services/UserService.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v14/dev)

ℹ Getting worse: Lines of Code in a Single File

The lines of code increases from 1719 to 1727, improve code health by reducing it to 1000. The number of Lines of Code in a single file. More Lines of Code lowers the code health.
using System.Globalization;
using System.Linq.Expressions;
using System.Security.Claims;
Expand Down Expand Up @@ -984,22 +984,32 @@

// We have to resolve the keys to ids to be compatible with the repository, this could be done in the factory,
// but I'd rather keep the ids out of the service API as much as possible.
int[]? startContentIds = GetIdsFromKeys(model.ContentStartNodeKeys, UmbracoObjectTypes.Document);
List<int>? startContentIds = GetIdsFromKeys(model.ContentStartNodeKeys, UmbracoObjectTypes.Document);

if (startContentIds is null || startContentIds.Length != model.ContentStartNodeKeys.Count)
if (startContentIds is null || startContentIds.Count != model.ContentStartNodeKeys.Count)
{
scope.Complete();
return Attempt.FailWithStatus<IUser?, UserOperationStatus>(UserOperationStatus.ContentStartNodeNotFound, existingUser);
}

int[]? startMediaIds = GetIdsFromKeys(model.MediaStartNodeKeys, UmbracoObjectTypes.Media);
List<int>? startMediaIds = GetIdsFromKeys(model.MediaStartNodeKeys, UmbracoObjectTypes.Media);

if (startMediaIds is null || startMediaIds.Length != model.MediaStartNodeKeys.Count)
if (startMediaIds is null || startMediaIds.Count != model.MediaStartNodeKeys.Count)
{
scope.Complete();
return Attempt.FailWithStatus<IUser?, UserOperationStatus>(UserOperationStatus.MediaStartNodeNotFound, existingUser);
}

if (model.HasContentRootAccess)
{
startContentIds.Add(Constants.System.Root);
}

if (model.HasMediaRootAccess)
{
startMediaIds.Add(Constants.System.Root);
}

Check warning on line 1012 in src/Umbraco.Core/Services/UserService.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v14/dev)

❌ Getting worse: Complex Method

UpdateAsync increases in cyclomatic complexity from 14 to 16, threshold = 9. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.
Attempt<string?> isAuthorized = _userEditorAuthorizationHelper.IsAuthorized(
performingUser,
existingUser,
Expand Down Expand Up @@ -1085,15 +1095,15 @@
UserUpdateModel source,
ISet<IUserGroup> sourceUserGroups,
IUser target,
int[]? startContentIds,
int[]? startMediaIds)
List<int> startContentIds,
List<int> startMediaIds)
{
target.Name = source.Name;
target.Language = source.LanguageIsoCode;
target.Email = source.Email;
target.Username = source.UserName;
target.StartContentIds = startContentIds;
target.StartMediaIds = startMediaIds;
target.StartContentIds = startContentIds.ToArray();
target.StartMediaIds = startMediaIds.ToArray();

target.ClearGroups();
foreach (IUserGroup group in sourceUserGroups)
Expand Down Expand Up @@ -1152,13 +1162,13 @@

private static bool IsEmailValid(string email) => new EmailAddressAttribute().IsValid(email);

private int[]? GetIdsFromKeys(IEnumerable<Guid>? guids, UmbracoObjectTypes type)
private List<int>? GetIdsFromKeys(IEnumerable<Guid>? guids, UmbracoObjectTypes type)
{
int[]? keys = guids?
var keys = guids?
.Select(x => _entityService.GetId(x, type))
.Where(x => x.Success)
.Select(x => x.Result)
.ToArray();
.ToList();

return keys;
}
Expand Down
Loading
Loading