From b5391e0046f399e66b5d3cc417f0113397156741 Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Mon, 12 Sep 2022 15:43:50 -0700 Subject: [PATCH 01/25] Add flexible roles to users module --- .../OrchardCore.Users/AdminMenu.cs | 15 +- .../Controllers/AdminController.cs | 148 +++++++----------- .../Drivers/UserDisplayDriver.cs | 45 +++--- .../Drivers/UserInformationDisplayDriver.cs | 108 ++++++++----- .../UserProfileSettingsDisplayDriver.cs | 72 +++++++++ .../Drivers/UserRoleDisplayDriver.cs | 75 ++++----- .../OrchardCore.Users/Permissions.cs | 60 +++++-- .../DefaultUsersAdminListFilterProvider.cs | 16 +- .../DefaultUsersAdminListQueryService.cs | 9 +- .../Services/RoleAuthorizationHandler.cs | 136 ++++++++++++++++ .../Services/RolesAdminListFilterProvider.cs | 40 +++++ .../Services/UserAuthorizationHandler.cs | 113 ------------- .../OrchardCore.Users/Startup.cs | 23 ++- .../OrchardCore.Users/UserMigrations.cs | 39 +++++ .../OrchardCore.Users/UserRolePermissions.cs | 55 +++++++ .../ViewModels/EditUserEmailViewModel.cs | 15 ++ .../ViewModels/EditUserNameViewModel.cs | 13 ++ .../UserProfileSettingsViewModel.cs | 8 + .../Views/Admin/Index.cshtml | 5 +- ...tionItemText-UserProfileSettings.Id.cshtml | 1 + .../OrchardCore.Users/Views/RolesMeta.cshtml | 8 + .../Views/User.SummaryAdmin.cshtml | 40 ++--- .../Views/UserButtons.cshtml | 18 ++- .../Views/UserEmail.Edit.cshtml | 10 ++ .../OrchardCore.Users/Views/UserMenu.cshtml | 2 +- .../Views/UserName.Edit.cshtml | 10 ++ .../Views/UserProfileSettings.Edit.cshtml | 15 ++ .../Views/UsersAdminList.cshtml | 24 +-- .../Views/UsersAdminListCreate.cshtml | 12 +- .../Services/RoleServiceExtensions.cs | 34 ++++ .../CommonPermissions.cs | 48 +++++- .../IUsersAdminListFilter.cs | 12 ++ .../Indexes/UserRoleIndex.cs | 37 +++++ .../Models/UserProfileSettings.cs | 8 + 34 files changed, 893 insertions(+), 381 deletions(-) create mode 100644 src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserProfileSettingsDisplayDriver.cs create mode 100644 src/OrchardCore.Modules/OrchardCore.Users/Services/RoleAuthorizationHandler.cs create mode 100644 src/OrchardCore.Modules/OrchardCore.Users/Services/RolesAdminListFilterProvider.cs delete mode 100644 src/OrchardCore.Modules/OrchardCore.Users/Services/UserAuthorizationHandler.cs create mode 100644 src/OrchardCore.Modules/OrchardCore.Users/UserMigrations.cs create mode 100644 src/OrchardCore.Modules/OrchardCore.Users/UserRolePermissions.cs create mode 100644 src/OrchardCore.Modules/OrchardCore.Users/ViewModels/EditUserEmailViewModel.cs create mode 100644 src/OrchardCore.Modules/OrchardCore.Users/ViewModels/EditUserNameViewModel.cs create mode 100644 src/OrchardCore.Modules/OrchardCore.Users/ViewModels/UserProfileSettingsViewModel.cs create mode 100644 src/OrchardCore.Modules/OrchardCore.Users/Views/NavigationItemText-UserProfileSettings.Id.cshtml create mode 100644 src/OrchardCore.Modules/OrchardCore.Users/Views/RolesMeta.cshtml create mode 100644 src/OrchardCore.Modules/OrchardCore.Users/Views/UserEmail.Edit.cshtml create mode 100644 src/OrchardCore.Modules/OrchardCore.Users/Views/UserName.Edit.cshtml create mode 100644 src/OrchardCore.Modules/OrchardCore.Users/Views/UserProfileSettings.Edit.cshtml create mode 100644 src/OrchardCore/OrchardCore.Users.Core/IUsersAdminListFilter.cs create mode 100644 src/OrchardCore/OrchardCore.Users.Core/Indexes/UserRoleIndex.cs create mode 100644 src/OrchardCore/OrchardCore.Users.Core/Models/UserProfileSettings.cs diff --git a/src/OrchardCore.Modules/OrchardCore.Users/AdminMenu.cs b/src/OrchardCore.Modules/OrchardCore.Users/AdminMenu.cs index bc590dfebd7..62c6ae0e4ec 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/AdminMenu.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/AdminMenu.cs @@ -29,7 +29,7 @@ public Task BuildNavigationAsync(string name, NavigationBuilder builder) .Add(S["Users"], S["Users"].PrefixPosition(), users => users .AddClass("users").Id("users") .Action("Index", "Admin", "OrchardCore.Users") - .Permission(Permissions.ViewUsers) + .Permission(CommonPermissions.ListUsers) .Resource(new User()) .LocalNav() ) @@ -42,6 +42,19 @@ public Task BuildNavigationAsync(string name, NavigationBuilder builder) ) ); + builder + .Add(S["Configuration"], configuration => configuration + .Add(S["Settings"], settings => settings + .Add(S["User Profile Settings"], S["User Profile Settings"].PrefixPosition(), entry => entry + .AddClass("UserProfileSettings") + .Id("UserProfileSettings") + .Action("Index", "Admin", new { area = "OrchardCore.Settings", groupId = UserProfileSettingsDisplayDriver.GroupId }) + .Permission(CommonPermissions.ManageUserProfileSettings) + .LocalNav() + ) + ) + ); + return Task.CompletedTask; } } diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Controllers/AdminController.cs b/src/OrchardCore.Modules/OrchardCore.Users/Controllers/AdminController.cs index ae1d6ca83bd..2c3a1d25dd1 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Controllers/AdminController.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Controllers/AdminController.cs @@ -62,7 +62,7 @@ public AdminController( INotifier notifier, IOptions pagerOptions, IShapeFactory shapeFactory, - ILogger logger, + ILogger logger, IHtmlLocalizer htmlLocalizer, IStringLocalizer stringLocalizer, IUpdateModelAccessor updateModelAccessor) @@ -92,16 +92,17 @@ public async Task Index([ModelBinder(BinderType = typeof(UserFilte // Check a dummy user account to see if the current user has permission to view users. var authUser = new User(); - if (!await _authorizationService.AuthorizeAsync(User, Permissions.ViewUsers, authUser)) + if (!await _authorizationService.AuthorizeAsync(User, CommonPermissions.ListUsers, authUser)) { return Forbid(); } - var options = new UserIndexOptions(); - - // Populate route values to maintain previous route data when generating page links - // await _userOptionsDisplayManager.UpdateEditorAsync(options, _updateModelAccessor.ModelUpdater, false); - options.FilterResult = queryFilterResult; + var options = new UserIndexOptions + { + // Populate route values to maintain previous route data when generating page links + // await _userOptionsDisplayManager.UpdateEditorAsync(options, _updateModelAccessor.ModelUpdater, false); + FilterResult = queryFilterResult + }; options.FilterResult.MapTo(options); // With the options populated we filter the query, allowing the filters to alter the options. @@ -135,8 +136,7 @@ public async Task Index([ModelBinder(BinderType = typeof(UserFilte { UserId = user.UserId, Shape = await _userDisplayManager.BuildDisplayAsync(user, updater: _updateModelAccessor.ModelUpdater, displayType: "SummaryAdmin") - } - ); + }); } options.UserFilters = new List() @@ -221,14 +221,12 @@ public async Task IndexFilterPOST(UserIndexOptions options) public async Task IndexPOST(UserIndexOptions options, IEnumerable itemIds) { // Check a dummy user account to see if the current user has permission to manage it. - var authUser = new User(); - - if (!await _authorizationService.AuthorizeAsync(User, Permissions.ManageUsers, authUser)) + if (!await _authorizationService.AuthorizeAsync(User, CommonPermissions.ListUsers, new User())) { return Forbid(); } - if (itemIds?.Count() > 0) + if (itemIds != null && itemIds.Any()) { var checkedUsers = await _session.Query().Where(x => x.UserId.IsIn(itemIds)).ListAsync(); @@ -236,74 +234,56 @@ public async Task IndexPOST(UserIndexOptions options, IEnumerable< // To prevent html injection we authorize each user before performing any operations. foreach (var user in checkedUsers) { - if (!await _authorizationService.AuthorizeAsync(User, Permissions.ManageUsers, user)) - { - return Forbid(); - } - } + var canEditUser = await _authorizationService.AuthorizeAsync(User, CommonPermissions.EditUsers, user); + var isSameUser = String.Equals(user.UserId, User.FindFirstValue(ClaimTypes.NameIdentifier), StringComparison.OrdinalIgnoreCase); - switch (options.BulkAction) - { - case UsersBulkAction.None: - break; - case UsersBulkAction.Approve: - foreach (var user in checkedUsers) - { - if (!await _userManager.IsEmailConfirmedAsync(user)) + switch (options.BulkAction) + { + case UsersBulkAction.None: break; + case UsersBulkAction.Approve: + if (canEditUser && !await _userManager.IsEmailConfirmedAsync(user)) { var token = await _userManager.GenerateEmailConfirmationTokenAsync(user); await _userManager.ConfirmEmailAsync(user, token); await _notifier.SuccessAsync(H["User {0} successfully approved.", user.UserName]); } - } - break; - case UsersBulkAction.Delete: - foreach (var user in checkedUsers) - { - if (String.Equals(user.UserId, User.FindFirstValue(ClaimTypes.NameIdentifier), StringComparison.OrdinalIgnoreCase)) + break; + case UsersBulkAction.Delete: + if (!isSameUser && await _authorizationService.AuthorizeAsync(User, CommonPermissions.DeleteUsers, user)) { - continue; + await _userManager.DeleteAsync(user); + await _notifier.SuccessAsync(H["User {0} successfully deleted.", user.UserName]); } - await _userManager.DeleteAsync(user); - await _notifier.SuccessAsync(H["User {0} successfully deleted.", user.UserName]); - } - break; - case UsersBulkAction.Disable: - foreach (var user in checkedUsers) - { - if (String.Equals(user.UserId, User.FindFirstValue(ClaimTypes.NameIdentifier), StringComparison.OrdinalIgnoreCase)) + break; + case UsersBulkAction.Disable: + if (!isSameUser && canEditUser) { - continue; + user.IsEnabled = false; + await _userManager.UpdateAsync(user); + await _notifier.SuccessAsync(H["User {0} successfully disabled.", user.UserName]); } - user.IsEnabled = false; - await _userManager.UpdateAsync(user); - await _notifier.SuccessAsync(H["User {0} successfully disabled.", user.UserName]); - } - break; - case UsersBulkAction.Enable: - foreach (var user in checkedUsers) - { - if (String.Equals(user.UserId, User.FindFirstValue(ClaimTypes.NameIdentifier), StringComparison.OrdinalIgnoreCase)) + break; + case UsersBulkAction.Enable: + if (!isSameUser && canEditUser) { - continue; + user.IsEnabled = true; + await _userManager.UpdateAsync(user); + await _notifier.SuccessAsync(H["User {0} successfully enabled.", user.UserName]); } - user.IsEnabled = true; - await _userManager.UpdateAsync(user); - await _notifier.SuccessAsync(H["User {0} successfully enabled.", user.UserName]); - } - break; - default: - throw new ArgumentOutOfRangeException(); + break; + default: + throw new ArgumentOutOfRangeException(nameof(options.BulkAction), "Invalid bulk options."); + } } - } + } return RedirectToAction(nameof(Index)); } public async Task Create() { var user = new User(); - if (!await _authorizationService.AuthorizeAsync(User, Permissions.ViewUsers, user)) + if (!await _authorizationService.AuthorizeAsync(User, CommonPermissions.EditUsers, user)) { return Forbid(); } @@ -319,7 +299,7 @@ public async Task CreatePost([Bind(Prefix = "User.Password")] str { var user = new User(); - if (!await _authorizationService.AuthorizeAsync(User, Permissions.ViewUsers, user)) + if (!await _authorizationService.AuthorizeAsync(User, CommonPermissions.EditUsers, user)) { return Forbid(); } @@ -350,20 +330,19 @@ public async Task Edit(string id, string returnUrl) if (String.IsNullOrEmpty(id)) { id = User.FindFirstValue(ClaimTypes.NameIdentifier); - if (!await _authorizationService.AuthorizeAsync(User, Permissions.ManageOwnUserInformation)) + if (!await _authorizationService.AuthorizeAsync(User, Permissions.EditOwnUserInformation)) { return Forbid(); } editingOwnUser = true; } - var user = await _userManager.FindByIdAsync(id) as User; - if (user == null) + if (await _userManager.FindByIdAsync(id) is not User user) { return NotFound(); } - if (!editingOwnUser && !await _authorizationService.AuthorizeAsync(User, Permissions.ViewUsers, user)) + if (!editingOwnUser && !await _authorizationService.AuthorizeAsync(User, CommonPermissions.EditUsers, user)) { return Forbid(); } @@ -385,19 +364,18 @@ public async Task EditPost(string id, string returnUrl) { editingOwnUser = true; id = User.FindFirstValue(ClaimTypes.NameIdentifier); - if (!await _authorizationService.AuthorizeAsync(User, Permissions.ManageOwnUserInformation)) + if (!await _authorizationService.AuthorizeAsync(User, Permissions.EditOwnUserInformation)) { return Forbid(); } } - var user = await _userManager.FindByIdAsync(id) as User; - if (user == null) + if (await _userManager.FindByIdAsync(id) is not User user) { return NotFound(); } - if (!editingOwnUser && !await _authorizationService.AuthorizeAsync(User, Permissions.ViewUsers, user)) + if (!editingOwnUser && !await _authorizationService.AuthorizeAsync(User, CommonPermissions.EditUsers, user)) { return Forbid(); } @@ -413,7 +391,7 @@ public async Task EditPost(string id, string returnUrl) foreach (var error in result.Errors) { - ModelState.AddModelError(string.Empty, error.Description); + ModelState.AddModelError(String.Empty, error.Description); } if (!ModelState.IsValid) @@ -451,14 +429,12 @@ public async Task EditPost(string id, string returnUrl) [HttpPost] public async Task Delete(string id) { - var user = await _userManager.FindByIdAsync(id) as User; - - if (user == null) + if (await _userManager.FindByIdAsync(id) is not User user) { return NotFound(); } - if (!await _authorizationService.AuthorizeAsync(User, Permissions.ManageUsers, user)) + if (!await _authorizationService.AuthorizeAsync(User, CommonPermissions.DeleteUsers, user)) { return Forbid(); } @@ -486,14 +462,12 @@ public async Task Delete(string id) public async Task EditPassword(string id) { - var user = await _userManager.FindByIdAsync(id) as User; - - if (user == null) + if (await _userManager.FindByIdAsync(id) is not User user) { return NotFound(); } - if (!await _authorizationService.AuthorizeAsync(User, Permissions.ManageUsers, user)) + if (!await _authorizationService.AuthorizeAsync(User, CommonPermissions.EditUsers, user)) { return Forbid(); } @@ -506,14 +480,12 @@ public async Task EditPassword(string id) [HttpPost] public async Task EditPassword(ResetPasswordViewModel model) { - var user = await _userManager.FindByEmailAsync(model.Email) as User; - - if (user == null) + if (await _userManager.FindByEmailAsync(model.Email) is not User user) { return NotFound(); } - if (!await _authorizationService.AuthorizeAsync(User, Permissions.ManageUsers, user)) + if (!await _authorizationService.AuthorizeAsync(User, CommonPermissions.EditUsers, user)) { return Forbid(); } @@ -536,16 +508,14 @@ public async Task EditPassword(ResetPasswordViewModel model) [HttpPost] public async Task Unlock(string id) { - if (!await _authorizationService.AuthorizeAsync(User, Permissions.ManageUsers)) + if (await _userManager.FindByIdAsync(id) is not User user) { - return Forbid(); + return NotFound(); } - var user = await _userManager.FindByIdAsync(id) as User; - - if (user == null) + if (!await _authorizationService.AuthorizeAsync(User, CommonPermissions.EditUsers, user)) { - return NotFound(); + return Forbid(); } await _userManager.ResetAccessFailedCountAsync(user); diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserDisplayDriver.cs b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserDisplayDriver.cs index 783e4383dc6..62d0d12f0c3 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserDisplayDriver.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserDisplayDriver.cs @@ -7,16 +7,16 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Identity; using Microsoft.AspNetCore.Mvc.Localization; +using Microsoft.Extensions.Localization; using Microsoft.Extensions.Logging; using OrchardCore.DisplayManagement.Handlers; using OrchardCore.DisplayManagement.Notify; using OrchardCore.DisplayManagement.Views; using OrchardCore.Modules; +using OrchardCore.Mvc.ModelBinding; using OrchardCore.Users.Handlers; using OrchardCore.Users.Models; using OrchardCore.Users.ViewModels; -using OrchardCore.Mvc.ModelBinding; -using Microsoft.Extensions.Localization; namespace OrchardCore.Users.Drivers { @@ -60,28 +60,31 @@ public override IDisplayResult Display(User user) ); } - public override Task EditAsync(User user, BuildEditorContext context) + public override async Task EditAsync(User user, BuildEditorContext context) { - return Task.FromResult(Initialize("UserFields_Edit", async model => - { - model.EmailConfirmed = user.EmailConfirmed; - model.IsEnabled = user.IsEnabled; - model.IsNewRequest = context.IsNew; - // The current user cannot disable themselves, nor can a user without permission to manage this user disable them. - model.IsEditingDisabled = !await _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext.User, Permissions.ManageUsers, user) || - String.Equals(_httpContextAccessor.HttpContext.User.FindFirstValue(ClaimTypes.NameIdentifier), user.UserId, StringComparison.OrdinalIgnoreCase); - }) - .Location("Content:1.5") - .RenderWhen(() => _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext.User, Permissions.ViewUsers, user))); + if (!await _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext.User, CommonPermissions.EditUsers, user)) + { + return null; + } + + return Initialize("UserFields_Edit", model => + { + model.EmailConfirmed = user.EmailConfirmed; + model.IsEnabled = user.IsEnabled; + model.IsNewRequest = context.IsNew; + // The current user cannot disable themselves, nor can a user without permission to manage this user disable them. + model.IsEditingDisabled = IsCurrentUser(user); + }) + .Location("Content:1.5"); } public override async Task UpdateAsync(User user, UpdateEditorContext context) { // To prevent html injection when updating the user must meet all authorization requirements. - if (!await _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext.User, Permissions.ManageUsers, user)) + if (!await _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext.User, CommonPermissions.EditUsers, user)) { // When the user is only editing their profile never update this part of the user. - return Edit(user); + return await EditAsync(user, context); } var model = new EditUserViewModel(); @@ -90,7 +93,7 @@ public override async Task UpdateAsync(User user, UpdateEditorCo if (context.IsNew) { - if (string.IsNullOrWhiteSpace(model.Password)) + if (String.IsNullOrWhiteSpace(model.Password)) { context.Updater.ModelState.AddModelError(Prefix, nameof(model.Password), S["A password is required"]); } @@ -106,8 +109,7 @@ public override async Task UpdateAsync(User user, UpdateEditorCo return await EditAsync(user, context); } - var isEditingDisabled = !await _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext.User, Permissions.ManageUsers, user) || - String.Equals(_httpContextAccessor.HttpContext.User.FindFirstValue(ClaimTypes.NameIdentifier), user.UserId, StringComparison.OrdinalIgnoreCase); + var isEditingDisabled = IsCurrentUser(user); if (!isEditingDisabled && !model.IsEnabled && user.IsEnabled) { @@ -145,5 +147,10 @@ public override async Task UpdateAsync(User user, UpdateEditorCo return await EditAsync(user, context); } + + private bool IsCurrentUser(User user) + { + return String.Equals(_httpContextAccessor.HttpContext.User.FindFirstValue(ClaimTypes.NameIdentifier), user.UserId, StringComparison.OrdinalIgnoreCase); + } } } diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserInformationDisplayDriver.cs b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserInformationDisplayDriver.cs index be17783ca2d..2f172107af2 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserInformationDisplayDriver.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserInformationDisplayDriver.cs @@ -5,6 +5,8 @@ using Microsoft.AspNetCore.Http; using OrchardCore.DisplayManagement.Handlers; using OrchardCore.DisplayManagement.Views; +using OrchardCore.Entities; +using OrchardCore.Settings; using OrchardCore.Users.Models; using OrchardCore.Users.ViewModels; @@ -14,73 +16,103 @@ public class UserInformationDisplayDriver : DisplayDriver { private readonly IHttpContextAccessor _httpContextAccessor; private readonly IAuthorizationService _authorizationService; + private readonly ISiteService _siteService; public UserInformationDisplayDriver( IHttpContextAccessor httpContextAccessor, - IAuthorizationService authorizationService) + IAuthorizationService authorizationService, + ISiteService siteService) { _httpContextAccessor = httpContextAccessor; _authorizationService = authorizationService; + _siteService = siteService; } - public override IDisplayResult Edit(User user) + public override async Task EditAsync(User user, BuildEditorContext context) { - return Initialize("UserInformationFields_Edit", async model => + if (!await _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext.User, CommonPermissions.EditUsers, user)) { - model.UserName = user.UserName; - model.Email = user.Email; - model.IsEditingDisabled = !await AuthorizeUpdateAsync(user); - }) - .Location("Content:1") - .RenderWhen(() => AuthorizeEditAsync(user)); + return null; + } + + var site = await _siteService.GetSiteSettingsAsync(); + var settings = site.As(); + + return Combine( + Initialize("UserName_Edit", async model => + { + model.UserName = user.UserName; + + model.AllowEditing = context.IsNew || (settings.AllowChangingUsername && await CanEditUserInfoAsync(user)); + + }).Location("Content:1"), + + Initialize("UserEmail_Edit", async model => + { + model.Email = user.Email; + + model.AllowEditing = context.IsNew || (settings.AllowChangingEmail && await CanEditUserInfoAsync(user)); + + }).Location("Content:1.3") + ); } + public override async Task UpdateAsync(User user, UpdateEditorContext context) { - if (!await AuthorizeUpdateAsync(user)) + if (!await _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext.User, CommonPermissions.EditUsers, user)) { - return Edit(user); + return null; } - var model = new EditUserInformationViewModel(); + var userNameModel = new EditUserNameViewModel(); + var emailModel = new EditUserEmailViewModel(); - if (await context.Updater.TryUpdateModelAsync(model, Prefix)) - { - // Do not use the user manager to set these values, or validate them here, as they will validate at the incorrect time. - // After this driver runs the IUserService.UpdateAsync or IUserService.CreateAsync method will - // validate the user and provide the correct error messages based on the entire user objects values. + // Do not use the user manager to set these values, or validate them here, as they will validate at the incorrect time. + // After this driver runs the IUserService.UpdateAsync or IUserService.CreateAsync method will + // validate the user and provide the correct error messages based on the entire user objects values. - // Custom properties should still be validated in the driver. + // Custom properties should still be validated in the driver. - user.UserName = model.UserName; - user.Email = model.Email; - } + var site = await _siteService.GetSiteSettingsAsync(); + var settings = site.As(); - return Edit(user); - } + if (context.IsNew) + { + if (await context.Updater.TryUpdateModelAsync(userNameModel, Prefix)) + { + user.UserName = userNameModel.UserName; + } - private Task AuthorizeUpdateAsync(User user) - { - // When the current user matches this user we can ask for ManageOwnUserInformation - if (String.Equals(user.UserId, _httpContextAccessor.HttpContext.User.FindFirstValue(ClaimTypes.NameIdentifier), StringComparison.OrdinalIgnoreCase)) + if (await context.Updater.TryUpdateModelAsync(emailModel, Prefix)) + { + user.Email = emailModel.Email; + } + } + else { - return _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext.User, Permissions.ManageOwnUserInformation); + if (settings.AllowChangingUsername && await CanEditUserInfoAsync(user) && await context.Updater.TryUpdateModelAsync(userNameModel, Prefix)) + { + user.UserName = userNameModel.UserName; + } + + if (settings.AllowChangingEmail && await CanEditUserInfoAsync(user) && await context.Updater.TryUpdateModelAsync(emailModel, Prefix)) + { + user.Email = emailModel.Email; + } } - // Otherwise we require permission to manage this users information. - return _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext.User, Permissions.ManageUsers, user); + return await EditAsync(user, context); } - private Task AuthorizeEditAsync(User user) + private async Task CanEditUserInfoAsync(User user) { - // When the current user matches this user we can ask for ManageOwnUserInformation - if (String.Equals(user.UserId, _httpContextAccessor.HttpContext.User.FindFirstValue(ClaimTypes.NameIdentifier), StringComparison.OrdinalIgnoreCase)) - { - return _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext.User, Permissions.ManageOwnUserInformation); - } + return !IsCurrentUser(user) || await _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext.User, Permissions.EditOwnUserInformation); + } - // Otherwise we require permission to manage this users information. - return _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext.User, Permissions.ViewUsers, user); + private bool IsCurrentUser(User user) + { + return String.Equals(_httpContextAccessor.HttpContext.User.FindFirstValue(ClaimTypes.NameIdentifier), user.UserId, StringComparison.OrdinalIgnoreCase); } } } diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserProfileSettingsDisplayDriver.cs b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserProfileSettingsDisplayDriver.cs new file mode 100644 index 00000000000..45cee92de45 --- /dev/null +++ b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserProfileSettingsDisplayDriver.cs @@ -0,0 +1,72 @@ +using System; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Http; +using OrchardCore.DisplayManagement.Entities; +using OrchardCore.DisplayManagement.Handlers; +using OrchardCore.DisplayManagement.ModelBinding; +using OrchardCore.DisplayManagement.Views; +using OrchardCore.Environment.Shell; +using OrchardCore.Settings; +using OrchardCore.Users.Models; +using OrchardCore.Users.ViewModels; + +namespace OrchardCore.Users.Drivers; + +public class UserProfileSettingsDisplayDriver : SectionDisplayDriver +{ + public const string GroupId = "user-profile"; + + private readonly IHttpContextAccessor _httpContextAccessor; + private readonly IAuthorizationService _authorizationService; + private readonly IShellHost _shellHost; + private readonly ShellSettings _shellSettings; + + public UserProfileSettingsDisplayDriver(IHttpContextAccessor httpContextAccessor, + IAuthorizationService authorizationService, + IShellHost shellHost, + ShellSettings shellSettings) + { + _httpContextAccessor = httpContextAccessor; + _authorizationService = authorizationService; + _shellHost = shellHost; + _shellSettings = shellSettings; + } + + public override IDisplayResult Edit(UserProfileSettings settings, BuildEditorContext context) + { + var user = _httpContextAccessor.HttpContext?.User; + + return Initialize("UserProfileSettings_Edit", model => + { + model.AllowChangingEmail = settings.AllowChangingEmail; + model.AllowChangingUsername = settings.AllowChangingUsername; + + }).Location("Content:5").OnGroup(GroupId) + .RenderWhen(() => _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext?.User, CommonPermissions.ManageUserProfileSettings)); + } + + + public override async Task UpdateAsync(UserProfileSettings settings, IUpdateModel updater, BuildEditorContext context) + { + if (!GroupId.Equals(context.GroupId, StringComparison.OrdinalIgnoreCase) || !await _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext?.User, CommonPermissions.ManageUserProfileSettings)) + { + return null; + } + + var model = new UserProfileSettingsViewModel(); + + await updater.TryUpdateModelAsync(model, Prefix); + + if (settings.AllowChangingEmail != model.AllowChangingEmail || settings.AllowChangingUsername != model.AllowChangingUsername) + { + settings.AllowChangingEmail = model.AllowChangingEmail; + settings.AllowChangingUsername = model.AllowChangingUsername; + + // Release the tenant to apply the settings + await _shellHost.ReleaseShellContextAsync(_shellSettings); + } + + return Edit(settings, context); + } +} diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserRoleDisplayDriver.cs b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserRoleDisplayDriver.cs index b713cf5bfa6..fad410545a7 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserRoleDisplayDriver.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserRoleDisplayDriver.cs @@ -2,7 +2,6 @@ using System.Collections.Generic; using System.Linq; using System.Security.Claims; -using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Http; @@ -11,6 +10,7 @@ using OrchardCore.DisplayManagement.Handlers; using OrchardCore.DisplayManagement.Notify; using OrchardCore.DisplayManagement.Views; +using OrchardCore.Security; using OrchardCore.Security.Services; using OrchardCore.Users.Models; using OrchardCore.Users.ViewModels; @@ -47,44 +47,46 @@ public UserRoleDisplayDriver( H = htmlLocalizer; } + public override IDisplayResult Display(User user) + { + return Initialize("RolesMeta", model => model.User = user) + .Location("SummaryAdmin", "Meta"); + } public override IDisplayResult Edit(User user) { // This view is always rendered, however there will be no editable roles if the user does not have permission to edit them. return Initialize("UserRoleFields_Edit", async model => { - // The current user can only view their roles if they have list users, to prevent listing roles when managing their own profile. + // The current user can only view their roles if they have assign role, to prevent listing roles when managing their own profile. if (String.Equals(_httpContextAccessor.HttpContext.User.FindFirstValue(ClaimTypes.NameIdentifier), user.UserId, StringComparison.OrdinalIgnoreCase) && - !await _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext.User, Permissions.ViewUsers)) + !await _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext.User, CommonPermissions.AssignRole)) { return; } - var roleNames = await GetRoleNamesAsync(); + var roles = await GetRoleAsync(); // When a user is in a role that the current user cannot manage the role is shown but selection is disabled. - var authorizedRoleNames = await GetAuthorizedRoleNamesAsync(roleNames); - var userRoleNames = await _userRoleStore.GetRolesAsync(user, default(CancellationToken)); + var authorizedRoleNames = await GetAuthorizedRoleNamesAsync(roles); + var userRoleNames = await _userRoleStore.GetRolesAsync(user, default); var roleEntries = new List(); - foreach (var roleName in roleNames) + foreach (var role in roles) { var roleEntry = new RoleEntry { - Role = roleName, - IsSelected = userRoleNames.Contains(roleName, StringComparer.OrdinalIgnoreCase) + Role = role.RoleName, + IsSelected = userRoleNames.Contains(role.RoleName, StringComparer.OrdinalIgnoreCase), + IsEditingDisabled = !authorizedRoleNames.Contains(role.RoleName, StringComparer.OrdinalIgnoreCase) }; - if (!authorizedRoleNames.Contains(roleName, StringComparer.OrdinalIgnoreCase)) - { - roleEntry.IsEditingDisabled = true; - } - roleEntries.Add(roleEntry); } model.Roles = roleEntries.ToArray(); }) - .Location("Content:1.10"); + .Location("Content:1.10") + .RenderWhen(() => _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext.User, CommonPermissions.EditUsers, user)); } public override async Task UpdateAsync(User user, UpdateEditorContext context) @@ -99,18 +101,21 @@ public override async Task UpdateAsync(User user, UpdateEditorCo if (await context.Updater.TryUpdateModelAsync(model, Prefix)) { + var roles = await GetRoleAsync(); // Authorize each role in the model to prevent html injection. - var authorizedRoleNames = await GetAuthorizedRoleNamesAsync(model.Roles.Select(x => x.Role)); - var userRoleNames = await _userRoleStore.GetRolesAsync(user, default(CancellationToken)); + var authorizedRoleNames = await GetAuthorizedRoleNamesAsync(roles); + var userRoleNames = await _userRoleStore.GetRolesAsync(user, default); - var authorizedSelectedRoleNames = await GetAuthorizedRoleNamesAsync(model.Roles.Where(x => x.IsSelected).Select(x => x.Role)); + var selectedRoleNames = model.Roles.Where(x => x.IsSelected).Select(x => x.Role); + var selectedRoles = roles.Where(x => selectedRoleNames.Contains(x.RoleName, StringComparer.OrdinalIgnoreCase)); + var authorizedSelectedRoleNames = await GetAuthorizedRoleNamesAsync(selectedRoles); if (context.IsNew) { // Only add authorized new roles. foreach (var role in authorizedSelectedRoleNames) { - await _userRoleStore.AddToRoleAsync(user, _userManager.NormalizeName(role), default(CancellationToken)); + await _userRoleStore.AddToRoleAsync(user, _userManager.NormalizeName(role), default); } } else @@ -125,6 +130,7 @@ public override async Task UpdateAsync(User user, UpdateEditorCo rolesToRemove.Add(role); } } + foreach (var role in rolesToRemove) { if (String.Equals(role, AdministratorRole, StringComparison.OrdinalIgnoreCase)) @@ -136,23 +142,18 @@ public override async Task UpdateAsync(User user, UpdateEditorCo await _notifier.WarningAsync(H["Cannot remove administrator role from the only administrator."]); continue; } - else - { - await _userRoleStore.RemoveFromRoleAsync(user, _userManager.NormalizeName(role), default(CancellationToken)); - } - } - else - { - await _userRoleStore.RemoveFromRoleAsync(user, _userManager.NormalizeName(role), default(CancellationToken)); } + + await _userRoleStore.RemoveFromRoleAsync(user, _userManager.NormalizeName(role), default); } // Add new roles foreach (var role in authorizedSelectedRoleNames) { - if (!await _userRoleStore.IsInRoleAsync(user, _userManager.NormalizeName(role), default(CancellationToken))) + var normalName = _userManager.NormalizeName(role); + if (!await _userRoleStore.IsInRoleAsync(user, normalName, default)) { - await _userRoleStore.AddToRoleAsync(user, _userManager.NormalizeName(role), default(CancellationToken)); + await _userRoleStore.AddToRoleAsync(user, normalName, default); } } } @@ -161,20 +162,22 @@ public override async Task UpdateAsync(User user, UpdateEditorCo return Edit(user); } - private async Task> GetRoleNamesAsync() + private async Task> GetRoleAsync() { - var roleNames = await _roleService.GetRoleNamesAsync(); - return roleNames.Except(new[] { "Anonymous", "Authenticated" }, StringComparer.OrdinalIgnoreCase); + var roles = await _roleService.GetRolesAsync(); + var exclude = new[] { "Anonymous", "Authenticated" }; + + return roles.Where(role => !exclude.Any(x => x.Equals(role.RoleName, StringComparison.OrdinalIgnoreCase))); } - private async Task> GetAuthorizedRoleNamesAsync(IEnumerable roleNames) + private async Task> GetAuthorizedRoleNamesAsync(IEnumerable roles) { var authorizedRoleNames = new List(); - foreach (var roleName in roleNames) + foreach (var role in roles) { - if (await _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext.User, OrchardCore.Roles.CommonPermissions.CreatePermissionForAssignRole(roleName))) + if (await _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext.User, CommonPermissions.AssignRole, role)) { - authorizedRoleNames.Add(roleName); + authorizedRoleNames.Add(role.RoleName); } } diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Permissions.cs b/src/OrchardCore.Modules/OrchardCore.Users/Permissions.cs index 0bf40ce27bb..ffcef961def 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Permissions.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Permissions.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; +using Microsoft.Extensions.DependencyInjection; using OrchardCore.Security.Permissions; using OrchardCore.Security.Services; @@ -11,31 +12,50 @@ public class Permissions : IPermissionProvider { public static readonly Permission ManageUsers = CommonPermissions.ManageUsers; public static readonly Permission ViewUsers = CommonPermissions.ViewUsers; + public static readonly Permission EditOwnUserInformation = new("ManageOwnUserInformation", "Edit own user information", new Permission[] { ManageUsers }); - public static readonly Permission ManageOwnUserInformation = new Permission("ManageOwnUserInformation", "Manage own user information", new Permission[] { ManageUsers }); + private readonly IServiceProvider _serviceProvider; - private readonly IRoleService _roleService; + private IRoleService _roleService; - public Permissions(IRoleService roleService) + public Permissions(IServiceProvider serviceProvider) { - _roleService = roleService; + _serviceProvider = serviceProvider; } public async Task> GetPermissionsAsync() { - var list = new List + var list = new List() { - ManageUsers, - ManageOwnUserInformation, - ViewUsers + EditOwnUserInformation }; - var roles = (await _roleService.GetRoleNamesAsync()) - .Except(new[] { "Anonymous", "Authenticated" }, StringComparer.OrdinalIgnoreCase); + // lazy resolve the RoleService and it may not be available since the Users modules does not have direct dependeny on the Roles module + _roleService ??= _serviceProvider.GetService(); - foreach (var role in roles) + if (_roleService != null) { - list.Add(CommonPermissions.CreatePermissionForManageUsersInRole(role)); + var roles = (await _roleService.GetRoleNamesAsync()) + .Except(new[] { "Anonymous", "Authenticated" }, StringComparer.OrdinalIgnoreCase) + .OrderBy(x => x).ToList(); + + list.Add(CommonPermissions.ListUsers); + foreach (var role in roles) + { + list.Add(CommonPermissions.CreateListUsersInRolePermission(role)); + } + + list.Add(CommonPermissions.EditUsers); + foreach (var role in roles) + { + list.Add(CommonPermissions.CreateEditUsersInRolePermission(role)); + } + + list.Add(CommonPermissions.DeleteUsers); + foreach (var role in roles) + { + list.Add(CommonPermissions.CreateDeleteUsersInRolePermission(role)); + } } return list; @@ -46,23 +66,29 @@ public IEnumerable GetDefaultStereotypes() return new[] { new PermissionStereotype { Name = "Administrator", - Permissions = new[] { ManageUsers } + Permissions = new[] { + ManageUsers, + CommonPermissions.ListUsers, + CommonPermissions.EditUsers, + CommonPermissions.DeleteUsers, + CommonPermissions.ManageUserProfileSettings + } }, new PermissionStereotype { Name = "Editor", - Permissions = new[] { ManageOwnUserInformation } + Permissions = new[] { EditOwnUserInformation } }, new PermissionStereotype { Name = "Moderator", - Permissions = new[] { ManageOwnUserInformation } + Permissions = new[] { EditOwnUserInformation } }, new PermissionStereotype { Name = "Contributor", - Permissions = new[] { ManageOwnUserInformation } + Permissions = new[] { EditOwnUserInformation } }, new PermissionStereotype { Name = "Author", - Permissions = new[] { ManageOwnUserInformation } + Permissions = new[] { EditOwnUserInformation } } }; } diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Services/DefaultUsersAdminListFilterProvider.cs b/src/OrchardCore.Modules/OrchardCore.Users/Services/DefaultUsersAdminListFilterProvider.cs index ce0e6791ba0..e12748f5d0c 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Services/DefaultUsersAdminListFilterProvider.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Services/DefaultUsersAdminListFilterProvider.cs @@ -1,7 +1,10 @@ using System; using System.Threading.Tasks; +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.DependencyInjection; +using OrchardCore.Security.Services; using OrchardCore.Users.Indexes; using OrchardCore.Users.Models; using OrchardCore.Users.ViewModels; @@ -43,15 +46,12 @@ public void Build(QueryEngineBuilder builder) }) .MapFrom((model) => { - switch (model.Filter) + return model.Filter switch { - case UsersFilter.Enabled: - return (true, model.Filter.ToString()); - case UsersFilter.Disabled: - return (true, model.Filter.ToString()); - } - - return (false, String.Empty); + UsersFilter.Enabled => (true, model.Filter.ToString()), + UsersFilter.Disabled => (true, model.Filter.ToString()), + _ => (false, String.Empty) + }; }) ) .WithNamedTerm("sort", builder => builder diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Services/DefaultUsersAdminListQueryService.cs b/src/OrchardCore.Modules/OrchardCore.Users/Services/DefaultUsersAdminListQueryService.cs index f5c7b1a753a..d5b90eaf707 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Services/DefaultUsersAdminListQueryService.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Services/DefaultUsersAdminListQueryService.cs @@ -1,7 +1,9 @@ using System; +using System.Collections.Generic; using System.Threading.Tasks; using Microsoft.Extensions.Logging; using OrchardCore.DisplayManagement.ModelBinding; +using OrchardCore.Modules; using OrchardCore.Users.Models; using OrchardCore.Users.ViewModels; using YesSql; @@ -13,15 +15,18 @@ public class DefaultUsersAdminListQueryService : IUsersAdminListQueryService private readonly ISession _session; private readonly IServiceProvider _serviceProvider; private readonly ILogger _logger; + private readonly IEnumerable _usersAdminListFilters; public DefaultUsersAdminListQueryService( ISession session, IServiceProvider serviceProvider, - ILogger logger) + ILogger logger, + IEnumerable usersAdminListFilters) { _session = session; _serviceProvider = serviceProvider; _logger = logger; + _usersAdminListFilters = usersAdminListFilters; } public async Task> QueryAsync(UserIndexOptions options, IUpdateModel updater) @@ -31,6 +36,8 @@ public async Task> QueryAsync(UserIndexOptions options, IUpdateMode query = await options.FilterResult.ExecuteAsync(new UserQueryContext(_serviceProvider, query)); + await _usersAdminListFilters.InvokeAsync((filter, model, query, updater) => filter.FilterAsync(model, query, updater), options, query, updater, _logger); + return query; } } diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Services/RoleAuthorizationHandler.cs b/src/OrchardCore.Modules/OrchardCore.Users/Services/RoleAuthorizationHandler.cs new file mode 100644 index 00000000000..507bf646f11 --- /dev/null +++ b/src/OrchardCore.Modules/OrchardCore.Users/Services/RoleAuthorizationHandler.cs @@ -0,0 +1,136 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Security.Claims; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Authorization; +using Microsoft.Extensions.DependencyInjection; +using OrchardCore.Security; +using OrchardCore.Security.Permissions; +using OrchardCore.Security.Services; +using OrchardCore.Users.Models; + +namespace OrchardCore.Users.Services; + +public class RoleAuthorizationHandler : AuthorizationHandler +{ + private readonly IServiceProvider _serviceProvider; + + private IAuthorizationService _authorizationService; + private IRoleService _roleService; + + public RoleAuthorizationHandler(IServiceProvider serviceProvider) + { + _serviceProvider = serviceProvider; + } + + protected override async Task HandleRequirementAsync(AuthorizationHandlerContext context, PermissionRequirement requirement) + { + if (context.HasSucceeded) + { + // This handler is not revoking any pre-existing grants. + return; + } + + if (context.Resource == null) + { + return; + } + + // Lazy load to prevent circular dependencies + _authorizationService ??= _serviceProvider?.GetService(); + _roleService ??= _serviceProvider?.GetService(); + + if (context.Resource is IRole role) + { + if (await TryAuthenticateAsync(context, requirement.Permission, role.RoleName)) + { + context.Succeed(requirement); + + return; + } + } + + if (context.Resource is User user) + { + var currentUserId = context.User.FindFirstValue(ClaimTypes.NameIdentifier); + + if (String.Equals(requirement.Permission.Name, CommonPermissions.EditUsers.Name, StringComparison.OrdinalIgnoreCase) + && user.UserId != null && user.UserId.Equals(currentUserId, StringComparison.OrdinalIgnoreCase) + && await _authorizationService.AuthorizeAsync(context.User, Permissions.EditOwnUserInformation)) + { + context.Succeed(requirement); + + return; + } + + if (await _authorizationService.AuthorizeAsync(context.User, requirement.Permission)) + { + context.Succeed(requirement); + + return; + } + + var roleNames = user.RoleNames ?? Enumerable.Empty(); + + if (!roleNames.Any()) + { + // if the provided user does not have role mean it's a dummy user and we'll check to see if the current user has access to any permission variant + roleNames = await AvailableRolesAsync(); + } + + foreach (var roleName in roleNames) + { + if (await TryAuthenticateAsync(context, requirement.Permission, roleName)) + { + context.Succeed(requirement); + + return; + } + } + + } + } + + private IEnumerable _roleNames; + + private async Task> AvailableRolesAsync() + { + if (_roleNames == null) + { + _roleNames = (await _roleService.GetRoleNamesAsync()) + .Except(new[] { "Anonymous", "Authenticated" }, StringComparer.OrdinalIgnoreCase); + } + + return _roleNames; + } + + private async Task TryAuthenticateAsync(AuthorizationHandlerContext context, Permission permission, string roleName) + { + if (String.Equals(permission.Name, CommonPermissions.ListUsers.Name, StringComparison.OrdinalIgnoreCase) + && await _authorizationService.AuthorizeAsync(context.User, CommonPermissions.CreateListUsersInRolePermission(roleName))) + { + return true; + } + + if (String.Equals(permission.Name, CommonPermissions.EditUsers.Name, StringComparison.OrdinalIgnoreCase) + && await _authorizationService.AuthorizeAsync(context.User, CommonPermissions.CreateEditUsersInRolePermission(roleName))) + { + return true; + } + + if (String.Equals(permission.Name, CommonPermissions.DeleteUsers.Name, StringComparison.OrdinalIgnoreCase) + && await _authorizationService.AuthorizeAsync(context.User, CommonPermissions.CreateDeleteUsersInRolePermission(roleName))) + { + return true; + } + + if (String.Equals(permission.Name, CommonPermissions.AssignRole.Name, StringComparison.OrdinalIgnoreCase) + && await _authorizationService.AuthorizeAsync(context.User, CommonPermissions.CreateAssignUserToRolePermission(roleName))) + { + return true; + } + + return false; + } +} diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Services/RolesAdminListFilterProvider.cs b/src/OrchardCore.Modules/OrchardCore.Users/Services/RolesAdminListFilterProvider.cs new file mode 100644 index 00000000000..e206ad2881e --- /dev/null +++ b/src/OrchardCore.Modules/OrchardCore.Users/Services/RolesAdminListFilterProvider.cs @@ -0,0 +1,40 @@ +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; +using OrchardCore.Security.Services; +using OrchardCore.Users.Indexes; +using OrchardCore.Users.Models; +using YesSql.Filters.Query; +using YesSql.Services; + +namespace OrchardCore.Users.Services; + +public class RolesAdminListFilterProvider : IUsersAdminListFilterProvider +{ + public void Build(QueryEngineBuilder builder) + { + builder.WithNamedTerm("role-restriction", builder => builder + .OneCondition(async (contentType, query, ctx) => + { + var context = (UserQueryContext)ctx; + + var httpContextAccessor = context.ServiceProvider.GetRequiredService(); + var authorizationService = context.ServiceProvider.GetRequiredService(); + var roleService = context.ServiceProvider.GetRequiredService(); + + var user = httpContextAccessor.HttpContext?.User; + + if (user != null && !await authorizationService.AuthorizeAsync(user, CommonPermissions.ListUsers)) + { + // at this point the user cannot see all users, so lets see what role does he have access too and filter by them + var accessiableRoles = await roleService.GetAccessibleRoleNamesAsync(authorizationService, user, CommonPermissions.ListUsers); + + query.With(index => index.Role.IsIn(accessiableRoles)); + } + + return query; + }) + .AlwaysRun() + ); + } +} diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Services/UserAuthorizationHandler.cs b/src/OrchardCore.Modules/OrchardCore.Users/Services/UserAuthorizationHandler.cs deleted file mode 100644 index bf75b19789c..00000000000 --- a/src/OrchardCore.Modules/OrchardCore.Users/Services/UserAuthorizationHandler.cs +++ /dev/null @@ -1,113 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using System.Security.Claims; -using System.Threading.Tasks; -using Microsoft.AspNetCore.Authorization; -using Microsoft.AspNetCore.Identity; -using Microsoft.Extensions.DependencyInjection; -using OrchardCore.Security; -using OrchardCore.Security.Services; - -namespace OrchardCore.Users.Services -{ - /// - /// This provides authorization when the permission request is ManageUsers and the current authenticated User - /// has permission to edit any of the roles the user being managed is a member of. - /// Callers should supply a resource - /// - /// - /// This handler will not be triggered when only requesting ManagerUsers permission with supplying an . - /// - public class UserAuthorizationHandler : AuthorizationHandler - { - private readonly IServiceProvider _serviceProvider; - private readonly UserManager _userManager; - private readonly IRoleService _roleService; - - public UserAuthorizationHandler( - IServiceProvider serviceProvider, - UserManager userManager, - IRoleService roleService) - { - _serviceProvider = serviceProvider; - _userManager = userManager; - _roleService = roleService; - } - - protected override async Task HandleRequirementAsync(AuthorizationHandlerContext context, PermissionRequirement requirement) - { - if (context.HasSucceeded) - { - // This handler is not revoking any pre-existing grants. - return; - } - - if (context.Resource == null) - { - return; - } - - if (!String.Equals(requirement.Permission.Name, Permissions.ManageUsers.Name, StringComparison.OrdinalIgnoreCase)) - { - return; - } - - var user = context.Resource as IUser; - - if (user == null) - { - return; - } - - // Lazy load to prevent circular dependencies - var authorizationService = _serviceProvider.GetService(); - - var userRoleNames = await _userManager.GetRolesAsync(user); - - if (userRoleNames.Any()) - { - // When the user has roles we check to see if any of these roles are not authorized before succeeding. - if (await AuthorizeRolesAsync(authorizationService, context.User, userRoleNames)) - { - context.Succeed(requirement); - } - } - else - { - var roleNames = await _roleService.GetRoleNamesAsync(); - // When the user is in no roles, we check to see if the current user can manage any roles. - if (await HasAuthorizationToManageAnyRoleAsync(authorizationService, context.User, roleNames)) - { - context.Succeed(requirement); - } - } - } - - private async Task AuthorizeRolesAsync(IAuthorizationService authorizationService, ClaimsPrincipal user, IEnumerable roleNames) - { - foreach (var roleName in roleNames) - { - if (!await authorizationService.AuthorizeAsync(user, CommonPermissions.CreatePermissionForManageUsersInRole(roleName))) - { - return false; - } - } - - return true; - } - - private async Task HasAuthorizationToManageAnyRoleAsync(IAuthorizationService authorizationService, ClaimsPrincipal user, IEnumerable roleNames) - { - foreach (var roleName in roleNames) - { - if (await authorizationService.AuthorizeAsync(user, CommonPermissions.CreatePermissionForManageUsersInRole(roleName))) - { - return true; - } - } - - return false; - } - } -} diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Startup.cs b/src/OrchardCore.Modules/OrchardCore.Users/Startup.cs index 666b3c07820..230db66117d 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Startup.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Startup.cs @@ -192,7 +192,6 @@ public override void ConfigureServices(IServiceCollection services) }); services.AddSingleton(); - services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); services.AddScoped(); @@ -202,12 +201,9 @@ public override void ConfigureServices(IServiceCollection services) services.AddScoped(); services.AddSingleton(); - services.AddScoped(); - services.AddScoped(); services.AddScoped(); services.AddScoped(); - services.AddScoped(); services.AddScoped(); services.AddScoped(); @@ -216,7 +212,6 @@ public override void ConfigureServices(IServiceCollection services) services.AddScoped, LoginSettingsDisplayDriver>(); services.AddScoped, UserDisplayDriver>(); - services.AddScoped, UserRoleDisplayDriver>(); services.AddScoped, UserInformationDisplayDriver>(); services.AddScoped, UserButtonsDisplayDriver>(); @@ -247,6 +242,24 @@ public override void ConfigureServices(IServiceCollection services) services.AddScoped(); services.AddTransient, UserOptionsConfiguration>(); + + services.AddScoped, UserProfileSettingsDisplayDriver>(); + } + } + + [Feature("OrchardCore.Users")] + [RequireFeatures("OrchardCore.Roles")] + public class RolesStartup : StartupBase + { + public override void ConfigureServices(IServiceCollection services) + { + services.AddScoped(); + services.AddSingleton(); + services.AddScoped, UserRoleDisplayDriver>(); + services.AddScoped(); + services.AddScoped(); + services.AddSingleton(); + services.AddSingleton(); } } diff --git a/src/OrchardCore.Modules/OrchardCore.Users/UserMigrations.cs b/src/OrchardCore.Modules/OrchardCore.Users/UserMigrations.cs new file mode 100644 index 00000000000..68a8e39b498 --- /dev/null +++ b/src/OrchardCore.Modules/OrchardCore.Users/UserMigrations.cs @@ -0,0 +1,39 @@ +using System.Threading.Tasks; +using OrchardCore.Data.Migration; +using OrchardCore.Users.Indexes; +using OrchardCore.Users.Models; +using YesSql; +using YesSql.Sql; + +namespace OrchardCore.Users; + +public class UserMigrations : DataMigration +{ + private readonly ISession _session; + + public UserMigrations(ISession session) + { + _session = session; + } + + public async Task CreateAsync() + { + SchemaBuilder.CreateMapIndexTable(table => table + .Column("UserId", column => column.WithLength(26)) + .Column("Role") + ); + + // first save exting changes to make sure that the index is created before we populate it + await _session.SaveChangesAsync(); + + var users = await _session.Query().ListAsync(); + + foreach (var user in users) + { + // this will force the index provider to update the indexes + _session.Save(user); + } + + return 1; + } +} diff --git a/src/OrchardCore.Modules/OrchardCore.Users/UserRolePermissions.cs b/src/OrchardCore.Modules/OrchardCore.Users/UserRolePermissions.cs new file mode 100644 index 00000000000..40e25691366 --- /dev/null +++ b/src/OrchardCore.Modules/OrchardCore.Users/UserRolePermissions.cs @@ -0,0 +1,55 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; +using OrchardCore.Security.Permissions; +using OrchardCore.Security.Services; + +namespace OrchardCore.Users +{ + public class UserRolePermissions : IPermissionProvider + { + public static readonly Permission ManageUsers = CommonPermissions.ManageUsers; + public static readonly Permission ViewUsers = CommonPermissions.ViewUsers; + public static readonly Permission EditOwnUserInformation = new("ManageOwnUserInformation", "Edit own user information", new Permission[] { ManageUsers }); + + private readonly IRoleService _roleService; + + public UserRolePermissions(IRoleService roleService) + { + _roleService = roleService; + } + + public async Task> GetPermissionsAsync() + { + var list = new List() + { + CommonPermissions.AssignRole + }; + + var roles = (await _roleService.GetRoleNamesAsync()) + .Except(new[] { "Anonymous", "Authenticated" }, StringComparer.OrdinalIgnoreCase) + .OrderBy(x => x).ToList(); + + foreach (var role in roles) + { + list.Add(CommonPermissions.CreateAssignUserToRolePermission(role)); + } + + return list; + } + + public IEnumerable GetDefaultStereotypes() + { + return new[] { + new PermissionStereotype { + Name = "Administrator", + Permissions = new[] { + CommonPermissions.AssignRole, + } + } + }; + } + } +} + diff --git a/src/OrchardCore.Modules/OrchardCore.Users/ViewModels/EditUserEmailViewModel.cs b/src/OrchardCore.Modules/OrchardCore.Users/ViewModels/EditUserEmailViewModel.cs new file mode 100644 index 00000000000..ae1e83b6545 --- /dev/null +++ b/src/OrchardCore.Modules/OrchardCore.Users/ViewModels/EditUserEmailViewModel.cs @@ -0,0 +1,15 @@ +using System.ComponentModel.DataAnnotations; +using Microsoft.AspNetCore.Mvc.ModelBinding; + +namespace OrchardCore.Users.ViewModels; + +public class EditUserEmailViewModel +{ + [Required(ErrorMessage = "Email is required.")] + [Email.EmailAddress(ErrorMessage = "Invalid Email.")] + public string Email { get; set; } + + [BindNever] + public bool AllowEditing { get; set; } +} + diff --git a/src/OrchardCore.Modules/OrchardCore.Users/ViewModels/EditUserNameViewModel.cs b/src/OrchardCore.Modules/OrchardCore.Users/ViewModels/EditUserNameViewModel.cs new file mode 100644 index 00000000000..0cfad2eaf04 --- /dev/null +++ b/src/OrchardCore.Modules/OrchardCore.Users/ViewModels/EditUserNameViewModel.cs @@ -0,0 +1,13 @@ +using System.ComponentModel.DataAnnotations; +using Microsoft.AspNetCore.Mvc.ModelBinding; + +namespace OrchardCore.Users.ViewModels; + +public class EditUserNameViewModel +{ + [Required] + public string UserName { get; set; } + + [BindNever] + public bool AllowEditing { get; set; } +} diff --git a/src/OrchardCore.Modules/OrchardCore.Users/ViewModels/UserProfileSettingsViewModel.cs b/src/OrchardCore.Modules/OrchardCore.Users/ViewModels/UserProfileSettingsViewModel.cs new file mode 100644 index 00000000000..688c34d7e9d --- /dev/null +++ b/src/OrchardCore.Modules/OrchardCore.Users/ViewModels/UserProfileSettingsViewModel.cs @@ -0,0 +1,8 @@ +namespace OrchardCore.Users.ViewModels; + +public class UserProfileSettingsViewModel +{ + public bool AllowChangingUsername { get; set; } + + public bool AllowChangingEmail { get; set; } +} diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Views/Admin/Index.cshtml b/src/OrchardCore.Modules/OrchardCore.Users/Views/Admin/Index.cshtml index 6c001d65908..6a1a0745287 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Views/Admin/Index.cshtml +++ b/src/OrchardCore.Modules/OrchardCore.Users/Views/Admin/Index.cshtml @@ -1,8 +1,5 @@ @model UsersIndexViewModel -@{ - int startIndex = (Model.Pager.Page - 1) * (Model.Pager.PageSize) + 1; - int endIndex = startIndex + Model.Users.Count - 1; -} +

@RenderTitleSegments(T["Users"])

 diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Views/NavigationItemText-UserProfileSettings.Id.cshtml b/src/OrchardCore.Modules/OrchardCore.Users/Views/NavigationItemText-UserProfileSettings.Id.cshtml new file mode 100644 index 00000000000..081b06a99fd --- /dev/null +++ b/src/OrchardCore.Modules/OrchardCore.Users/Views/NavigationItemText-UserProfileSettings.Id.cshtml @@ -0,0 +1 @@ +@T["User Profile Settings"] diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Views/RolesMeta.cshtml b/src/OrchardCore.Modules/OrchardCore.Users/Views/RolesMeta.cshtml new file mode 100644 index 00000000000..d770289aa1a --- /dev/null +++ b/src/OrchardCore.Modules/OrchardCore.Users/Views/RolesMeta.cshtml @@ -0,0 +1,8 @@ +@model SummaryAdminUserViewModel + +
+ @foreach (var roleName in Model.User.RoleNames) + { + @roleName + } +
diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Views/User.SummaryAdmin.cshtml b/src/OrchardCore.Modules/OrchardCore.Users/Views/User.SummaryAdmin.cshtml index 021b8dd1cce..7b09b3cb94a 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Views/User.SummaryAdmin.cshtml +++ b/src/OrchardCore.Modules/OrchardCore.Users/Views/User.SummaryAdmin.cshtml @@ -1,25 +1,27 @@ -
- @if (Model.Actions != null) - { -
- @await DisplayAsync(Model.Actions) -
- } +
+
+ @if (Model.Header != null) + { +
@await DisplayAsync(Model.Header)
+ } - @if (Model.Header != null) - { -
@await DisplayAsync(Model.Header)
- } + @if (Model.Meta != null) + { +
+ @await DisplayAsync(Model.Meta) +
+ } + + @if (Model.Content != null) + { +
@await DisplayAsync(Model.Content)
+ } +
- @if (Model.Meta != null) + @if (Model.Actions != null) { -
- @await DisplayAsync(Model.Meta) +
+ @await DisplayAsync(Model.Actions)
}
- -@if (Model.Content != null) -{ -
@await DisplayAsync(Model.Content)
-} diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Views/UserButtons.cshtml b/src/OrchardCore.Modules/OrchardCore.Users/Views/UserButtons.cshtml index 8ec88436144..89ea847274d 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Views/UserButtons.cshtml +++ b/src/OrchardCore.Modules/OrchardCore.Users/Views/UserButtons.cshtml @@ -8,17 +8,14 @@ @{ var user = Model.User as User; var currentUser = user.UserName == User.Identity.Name; - var hasManageUser = await AuthorizationService.AuthorizeAsync(User, Permissions.ManageUsers, user); + var canEdit = await AuthorizationService.AuthorizeAsync(User, CommonPermissions.EditUsers, user); var isLockedOut = await UserManager.IsLockedOutAsync(user); } -@T["Edit"] -@if (hasManageUser) +@if (canEdit) { - if (!currentUser) - { - @T["Delete"] - } + @T["Edit"] + @T["Edit Password"] if (isLockedOut) { @@ -26,7 +23,12 @@ } } -@if (!user.EmailConfirmed && Site.As().UsersMustValidateEmail && hasManageUser) +@if (!currentUser && await AuthorizationService.AuthorizeAsync(User, CommonPermissions.DeleteUsers, user)) +{ + @T["Delete"] +} + +@if (canEdit && !user.EmailConfirmed && Site.As().UsersMustValidateEmail) {
diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Views/UserEmail.Edit.cshtml b/src/OrchardCore.Modules/OrchardCore.Users/Views/UserEmail.Edit.cshtml new file mode 100644 index 00000000000..f8177c2a12e --- /dev/null +++ b/src/OrchardCore.Modules/OrchardCore.Users/Views/UserEmail.Edit.cshtml @@ -0,0 +1,10 @@ +@model OrchardCore.Users.ViewModels.EditUserEmailViewModel + +
+ +
+ + + @T["The email is invalid."] +
+
diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Views/UserMenu.cshtml b/src/OrchardCore.Modules/OrchardCore.Users/Views/UserMenu.cshtml index 1eda4f04865..32892441353 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Views/UserMenu.cshtml +++ b/src/OrchardCore.Modules/OrchardCore.Users/Views/UserMenu.cshtml @@ -7,7 +7,7 @@
  • @T["Signed in as"]@User.Identity.Name
    • - @if (await AuthorizationService.AuthorizeAsync(User, Permissions.ManageOwnUserInformation)) + @if (await AuthorizationService.AuthorizeAsync(User, Permissions.EditOwnUserInformation)) {
  •   @T["Profile"]
    • } diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Views/UserName.Edit.cshtml b/src/OrchardCore.Modules/OrchardCore.Users/Views/UserName.Edit.cshtml new file mode 100644 index 00000000000..2a9c3329a86 --- /dev/null +++ b/src/OrchardCore.Modules/OrchardCore.Users/Views/UserName.Edit.cshtml @@ -0,0 +1,10 @@ +@model OrchardCore.Users.ViewModels.EditUserNameViewModel + +
    + +
    + + + @T["The user name is required."] +
    +
    \ No newline at end of file diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Views/UserProfileSettings.Edit.cshtml b/src/OrchardCore.Modules/OrchardCore.Users/Views/UserProfileSettings.Edit.cshtml new file mode 100644 index 00000000000..02698abdac0 --- /dev/null +++ b/src/OrchardCore.Modules/OrchardCore.Users/Views/UserProfileSettings.Edit.cshtml @@ -0,0 +1,15 @@ +@model OrchardCore.Users.ViewModels.UserProfileSettingsViewModel + +
    +
    + + +
    +
    + +
    +
    + + +
    +
    diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Views/UsersAdminList.cshtml b/src/OrchardCore.Modules/OrchardCore.Users/Views/UsersAdminList.cshtml index f509d234a7d..149f19d4b07 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Views/UsersAdminList.cshtml +++ b/src/OrchardCore.Modules/OrchardCore.Users/Views/UsersAdminList.cshtml @@ -5,7 +5,7 @@ - + @Html.HiddenFor(o => o.Options.BulkAction)
    @@ -53,7 +53,7 @@ @await DisplayAsync(Model.Pager) +
    +
    + + +
    +
    + +
    +
    + + +
    +
    +
    @@ -102,7 +116,7 @@ ' });\n' + ' default:\n' + ' log("Warning", "Provider " + loginProvider + " was not handled");\n' + - ' break;\n' + + ' break;\n' + '}\n' + '*/\n' ); @@ -117,23 +131,23 @@ editor.options.readOnly = "nocursor"; } } -$(function () { - var textArea = document.getElementById('@Html.IdFor(x => x.SyncRolesScript)'); - var syncRolesCheckbox = document.getElementById('@Html.IdFor(x => x.UseScriptToSyncRoles)'); - if (textArea == null) { - return; - } - var editor = CodeMirror.fromTextArea(textArea, { - autoRefresh: true, - lineNumbers: true, - styleActiveLine: true, - matchBrackets: true, - autoCloseTags: true, - mode: "javascript" + $(function () { + var textArea = document.getElementById('@Html.IdFor(x => x.SyncRolesScript)'); + var syncRolesCheckbox = document.getElementById('@Html.IdFor(x => x.UseScriptToSyncRoles)'); + if (textArea == null) { + return; + } + var editor = CodeMirror.fromTextArea(textArea, { + autoRefresh: true, + lineNumbers: true, + styleActiveLine: true, + matchBrackets: true, + autoCloseTags: true, + mode: "javascript" + }); + $('#@Html.IdFor(x => x.SyncRolesScript)').data('editor', editor); + resetScript(editor.doc.getValue() != ''); + syncRolesCheckbox.addEventListener("change", toggleEditorState); + toggleEditorState(); }); - $('#@Html.IdFor(x => x.SyncRolesScript)').data('editor', editor); - resetScript(editor.doc.getValue() != ''); - syncRolesCheckbox.addEventListener("change", toggleEditorState); - toggleEditorState(); -}); diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Views/UserProfileSettings.Edit.cshtml b/src/OrchardCore.Modules/OrchardCore.Users/Views/UserProfileSettings.Edit.cshtml deleted file mode 100644 index 02698abdac0..00000000000 --- a/src/OrchardCore.Modules/OrchardCore.Users/Views/UserProfileSettings.Edit.cshtml +++ /dev/null @@ -1,15 +0,0 @@ -@model OrchardCore.Users.ViewModels.UserProfileSettingsViewModel - -
    -
    - - -
    -
    - -
    -
    - - -
    -
    diff --git a/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Security/Services/RoleServiceExtensions.cs b/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Security/Services/RoleServiceExtensions.cs index 1c161bb1c71..0019052606d 100644 --- a/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Security/Services/RoleServiceExtensions.cs +++ b/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Security/Services/RoleServiceExtensions.cs @@ -10,6 +10,12 @@ namespace OrchardCore.Security.Services { public static class RoleServiceExtensions { + public static HashSet SystemRoleNames = new() + { + "Anonymous", + "Authenticated" + }; + public static async Task> GetRoleNamesAsync(this IRoleService roleService) { var roles = await roleService.GetRolesAsync(); @@ -24,7 +30,7 @@ public static async Task> GetAccessibleRolesAsync(this IRoleS var accessibleRoles = new List(); foreach (var role in roles) { - if (new[] { "Anonymous", "Authenticated" }.Contains(role.RoleName, StringComparer.OrdinalIgnoreCase)) + if (SystemRoleNames.Contains(role.RoleName, StringComparer.OrdinalIgnoreCase)) { continue; } diff --git a/src/OrchardCore/OrchardCore.Users.Core/Indexes/UserRoleIndex.cs b/src/OrchardCore/OrchardCore.Users.Core/Indexes/UserRoleIndex.cs index 63ba6aefb15..b8134d09a3b 100644 --- a/src/OrchardCore/OrchardCore.Users.Core/Indexes/UserRoleIndex.cs +++ b/src/OrchardCore/OrchardCore.Users.Core/Indexes/UserRoleIndex.cs @@ -1,5 +1,5 @@ -using System.Collections.Generic; using System.Linq; +using Microsoft.AspNetCore.Identity; using OrchardCore.Users.Models; using YesSql.Indexes; @@ -14,24 +14,24 @@ public class UserRoleIndex : MapIndex public class UserRoleIndexProvider : IndexProvider { + private readonly ILookupNormalizer _keyNormalizer; + + public UserRoleIndexProvider(ILookupNormalizer keyNormalizer) + { + _keyNormalizer = keyNormalizer; + } + public override void Describe(DescribeContext context) { context.For() .Map(user => { - var indexes = new List(); - - foreach (var role in user.RoleNames.Distinct()) - { - indexes.Add(new UserRoleIndex() + return user.RoleNames.Distinct() + .Select(roleName => new UserRoleIndex() { UserId = user.UserId, - Role = role, + Role = _keyNormalizer.NormalizeName(roleName), }); - } - - return indexes; }); } - } diff --git a/src/OrchardCore/OrchardCore.Users.Core/Models/UserProfileSettings.cs b/src/OrchardCore/OrchardCore.Users.Core/Models/UserProfileSettings.cs deleted file mode 100644 index 3e980305602..00000000000 --- a/src/OrchardCore/OrchardCore.Users.Core/Models/UserProfileSettings.cs +++ /dev/null @@ -1,8 +0,0 @@ -namespace OrchardCore.Users.Models; - -public class UserProfileSettings -{ - public bool AllowChangingUsername { get; set; } - - public bool AllowChangingEmail { get; set; } -} From 8006a36adde360e5aea71af0e0ac8c277e916c39 Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Thu, 22 Dec 2022 13:16:53 -0800 Subject: [PATCH 09/25] Fix permission naming --- .../Services/RolesAdminListFilterProvider.cs | 2 +- .../OrchardCore.Users/UserRolePermissions.cs | 4 ---- src/OrchardCore/OrchardCore.Users.Core/CommonPermissions.cs | 4 ++-- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Services/RolesAdminListFilterProvider.cs b/src/OrchardCore.Modules/OrchardCore.Users/Services/RolesAdminListFilterProvider.cs index ac0c07a3b7c..39548fe9ab3 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Services/RolesAdminListFilterProvider.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Services/RolesAdminListFilterProvider.cs @@ -29,7 +29,7 @@ public void Build(QueryEngineBuilder builder) // At this point the user cannot see all users, so lets see what role does he have access too and filter by them. var accessibleRoles = await roleService.GetAccessibleRoleNamesAsync(authorizationService, user, CommonPermissions.ListUsers); - query.With(index => index.Role.IsIn(accessibleRoles)); + query.With(index => index.RoleName.IsIn(accessibleRoles)); } return query; diff --git a/src/OrchardCore.Modules/OrchardCore.Users/UserRolePermissions.cs b/src/OrchardCore.Modules/OrchardCore.Users/UserRolePermissions.cs index beb623ac521..0ef598f9b4f 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/UserRolePermissions.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/UserRolePermissions.cs @@ -27,13 +27,9 @@ public async Task> GetPermissionsAsync() foreach (var role in roles) { list.Add(CommonPermissions.CreateListUsersInRolePermission(role)); - list.Add(CommonPermissions.CreateEditUsersInRolePermission(role)); - list.Add(CommonPermissions.CreateDeleteUsersInRolePermission(role)); - list.Add(CommonPermissions.CreateAssignUsersToRolePermission(role)); - list.Add(CommonPermissions.CreatePermissionForManageUsersInRole(role)); } diff --git a/src/OrchardCore/OrchardCore.Users.Core/CommonPermissions.cs b/src/OrchardCore/OrchardCore.Users.Core/CommonPermissions.cs index 15061c80e21..911f86d5a24 100644 --- a/src/OrchardCore/OrchardCore.Users.Core/CommonPermissions.cs +++ b/src/OrchardCore/OrchardCore.Users.Core/CommonPermissions.cs @@ -32,10 +32,10 @@ public static Permission CreateDeleteUsersInRolePermission(string name) => CreateDynamicPermission(name, new Permission("DeleteUsersInRole_{0}", "Delete users in role - {0}", new[] { DeleteUsers }, true)); public static Permission CreateAssignUsersToRolePermission(string name) => - CreateDynamicPermission(name, new Permission("AssignUsersInRole_{0}", "Delete users in role - {0}", new[] { AssignRole }, true)); + CreateDynamicPermission(name, new Permission("AssignUsersInRole_{0}", "Assign users in role - {0}", new[] { AssignRole }, true)); public static Permission CreatePermissionForManageUsersInRole(string name) => - CreateDynamicPermission(name, new Permission("ManageUsersInRole_{0}", "Manage users to {0} role", new[] { ManageUsers }, true)); + CreateDynamicPermission(name, new Permission("ManageUsersInRole_{0}", "Manage users in {0} role", new[] { ManageUsers }, true)); // Dynamic permission template. From ee4e7bcf77b05cc945c96daf6732a4c6313a65d9 Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Thu, 22 Dec 2022 13:23:55 -0800 Subject: [PATCH 10/25] Remove UserRoleIdex and use UserByRoleNameIndex instead --- .../OrchardCore.Users/Startup.cs | 2 - .../OrchardCore.Users/UserMigrations.cs | 68 ------------------- .../OrchardCore.Users/UserRolePermissions.cs | 2 +- .../Indexes/UserRoleIndex.cs | 37 ---------- 4 files changed, 1 insertion(+), 108 deletions(-) delete mode 100644 src/OrchardCore.Modules/OrchardCore.Users/UserMigrations.cs delete mode 100644 src/OrchardCore/OrchardCore.Users.Core/Indexes/UserRoleIndex.cs diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Startup.cs b/src/OrchardCore.Modules/OrchardCore.Users/Startup.cs index 9fa00108f22..b2e588e1431 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Startup.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Startup.cs @@ -239,8 +239,6 @@ public override void ConfigureServices(IServiceCollection services) services.AddSingleton(); services.AddScoped, UserRoleDisplayDriver>(); services.AddScoped(); - services.AddScoped(); - services.AddSingleton(); services.AddScoped(); services.AddSingleton(); } diff --git a/src/OrchardCore.Modules/OrchardCore.Users/UserMigrations.cs b/src/OrchardCore.Modules/OrchardCore.Users/UserMigrations.cs deleted file mode 100644 index 605bd14d06e..00000000000 --- a/src/OrchardCore.Modules/OrchardCore.Users/UserMigrations.cs +++ /dev/null @@ -1,68 +0,0 @@ -using System.Linq; -using Microsoft.Extensions.DependencyInjection; -using OrchardCore.Data.Migration; -using OrchardCore.Environment.Shell.Scope; -using OrchardCore.Users.Indexes; -using OrchardCore.Users.Models; -using YesSql; -using YesSql.Services; -using YesSql.Sql; - -namespace OrchardCore.Users; - -public class UserMigrations : DataMigration -{ - public int Create() - { - SchemaBuilder.CreateMapIndexTable(table => table - .Column("UserId", column => column.WithLength(26)) - .Column("Role", column => column.WithLength(255)) - ); - - SchemaBuilder.AlterIndexTable(table => table - .CreateIndex("IDX_UserRoleIndex_DocumentId", - "DocumentId", - "UserId", - "Role") - ); - - ShellScope.AddDeferredTask(async scope => - { - var counter = 0; - var batchSize = 500; - - var session = scope.ServiceProvider.GetRequiredService(); - - while (true) - { - var users = await session.Query() - .OrderBy(x => x.DocumentId) - .Skip(counter++ * batchSize) - .Take(batchSize) - .ListAsync(); - - var totalUsers = users.Count(); - - if (totalUsers > 0) - { - foreach (var user in users) - { - // Saving the user will allow all index providers to run. - session.Save(user); - } - - await session.SaveChangesAsync(); - } - - - if (totalUsers < batchSize) - { - // If there are less users in the database than the batchSize, nothing left to do. - break; - } - } - }); - - return 1; - } -} diff --git a/src/OrchardCore.Modules/OrchardCore.Users/UserRolePermissions.cs b/src/OrchardCore.Modules/OrchardCore.Users/UserRolePermissions.cs index 0ef598f9b4f..33300db8181 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/UserRolePermissions.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/UserRolePermissions.cs @@ -20,7 +20,7 @@ public async Task> GetPermissionsAsync() { var roles = (await _roleService.GetRoleNamesAsync()) .Except(new[] { "Anonymous", "Authenticated" }, StringComparer.OrdinalIgnoreCase) - .OrderBy(x => x).ToList(); + .OrderBy(x => x); var list = new List(); diff --git a/src/OrchardCore/OrchardCore.Users.Core/Indexes/UserRoleIndex.cs b/src/OrchardCore/OrchardCore.Users.Core/Indexes/UserRoleIndex.cs deleted file mode 100644 index b8134d09a3b..00000000000 --- a/src/OrchardCore/OrchardCore.Users.Core/Indexes/UserRoleIndex.cs +++ /dev/null @@ -1,37 +0,0 @@ -using System.Linq; -using Microsoft.AspNetCore.Identity; -using OrchardCore.Users.Models; -using YesSql.Indexes; - -namespace OrchardCore.Users.Indexes; - -public class UserRoleIndex : MapIndex -{ - public string UserId { get; set; } - - public string Role { get; set; } -} - -public class UserRoleIndexProvider : IndexProvider -{ - private readonly ILookupNormalizer _keyNormalizer; - - public UserRoleIndexProvider(ILookupNormalizer keyNormalizer) - { - _keyNormalizer = keyNormalizer; - } - - public override void Describe(DescribeContext context) - { - context.For() - .Map(user => - { - return user.RoleNames.Distinct() - .Select(roleName => new UserRoleIndex() - { - UserId = user.UserId, - Role = _keyNormalizer.NormalizeName(roleName), - }); - }); - } -} From 06c22437196d99098f55810a51c157826f7426e5 Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Thu, 22 Dec 2022 13:49:01 -0800 Subject: [PATCH 11/25] Remove ManageUserProfileSettings permission since it is no longer needed --- src/OrchardCore.Modules/OrchardCore.Users/Permissions.cs | 2 -- src/OrchardCore/OrchardCore.Users.Core/CommonPermissions.cs | 4 +++- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Permissions.cs b/src/OrchardCore.Modules/OrchardCore.Users/Permissions.cs index c187215ff50..ae1e4512b5a 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Permissions.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Permissions.cs @@ -21,7 +21,6 @@ public Task> GetPermissionsAsync() CommonPermissions.EditUsers, CommonPermissions.DeleteUsers, CommonPermissions.AssignRole, - CommonPermissions.ManageUserProfileSettings, }); } @@ -38,7 +37,6 @@ public IEnumerable GetDefaultStereotypes() CommonPermissions.EditUsers, CommonPermissions.DeleteUsers, CommonPermissions.AssignRole, - CommonPermissions.ManageUserProfileSettings, } }, new PermissionStereotype { diff --git a/src/OrchardCore/OrchardCore.Users.Core/CommonPermissions.cs b/src/OrchardCore/OrchardCore.Users.Core/CommonPermissions.cs index 911f86d5a24..348de3ea656 100644 --- a/src/OrchardCore/OrchardCore.Users.Core/CommonPermissions.cs +++ b/src/OrchardCore/OrchardCore.Users.Core/CommonPermissions.cs @@ -17,10 +17,12 @@ public class CommonPermissions public static readonly Permission ViewUsers = new("View Users", "View user's profile", new[] { ManageUsers }); public static readonly Permission EditUsers = new("EditUsers", "Edit any user", new[] { ManageUsers }, true); + public static readonly Permission DeleteUsers = new("DeleteUsers", "Delete any user", new[] { ManageUsers }, true); + public static readonly Permission ListUsers = new("ListUsers", "List all Users", new[] { EditUsers, DeleteUsers }); + public static readonly Permission AssignRole = new("AssignRole", "Assign any role", new[] { EditUsers }, true); - public static readonly Permission ManageUserProfileSettings = new("ManageUserProfileSettings", "Manage user profile settings", new[] { ManageUsers }, true); public static Permission CreateListUsersInRolePermission(string name) => CreateDynamicPermission(name, new Permission("ListUsersInRole_{0}", "List users in role - {0}", new[] { ListUsers })); From 3bf2b903bd4651baad4d9bbecc4f17e373bf5f82 Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Wed, 4 Jan 2023 16:29:47 -0800 Subject: [PATCH 12/25] Clean up and add documenations --- .../Controllers/AdminController.cs | 4 +-- .../Drivers/UserRoleDisplayDriver.cs | 4 +-- .../OrchardCore.Users/Permissions.cs | 2 -- .../Services/RoleAuthorizationHandler.cs | 2 +- .../OrchardCore.Users/UserRolePermissions.cs | 18 +++++++++++-- .../Views/UsersAdminListCreate.cshtml | 2 +- .../CommonPermissions.cs | 15 +++++------ src/docs/releases/1.6.0.md | 27 +++++++++++++++++++ 8 files changed, 56 insertions(+), 18 deletions(-) create mode 100644 src/docs/releases/1.6.0.md diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Controllers/AdminController.cs b/src/OrchardCore.Modules/OrchardCore.Users/Controllers/AdminController.cs index a017c360324..bc58437fe7f 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Controllers/AdminController.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Controllers/AdminController.cs @@ -283,7 +283,7 @@ public async Task Create() { var user = new User(); - if (!await _authorizationService.AuthorizeAsync(User, CommonPermissions.EditUsers, user) || !await _authorizationService.AuthorizeAsync(User, CommonPermissions.AssignRole, user)) + if (!await _authorizationService.AuthorizeAsync(User, CommonPermissions.EditUsers, user) || !await _authorizationService.AuthorizeAsync(User, CommonPermissions.AssignUsersToRole, user)) { return Forbid(); } @@ -299,7 +299,7 @@ public async Task CreatePost([Bind(Prefix = "User.Password")] str { var user = new User(); - if (!await _authorizationService.AuthorizeAsync(User, CommonPermissions.EditUsers, user) || !await _authorizationService.AuthorizeAsync(User, CommonPermissions.AssignRole, user)) + if (!await _authorizationService.AuthorizeAsync(User, CommonPermissions.EditUsers, user) || !await _authorizationService.AuthorizeAsync(User, CommonPermissions.AssignUsersToRole, user)) { return Forbid(); } diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserRoleDisplayDriver.cs b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserRoleDisplayDriver.cs index d297b5e57a3..1dd0709e8dc 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserRoleDisplayDriver.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserRoleDisplayDriver.cs @@ -59,7 +59,7 @@ public override IDisplayResult Edit(User user) { // The current user can only view their roles if they have assign role, to prevent listing roles when managing their own profile. if (String.Equals(_httpContextAccessor.HttpContext.User.FindFirstValue(ClaimTypes.NameIdentifier), user.UserId, StringComparison.OrdinalIgnoreCase) && - !await _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext.User, CommonPermissions.AssignRole)) + !await _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext.User, CommonPermissions.AssignUsersToRole)) { return; } @@ -175,7 +175,7 @@ private async Task> GetAuthorizedRoleNamesAsync(IEnumerable< var authorizedRoleNames = new List(); foreach (var role in roles) { - if (await _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext.User, CommonPermissions.AssignRole, role)) + if (await _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext.User, CommonPermissions.AssignUsersToRole, role)) { authorizedRoleNames.Add(role.RoleName); } diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Permissions.cs b/src/OrchardCore.Modules/OrchardCore.Users/Permissions.cs index ae1e4512b5a..fe429fd5a2b 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Permissions.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Permissions.cs @@ -20,7 +20,6 @@ public Task> GetPermissionsAsync() CommonPermissions.ListUsers, CommonPermissions.EditUsers, CommonPermissions.DeleteUsers, - CommonPermissions.AssignRole, }); } @@ -36,7 +35,6 @@ public IEnumerable GetDefaultStereotypes() CommonPermissions.ListUsers, CommonPermissions.EditUsers, CommonPermissions.DeleteUsers, - CommonPermissions.AssignRole, } }, new PermissionStereotype { diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Services/RoleAuthorizationHandler.cs b/src/OrchardCore.Modules/OrchardCore.Users/Services/RoleAuthorizationHandler.cs index 87cad69667f..9cdc2307fd9 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Services/RoleAuthorizationHandler.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Services/RoleAuthorizationHandler.cs @@ -122,7 +122,7 @@ private async Task TryAuthenticateAsync(AuthorizationHandlerContext contex return true; } - if (String.Equals(permission.Name, CommonPermissions.AssignRole.Name, StringComparison.OrdinalIgnoreCase) + if (String.Equals(permission.Name, CommonPermissions.AssignUsersToRole.Name, StringComparison.OrdinalIgnoreCase) && await _authorizationService.AuthorizeAsync(context.User, CommonPermissions.CreateAssignUsersToRolePermission(roleName))) { return true; diff --git a/src/OrchardCore.Modules/OrchardCore.Users/UserRolePermissions.cs b/src/OrchardCore.Modules/OrchardCore.Users/UserRolePermissions.cs index 33300db8181..50e57920b80 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/UserRolePermissions.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/UserRolePermissions.cs @@ -22,7 +22,10 @@ public async Task> GetPermissionsAsync() .Except(new[] { "Anonymous", "Authenticated" }, StringComparer.OrdinalIgnoreCase) .OrderBy(x => x); - var list = new List(); + var list = new List() + { + CommonPermissions.AssignUsersToRole, + }; foreach (var role in roles) { @@ -36,5 +39,16 @@ public async Task> GetPermissionsAsync() return list; } - public IEnumerable GetDefaultStereotypes() => Enumerable.Empty(); + public IEnumerable GetDefaultStereotypes() + { + return new[] { + new PermissionStereotype + { + Name = "Administrator", + Permissions = new[] { + CommonPermissions.AssignUsersToRole, + } + } + }; + } } diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Views/UsersAdminListCreate.cshtml b/src/OrchardCore.Modules/OrchardCore.Users/Views/UsersAdminListCreate.cshtml index 7de522772b1..0887ea6663c 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Views/UsersAdminListCreate.cshtml +++ b/src/OrchardCore.Modules/OrchardCore.Users/Views/UsersAdminListCreate.cshtml @@ -6,7 +6,7 @@ } @if (await AuthorizationService.AuthorizeAsync(User, CommonPermissions.EditUsers, dummyUser) -&& await AuthorizationService.AuthorizeAsync(User, CommonPermissions.AssignRole, dummyUser)) +&& await AuthorizationService.AuthorizeAsync(User, CommonPermissions.AssignUsersToRole, dummyUser)) { @T["Add User"] } diff --git a/src/OrchardCore/OrchardCore.Users.Core/CommonPermissions.cs b/src/OrchardCore/OrchardCore.Users.Core/CommonPermissions.cs index 348de3ea656..473877e4a41 100644 --- a/src/OrchardCore/OrchardCore.Users.Core/CommonPermissions.cs +++ b/src/OrchardCore/OrchardCore.Users.Core/CommonPermissions.cs @@ -12,7 +12,7 @@ public class CommonPermissions public static readonly Permission ManageUsers = new("ManageUsers", "Manage security settings and all users", true); /// - /// View users only allows listing or viewing a users profile. + /// View users only allows viewing a users profile. /// public static readonly Permission ViewUsers = new("View Users", "View user's profile", new[] { ManageUsers }); @@ -20,27 +20,26 @@ public class CommonPermissions public static readonly Permission DeleteUsers = new("DeleteUsers", "Delete any user", new[] { ManageUsers }, true); - public static readonly Permission ListUsers = new("ListUsers", "List all Users", new[] { EditUsers, DeleteUsers }); + public static readonly Permission ListUsers = new("ListUsers", "List all users", new[] { EditUsers, DeleteUsers }); - public static readonly Permission AssignRole = new("AssignRole", "Assign any role", new[] { EditUsers }, true); + public static readonly Permission AssignUsersToRole = new("AssignUsersToRole", "Assign users to any role", new[] { EditUsers }, true); public static Permission CreateListUsersInRolePermission(string name) => - CreateDynamicPermission(name, new Permission("ListUsersInRole_{0}", "List users in role - {0}", new[] { ListUsers })); + CreateDynamicPermission(name, new Permission("ListUsersInRole_{0}", "List users in {0} role", new[] { ListUsers })); public static Permission CreateEditUsersInRolePermission(string name) => - CreateDynamicPermission(name, new Permission("EditUsersInRole_{0}", "Edit users in role - {0}", new[] { EditUsers }, true)); + CreateDynamicPermission(name, new Permission("EditUsersInRole_{0}", "Edit users in {0} role", new[] { EditUsers }, true)); public static Permission CreateDeleteUsersInRolePermission(string name) => - CreateDynamicPermission(name, new Permission("DeleteUsersInRole_{0}", "Delete users in role - {0}", new[] { DeleteUsers }, true)); + CreateDynamicPermission(name, new Permission("DeleteUsersInRole_{0}", "Delete users in {0} role", new[] { DeleteUsers }, true)); public static Permission CreateAssignUsersToRolePermission(string name) => - CreateDynamicPermission(name, new Permission("AssignUsersInRole_{0}", "Assign users in role - {0}", new[] { AssignRole }, true)); + CreateDynamicPermission(name, new Permission("AssignUsersToRole_{0}", "Assign users to {0} role", new[] { AssignUsersToRole }, true)); public static Permission CreatePermissionForManageUsersInRole(string name) => CreateDynamicPermission(name, new Permission("ManageUsersInRole_{0}", "Manage users in {0} role", new[] { ManageUsers }, true)); // Dynamic permission template. - private static Permission CreateDynamicPermission(string name, Permission permission) => new( String.Format(permission.Name, name), diff --git a/src/docs/releases/1.6.0.md b/src/docs/releases/1.6.0.md new file mode 100644 index 00000000000..789e6c06bfb --- /dev/null +++ b/src/docs/releases/1.6.0.md @@ -0,0 +1,27 @@ +# Orchard Core 1.6.0 + +Release date: Not yet released + +## Breaking Changes + +### Users + +The `OrchardCore.Users` module now provides multiple new permissions for added control on user related functions. + +In previous versions, `View Users` permission was enough to show the `Users` menu item. However, with the newly added permissions, `ListUsers` or at least one `ListUsersInRole_{0}` variation is required to see the `Users` menu. The `View Users` permission now only allows you to view user's profile. Here is a list of the new permissions + +| Permission | Description | +| --- | --- | +| `ListUsers` | allows users to list all users. | +| `EditUsers` | allow users to edit any user. | +| `DeleteUsers` | allows users to delete any user. | + + The following permissions will be available if `OrchardCore.Roles` feature in enabled. + + | Permission | Description | + | --- | --- | + | `AssignUsersToRole` | allows users to assign users to any role. | + | `ListUsersInRole_{0}` | allows users to list users in a given role. | + | `EditUsersInRole_{0}` | allows users to edit users in a given role. | + | `DeleteUsersInRole_{0}` | allows users to delete users in a given role. | + | `AssignUsersToRole_{0}` | allows users to assign users to a given role. | From 918b1e1f75c8fdb71aa83f7b56a0adf438d47b29 Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Mon, 9 Jan 2023 18:17:51 -0800 Subject: [PATCH 13/25] UI Fix --- .../Drivers/UserRoleDisplayDriver.cs | 35 ++++++++++--------- .../ViewModels/EditUserRoleViewModel.cs | 5 +-- .../Views/UserRoleFields.Edit.cshtml | 4 +-- 3 files changed, 21 insertions(+), 23 deletions(-) diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserRoleDisplayDriver.cs b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserRoleDisplayDriver.cs index 1dd0709e8dc..8d113ac87dc 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserRoleDisplayDriver.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserRoleDisplayDriver.cs @@ -67,17 +67,16 @@ public override IDisplayResult Edit(User user) var roles = await GetRoleAsync(); // When a user is in a role that the current user cannot manage the role is shown but selection is disabled. - var authorizedRoleNames = await GetAuthorizedRoleNamesAsync(roles); + var authorizedRoleNames = await GetAccessibleRoleNamesAsync(roles); var userRoleNames = await _userRoleStore.GetRolesAsync(user, default); var roleEntries = new List(); - foreach (var role in roles) + foreach (var roleName in authorizedRoleNames) { var roleEntry = new RoleEntry { - Role = role.RoleName, - IsSelected = userRoleNames.Contains(role.RoleName, StringComparer.OrdinalIgnoreCase), - IsEditingDisabled = !authorizedRoleNames.Contains(role.RoleName, StringComparer.OrdinalIgnoreCase) + Role = roleName, + IsSelected = userRoleNames.Contains(roleName, StringComparer.OrdinalIgnoreCase), }; roleEntries.Add(roleEntry); @@ -86,7 +85,7 @@ public override IDisplayResult Edit(User user) model.Roles = roleEntries.ToArray(); }) .Location("Content:1.10") - .RenderWhen(() => _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext.User, CommonPermissions.EditUsers, user)); + .RenderWhen(async () => await _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext.User, CommonPermissions.EditUsers, user)); } public override async Task UpdateAsync(User user, UpdateEditorContext context) @@ -103,29 +102,29 @@ public override async Task UpdateAsync(User user, UpdateEditorCo { var roles = await GetRoleAsync(); // Authorize each role in the model to prevent html injection. - var authorizedRoleNames = await GetAuthorizedRoleNamesAsync(roles); - var userRoleNames = await _userRoleStore.GetRolesAsync(user, default); + var accessibleRoleNames = await GetAccessibleRoleNamesAsync(roles); + var currentUserRoleNames = await _userRoleStore.GetRolesAsync(user, default); var selectedRoleNames = model.Roles.Where(x => x.IsSelected).Select(x => x.Role); var selectedRoles = roles.Where(x => selectedRoleNames.Contains(x.RoleName, StringComparer.OrdinalIgnoreCase)); - var authorizedSelectedRoleNames = await GetAuthorizedRoleNamesAsync(selectedRoles); + var accessibleAndSelectedRoleNames = await GetAccessibleRoleNamesAsync(selectedRoles); if (context.IsNew) { // Only add authorized new roles. - foreach (var role in authorizedSelectedRoleNames) + foreach (var role in accessibleAndSelectedRoleNames) { await _userRoleStore.AddToRoleAsync(user, _userManager.NormalizeName(role), default); } } else { - // Remove roles in two steps to prevent an iteration on a modified collection + // Remove roles in two steps to prevent an iteration on a modified collection. var rolesToRemove = new List(); - foreach (var role in userRoleNames) + foreach (var role in currentUserRoleNames) { // When the user has permission to manage the role and it is no longer selected the role can be removed. - if (authorizedRoleNames.Contains(role, StringComparer.OrdinalIgnoreCase) && !authorizedSelectedRoleNames.Contains(role, StringComparer.OrdinalIgnoreCase)) + if (accessibleRoleNames.Contains(role, StringComparer.OrdinalIgnoreCase) && !accessibleAndSelectedRoleNames.Contains(role, StringComparer.OrdinalIgnoreCase)) { rolesToRemove.Add(role); } @@ -136,10 +135,11 @@ public override async Task UpdateAsync(User user, UpdateEditorCo if (String.Equals(role, AdministratorRole, StringComparison.OrdinalIgnoreCase)) { var usersOfAdminRole = (await _userManager.GetUsersInRoleAsync(AdministratorRole)).Cast(); - // Make sure we always have at least one administrator account + // Make sure we always have at least one administrator account. if (usersOfAdminRole.Count() == 1 && String.Equals(user.UserId, usersOfAdminRole.First().UserId, StringComparison.OrdinalIgnoreCase)) { await _notifier.WarningAsync(H["Cannot remove administrator role from the only administrator."]); + continue; } } @@ -147,8 +147,8 @@ public override async Task UpdateAsync(User user, UpdateEditorCo await _userRoleStore.RemoveFromRoleAsync(user, _userManager.NormalizeName(role), default); } - // Add new roles - foreach (var role in authorizedSelectedRoleNames) + // Add new roles. + foreach (var role in accessibleAndSelectedRoleNames) { var normalizedName = _userManager.NormalizeName(role); if (!await _userRoleStore.IsInRoleAsync(user, normalizedName, default)) @@ -170,9 +170,10 @@ private async Task> GetRoleAsync() return roles.Where(role => !exclude.Any(x => x.Equals(role.RoleName, StringComparison.OrdinalIgnoreCase))); } - private async Task> GetAuthorizedRoleNamesAsync(IEnumerable roles) + private async Task> GetAccessibleRoleNamesAsync(IEnumerable roles) { var authorizedRoleNames = new List(); + foreach (var role in roles) { if (await _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext.User, CommonPermissions.AssignUsersToRole, role)) diff --git a/src/OrchardCore.Modules/OrchardCore.Users/ViewModels/EditUserRoleViewModel.cs b/src/OrchardCore.Modules/OrchardCore.Users/ViewModels/EditUserRoleViewModel.cs index de009146558..06f74564e9d 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/ViewModels/EditUserRoleViewModel.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/ViewModels/EditUserRoleViewModel.cs @@ -1,5 +1,4 @@ using System; -using Microsoft.AspNetCore.Mvc.ModelBinding; namespace OrchardCore.Users.ViewModels { @@ -11,9 +10,7 @@ public class EditUserRoleViewModel public class RoleEntry { public string Role { get; set; } - public bool IsSelected { get; set; } - [BindNever] - public bool IsEditingDisabled { get; set; } + public bool IsSelected { get; set; } } } diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Views/UserRoleFields.Edit.cshtml b/src/OrchardCore.Modules/OrchardCore.Users/Views/UserRoleFields.Edit.cshtml index d7feb9a3e4c..1767775d280 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Views/UserRoleFields.Edit.cshtml +++ b/src/OrchardCore.Modules/OrchardCore.Users/Views/UserRoleFields.Edit.cshtml @@ -1,13 +1,13 @@ @model EditUserRoleViewModel -@if (Model.Roles.Any()) +@if (Model.Roles.Length > 0) {
    @T["Roles"]
    @for (var i = 0; i < Model.Roles.Length; i++) {
    - +
    From b5bc9177c990d463c0d91172a7456390c64c73ef Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Thu, 12 Jan 2023 10:15:08 -0800 Subject: [PATCH 14/25] Minor adjustments Fix #12749 --- .../Drivers/UserDisplayDriver.cs | 12 +++++++---- .../Drivers/UserRoleDisplayDriver.cs | 19 +++++++++++------ .../OrchardCore.Users/Views/RolesMeta.cshtml | 8 ------- .../Views/UserRolesMeta.cshtml | 21 +++++++++++++++++++ 4 files changed, 42 insertions(+), 18 deletions(-) delete mode 100644 src/OrchardCore.Modules/OrchardCore.Users/Views/RolesMeta.cshtml create mode 100644 src/OrchardCore.Modules/OrchardCore.Users/Views/UserRolesMeta.cshtml diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserDisplayDriver.cs b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserDisplayDriver.cs index fcc6c876513..43bda03974c 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserDisplayDriver.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserDisplayDriver.cs @@ -27,7 +27,7 @@ public class UserDisplayDriver : DisplayDriver private readonly IHttpContextAccessor _httpContextAccessor; private readonly INotifier _notifier; private readonly IAuthorizationService _authorizationService; - private IEnumerable _userEventHandlers; + private readonly IEnumerable _userEventHandlers; private readonly ILogger _logger; private readonly IHtmlLocalizer H; private readonly IStringLocalizer S; @@ -113,10 +113,14 @@ public override async Task UpdateAsync(User user, UpdateEditorCo if (!isEditingDisabled && !model.IsEnabled && user.IsEnabled) { - var usersOfAdminRole = (await _userManager.GetUsersInRoleAsync(AdministratorRole)).Cast(); - if (usersOfAdminRole.Count() == 1 && String.Equals(user.UserId, usersOfAdminRole.First().UserId, StringComparison.OrdinalIgnoreCase)) + var enabledUsersOfAdminRole = (await _userManager.GetUsersInRoleAsync(AdministratorRole)) + .Cast() + .Where(user => user.IsEnabled) + .ToList(); + + if (enabledUsersOfAdminRole.Count == 1 && user.UserId == enabledUsersOfAdminRole.First().UserId) { - await _notifier.WarningAsync(H["Cannot disable the only administrator."]); + await _notifier.WarningAsync(H["Cannot disable the only enabled administrator."]); } else { diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserRoleDisplayDriver.cs b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserRoleDisplayDriver.cs index 8d113ac87dc..cdd8cb4df1a 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserRoleDisplayDriver.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserRoleDisplayDriver.cs @@ -49,7 +49,7 @@ public UserRoleDisplayDriver( public override IDisplayResult Display(User user) { - return Initialize("RolesMeta", model => model.User = user) + return Initialize("UserRolesMeta", model => model.User = user) .Location("SummaryAdmin", "Meta"); } public override IDisplayResult Edit(User user) @@ -91,10 +91,13 @@ public override IDisplayResult Edit(User user) public override async Task UpdateAsync(User user, UpdateEditorContext context) { var model = new EditUserRoleViewModel(); + var currentUserId = _httpContextAccessor.HttpContext.User.FindFirstValue(ClaimTypes.NameIdentifier); // The current user cannot alter their own roles. This prevents them removing access to the site for themselves. - if (String.Equals(_httpContextAccessor.HttpContext.User.FindFirstValue(ClaimTypes.NameIdentifier), user.UserId, StringComparison.OrdinalIgnoreCase)) + if (currentUserId == user.UserId && !await _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext.User, StandardPermissions.SiteOwner)) { + await _notifier.WarningAsync(H["Cannot update your own roles."]); + return Edit(user); } @@ -134,11 +137,15 @@ public override async Task UpdateAsync(User user, UpdateEditorCo { if (String.Equals(role, AdministratorRole, StringComparison.OrdinalIgnoreCase)) { - var usersOfAdminRole = (await _userManager.GetUsersInRoleAsync(AdministratorRole)).Cast(); - // Make sure we always have at least one administrator account. - if (usersOfAdminRole.Count() == 1 && String.Equals(user.UserId, usersOfAdminRole.First().UserId, StringComparison.OrdinalIgnoreCase)) + var enabledUsersOfAdminRole = (await _userManager.GetUsersInRoleAsync(AdministratorRole)) + .Cast() + .Where(user => user.IsEnabled) + .ToList(); + + // Make sure we always have at least one enabled administrator account. + if (enabledUsersOfAdminRole.Count == 1 && user.UserId == enabledUsersOfAdminRole.First().UserId) { - await _notifier.WarningAsync(H["Cannot remove administrator role from the only administrator."]); + await _notifier.WarningAsync(H[$"Cannot remove {AdministratorRole} role from the only enabled administrator."]); continue; } diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Views/RolesMeta.cshtml b/src/OrchardCore.Modules/OrchardCore.Users/Views/RolesMeta.cshtml deleted file mode 100644 index eca7c9cd3aa..00000000000 --- a/src/OrchardCore.Modules/OrchardCore.Users/Views/RolesMeta.cshtml +++ /dev/null @@ -1,8 +0,0 @@ -@model SummaryAdminUserViewModel - -
    - @foreach (var roleName in Model.User.RoleNames) - { - @roleName - } -
    diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Views/UserRolesMeta.cshtml b/src/OrchardCore.Modules/OrchardCore.Users/Views/UserRolesMeta.cshtml new file mode 100644 index 00000000000..a34b3474cfd --- /dev/null +++ b/src/OrchardCore.Modules/OrchardCore.Users/Views/UserRolesMeta.cshtml @@ -0,0 +1,21 @@ +@using OrchardCore.Security.Services +@model SummaryAdminUserViewModel + +@inject IRoleService RoleService + +@{ + var roleNames = await RoleService.GetRoleNamesAsync(); +} + +
    + @foreach (var normalizedRoleName in Model.User.RoleNames) + { + var roleName = roleNames.FirstOrDefault(roleName => roleName.Equals(normalizedRoleName, StringComparison.OrdinalIgnoreCase)); + if (roleName == null) + { + continue; + } + + @roleName + } +
    From bda7efd6dde90e64b44111729bbe92e290cb0221 Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Thu, 12 Jan 2023 10:31:44 -0800 Subject: [PATCH 15/25] Remove unused usings --- .../Extensions/UsersServiceCollectionExtensions.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/OrchardCore/OrchardCore.Users.Core/Extensions/UsersServiceCollectionExtensions.cs b/src/OrchardCore/OrchardCore.Users.Core/Extensions/UsersServiceCollectionExtensions.cs index 9f19d3e2f02..e67b3b782cb 100644 --- a/src/OrchardCore/OrchardCore.Users.Core/Extensions/UsersServiceCollectionExtensions.cs +++ b/src/OrchardCore/OrchardCore.Users.Core/Extensions/UsersServiceCollectionExtensions.cs @@ -1,8 +1,6 @@ using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.DependencyInjection.Extensions; -using OrchardCore.Roles.Services; using OrchardCore.Security; -using OrchardCore.Security.Services; using OrchardCore.Users; using OrchardCore.Users.Handlers; using OrchardCore.Users.Indexes; From 306a9c97d2fc304ad94fe8b5bb9e7df1715f477a Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Fri, 13 Jan 2023 08:24:16 -0800 Subject: [PATCH 16/25] Adding RoleHelper for system roles --- .../Drivers/UserRoleDisplayDriver.cs | 3 +-- .../OrchardCore.Users/Startup.cs | 4 ++-- .../OrchardCore.Users/UserRolePermissions.cs | 19 +++++++++---------- .../Security/Services/RoleHelper.cs | 14 ++++++++++++++ .../Services/RoleServiceExtensions.cs | 9 +-------- .../UsersServiceCollectionExtensions.cs | 4 ++-- 6 files changed, 29 insertions(+), 24 deletions(-) create mode 100644 src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Security/Services/RoleHelper.cs diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserRoleDisplayDriver.cs b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserRoleDisplayDriver.cs index cdd8cb4df1a..81e8202b118 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserRoleDisplayDriver.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserRoleDisplayDriver.cs @@ -172,9 +172,8 @@ public override async Task UpdateAsync(User user, UpdateEditorCo private async Task> GetRoleAsync() { var roles = await _roleService.GetRolesAsync(); - var exclude = new[] { "Anonymous", "Authenticated" }; - return roles.Where(role => !exclude.Any(x => x.Equals(role.RoleName, StringComparison.OrdinalIgnoreCase))); + return roles.Where(role => !RoleHelper.SystemRoleNames.Contains(role.RoleName)); } private async Task> GetAccessibleRoleNamesAsync(IEnumerable roles) diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Startup.cs b/src/OrchardCore.Modules/OrchardCore.Users/Startup.cs index cde88475437..7a69d2af58e 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Startup.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Startup.cs @@ -14,6 +14,7 @@ using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.Options; using OrchardCore.Admin; +using OrchardCore.Data; using OrchardCore.Data.Migration; using OrchardCore.DisplayManagement.Handlers; using OrchardCore.DisplayManagement.Theming; @@ -42,7 +43,6 @@ using OrchardCore.Users.Services; using OrchardCore.Users.ViewModels; using YesSql.Filters.Query; -using YesSql.Indexes; namespace OrchardCore.Users { @@ -235,7 +235,7 @@ public class RolesStartup : StartupBase public override void ConfigureServices(IServiceCollection services) { services.AddScoped(); - services.AddSingleton(); + services.AddIndexProvider(); services.AddScoped, UserRoleDisplayDriver>(); services.AddScoped(); services.AddScoped(); diff --git a/src/OrchardCore.Modules/OrchardCore.Users/UserRolePermissions.cs b/src/OrchardCore.Modules/OrchardCore.Users/UserRolePermissions.cs index 50e57920b80..7e0c7deb268 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/UserRolePermissions.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/UserRolePermissions.cs @@ -1,4 +1,3 @@ -using System; using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; @@ -18,22 +17,22 @@ public UserRolePermissions(IRoleService roleService) public async Task> GetPermissionsAsync() { - var roles = (await _roleService.GetRoleNamesAsync()) - .Except(new[] { "Anonymous", "Authenticated" }, StringComparer.OrdinalIgnoreCase) - .OrderBy(x => x); + var roleNames = (await _roleService.GetRoleNamesAsync()) + .Where(roleName => !RoleHelper.SystemRoleNames.Contains(roleName)) + .OrderBy(roleName => roleName); var list = new List() { CommonPermissions.AssignUsersToRole, }; - foreach (var role in roles) + foreach (var roleName in roleNames) { - list.Add(CommonPermissions.CreateListUsersInRolePermission(role)); - list.Add(CommonPermissions.CreateEditUsersInRolePermission(role)); - list.Add(CommonPermissions.CreateDeleteUsersInRolePermission(role)); - list.Add(CommonPermissions.CreateAssignUsersToRolePermission(role)); - list.Add(CommonPermissions.CreatePermissionForManageUsersInRole(role)); + list.Add(CommonPermissions.CreateListUsersInRolePermission(roleName)); + list.Add(CommonPermissions.CreateEditUsersInRolePermission(roleName)); + list.Add(CommonPermissions.CreateDeleteUsersInRolePermission(roleName)); + list.Add(CommonPermissions.CreateAssignUsersToRolePermission(roleName)); + list.Add(CommonPermissions.CreatePermissionForManageUsersInRole(roleName)); } return list; diff --git a/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Security/Services/RoleHelper.cs b/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Security/Services/RoleHelper.cs new file mode 100644 index 00000000000..a360c7e189e --- /dev/null +++ b/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Security/Services/RoleHelper.cs @@ -0,0 +1,14 @@ +using System; +using System.Collections.Generic; + +namespace OrchardCore.Security.Services +{ + public class RoleHelper + { + public static readonly HashSet SystemRoleNames = new(StringComparer.OrdinalIgnoreCase) + { + "Anonymous", + "Authenticated" + }; + } +} diff --git a/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Security/Services/RoleServiceExtensions.cs b/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Security/Services/RoleServiceExtensions.cs index 0019052606d..94e36ac220a 100644 --- a/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Security/Services/RoleServiceExtensions.cs +++ b/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Security/Services/RoleServiceExtensions.cs @@ -1,4 +1,3 @@ -using System; using System.Collections.Generic; using System.Linq; using System.Security.Claims; @@ -10,12 +9,6 @@ namespace OrchardCore.Security.Services { public static class RoleServiceExtensions { - public static HashSet SystemRoleNames = new() - { - "Anonymous", - "Authenticated" - }; - public static async Task> GetRoleNamesAsync(this IRoleService roleService) { var roles = await roleService.GetRolesAsync(); @@ -30,7 +23,7 @@ public static async Task> GetAccessibleRolesAsync(this IRoleS var accessibleRoles = new List(); foreach (var role in roles) { - if (SystemRoleNames.Contains(role.RoleName, StringComparer.OrdinalIgnoreCase)) + if (RoleHelper.SystemRoleNames.Contains(role.RoleName)) { continue; } diff --git a/src/OrchardCore/OrchardCore.Users.Core/Extensions/UsersServiceCollectionExtensions.cs b/src/OrchardCore/OrchardCore.Users.Core/Extensions/UsersServiceCollectionExtensions.cs index dbf9047fd43..4851efcf06e 100644 --- a/src/OrchardCore/OrchardCore.Users.Core/Extensions/UsersServiceCollectionExtensions.cs +++ b/src/OrchardCore/OrchardCore.Users.Core/Extensions/UsersServiceCollectionExtensions.cs @@ -3,6 +3,7 @@ using OrchardCore.Data; using OrchardCore.Roles.Services; using OrchardCore.Security; +using OrchardCore.Security.Services; using OrchardCore.Users; using OrchardCore.Users.Handlers; using OrchardCore.Users.Indexes; @@ -37,9 +38,8 @@ public static IServiceCollection AddUsers(this IServiceCollection services) services.TryAddScoped>(); services.TryAddScoped(); - + services.AddIndexProvider(); - services.AddIndexProvider(); services.AddIndexProvider(); services.AddIndexProvider(); From 0589f60a2d4226cbdb0bc94cab8faf0ab815580f Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Sun, 15 Jan 2023 13:54:22 -0800 Subject: [PATCH 17/25] Cleanup and add display action for user. --- .../Controllers/AdminController.cs | 51 +++++++++++------ .../Drivers/UserDisplayDriver.cs | 1 + .../Drivers/UserRoleDisplayDriver.cs | 14 +++-- .../OrchardCore.Users/Permissions.cs | 2 +- .../Services/RoleAuthorizationHandler.cs | 56 ++++++++----------- .../OrchardCore.Users/Startup.cs | 7 ++- .../OrchardCore.Users/UserRolePermissions.cs | 6 +- .../Views/Admin/Display.cshtml | 1 + .../Views/User.DetailAdmin.cshtml | 23 ++++++++ .../Views/UserButtons.cshtml | 29 ++++++---- .../OrchardCore.Users/Views/UserInfo.cshtml | 15 +++++ .../OrchardCore.Users/Views/UserMenu.cshtml | 8 +++ .../OrchardCore.Users/Views/UserRoles.cshtml | 22 ++++++++ .../Views/UsersAdminListCreate.cshtml | 6 +- .../CommonPermissions.cs | 28 +++++----- src/docs/releases/1.6.0.md | 20 ++++--- 16 files changed, 189 insertions(+), 100 deletions(-) create mode 100644 src/OrchardCore.Modules/OrchardCore.Users/Views/Admin/Display.cshtml create mode 100644 src/OrchardCore.Modules/OrchardCore.Users/Views/User.DetailAdmin.cshtml create mode 100644 src/OrchardCore.Modules/OrchardCore.Users/Views/UserInfo.cshtml create mode 100644 src/OrchardCore.Modules/OrchardCore.Users/Views/UserRoles.cshtml diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Controllers/AdminController.cs b/src/OrchardCore.Modules/OrchardCore.Users/Controllers/AdminController.cs index bc58437fe7f..994d269938d 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Controllers/AdminController.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Controllers/AdminController.cs @@ -171,11 +171,11 @@ public async Task Index([ModelBinder(BinderType = typeof(UserFilte options.UserRoleFilters = new List() { new SelectListItem() { Text = S["All roles"], Value = String.Empty, Selected = (options.SelectedRole == String.Empty) }, - new SelectListItem() { Text = S["Authenticated (no roles)"], Value = "Authenticated", Selected = (String.Equals(options.SelectedRole, "Authenticated", StringComparison.OrdinalIgnoreCase)) } + new SelectListItem() { Text = S["Authenticated (no roles)"], Value = "Authenticated", Selected = String.Equals(options.SelectedRole, "Authenticated", StringComparison.OrdinalIgnoreCase) } }; // TODO Candidate for dynamic localization. - options.UserRoleFilters.AddRange(allRoles.Select(x => new SelectListItem { Text = x, Value = x, Selected = (String.Equals(options.SelectedRole, x, StringComparison.OrdinalIgnoreCase)) })); + options.UserRoleFilters.AddRange(allRoles.Select(x => new SelectListItem { Text = x, Value = x, Selected = String.Equals(options.SelectedRole, x, StringComparison.OrdinalIgnoreCase) })); // Populate options pager summary values. var startIndex = (pagerShape.Page - 1) * (pagerShape.PageSize) + 1; @@ -184,7 +184,7 @@ public async Task Index([ModelBinder(BinderType = typeof(UserFilte options.UsersCount = userEntries.Count; options.TotalItemCount = pagerShape.TotalItemCount; - var header = await _userOptionsDisplayManager.BuildEditorAsync(options, _updateModelAccessor.ModelUpdater, false, "", ""); + var header = await _userOptionsDisplayManager.BuildEditorAsync(options, _updateModelAccessor.ModelUpdater, false, String.Empty, String.Empty); var shapeViewModel = await _shapeFactory.CreateAsync("UsersAdminList", viewModel => { @@ -208,7 +208,7 @@ public async Task IndexFilterPOST(UserIndexOptions options) } // Evaluate the values provided in the form post and map them to the filter result and route values. - await _userOptionsDisplayManager.UpdateEditorAsync(options, _updateModelAccessor.ModelUpdater, false, "", ""); + await _userOptionsDisplayManager.UpdateEditorAsync(options, _updateModelAccessor.ModelUpdater, false, String.Empty, String.Empty); // The route value must always be added after the editors have updated the models. options.RouteValues.TryAdd("q", options.FilterResult.ToString()); @@ -235,7 +235,7 @@ public async Task IndexPOST(UserIndexOptions options, IEnumerable< foreach (var user in checkedUsers) { var canEditUser = await _authorizationService.AuthorizeAsync(User, CommonPermissions.EditUsers, user); - var isSameUser = String.Equals(user.UserId, User.FindFirstValue(ClaimTypes.NameIdentifier), StringComparison.OrdinalIgnoreCase); + var isSameUser = user.UserId == User.FindFirstValue(ClaimTypes.NameIdentifier); switch (options.BulkAction) { @@ -283,12 +283,12 @@ public async Task Create() { var user = new User(); - if (!await _authorizationService.AuthorizeAsync(User, CommonPermissions.EditUsers, user) || !await _authorizationService.AuthorizeAsync(User, CommonPermissions.AssignUsersToRole, user)) + if (!await _authorizationService.AuthorizeAsync(User, CommonPermissions.EditUsers, user)) { return Forbid(); } - var shape = await _userDisplayManager.BuildEditorAsync(user, updater: _updateModelAccessor.ModelUpdater, isNew: true, "", ""); + var shape = await _userDisplayManager.BuildEditorAsync(user, updater: _updateModelAccessor.ModelUpdater, isNew: true, String.Empty, String.Empty); return View(shape); } @@ -299,12 +299,12 @@ public async Task CreatePost([Bind(Prefix = "User.Password")] str { var user = new User(); - if (!await _authorizationService.AuthorizeAsync(User, CommonPermissions.EditUsers, user) || !await _authorizationService.AuthorizeAsync(User, CommonPermissions.AssignUsersToRole, user)) + if (!await _authorizationService.AuthorizeAsync(User, CommonPermissions.EditUsers, user)) { return Forbid(); } - var shape = await _userDisplayManager.UpdateEditorAsync(user, updater: _updateModelAccessor.ModelUpdater, isNew: true, "", ""); + var shape = await _userDisplayManager.UpdateEditorAsync(user, updater: _updateModelAccessor.ModelUpdater, isNew: true, String.Empty, String.Empty); if (!ModelState.IsValid) { @@ -347,7 +347,7 @@ public async Task Edit(string id, string returnUrl) return Forbid(); } - var shape = await _userDisplayManager.BuildEditorAsync(user, updater: _updateModelAccessor.ModelUpdater, isNew: false, "", ""); + var shape = await _userDisplayManager.BuildEditorAsync(user, updater: _updateModelAccessor.ModelUpdater, isNew: false, String.Empty, String.Empty); ViewData["ReturnUrl"] = returnUrl; @@ -380,7 +380,7 @@ public async Task EditPost(string id, string returnUrl) return Forbid(); } - var shape = await _userDisplayManager.UpdateEditorAsync(user, updater: _updateModelAccessor.ModelUpdater, isNew: false, "", ""); + var shape = await _userDisplayManager.UpdateEditorAsync(user, updater: _updateModelAccessor.ModelUpdater, isNew: false, String.Empty, String.Empty); if (!ModelState.IsValid) { @@ -399,7 +399,7 @@ public async Task EditPost(string id, string returnUrl) return View(shape); } - if (String.Equals(User.FindFirstValue(ClaimTypes.NameIdentifier), user.UserId, StringComparison.OrdinalIgnoreCase)) + if (User.FindFirstValue(ClaimTypes.NameIdentifier) == user.UserId) { await _signInManager.RefreshSignInAsync(user); } @@ -415,15 +415,30 @@ public async Task EditPost(string id, string returnUrl) return RedirectToAction(nameof(Edit)); } - else + + if (!String.IsNullOrEmpty(returnUrl)) { - if (!String.IsNullOrEmpty(returnUrl)) - { - return this.LocalRedirect(returnUrl, true); - } + return this.LocalRedirect(returnUrl, true); + } + + return RedirectToAction(nameof(Index)); + } - return RedirectToAction(nameof(Index)); + public async Task Display(string id) + { + if (await _userManager.FindByIdAsync(id) is not User user) + { + return NotFound(); } + + if (!await _authorizationService.AuthorizeAsync(User, CommonPermissions.ViewUsers, user)) + { + return Forbid(); + } + + var model = await _userDisplayManager.BuildDisplayAsync(user, _updateModelAccessor.ModelUpdater, "DetailAdmin"); + + return View(model); } [HttpPost] diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserDisplayDriver.cs b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserDisplayDriver.cs index 43bda03974c..2d181fc0ec9 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserDisplayDriver.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserDisplayDriver.cs @@ -56,6 +56,7 @@ public override IDisplayResult Display(User user) { return Combine( Initialize("UserFields", model => model.User = user).Location("SummaryAdmin", "Header:1"), + Initialize("UserInfo", model => model.User = user).Location("DetailAdmin", "Content:5"), Initialize("UserButtons", model => model.User = user).Location("SummaryAdmin", "Actions:1") ); } diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserRoleDisplayDriver.cs b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserRoleDisplayDriver.cs index 81e8202b118..e589c095e07 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserRoleDisplayDriver.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserRoleDisplayDriver.cs @@ -49,9 +49,15 @@ public UserRoleDisplayDriver( public override IDisplayResult Display(User user) { - return Initialize("UserRolesMeta", model => model.User = user) - .Location("SummaryAdmin", "Meta"); + return Combine( + Initialize("UserRolesMeta", model => model.User = user) + .Location("SummaryAdmin", "Meta"), + + Initialize("UserRoles", model => model.User = user) + .Location("DetailAdmin", "Content:10") + ); } + public override IDisplayResult Edit(User user) { // This view is always rendered, however there will be no editable roles if the user does not have permission to edit them. @@ -59,7 +65,7 @@ public override IDisplayResult Edit(User user) { // The current user can only view their roles if they have assign role, to prevent listing roles when managing their own profile. if (String.Equals(_httpContextAccessor.HttpContext.User.FindFirstValue(ClaimTypes.NameIdentifier), user.UserId, StringComparison.OrdinalIgnoreCase) && - !await _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext.User, CommonPermissions.AssignUsersToRole)) + !await _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext.User, CommonPermissions.AssignRoleToUsers)) { return; } @@ -182,7 +188,7 @@ private async Task> GetAccessibleRoleNamesAsync(IEnumerable< foreach (var role in roles) { - if (await _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext.User, CommonPermissions.AssignUsersToRole, role)) + if (await _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext.User, CommonPermissions.AssignRoleToUsers, role)) { authorizedRoleNames.Add(role.RoleName); } diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Permissions.cs b/src/OrchardCore.Modules/OrchardCore.Users/Permissions.cs index d46e4667a3b..ccb4605d8d5 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Permissions.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Permissions.cs @@ -8,7 +8,7 @@ public class Permissions : IPermissionProvider { public static readonly Permission ManageUsers = CommonPermissions.ManageUsers; public static readonly Permission ViewUsers = CommonPermissions.ViewUsers; - public static readonly Permission ManageOwnUserInformation = new("ManageOwnUserInformation", "Edit own user information", new Permission[] { ManageUsers }); + public static readonly Permission ManageOwnUserInformation = new("ManageOwnUserInformation", "Edit own user information", new Permission[] { CommonPermissions.EditUsers }); public Task> GetPermissionsAsync() { diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Services/RoleAuthorizationHandler.cs b/src/OrchardCore.Modules/OrchardCore.Users/Services/RoleAuthorizationHandler.cs index 9cdc2307fd9..6eca9cc2de5 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Services/RoleAuthorizationHandler.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Services/RoleAuthorizationHandler.cs @@ -1,5 +1,4 @@ using System; -using System.Collections.Generic; using System.Linq; using System.Security.Claims; using System.Threading.Tasks; @@ -43,7 +42,9 @@ protected override async Task HandleRequirementAsync(AuthorizationHandlerContext if (context.Resource is IRole role) { - if (await TryAuthenticateAsync(context, requirement.Permission, role.RoleName)) + var variantPermission = GetPermissionVariation(requirement.Permission, role.RoleName); + + if (variantPermission != null && await _authorizationService.AuthorizeAsync(context.User, variantPermission)) { context.Succeed(requirement); @@ -55,8 +56,8 @@ protected override async Task HandleRequirementAsync(AuthorizationHandlerContext { var currentUserId = context.User.FindFirstValue(ClaimTypes.NameIdentifier); - if (String.Equals(requirement.Permission.Name, CommonPermissions.EditUsers.Name, StringComparison.OrdinalIgnoreCase) - && user.UserId != null && user.UserId.Equals(currentUserId, StringComparison.OrdinalIgnoreCase) + if (requirement.Permission.Name == CommonPermissions.EditUsers.Name + && user.UserId == currentUserId && await _authorizationService.AuthorizeAsync(context.User, Permissions.ManageOwnUserInformation)) { context.Succeed(requirement); @@ -75,14 +76,16 @@ protected override async Task HandleRequirementAsync(AuthorizationHandlerContext if (!roleNames.Any()) { - // When the given user does not have any role, means it's a dummy user. - roleNames = await AvailableRolesAsync(); + // When the user is in no roles, we check to see if the current user can manage any roles. + roleNames = (await _roleService.GetRoleNamesAsync()).Where(roleName => !RoleHelper.SystemRoleNames.Contains(roleName)); } // Check every role to see if the current user has permission to at least one role. foreach (var roleName in roleNames) { - if (await TryAuthenticateAsync(context, requirement.Permission, roleName)) + var variantPermission = GetPermissionVariation(requirement.Permission, roleName); + + if (variantPermission != null && await _authorizationService.AuthorizeAsync(context.User, variantPermission)) { context.Succeed(requirement); @@ -92,48 +95,33 @@ protected override async Task HandleRequirementAsync(AuthorizationHandlerContext } } - private IEnumerable _roleNames; - - private async Task> AvailableRolesAsync() - { - _roleNames ??= (await _roleService.GetRoleNamesAsync()) - .Except(new[] { "Anonymous", "Authenticated" }, StringComparer.OrdinalIgnoreCase); - - return _roleNames; - } - - private async Task TryAuthenticateAsync(AuthorizationHandlerContext context, Permission permission, string roleName) + private Permission GetPermissionVariation(Permission permission, string roleName) { - if (String.Equals(permission.Name, CommonPermissions.ListUsers.Name, StringComparison.OrdinalIgnoreCase) - && await _authorizationService.AuthorizeAsync(context.User, CommonPermissions.CreateListUsersInRolePermission(roleName))) + if (permission.Name == CommonPermissions.ListUsers.Name) { - return true; + return CommonPermissions.CreateListUsersInRolePermission(roleName); } - if (String.Equals(permission.Name, CommonPermissions.EditUsers.Name, StringComparison.OrdinalIgnoreCase) - && await _authorizationService.AuthorizeAsync(context.User, CommonPermissions.CreateEditUsersInRolePermission(roleName))) + if (permission.Name == CommonPermissions.EditUsers.Name) { - return true; + return CommonPermissions.CreateEditUsersInRolePermission(roleName); } - if (String.Equals(permission.Name, CommonPermissions.DeleteUsers.Name, StringComparison.OrdinalIgnoreCase) - && await _authorizationService.AuthorizeAsync(context.User, CommonPermissions.CreateDeleteUsersInRolePermission(roleName))) + if (permission.Name == CommonPermissions.DeleteUsers.Name) { - return true; + return CommonPermissions.CreateDeleteUsersInRolePermission(roleName); } - if (String.Equals(permission.Name, CommonPermissions.AssignUsersToRole.Name, StringComparison.OrdinalIgnoreCase) - && await _authorizationService.AuthorizeAsync(context.User, CommonPermissions.CreateAssignUsersToRolePermission(roleName))) + if (permission.Name == CommonPermissions.AssignRoleToUsers.Name) { - return true; + return CommonPermissions.CreateAssignRoleToUsersPermission(roleName); } - if (String.Equals(permission.Name, CommonPermissions.ManageUsers.Name, StringComparison.OrdinalIgnoreCase) - && await _authorizationService.AuthorizeAsync(context.User, CommonPermissions.CreatePermissionForManageUsersInRole(roleName))) + if (permission.Name == CommonPermissions.ManageUsers.Name) { - return true; + return CommonPermissions.CreatePermissionForManageUsersInRole(roleName); } - return false; + return null; } } diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Startup.cs b/src/OrchardCore.Modules/OrchardCore.Users/Startup.cs index 7a69d2af58e..f021b5497e0 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Startup.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Startup.cs @@ -135,7 +135,12 @@ public override void Configure(IApplicationBuilder builder, IEndpointRouteBuilde pattern: _adminOptions.AdminUrlPrefix + "/Users/Unlock/{id}", defaults: new { controller = adminControllerName, action = nameof(AdminController.Unlock) } ); - + routes.MapAreaControllerRoute( + name: "UsersDisplay", + areaName: "OrchardCore.Users", + pattern: _adminOptions.AdminUrlPrefix + "/Users/Display/{id}", + defaults: new { controller = adminControllerName, action = nameof(AdminController.Display) } + ); builder.UseAuthorization(); } diff --git a/src/OrchardCore.Modules/OrchardCore.Users/UserRolePermissions.cs b/src/OrchardCore.Modules/OrchardCore.Users/UserRolePermissions.cs index 7e0c7deb268..5929d19a1f8 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/UserRolePermissions.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/UserRolePermissions.cs @@ -23,7 +23,7 @@ public async Task> GetPermissionsAsync() var list = new List() { - CommonPermissions.AssignUsersToRole, + CommonPermissions.AssignRoleToUsers, }; foreach (var roleName in roleNames) @@ -31,7 +31,7 @@ public async Task> GetPermissionsAsync() list.Add(CommonPermissions.CreateListUsersInRolePermission(roleName)); list.Add(CommonPermissions.CreateEditUsersInRolePermission(roleName)); list.Add(CommonPermissions.CreateDeleteUsersInRolePermission(roleName)); - list.Add(CommonPermissions.CreateAssignUsersToRolePermission(roleName)); + list.Add(CommonPermissions.CreateAssignRoleToUsersPermission(roleName)); list.Add(CommonPermissions.CreatePermissionForManageUsersInRole(roleName)); } @@ -45,7 +45,7 @@ public IEnumerable GetDefaultStereotypes() { Name = "Administrator", Permissions = new[] { - CommonPermissions.AssignUsersToRole, + CommonPermissions.AssignRoleToUsers, } } }; diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Views/Admin/Display.cshtml b/src/OrchardCore.Modules/OrchardCore.Users/Views/Admin/Display.cshtml new file mode 100644 index 00000000000..32eceece3e6 --- /dev/null +++ b/src/OrchardCore.Modules/OrchardCore.Users/Views/Admin/Display.cshtml @@ -0,0 +1 @@ +@await DisplayAsync(Model) diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Views/User.DetailAdmin.cshtml b/src/OrchardCore.Modules/OrchardCore.Users/Views/User.DetailAdmin.cshtml new file mode 100644 index 00000000000..52ca4c01b81 --- /dev/null +++ b/src/OrchardCore.Modules/OrchardCore.Users/Views/User.DetailAdmin.cshtml @@ -0,0 +1,23 @@ +@using OrchardCore.Mvc.Utilities; +@using OrchardCore.ContentManagement + +

    @RenderTitleSegments(T["View User"])

     + +
    +
    + @await DisplayAsync(Model.Header) + @if (Model.Meta != null) + { +
    + @await DisplayAsync(Model.Meta) +
    + } +
    + @await DisplayAsync(Model.Content) + @if (Model.Footer != null) + { +
    + @await DisplayAsync(Model.Footer) +
    + } +
    diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Views/UserButtons.cshtml b/src/OrchardCore.Modules/OrchardCore.Users/Views/UserButtons.cshtml index 89ea847274d..5401fff36df 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Views/UserButtons.cshtml +++ b/src/OrchardCore.Modules/OrchardCore.Users/Views/UserButtons.cshtml @@ -6,32 +6,39 @@ @inject UserManager UserManager @inject IAuthorizationService AuthorizationService @{ - var user = Model.User as User; - var currentUser = user.UserName == User.Identity.Name; - var canEdit = await AuthorizationService.AuthorizeAsync(User, CommonPermissions.EditUsers, user); - var isLockedOut = await UserManager.IsLockedOutAsync(user); + var isCurrentUser = Model.User.UserName == User.Identity.Name; + var canEdit = await AuthorizationService.AuthorizeAsync(User, CommonPermissions.EditUsers, Model.User); + var isLockedOut = await UserManager.IsLockedOutAsync(Model.User); } @if (canEdit) { - @T["Edit"] + @T["Edit"] +} + +@if (!isCurrentUser && await AuthorizationService.AuthorizeAsync(User, CommonPermissions.ViewUsers, Model.User)) +{ + @T["View"] +} - @T["Edit Password"] +@if (canEdit) +{ + @T["Edit Password"] if (isLockedOut) { - @T["Unlock"] + @T["Unlock"] } } -@if (!currentUser && await AuthorizationService.AuthorizeAsync(User, CommonPermissions.DeleteUsers, user)) +@if (!isCurrentUser && await AuthorizationService.AuthorizeAsync(User, CommonPermissions.DeleteUsers, Model.User)) { - @T["Delete"] + @T["Delete"] } -@if (canEdit && !user.EmailConfirmed && Site.As().UsersMustValidateEmail) +@if (canEdit && !Model.User.EmailConfirmed && Site.As().UsersMustValidateEmail) { - + } diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Views/UserInfo.cshtml b/src/OrchardCore.Modules/OrchardCore.Users/Views/UserInfo.cshtml new file mode 100644 index 00000000000..3f9adae94aa --- /dev/null +++ b/src/OrchardCore.Modules/OrchardCore.Users/Views/UserInfo.cshtml @@ -0,0 +1,15 @@ +@model SummaryAdminUserViewModel + +

    @T["User Info"]

    + +
    +
    @T["Username"]
    +
    @Model.User.UserName
    + +
    @T["Email"]
    +
    @Model.User.Email
    + +
    @T["Status"]
    +
    @(Model.User.IsEnabled ? T["Enabled"] : T["Disabled"])
    + +
    diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Views/UserMenu.cshtml b/src/OrchardCore.Modules/OrchardCore.Users/Views/UserMenu.cshtml index 1eda4f04865..b09a57abe56 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Views/UserMenu.cshtml +++ b/src/OrchardCore.Modules/OrchardCore.Users/Views/UserMenu.cshtml @@ -1,4 +1,8 @@ +@using System.Security.Claims @inject IAuthorizationService AuthorizationService +@{ + var userId = User.FindFirstValue(ClaimTypes.NameIdentifier); +}
  •   @T["Profile"]
    • } + else if (await AuthorizationService.AuthorizeAsync(User, CommonPermissions.ViewUsers)) + { +
  •   @T["Profile"]
    • + }
  •   @T["Change password"]
diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Views/UserRoles.cshtml b/src/OrchardCore.Modules/OrchardCore.Users/Views/UserRoles.cshtml new file mode 100644 index 00000000000..531fdd3b82b --- /dev/null +++ b/src/OrchardCore.Modules/OrchardCore.Users/Views/UserRoles.cshtml @@ -0,0 +1,22 @@ +@using OrchardCore.Security.Services +@model SummaryAdminUserViewModel + +@inject IRoleService RoleService + +@{ + var roleNames = await RoleService.GetRoleNamesAsync(); +} + +

@T["Roles"]

+
    + @foreach (var normalizedRoleName in Model.User.RoleNames) + { + var roleName = roleNames.FirstOrDefault(roleName => roleName.Equals(normalizedRoleName, StringComparison.OrdinalIgnoreCase)); + if (roleName == null) + { + continue; + } + +
  • @roleName
    • + } +
diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Views/UsersAdminListCreate.cshtml b/src/OrchardCore.Modules/OrchardCore.Users/Views/UsersAdminListCreate.cshtml index 0887ea6663c..2dea19a71b6 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Views/UsersAdminListCreate.cshtml +++ b/src/OrchardCore.Modules/OrchardCore.Users/Views/UsersAdminListCreate.cshtml @@ -1,12 +1,8 @@ @model UserIndexOptions @using OrchardCore.Users.Models @inject IAuthorizationService AuthorizationService -@{ - var dummyUser = new User(); -} -@if (await AuthorizationService.AuthorizeAsync(User, CommonPermissions.EditUsers, dummyUser) -&& await AuthorizationService.AuthorizeAsync(User, CommonPermissions.AssignUsersToRole, dummyUser)) +@if (await AuthorizationService.AuthorizeAsync(User, CommonPermissions.EditUsers, new User())) { @T["Add User"] } diff --git a/src/OrchardCore/OrchardCore.Users.Core/CommonPermissions.cs b/src/OrchardCore/OrchardCore.Users.Core/CommonPermissions.cs index 473877e4a41..3c3c6d4d637 100644 --- a/src/OrchardCore/OrchardCore.Users.Core/CommonPermissions.cs +++ b/src/OrchardCore/OrchardCore.Users.Core/CommonPermissions.cs @@ -12,9 +12,9 @@ public class CommonPermissions public static readonly Permission ManageUsers = new("ManageUsers", "Manage security settings and all users", true); /// - /// View users only allows viewing a users profile. + /// Allows viewing user profiles. /// - public static readonly Permission ViewUsers = new("View Users", "View user's profile", new[] { ManageUsers }); + public static readonly Permission ViewUsers = new("View Users", "View user profiles", new[] { ManageUsers }); public static readonly Permission EditUsers = new("EditUsers", "Edit any user", new[] { ManageUsers }, true); @@ -22,28 +22,28 @@ public class CommonPermissions public static readonly Permission ListUsers = new("ListUsers", "List all users", new[] { EditUsers, DeleteUsers }); - public static readonly Permission AssignUsersToRole = new("AssignUsersToRole", "Assign users to any role", new[] { EditUsers }, true); + public static readonly Permission AssignRoleToUsers = new("AssignRoleToUsers", "Assign any role to users", new[] { EditUsers }, true); - public static Permission CreateListUsersInRolePermission(string name) => - CreateDynamicPermission(name, new Permission("ListUsersInRole_{0}", "List users in {0} role", new[] { ListUsers })); + public static Permission CreateEditUsersInRolePermission(string roleName) => + CreateDynamicPermission(roleName, new Permission("EditUsersInRole_{0}", "Edit users in {0} role", new[] { EditUsers }, true)); - public static Permission CreateEditUsersInRolePermission(string name) => - CreateDynamicPermission(name, new Permission("EditUsersInRole_{0}", "Edit users in {0} role", new[] { EditUsers }, true)); + public static Permission CreateDeleteUsersInRolePermission(string roleName) => + CreateDynamicPermission(roleName, new Permission("DeleteUsersInRole_{0}", "Delete users in {0} role", new[] { DeleteUsers }, true)); - public static Permission CreateDeleteUsersInRolePermission(string name) => - CreateDynamicPermission(name, new Permission("DeleteUsersInRole_{0}", "Delete users in {0} role", new[] { DeleteUsers }, true)); + public static Permission CreateListUsersInRolePermission(string roleName) => + CreateDynamicPermission(roleName, new Permission("ListUsersInRole_{0}", "List users in {0} role", new[] { ListUsers })); - public static Permission CreateAssignUsersToRolePermission(string name) => - CreateDynamicPermission(name, new Permission("AssignUsersToRole_{0}", "Assign users to {0} role", new[] { AssignUsersToRole }, true)); + public static Permission CreateAssignRoleToUsersPermission(string roleName) => + CreateDynamicPermission(roleName, new Permission("AssignRoleToUsers_{0}", "Assign {0} role to users", new[] { AssignRoleToUsers }, true)); public static Permission CreatePermissionForManageUsersInRole(string name) => CreateDynamicPermission(name, new Permission("ManageUsersInRole_{0}", "Manage users in {0} role", new[] { ManageUsers }, true)); // Dynamic permission template. - private static Permission CreateDynamicPermission(string name, Permission permission) + private static Permission CreateDynamicPermission(string roleName, Permission permission) => new( - String.Format(permission.Name, name), - String.Format(permission.Description, name), + String.Format(permission.Name, roleName), + String.Format(permission.Description, roleName), permission.ImpliedBy, permission.IsSecurityCritical ); diff --git a/src/docs/releases/1.6.0.md b/src/docs/releases/1.6.0.md index bb4eecfe783..787d799feff 100644 --- a/src/docs/releases/1.6.0.md +++ b/src/docs/releases/1.6.0.md @@ -8,23 +8,25 @@ Release date: Not yet released The `OrchardCore.Users` module now provides multiple new permissions for added control on user related functions. -In previous versions, `View Users` permission was enough to show the `Users` menu item. However, with the newly added permissions, `ListUsers` or at least one `ListUsersInRole_{0}` variation is required to see the `Users` menu. The `View Users` permission now only allows you to view user's profile. Here is a list of the new permissions +In previous versions, `View Users` permission was enough to show the `Users` menu item. However, with the newly added permissions, `ListUsers` or at least one `ListUsersInRole_{0}` variation is required to see the `Users` menu. The `View Users` permission now only allows you to view user's profile. The permission `ManageUsers` is a super user permission which grants all access to manage user including user settings. + +Here is a list of the new permissions | Permission | Description | | --- | --- | -| `ListUsers` | allows users to list all users. | -| `EditUsers` | allow users to edit any user. | -| `DeleteUsers` | allows users to delete any user. | +| `ListUsers` | allows listing all users. | +| `EditUsers` | allow editing any user. | +| `DeleteUsers` | allows deleting any user. | The following permissions will be available if `OrchardCore.Roles` feature in enabled. | Permission | Description | | --- | --- | - | `AssignUsersToRole` | allows users to assign users to any role. | - | `ListUsersInRole_{0}` | allows users to list users in a given role. | - | `EditUsersInRole_{0}` | allows users to edit users in a given role. | - | `DeleteUsersInRole_{0}` | allows users to delete users in a given role. | - | `AssignUsersToRole_{0}` | allows users to assign users to a given role. | + | `AssignRoleToUsers` | allows assigning any role to users. | + | `ListUsersInRole_{0}` | allows listing users in a given role. | + | `EditUsersInRole_{0}` | allows editing users in a given role. | + | `DeleteUsersInRole_{0}` | allows deleting users in a given role. | + | `AssignRoleToUsers_{0}` | allows assigning users to a given role. | ### Features and Recipes From a36081c5e42109db525b6f3d7510026dcecc6f52 Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Sun, 15 Jan 2023 13:56:19 -0800 Subject: [PATCH 18/25] use normalized name is the views --- .../OrchardCore.Users/Views/UserRoles.cshtml | 4 ++-- .../OrchardCore.Users/Views/UserRolesMeta.cshtml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Views/UserRoles.cshtml b/src/OrchardCore.Modules/OrchardCore.Users/Views/UserRoles.cshtml index 531fdd3b82b..8899109b246 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Views/UserRoles.cshtml +++ b/src/OrchardCore.Modules/OrchardCore.Users/Views/UserRoles.cshtml @@ -4,14 +4,14 @@ @inject IRoleService RoleService @{ - var roleNames = await RoleService.GetRoleNamesAsync(); + var roleNames = await RoleService.GetNormalizedRoleNamesAsync(); }

@T["Roles"]

    @foreach (var normalizedRoleName in Model.User.RoleNames) { - var roleName = roleNames.FirstOrDefault(roleName => roleName.Equals(normalizedRoleName, StringComparison.OrdinalIgnoreCase)); + var roleName = roleNames.FirstOrDefault(roleName => roleName == normalizedRoleName); if (roleName == null) { continue; diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Views/UserRolesMeta.cshtml b/src/OrchardCore.Modules/OrchardCore.Users/Views/UserRolesMeta.cshtml index a34b3474cfd..e317fb79913 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Views/UserRolesMeta.cshtml +++ b/src/OrchardCore.Modules/OrchardCore.Users/Views/UserRolesMeta.cshtml @@ -4,13 +4,13 @@ @inject IRoleService RoleService @{ - var roleNames = await RoleService.GetRoleNamesAsync(); + var roleNames = await RoleService.GetNormalizedRoleNamesAsync(); }
    @foreach (var normalizedRoleName in Model.User.RoleNames) { - var roleName = roleNames.FirstOrDefault(roleName => roleName.Equals(normalizedRoleName, StringComparison.OrdinalIgnoreCase)); + var roleName = roleNames.FirstOrDefault(roleName => roleName == normalizedRoleName); if (roleName == null) { continue; From 4e994a28c3934f6bb134db1df9e46bad4cf39999 Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Sun, 15 Jan 2023 14:00:43 -0800 Subject: [PATCH 19/25] Remove ununsed class --- .../ViewModels/UserProfileSettingsViewModel.cs | 8 -------- 1 file changed, 8 deletions(-) delete mode 100644 src/OrchardCore.Modules/OrchardCore.Users/ViewModels/UserProfileSettingsViewModel.cs diff --git a/src/OrchardCore.Modules/OrchardCore.Users/ViewModels/UserProfileSettingsViewModel.cs b/src/OrchardCore.Modules/OrchardCore.Users/ViewModels/UserProfileSettingsViewModel.cs deleted file mode 100644 index 688c34d7e9d..00000000000 --- a/src/OrchardCore.Modules/OrchardCore.Users/ViewModels/UserProfileSettingsViewModel.cs +++ /dev/null @@ -1,8 +0,0 @@ -namespace OrchardCore.Users.ViewModels; - -public class UserProfileSettingsViewModel -{ - public bool AllowChangingUsername { get; set; } - - public bool AllowChangingEmail { get; set; } -} From 72dda0800b5f5b40d3ea6cda3f2d34a8c08c1171 Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Sun, 15 Jan 2023 14:13:01 -0800 Subject: [PATCH 20/25] Update userid check --- .../OrchardCore.Users/Drivers/UserDisplayDriver.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserDisplayDriver.cs b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserDisplayDriver.cs index 2d181fc0ec9..416ebbc60bf 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserDisplayDriver.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserDisplayDriver.cs @@ -155,7 +155,7 @@ public override async Task UpdateAsync(User user, UpdateEditorCo private bool IsCurrentUser(User user) { - return String.Equals(_httpContextAccessor.HttpContext.User.FindFirstValue(ClaimTypes.NameIdentifier), user.UserId, StringComparison.OrdinalIgnoreCase); + return _httpContextAccessor.HttpContext.User.FindFirstValue(ClaimTypes.NameIdentifier) == user.UserId; } } } From ff61b3651fa4517269e7019aba6404691606b3a6 Mon Sep 17 00:00:00 2001 From: jtkech Date: Mon, 16 Jan 2023 05:16:53 +0100 Subject: [PATCH 21/25] Normalize user "Administrator" role on setup. --- .../OrchardCore.Users/Services/SetupEventHandler.cs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Services/SetupEventHandler.cs b/src/OrchardCore.Modules/OrchardCore.Users/Services/SetupEventHandler.cs index e4e2918f9cc..6af415f2317 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Services/SetupEventHandler.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Services/SetupEventHandler.cs @@ -1,7 +1,9 @@ using System; using System.Collections.Generic; using System.Threading.Tasks; +using Microsoft.AspNetCore.Identity; using OrchardCore.Abstractions.Setup; +using OrchardCore.Security; using OrchardCore.Setup.Events; using OrchardCore.Users.Models; @@ -13,10 +15,12 @@ namespace OrchardCore.Users.Services public class SetupEventHandler : ISetupEventHandler { private readonly IUserService _userService; + private readonly RoleManager _roleManager; - public SetupEventHandler(IUserService userService) + public SetupEventHandler(IUserService userService, RoleManager roleManager) { _userService = userService; + _roleManager = roleManager; } public Task Setup( @@ -29,10 +33,11 @@ Action reportError UserName = properties.TryGetValue(SetupConstants.AdminUsername, out var adminUserName) ? adminUserName?.ToString() : String.Empty, UserId = properties.TryGetValue(SetupConstants.AdminUserId, out var adminUserId) ? adminUserId?.ToString() : String.Empty, Email = properties.TryGetValue(SetupConstants.AdminEmail, out var adminEmail) ? adminEmail?.ToString() : String.Empty, - RoleNames = new string[] { "Administrator" }, EmailConfirmed = true }; + user.RoleNames.Add(_roleManager.NormalizeKey("Administrator")); + return _userService.CreateUserAsync(user, properties[SetupConstants.AdminPassword]?.ToString(), reportError); } } From 241092962d3fc0162b5c9562a5af8803de2a9bbf Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Mon, 16 Jan 2023 17:36:58 -0800 Subject: [PATCH 22/25] Update permissions --- .../OrchardCore.Users/AdminMenu.cs | 8 +++---- .../Controllers/AdminController.cs | 4 ++-- .../Controllers/RegistrationController.cs | 2 +- .../ChangeEmailSettingsDisplayDriver.cs | 4 ++-- .../Drivers/LoginSettingsDisplayDriver.cs | 4 ++-- .../RegistrationSettingsDisplayDriver.cs | 4 ++-- .../ResetPasswordSettingsDisplayDriver.cs | 4 ++-- .../Drivers/UserInformationDisplayDriver.cs | 5 ++--- .../Drivers/UserRoleDisplayDriver.cs | 8 +++---- .../OrchardCore.Users/Permissions.cs | 22 +++++++++---------- .../Services/RoleAuthorizationHandler.cs | 2 +- .../OrchardCore.Users/Views/UserMenu.cshtml | 2 +- .../CommonPermissions.cs | 2 ++ 13 files changed, 36 insertions(+), 35 deletions(-) diff --git a/src/OrchardCore.Modules/OrchardCore.Users/AdminMenu.cs b/src/OrchardCore.Modules/OrchardCore.Users/AdminMenu.cs index f6472e72e05..3aa32a383af 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/AdminMenu.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/AdminMenu.cs @@ -35,7 +35,7 @@ public Task BuildNavigationAsync(string name, NavigationBuilder builder) ) .Add(S["Settings"], settings => settings .Add(S["User Login"], S["User Login"].PrefixPosition(), login => login - .Permission(Permissions.ManageUsers) + .Permission(CommonPermissions.ManageUsers) .Action("Index", "Admin", new { area = "OrchardCore.Settings", groupId = LoginSettingsDisplayDriver.GroupId }) .LocalNav() ) @@ -67,7 +67,7 @@ public Task BuildNavigationAsync(string name, NavigationBuilder builder) .Add(S["Security"], security => security .Add(S["Settings"], settings => settings .Add(S["User Change email"], S["User Change email"].PrefixPosition(), registration => registration - .Permission(Permissions.ManageUsers) + .Permission(CommonPermissions.ManageUsers) .Action("Index", "Admin", new { area = "OrchardCore.Settings", groupId = ChangeEmailSettingsDisplayDriver.GroupId }) .LocalNav() ))); @@ -97,7 +97,7 @@ public Task BuildNavigationAsync(string name, NavigationBuilder builder) .Add(S["Security"], security => security .Add(S["Settings"], settings => settings .Add(S["User Registration"], S["User Registration"].PrefixPosition(), registration => registration - .Permission(Permissions.ManageUsers) + .Permission(CommonPermissions.ManageUsers) .Action("Index", "Admin", new { area = "OrchardCore.Settings", groupId = RegistrationSettingsDisplayDriver.GroupId }) .LocalNav() ))); @@ -127,7 +127,7 @@ public Task BuildNavigationAsync(string name, NavigationBuilder builder) .Add(S["Security"], security => security .Add(S["Settings"], settings => settings .Add(S["User Reset password"], S["User Reset password"].PrefixPosition(), password => password - .Permission(Permissions.ManageUsers) + .Permission(CommonPermissions.ManageUsers) .Action("Index", "Admin", new { area = "OrchardCore.Settings", groupId = ResetPasswordSettingsDisplayDriver.GroupId }) .LocalNav() ))); diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Controllers/AdminController.cs b/src/OrchardCore.Modules/OrchardCore.Users/Controllers/AdminController.cs index 994d269938d..06322ae21c2 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Controllers/AdminController.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Controllers/AdminController.cs @@ -330,7 +330,7 @@ public async Task Edit(string id, string returnUrl) if (String.IsNullOrEmpty(id)) { id = User.FindFirstValue(ClaimTypes.NameIdentifier); - if (!await _authorizationService.AuthorizeAsync(User, Permissions.ManageOwnUserInformation)) + if (!await _authorizationService.AuthorizeAsync(User, CommonPermissions.EditOwnUser)) { return Forbid(); } @@ -364,7 +364,7 @@ public async Task EditPost(string id, string returnUrl) { editingOwnUser = true; id = User.FindFirstValue(ClaimTypes.NameIdentifier); - if (!await _authorizationService.AuthorizeAsync(User, Permissions.ManageOwnUserInformation)) + if (!await _authorizationService.AuthorizeAsync(User, CommonPermissions.EditOwnUser)) { return Forbid(); } diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Controllers/RegistrationController.cs b/src/OrchardCore.Modules/OrchardCore.Users/Controllers/RegistrationController.cs index efd4e7c8c00..9983ebe6ed1 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Controllers/RegistrationController.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Controllers/RegistrationController.cs @@ -152,7 +152,7 @@ public IActionResult RegistrationPending(string returnUrl = null) [ValidateAntiForgeryToken] public async Task SendVerificationEmail(string id) { - if (!await _authorizationService.AuthorizeAsync(User, Permissions.ManageUsers)) + if (!await _authorizationService.AuthorizeAsync(User, CommonPermissions.ManageUsers)) { return Forbid(); } diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/ChangeEmailSettingsDisplayDriver.cs b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/ChangeEmailSettingsDisplayDriver.cs index c0f4f5a7d40..05747e29d2d 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/ChangeEmailSettingsDisplayDriver.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/ChangeEmailSettingsDisplayDriver.cs @@ -28,7 +28,7 @@ public override async Task EditAsync(ChangeEmailSettings setting { var user = _httpContextAccessor.HttpContext?.User; - if (!await _authorizationService.AuthorizeAsync(user, Permissions.ManageUsers)) + if (!await _authorizationService.AuthorizeAsync(user, CommonPermissions.ManageUsers)) { return null; } @@ -43,7 +43,7 @@ public override async Task UpdateAsync(ChangeEmailSettings secti { var user = _httpContextAccessor.HttpContext?.User; - if (!await _authorizationService.AuthorizeAsync(user, Permissions.ManageUsers)) + if (!await _authorizationService.AuthorizeAsync(user, CommonPermissions.ManageUsers)) { return null; } diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/LoginSettingsDisplayDriver.cs b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/LoginSettingsDisplayDriver.cs index 7122cf4e1d7..4d07548afa7 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/LoginSettingsDisplayDriver.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/LoginSettingsDisplayDriver.cs @@ -26,7 +26,7 @@ public override async Task EditAsync(LoginSettings settings, Bui { var user = _httpContextAccessor.HttpContext?.User; - if (!await _authorizationService.AuthorizeAsync(user, Permissions.ManageUsers)) + if (!await _authorizationService.AuthorizeAsync(user, CommonPermissions.ManageUsers)) { return null; } @@ -45,7 +45,7 @@ public override async Task EditAsync(LoginSettings settings, Bui public override async Task UpdateAsync(LoginSettings section, BuildEditorContext context) { - if (!await _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext?.User, Permissions.ManageUsers)) + if (!await _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext?.User, CommonPermissions.ManageUsers)) { return null; } diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/RegistrationSettingsDisplayDriver.cs b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/RegistrationSettingsDisplayDriver.cs index 2a686310cf3..aa604795de3 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/RegistrationSettingsDisplayDriver.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/RegistrationSettingsDisplayDriver.cs @@ -28,7 +28,7 @@ public override async Task EditAsync(RegistrationSettings settin { var user = _httpContextAccessor.HttpContext?.User; - if (!await _authorizationService.AuthorizeAsync(user, Permissions.ManageUsers)) + if (!await _authorizationService.AuthorizeAsync(user, CommonPermissions.ManageUsers)) { return null; } @@ -51,7 +51,7 @@ public override async Task UpdateAsync(RegistrationSettings sect { var user = _httpContextAccessor.HttpContext?.User; - if (!await _authorizationService.AuthorizeAsync(user, Permissions.ManageUsers)) + if (!await _authorizationService.AuthorizeAsync(user, CommonPermissions.ManageUsers)) { return null; } diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/ResetPasswordSettingsDisplayDriver.cs b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/ResetPasswordSettingsDisplayDriver.cs index 43fe1f732b9..a465d7e0eb3 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/ResetPasswordSettingsDisplayDriver.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/ResetPasswordSettingsDisplayDriver.cs @@ -29,7 +29,7 @@ public override async Task EditAsync(ResetPasswordSettings setti { var user = _httpContextAccessor.HttpContext?.User; - if (!await _authorizationService.AuthorizeAsync(user, Permissions.ManageUsers)) + if (!await _authorizationService.AuthorizeAsync(user, CommonPermissions.ManageUsers)) { return null; } @@ -45,7 +45,7 @@ public override async Task UpdateAsync(ResetPasswordSettings sec { var user = _httpContextAccessor.HttpContext?.User; - if (!await _authorizationService.AuthorizeAsync(user, Permissions.ManageUsers)) + if (!await _authorizationService.AuthorizeAsync(user, CommonPermissions.ManageUsers)) { return null; } diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserInformationDisplayDriver.cs b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserInformationDisplayDriver.cs index 210d86ac2f0..3665b562869 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserInformationDisplayDriver.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserInformationDisplayDriver.cs @@ -1,4 +1,3 @@ -using System; using System.Security.Claims; using System.Threading.Tasks; using Microsoft.AspNetCore.Authorization; @@ -107,12 +106,12 @@ public override async Task UpdateAsync(User user, UpdateEditorCo private async Task CanEditUserInfoAsync(User user) { - return !IsCurrentUser(user) || await _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext.User, Permissions.ManageOwnUserInformation); + return !IsCurrentUser(user) || await _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext.User, CommonPermissions.EditOwnUser); } private bool IsCurrentUser(User user) { - return String.Equals(_httpContextAccessor.HttpContext.User.FindFirstValue(ClaimTypes.NameIdentifier), user.UserId, StringComparison.OrdinalIgnoreCase); + return _httpContextAccessor.HttpContext.User.FindFirstValue(ClaimTypes.NameIdentifier) == user.UserId; } } } diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserRoleDisplayDriver.cs b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserRoleDisplayDriver.cs index e589c095e07..1e22f416dbf 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserRoleDisplayDriver.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserRoleDisplayDriver.cs @@ -19,7 +19,7 @@ namespace OrchardCore.Users.Drivers { public class UserRoleDisplayDriver : DisplayDriver { - private const string AdministratorRole = "Administrator"; + private const string _administratorRole = "Administrator"; private readonly UserManager _userManager; private readonly IRoleService _roleService; @@ -141,9 +141,9 @@ public override async Task UpdateAsync(User user, UpdateEditorCo foreach (var role in rolesToRemove) { - if (String.Equals(role, AdministratorRole, StringComparison.OrdinalIgnoreCase)) + if (String.Equals(role, _administratorRole, StringComparison.OrdinalIgnoreCase)) { - var enabledUsersOfAdminRole = (await _userManager.GetUsersInRoleAsync(AdministratorRole)) + var enabledUsersOfAdminRole = (await _userManager.GetUsersInRoleAsync(_administratorRole)) .Cast() .Where(user => user.IsEnabled) .ToList(); @@ -151,7 +151,7 @@ public override async Task UpdateAsync(User user, UpdateEditorCo // Make sure we always have at least one enabled administrator account. if (enabledUsersOfAdminRole.Count == 1 && user.UserId == enabledUsersOfAdminRole.First().UserId) { - await _notifier.WarningAsync(H[$"Cannot remove {AdministratorRole} role from the only enabled administrator."]); + await _notifier.WarningAsync(H[$"Cannot remove {_administratorRole} role from the only enabled administrator."]); continue; } diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Permissions.cs b/src/OrchardCore.Modules/OrchardCore.Users/Permissions.cs index ccb4605d8d5..351b74b7591 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Permissions.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Permissions.cs @@ -8,15 +8,15 @@ public class Permissions : IPermissionProvider { public static readonly Permission ManageUsers = CommonPermissions.ManageUsers; public static readonly Permission ViewUsers = CommonPermissions.ViewUsers; - public static readonly Permission ManageOwnUserInformation = new("ManageOwnUserInformation", "Edit own user information", new Permission[] { CommonPermissions.EditUsers }); + public static readonly Permission ManageOwnUserInformation = CommonPermissions.EditOwnUser; public Task> GetPermissionsAsync() { return Task.FromResult>(new List { - ManageUsers, - ViewUsers, - ManageOwnUserInformation, + CommonPermissions.ManageUsers, + CommonPermissions.ViewUsers, + CommonPermissions.EditOwnUser, CommonPermissions.ListUsers, CommonPermissions.EditUsers, CommonPermissions.DeleteUsers, @@ -29,9 +29,9 @@ public IEnumerable GetDefaultStereotypes() new PermissionStereotype { Name = "Administrator", Permissions = new[] { - ManageUsers, - ViewUsers, - ManageOwnUserInformation, + CommonPermissions.ManageUsers, + CommonPermissions.ViewUsers, + CommonPermissions.EditOwnUser, CommonPermissions.ListUsers, CommonPermissions.EditUsers, CommonPermissions.DeleteUsers, @@ -39,19 +39,19 @@ public IEnumerable GetDefaultStereotypes() }, new PermissionStereotype { Name = "Editor", - Permissions = new[] { ManageOwnUserInformation } + Permissions = new[] { CommonPermissions.EditOwnUser } }, new PermissionStereotype { Name = "Moderator", - Permissions = new[] { ManageOwnUserInformation } + Permissions = new[] { CommonPermissions.EditOwnUser } }, new PermissionStereotype { Name = "Contributor", - Permissions = new[] { ManageOwnUserInformation } + Permissions = new[] { CommonPermissions.EditOwnUser } }, new PermissionStereotype { Name = "Author", - Permissions = new[] { ManageOwnUserInformation } + Permissions = new[] { CommonPermissions.EditOwnUser } } }; } diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Services/RoleAuthorizationHandler.cs b/src/OrchardCore.Modules/OrchardCore.Users/Services/RoleAuthorizationHandler.cs index 6eca9cc2de5..ad094766b23 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Services/RoleAuthorizationHandler.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Services/RoleAuthorizationHandler.cs @@ -58,7 +58,7 @@ protected override async Task HandleRequirementAsync(AuthorizationHandlerContext if (requirement.Permission.Name == CommonPermissions.EditUsers.Name && user.UserId == currentUserId - && await _authorizationService.AuthorizeAsync(context.User, Permissions.ManageOwnUserInformation)) + && await _authorizationService.AuthorizeAsync(context.User, CommonPermissions.EditOwnUser)) { context.Succeed(requirement); diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Views/UserMenu.cshtml b/src/OrchardCore.Modules/OrchardCore.Users/Views/UserMenu.cshtml index b09a57abe56..38557e41adb 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Views/UserMenu.cshtml +++ b/src/OrchardCore.Modules/OrchardCore.Users/Views/UserMenu.cshtml @@ -11,7 +11,7 @@
    • @T["Signed in as"]@User.Identity.Name
      • - @if (await AuthorizationService.AuthorizeAsync(User, Permissions.ManageOwnUserInformation)) + @if (await AuthorizationService.AuthorizeAsync(User, CommonPermissions.EditOwnUser)) {
    •   @T["Profile"]
      • } diff --git a/src/OrchardCore/OrchardCore.Users.Core/CommonPermissions.cs b/src/OrchardCore/OrchardCore.Users.Core/CommonPermissions.cs index 3c3c6d4d637..ae01aea8003 100644 --- a/src/OrchardCore/OrchardCore.Users.Core/CommonPermissions.cs +++ b/src/OrchardCore/OrchardCore.Users.Core/CommonPermissions.cs @@ -24,6 +24,8 @@ public class CommonPermissions public static readonly Permission AssignRoleToUsers = new("AssignRoleToUsers", "Assign any role to users", new[] { EditUsers }, true); + public static readonly Permission EditOwnUser = new("ManageOwnUserInformation", "Edit own user information", new Permission[] { EditUsers }); + public static Permission CreateEditUsersInRolePermission(string roleName) => CreateDynamicPermission(roleName, new Permission("EditUsersInRole_{0}", "Edit users in {0} role", new[] { EditUsers }, true)); From d01a2f1d8d6bfca02242596dd22efc9693c8da0d Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Tue, 17 Jan 2023 08:25:11 -0800 Subject: [PATCH 23/25] Cleanup --- .../Drivers/UserRoleDisplayDriver.cs | 11 ++++++----- .../OrchardCore.Users/Views/UserRoles.cshtml | 11 +++++------ .../OrchardCore.Users/Views/UserRolesMeta.cshtml | 9 ++++----- 3 files changed, 15 insertions(+), 16 deletions(-) diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserRoleDisplayDriver.cs b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserRoleDisplayDriver.cs index 1e22f416dbf..fc8365d369d 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserRoleDisplayDriver.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserRoleDisplayDriver.cs @@ -64,8 +64,8 @@ public override IDisplayResult Edit(User user) return Initialize("UserRoleFields_Edit", async model => { // The current user can only view their roles if they have assign role, to prevent listing roles when managing their own profile. - if (String.Equals(_httpContextAccessor.HttpContext.User.FindFirstValue(ClaimTypes.NameIdentifier), user.UserId, StringComparison.OrdinalIgnoreCase) && - !await _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext.User, CommonPermissions.AssignRoleToUsers)) + if (_httpContextAccessor.HttpContext.User.FindFirstValue(ClaimTypes.NameIdentifier) == user.UserId + && !await _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext.User, CommonPermissions.AssignRoleToUsers)) { return; } @@ -97,10 +97,10 @@ public override IDisplayResult Edit(User user) public override async Task UpdateAsync(User user, UpdateEditorContext context) { var model = new EditUserRoleViewModel(); - var currentUserId = _httpContextAccessor.HttpContext.User.FindFirstValue(ClaimTypes.NameIdentifier); // The current user cannot alter their own roles. This prevents them removing access to the site for themselves. - if (currentUserId == user.UserId && !await _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext.User, StandardPermissions.SiteOwner)) + if (_httpContextAccessor.HttpContext.User.FindFirstValue(ClaimTypes.NameIdentifier) == user.UserId + && !await _authorizationService.AuthorizeAsync(_httpContextAccessor.HttpContext.User, StandardPermissions.SiteOwner)) { await _notifier.WarningAsync(H["Cannot update your own roles."]); @@ -133,7 +133,8 @@ public override async Task UpdateAsync(User user, UpdateEditorCo foreach (var role in currentUserRoleNames) { // When the user has permission to manage the role and it is no longer selected the role can be removed. - if (accessibleRoleNames.Contains(role, StringComparer.OrdinalIgnoreCase) && !accessibleAndSelectedRoleNames.Contains(role, StringComparer.OrdinalIgnoreCase)) + if (accessibleRoleNames.Contains(role, StringComparer.OrdinalIgnoreCase) + && !accessibleAndSelectedRoleNames.Contains(role, StringComparer.OrdinalIgnoreCase)) { rolesToRemove.Add(role); } diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Views/UserRoles.cshtml b/src/OrchardCore.Modules/OrchardCore.Users/Views/UserRoles.cshtml index 8899109b246..8814867d986 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Views/UserRoles.cshtml +++ b/src/OrchardCore.Modules/OrchardCore.Users/Views/UserRoles.cshtml @@ -1,22 +1,21 @@ +@using Microsoft.AspNetCore.Identity @using OrchardCore.Security.Services @model SummaryAdminUserViewModel @inject IRoleService RoleService - @{ - var roleNames = await RoleService.GetNormalizedRoleNamesAsync(); + var roles = (await RoleService.GetRolesAsync()).ToDictionary(x => x.RoleName, x => x, StringComparer.OrdinalIgnoreCase); }

      @T["Roles"]

        - @foreach (var normalizedRoleName in Model.User.RoleNames) + @foreach (var roleName in Model.User.RoleNames) { - var roleName = roleNames.FirstOrDefault(roleName => roleName == normalizedRoleName); - if (roleName == null) + if (!roles.TryGetValue(roleName, out var role)) { continue; } -
      • @roleName
        • +
      • @role.RoleName
        • }
      diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Views/UserRolesMeta.cshtml b/src/OrchardCore.Modules/OrchardCore.Users/Views/UserRolesMeta.cshtml index e317fb79913..0b586d0aa80 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Views/UserRolesMeta.cshtml +++ b/src/OrchardCore.Modules/OrchardCore.Users/Views/UserRolesMeta.cshtml @@ -4,18 +4,17 @@ @inject IRoleService RoleService @{ - var roleNames = await RoleService.GetNormalizedRoleNamesAsync(); + var roles = (await RoleService.GetRolesAsync()).ToDictionary(x => x.RoleName, x => x, StringComparer.OrdinalIgnoreCase); }
      - @foreach (var normalizedRoleName in Model.User.RoleNames) + @foreach (var roleName in Model.User.RoleNames) { - var roleName = roleNames.FirstOrDefault(roleName => roleName == normalizedRoleName); - if (roleName == null) + if (!roles.TryGetValue(roleName, out var role)) { continue; } - @roleName + @role.RoleName }
      From f46c1e3a28299b8a09e7635a5b275c45fcee639e Mon Sep 17 00:00:00 2001 From: jtkech Date: Wed, 18 Jan 2023 08:06:32 +0100 Subject: [PATCH 24/25] Minor change --- .../OrchardCore.Users/Controllers/AdminController.cs | 2 +- .../Security/Services/RoleHelper.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Controllers/AdminController.cs b/src/OrchardCore.Modules/OrchardCore.Users/Controllers/AdminController.cs index 06322ae21c2..7495c0c26c4 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Controllers/AdminController.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Controllers/AdminController.cs @@ -166,7 +166,7 @@ public async Task Index([ModelBinder(BinderType = typeof(UserFilte }; var allRoles = (await _roleService.GetRoleNamesAsync()) - .Except(new[] { "Anonymous", "Authenticated" }, StringComparer.OrdinalIgnoreCase); + .Except(RoleHelper.SystemRoleNames, StringComparer.OrdinalIgnoreCase); options.UserRoleFilters = new List() { diff --git a/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Security/Services/RoleHelper.cs b/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Security/Services/RoleHelper.cs index a360c7e189e..52ecad50a68 100644 --- a/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Security/Services/RoleHelper.cs +++ b/src/OrchardCore/OrchardCore.Infrastructure.Abstractions/Security/Services/RoleHelper.cs @@ -8,7 +8,7 @@ public class RoleHelper public static readonly HashSet SystemRoleNames = new(StringComparer.OrdinalIgnoreCase) { "Anonymous", - "Authenticated" + "Authenticated", }; } } From cbbf2412a9d88e02ea5a424af5809ee6fea8f43b Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Wed, 18 Jan 2023 08:21:46 -0800 Subject: [PATCH 25/25] Minor UI changes --- .../Drivers/UserRoleDisplayDriver.cs | 2 +- .../Views/User.SummaryAdmin.cshtml | 33 +++++++++++-------- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserRoleDisplayDriver.cs b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserRoleDisplayDriver.cs index fc8365d369d..e82c20343a5 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserRoleDisplayDriver.cs +++ b/src/OrchardCore.Modules/OrchardCore.Users/Drivers/UserRoleDisplayDriver.cs @@ -51,7 +51,7 @@ public override IDisplayResult Display(User user) { return Combine( Initialize("UserRolesMeta", model => model.User = user) - .Location("SummaryAdmin", "Meta"), + .Location("SummaryAdmin", "Description"), Initialize("UserRoles", model => model.User = user) .Location("DetailAdmin", "Content:10") diff --git a/src/OrchardCore.Modules/OrchardCore.Users/Views/User.SummaryAdmin.cshtml b/src/OrchardCore.Modules/OrchardCore.Users/Views/User.SummaryAdmin.cshtml index 7b09b3cb94a..79d58feb283 100644 --- a/src/OrchardCore.Modules/OrchardCore.Users/Views/User.SummaryAdmin.cshtml +++ b/src/OrchardCore.Modules/OrchardCore.Users/Views/User.SummaryAdmin.cshtml @@ -1,20 +1,27 @@
      -
      - @if (Model.Header != null) - { -
      @await DisplayAsync(Model.Header)
      - } +
      +
      + @if (Model.Header != null) + { +
      @await DisplayAsync(Model.Header)
      + } - @if (Model.Meta != null) - { -
      - @await DisplayAsync(Model.Meta) -
      - } + @if (Model.Meta != null) + { +
      + @await DisplayAsync(Model.Meta) +
      + } + + @if (Model.Content != null) + { +
      @await DisplayAsync(Model.Content)
      + } +
      - @if (Model.Content != null) + @if (Model.Description != null) { -
      @await DisplayAsync(Model.Content)
      +
      @await DisplayAsync(Model.Description)
      }