Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

Adding validation on $export type filters. #4617

Merged
merged 3 commits into from
Sep 19, 2024
Merged

Adding validation on $export type filters. #4617

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9c045a5
Adding validation on $export type filters.
v-iyamauchi Sep 16, 2024
db5153b
Merge branch 'main' of https://github.com/Microsoft/fhir-server
v-iyamauchi Sep 17, 2024
8e954ff
Addressing reviewer's comment.
v-iyamauchi Sep 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 118 additions & 1 deletion src/Microsoft.Health.Fhir.Core/Features/Operations/Export/CreateExportRequestHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,15 @@

using System;
using System.Collections.Generic;
using System.Globalization;
using System.Linq;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
using EnsureThat;
using Hl7.Fhir.Utility;
using MediatR;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Microsoft.Health.Core.Extensions;
using Microsoft.Health.Core.Features.Context;
Expand All @@ -18,10 +22,16 @@
using Microsoft.Health.Fhir.Core.Configs;
using Microsoft.Health.Fhir.Core.Exceptions;
using Microsoft.Health.Fhir.Core.Features.Context;
using Microsoft.Health.Fhir.Core.Features.Definition;
using Microsoft.Health.Fhir.Core.Features.Operations.Export.Models;
using Microsoft.Health.Fhir.Core.Features.Persistence;
using Microsoft.Health.Fhir.Core.Features.Search;
using Microsoft.Health.Fhir.Core.Features.Search.Registry;
using Microsoft.Health.Fhir.Core.Features.Security;
using Microsoft.Health.Fhir.Core.Messages.Export;
using Microsoft.Health.Fhir.Core.Models;
using Microsoft.Health.Fhir.ValueSets;
using StringExtensions = Microsoft.Health.Core.Extensions.StringExtensions;

