From 17fec7d6e16cacc54d605bc814e65184913ca50e Mon Sep 17 00:00:00 2001 From: sbwalker Date: Mon, 15 Jul 2024 10:07:48 -0400 Subject: [PATCH] resolve security issue in Search --- .../Modules/Admin/SearchResults/Index.razor | 1 - .../Infrastructure/Jobs/SearchIndexJob.cs | 2 +- Oqtane.Server/Services/SearchService.cs | 42 +++++++------------ Oqtane.Shared/Models/SearchQuery.cs | 2 - 4 files changed, 16 insertions(+), 31 deletions(-) diff --git a/Oqtane.Client/Modules/Admin/SearchResults/Index.razor b/Oqtane.Client/Modules/Admin/SearchResults/Index.razor index 01039a51..1b414fd1 100644 --- a/Oqtane.Client/Modules/Admin/SearchResults/Index.razor +++ b/Oqtane.Client/Modules/Admin/SearchResults/Index.razor @@ -100,7 +100,6 @@ { SiteId = PageState.Site.SiteId, Alias = PageState.Alias, - User = PageState.User, Keywords = _keywords, SortDirection = _searchSortDirection, SortField = _searchSortField, diff --git a/Oqtane.Server/Infrastructure/Jobs/SearchIndexJob.cs b/Oqtane.Server/Infrastructure/Jobs/SearchIndexJob.cs index b791d956..142f3d3b 100644 --- a/Oqtane.Server/Infrastructure/Jobs/SearchIndexJob.cs +++ b/Oqtane.Server/Infrastructure/Jobs/SearchIndexJob.cs @@ -97,7 +97,7 @@ namespace Oqtane.Infrastructure ContentModifiedOn = page.ModifiedOn, AdditionalContent = string.Empty, CreatedOn = DateTime.UtcNow, - IsDeleted = (removed || pageModules.Any(item => item.PageId == page.PageId)), + IsDeleted = removed, TenantId = tenantId }; searchContents.Add(searchContent); diff --git a/Oqtane.Server/Services/SearchService.cs b/Oqtane.Server/Services/SearchService.cs index 76d21431..1029fc2e 100644 --- a/Oqtane.Server/Services/SearchService.cs +++ b/Oqtane.Server/Services/SearchService.cs @@ -2,6 +2,7 @@ using System; using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; +using Microsoft.AspNetCore.Http; using Microsoft.Extensions.DependencyInjection; using Oqtane.Models; using Oqtane.Repository; @@ -16,16 +17,20 @@ namespace Oqtane.Services private readonly IServiceProvider _serviceProvider; private readonly ISettingRepository _settingRepository; - private readonly IPermissionRepository _permissionRepository; + private readonly IUserPermissions _userPermissions; + private readonly IHttpContextAccessor _accessor; + public SearchService( IServiceProvider serviceProvider, ISettingRepository settingRepository, - IPermissionRepository permissionRepository) + IUserPermissions userPermissions, + IHttpContextAccessor accessor) { _settingRepository = settingRepository; - _permissionRepository = permissionRepository; _serviceProvider = serviceProvider; + _userPermissions = userPermissions; + _accessor = accessor; } public async Task GetSearchResultsAsync(SearchQuery searchQuery) @@ -35,9 +40,12 @@ namespace Oqtane.Services var totalResults = 0; - // trim results based on permissions - var results = searchResults.Where(i => IsVisible(i, searchQuery)); + // trim results + var results = searchResults.Where(i => HasViewPermission(i, searchQuery)) + .OrderBy(i => i.Url).ThenByDescending(i => i.Score) + .DistinctBy(i => i.Url); + // sort results if (searchQuery.SortDirection == SearchSortDirections.Descending) { switch (searchQuery.SortField) @@ -69,20 +77,6 @@ namespace Oqtane.Services } } - // remove duplicated results based on page id for Page and Module types - results = results.DistinctBy(i => - { - if (i.EntityName == EntityNames.Page || i.EntityName == EntityNames.Module) - { - var pageId = i.SearchContentProperties.FirstOrDefault(p => p.Name == Constants.SearchPageIdPropertyName)?.Value ?? string.Empty; - return !string.IsNullOrEmpty(pageId) ? pageId : i.UniqueKey; - } - else - { - return i.UniqueKey; - } - }); - totalResults = results.Count(); return new SearchResults @@ -92,14 +86,14 @@ namespace Oqtane.Services }; } - private bool IsVisible(SearchContent searchContent, SearchQuery searchQuery) + private bool HasViewPermission(SearchContent searchContent, SearchQuery searchQuery) { var visible = true; foreach (var permission in searchContent.Permissions.Split(',')) { var entityName = permission.Split(":")[0]; var entityId = int.Parse(permission.Split(":")[1]); - if (!HasViewPermission(searchQuery.SiteId, searchQuery.User, entityName, entityId)) + if (!_userPermissions.IsAuthorized(_accessor.HttpContext.User, searchQuery.SiteId, entityName, entityId, PermissionNames.View)) { visible = false; break; @@ -108,12 +102,6 @@ namespace Oqtane.Services return visible; } - private bool HasViewPermission(int siteId, User user, string entityName, int entityId) - { - var permissions = _permissionRepository.GetPermissions(siteId, entityName, entityId).ToList(); - return UserSecurity.IsAuthorized(user, PermissionNames.View, permissions); - } - public async Task SaveSearchContentsAsync(List searchContents, Dictionary siteSettings) { var result = ""; diff --git a/Oqtane.Shared/Models/SearchQuery.cs b/Oqtane.Shared/Models/SearchQuery.cs index 691f8491..00a83533 100644 --- a/Oqtane.Shared/Models/SearchQuery.cs +++ b/Oqtane.Shared/Models/SearchQuery.cs @@ -10,8 +10,6 @@ namespace Oqtane.Models public Alias Alias { get; set; } - public User User { get; set; } - public string Keywords { get; set; } public List EntityNames { get; set; } = new List();