Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[ODS-6532] Use CTE-based queries to improve performance for authorization of page and partitions requests #1167

Merged
merged 7 commits into from
Oct 21, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ public abstract class DataManagementControllerBase<TResourceModel, TEntityInterf
protected Lazy<GetManyPipeline<TResourceModel, TAggregateRoot>> GetManyPipeline;
protected Lazy<PutPipeline<TResourceModel, TAggregateRoot>> PutPipeline;

private static readonly IContextStorage _contextStorage = new CallContextStorage();

protected DataManagementControllerBase(
IPipelineFactory pipelineFactory,
IEdFiProblemDetailsProvider problemDetailsProvider,
Expand Down Expand Up @@ -148,7 +150,13 @@ public virtual async Task<IActionResult> GetAll(
[FromQuery] Dictionary<string, string> additionalParameters = default)
{
//respond quickly to DOS style requests (should we catch these earlier? e.g. attribute filter?)


// Store alternative auth approach decision into call context
if (additionalParameters?.TryGetValue("useJoinAuth", out string useJoinAuth) == true)
{
_contextStorage.SetValue("UseJoinAuth", Convert.ToBoolean(useJoinAuth));
}

var queryParameters = new QueryParameters(urlQueryParametersRequest);

if (!QueryParametersValidator.IsValid(queryParameters, _defaultPageLimitSize, out string errorMessage))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ public class PartitionsController : ControllerBase
private readonly IOdsDatabaseConnectionStringProvider _odsDatabaseConnectionStringProvider;
private readonly IPartitionsQueryBuilderProvider _partitionsQueryBuilderProvider;

private static readonly IContextStorage _contextStorage = new CallContextStorage();

public PartitionsController(
DbProviderFactory dbProviderFactory,
IContextProvider<DataManagementResourceContext> dataManagementResourceContextProvider,
Expand All @@ -67,6 +69,12 @@ public async Task<IActionResult> Get(
[FromQuery] int number = 1,
[FromQuery] Dictionary<string, string> additionalParameters = default)
{
// Store alternative auth approach decision into call context
if (additionalParameters?.TryGetValue("useJoinAuth", out string useJoinAuth) == true)
{
_contextStorage.SetValue("UseJoinAuth", Convert.ToBoolean(useJoinAuth));
}

if (number is < 1 or > 200)
{
var problemDetails = new BadRequestParameterException(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,11 @@ public QueryBuilder GetQueryBuilder(
var cteCountQueryBuilder = cteQueryBuilder.Clone();
bool hasDistinct = cteCountQueryBuilder.HasDistinct();
cteCountQueryBuilder.ClearSelect();
cteCountQueryBuilder.ClearWith();
cteCountQueryBuilder.SelectRaw($"COUNT({(hasDistinct ? "DISTINCT " : null)}AggregateId) AS CountOfRows");

var queryBuilder = new QueryBuilder(_dialect).With("Numbered", cteQueryBuilder)
var queryBuilder = new QueryBuilder(_dialect)
.With("Numbered", cteQueryBuilder)
.With("Counts", cteCountQueryBuilder)
.From("Numbered, Counts")
.Select("AggregateId")
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,220 @@
// SPDX-License-Identifier: Apache-2.0
// Licensed to the Ed-Fi Alliance under one or more agreements.
// The Ed-Fi Alliance licenses this file to you under the Apache License, Version 2.0.
// See the LICENSE and NOTICES files in the project root for more information.

using System;
using System.Collections.Generic;
using System.Linq;
using EdFi.Ods.Common;
using EdFi.Ods.Api.Security.Authorization.Filtering;
using EdFi.Ods.Api.Security.AuthorizationStrategies.Relationships.Filters;
using EdFi.Ods.Common.Context;
using EdFi.Ods.Common.Database.Querying;
using EdFi.Ods.Common.Infrastructure.Filtering;
using EdFi.Ods.Common.Models.Domain;
using EdFi.Ods.Common.Providers.Queries;
using EdFi.Ods.Common.Security.Authorization;

namespace EdFi.Ods.Api.Security.Authorization.Repositories
{
/// <summary>
/// Provides an abstract implementation for applying authorization filters to queries on aggregate roots built using the <see cref="QueryBuilder"/>.
/// </summary>
public class AggregateRootQueryBuilderProviderCteAuthorizationDecorator : IAggregateRootQueryBuilderProvider
{
private readonly IAggregateRootQueryBuilderProvider _decoratedInstance;
private readonly IContextProvider<DataManagementAuthorizationPlan> _authorizationPlanContextProvider;
private readonly IAuthorizationFilterDefinitionProvider _authorizationFilterDefinitionProvider;

public AggregateRootQueryBuilderProviderCteAuthorizationDecorator(
IAggregateRootQueryBuilderProvider decoratedInstance,
IContextProvider<DataManagementAuthorizationPlan> authorizationPlanContextProvider,
IAuthorizationFilterDefinitionProvider authorizationFilterDefinitionProvider)
{
_decoratedInstance = decoratedInstance;
_authorizationPlanContextProvider = authorizationPlanContextProvider;
_authorizationFilterDefinitionProvider = authorizationFilterDefinitionProvider;
}

/// <summary>
/// Applies the authorization filtering criteria to the query created by the decorated instance.
/// </summary>
/// <param name="aggregateRootEntity"></param>
/// <param name="specification">An instance of the entity representing the parameters to the query.</param>
/// <param name="queryParameters">The parameter values to apply to the query.</param>
/// <param name="additionalQueryParameters">Additional parameters supplied by the API client that are resource-level properties or common parameters.</param>
/// <returns>The criteria created by the decorated instance.</returns>
public QueryBuilder GetQueryBuilder(
Entity aggregateRootEntity,
AggregateRootWithCompositeKey specification,
IQueryParameters queryParameters,
IDictionary<string, string> additionalQueryParameters)
{
var queryBuilder = _decoratedInstance.GetQueryBuilder(
aggregateRootEntity,
specification,
queryParameters,
additionalQueryParameters);

var authorizationPlan = _authorizationPlanContextProvider.Get();

// Process unless join-based auth has been indicated
bool shouldUseJoinAuth = additionalQueryParameters?.TryGetValue("UseJoinAuth", out string useJoinAuth) == true
&& Convert.ToBoolean(useJoinAuth);

if (shouldUseJoinAuth)
{
return queryBuilder;
}

var unsupportedAuthorizationFilters = new HashSet<string>();

// If there are multiple relationship-based authorization strategies with views (that are combined with OR), we must use left outer joins and null/not null checks
var relationshipBasedAuthViewJoinType = DetermineRelationshipBasedAuthViewJoinType();

ApplyAuthorizationStrategiesCombinedWithAndLogic();
ApplyAuthorizationStrategiesCombinedWithOrLogic();

return queryBuilder;

JoinType? DetermineRelationshipBasedAuthViewJoinType()
{
// NOTE: Relationship-based authorization filters are combined using OR, while custom auth-view filters are combined using AND
var countOfRelationshipBasedAuthorizationFilters = authorizationPlan.Filtering.Count(
af => af.Operator == FilterOperator.Or && af.Filters.Select(afd =>
{
if (_authorizationFilterDefinitionProvider.TryGetAuthorizationFilterDefinition(afd.FilterName, out var filterDetails))
{
return filterDetails;
};

unsupportedAuthorizationFilters.Add(afd.FilterName);

return null;
})
.Where(x => x != null)
.OfType<ViewBasedAuthorizationFilterDefinition>()
.Any());

return countOfRelationshipBasedAuthorizationFilters switch
{
0 => null,
1 => JoinType.InnerJoin,
_ => JoinType.LeftOuterJoin
};
}

void ApplyAuthorizationStrategiesCombinedWithAndLogic()
{
var andStrategies = authorizationPlan.Filtering.Where(x => x.Operator == FilterOperator.And).ToArray();

// Combine 'AND' strategies
foreach (var andStrategy in andStrategies)
{
if (!TryApplyFilters(queryBuilder, andStrategy.Filters, andStrategy.AuthorizationStrategy, JoinType.InnerJoin))
{
// All filters for AND strategies must be applied, and if not, this is an error condition
throw new Exception($"The following authorization filters are not recognized: {string.Join(" ", unsupportedAuthorizationFilters)}");
}
}
}

void ApplyAuthorizationStrategiesCombinedWithOrLogic()
{
var orStrategies = authorizationPlan.Filtering
.Where(x => x.Operator == FilterOperator.Or)
.ToArray();

bool disjunctionFiltersApplied = false;

// Combine 'OR' strategies
queryBuilder.Where(
disjunctionBuilder =>
{
foreach (var orStrategy in orStrategies)
{
disjunctionBuilder.OrWhere(
filtersConjunctionBuilder =>
{
// Combine filters with 'AND'
if (TryApplyFilters(
filtersConjunctionBuilder,
orStrategy.Filters,
orStrategy.AuthorizationStrategy,
relationshipBasedAuthViewJoinType ?? JoinType.InnerJoin))
{
disjunctionFiltersApplied = true;
}

return filtersConjunctionBuilder;
});
}

return disjunctionBuilder;
});

// If we have some OR strategies with filters defined, but no filters were applied, this is an error condition
if (orStrategies.SelectMany(s => s.Filters).Any() && !disjunctionFiltersApplied)
{
throw new Exception($"The following authorization filters are not recognized: {string.Join(" ", unsupportedAuthorizationFilters)}");
}
}

bool TryApplyFilters(
QueryBuilder conjunctionQueryBuilder,
IReadOnlyList<AuthorizationFilterContext> filters,
IAuthorizationStrategy authorizationStrategy,
JoinType joinType)
{
bool allFiltersCanBeApplied = true;

foreach (var filterDetails in filters)
{
if (!_authorizationFilterDefinitionProvider.TryGetAuthorizationFilterDefinition(
filterDetails.FilterName,
out var ignored))
{
unsupportedAuthorizationFilters.Add(filterDetails.FilterName);

allFiltersCanBeApplied = false;
}
}

if (!allFiltersCanBeApplied)
{
return false;
}

bool filtersApplied = false;

foreach (var filterContext in filters)
{
_authorizationFilterDefinitionProvider.TryGetAuthorizationFilterDefinition(
filterContext.FilterName,
out var filterDefinition);

var parameterValues = filterContext.ClaimParameterName == null
? new Dictionary<string, object>()
: new Dictionary<string, object>
{
{ filterContext.ClaimParameterName, filterContext.ClaimParameterValues }
};

// Apply the authorization strategy filter
filterDefinition.CriteriaApplicator(
queryBuilder,
conjunctionQueryBuilder,
filterContext.SubjectEndpointNames,
parameterValues,
joinType,
authorizationStrategy);

filtersApplied = true;
}

return filtersApplied;
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@ namespace EdFi.Ods.Api.Security.Authorization.Repositories
/// <summary>
/// Provides an abstract implementation for applying authorization filters to queries on aggregate roots built using the <see cref="QueryBuilder"/>.
/// </summary>
public class AggregateRootQueryBuilderProviderAuthorizationDecorator : IAggregateRootQueryBuilderProvider
public class AggregateRootQueryBuilderProviderJoinAuthorizationDecorator : IAggregateRootQueryBuilderProvider
{
private readonly IAggregateRootQueryBuilderProvider _decoratedInstance;
private readonly IContextProvider<DataManagementAuthorizationPlan> _authorizationPlanContextProvider;
private readonly IAuthorizationFilterDefinitionProvider _authorizationFilterDefinitionProvider;

public AggregateRootQueryBuilderProviderAuthorizationDecorator(
public AggregateRootQueryBuilderProviderJoinAuthorizationDecorator(
IAggregateRootQueryBuilderProvider decoratedInstance,
IContextProvider<DataManagementAuthorizationPlan> authorizationPlanContextProvider,
IAuthorizationFilterDefinitionProvider authorizationFilterDefinitionProvider)
Expand Down Expand Up @@ -59,6 +59,15 @@ public QueryBuilder GetQueryBuilder(

var authorizationPlan = _authorizationPlanContextProvider.Get();

// Process if join-based auth has been indicated
bool shouldUseJoinAuth = additionalQueryParameters?.TryGetValue("UseJoinAuth", out string useJoinAuth) == true
&& Convert.ToBoolean(useJoinAuth);

if (!shouldUseJoinAuth)
{
return queryBuilder;
}

var unsupportedAuthorizationFilters = new HashSet<string>();

// If there are multiple relationship-based authorization strategies with views (that are combined with OR), we must use left outer joins and null/not null checks
Expand Down
Loading
Loading