namespace Microsoft.Health.Fhir.Core.Features.Operations.Export
{
Expand All @@ -30,30 +40,45 @@ namespace Microsoft.Health.Fhir.Core.Features.Operations.Export
/// </summary>
public class CreateExportRequestHandler : IRequestHandler<CreateExportRequest, CreateExportResponse>
{
private static readonly HashSet<string> KnownSearchParameterModifiers = new HashSet<string>(
Enum.GetNames(typeof(SearchModifierCode)).Select(e => ((SearchModifierCode)Enum.Parse(typeof(SearchModifierCode), e)).GetLiteral()),
StringComparer.OrdinalIgnoreCase);

private readonly IClaimsExtractor _claimsExtractor;
private readonly IFhirOperationDataStore _fhirOperationDataStore;
private readonly IAuthorizationService<DataActions> _authorizationService;
private readonly ExportJobConfiguration _exportJobConfiguration;
private readonly RequestContextAccessor<IFhirRequestContext> _contextAccessor;
private readonly ISearchParameterDefinitionManager _searchParameterDefinitionManager;
private readonly ILogger<CreateExportRequestHandler> _logger;
private readonly bool _includeValidateTypeFiltersValidationDetails;

public CreateExportRequestHandler(
IClaimsExtractor claimsExtractor,
IFhirOperationDataStore fhirOperationDataStore,
IAuthorizationService<DataActions> authorizationService,
IOptions<ExportJobConfiguration> exportJobConfiguration,
RequestContextAccessor<IFhirRequestContext> fhirRequestContextAccessor)
RequestContextAccessor<IFhirRequestContext> fhirRequestContextAccessor,
ISearchParameterDefinitionManager searchParameterDefinitionManager,
ILogger<CreateExportRequestHandler> logger,
bool includeValidateTypeFiltersValidationDetails = false)
{
EnsureArg.IsNotNull(claimsExtractor, nameof(claimsExtractor));
EnsureArg.IsNotNull(fhirOperationDataStore, nameof(fhirOperationDataStore));
EnsureArg.IsNotNull(authorizationService, nameof(authorizationService));
EnsureArg.IsNotNull(exportJobConfiguration?.Value, nameof(exportJobConfiguration));
EnsureArg.IsNotNull(exportJobConfiguration?.Value, nameof(fhirRequestContextAccessor));
EnsureArg.IsNotNull(searchParameterDefinitionManager, nameof(searchParameterDefinitionManager));
EnsureArg.IsNotNull(logger, nameof(logger));

_claimsExtractor = claimsExtractor;
_fhirOperationDataStore = fhirOperationDataStore;
_authorizationService = authorizationService;
_exportJobConfiguration = exportJobConfiguration.Value;
_contextAccessor = fhirRequestContextAccessor;
_searchParameterDefinitionManager = searchParameterDefinitionManager;
_logger = logger;
_includeValidateTypeFiltersValidationDetails = includeValidateTypeFiltersValidationDetails;
}

public async Task<CreateExportResponse> Handle(CreateExportRequest request, CancellationToken cancellationToken)
Expand All @@ -72,6 +97,7 @@ public async Task<CreateExportResponse> Handle(CreateExportRequest request, Canc
StringExtensions.ComputeHash(_exportJobConfiguration.StorageAccountConnection);

var filters = ParseFilter(request.Filters);
ValidateTypeFilters(filters);

ExportJobFormatConfiguration formatConfiguration = ParseFormat(request.FormatName, request.ContainerName != null);

Expand Down Expand Up @@ -179,5 +205,96 @@ private ExportJobFormatConfiguration ParseFormat(string formatName, bool useCont

return formatConfiguration;
}

private void ValidateTypeFilters(IList<ExportJobFilter> filters)
{
if (filters == null || filters.Count == 0)
{
_logger.LogInformation("No type filters to validate.");
return;
}

var errors = new List<string[]>();
foreach (var filter in filters)
{
if (filter.Parameters == null || filter.Parameters.Count == 0)
{
continue;
}

foreach (var parameter in filter.Parameters)
{
string name = null;
string modifier = null;
if (!TrySplitParameterName(parameter.Item1, out name, out modifier)
|| string.IsNullOrWhiteSpace(name)
|| (!string.IsNullOrWhiteSpace(modifier) ? !KnownSearchParameterModifiers.Contains(modifier) : false))
{
errors.Add(new string[] { filter.ResourceType, parameter.Item1, "Unknown" });
continue;
}

// NOTE: some search parameters loaded into SearchParameterDefinitionManager's cache from the embedded resource (search-parameters.json)
// have the status with "0", inconsistent with the status in the data store. Allow the "0" status as Enabled for now.
SearchParameterInfo searchParameter = null;
if (_searchParameterDefinitionManager.TryGetSearchParameter(filter.ResourceType, name, out searchParameter)
&& (searchParameter.SearchParameterStatus == SearchParameterStatus.Enabled
|| searchParameter.SearchParameterStatus == 0))
{
continue;
}

errors.Add(new string[]
{
filter.ResourceType,
parameter.Item1,
searchParameter?.SearchParameterStatus.ToString() ?? "Unknown",
});
}
}

if (errors.Count > 0)
{
var errorMessage = new StringBuilder($"{errors.Count} invalid search parameter(s) found:{Environment.NewLine}");
errors.ForEach(e => errorMessage.AppendLine(
string.Format(CultureInfo.InvariantCulture, "[type: {0}, parameter: {1}, status: {2}]", e[0], e[1], e[2])));

var message = errorMessage.ToString();
_logger.LogError(message);

var ex = new BadRequestException(message);
if (_includeValidateTypeFiltersValidationDetails)
{
// Note: Test purpose only
ex.Data.Add(nameof(ValidateTypeFilters), errors);
}

throw ex;
}
}

private static bool TrySplitParameterName(string parameterName, out string name, out string modifier)
v-iyamauchi marked this conversation as resolved.
Show resolved Hide resolved
{
name = string.Empty;
modifier = string.Empty;
if (string.IsNullOrWhiteSpace(parameterName))
{
return false;
}

string[] s = parameterName.Split(':');
if (s.Length > 2)
{
return false;
}

name = s[0];
if (s.Length > 1)
{
modifier = s[1];
}

return true;
}
}
}
Loading
Loading