Skip to content

Commit 37343c0

Browse files
vadevekaCopilot
vadeveka
and
Copilot
authored
Handle unauthorized fields in aggregation (#2790)
## Why make this change? Closes #2776 Ensure authorization error thrown if fields in the groupBy argument or in the aggregation function are not allowed for the current role. ## What is this change? During groupBy argument parsing, check if the field is allowed access for current role. During aggregation function argument parsing, check if the field is allowed access for current role If no access, then throw authorization error ## How was this tested? - [x] Integration Tests ## Sample Request(s) Samples from development mode (stack traces will not be show in production mode) <img width="1385" height="463" alt="image" src="https://github.com/user-attachments/assets/f412e127-74bb-4ca9-8d58-36c8a08281c3" /> <img width="1479" height="452" alt="image" src="https://github.com/user-attachments/assets/14763f97-93ea-4ed0-85b9-8b001859d508" /> --------- Co-authored-by: Copilot <[email protected]>
1 parent 7b6259b commit 37343c0

File tree

5 files changed

+114
-5
lines changed

5 files changed

+114
-5
lines changed

‎config-generators/mssql-commands.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,7 @@ update BookNF --config "dab-config.MsSql.json" --permissions "TestNestedFilter_E
207207
update BookNF --config "dab-config.MsSql.json" --permissions "TestNestedFilter_ColumnForbidden:read"
208208
update BookNF --config "dab-config.MsSql.json" --permissions "TestNestedFilterChained_EntityReadForbidden:read"
209209
update BookNF --config "dab-config.MsSql.json" --permissions "TestNestedFilterChained_ColumnForbidden:read"
210+
update BookNF --config "dab-config.MsSql.json" --permissions "TestFieldExcludedForAggregation:read" --fields.exclude "publisher_id"
210211
update BookNF --config "dab-config.MsSql.json" --relationship publishers --target.entity PublisherNF --cardinality one
211212
update BookNF --config "dab-config.MsSql.json" --relationship websiteplacement --target.entity BookWebsitePlacement --cardinality one
212213
update BookNF --config "dab-config.MsSql.json" --relationship reviews --target.entity Review --cardinality many

‎src/Config/DataApiBuilderException.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ public class DataApiBuilderException : Exception
1818
public const string GRAPHQL_FILTER_FIELD_AUTHZ_FAILURE = "Access forbidden to a field referenced in the filter.";
1919
public const string AUTHORIZATION_FAILURE = "Authorization Failure: Access Not Allowed.";
2020
public const string GRAPHQL_MUTATION_FIELD_AUTHZ_FAILURE = "Unauthorized due to one or more fields in this mutation.";
21+
public const string GRAPHQL_GROUPBY_FIELD_AUTHZ_FAILURE = "Access forbidden to field '{0}' referenced in the groupBy argument.";
22+
public const string GRAPHQL_AGGREGATION_FIELD_AUTHZ_FAILURE = "Access forbidden to field '{0}' referenced in the aggregation function '{1}'.";
2123

2224
public enum SubStatusCodes
2325
{

‎src/Core/Resolvers/Sql Query Structures/SqlQueryStructure.cs‎

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -459,7 +459,7 @@ private SqlQueryStructure(
459459
{
460460
if (isGroupByQuery)
461461
{
462-
ProcessGroupByField(queryField, ctx);
462+
ProcessGroupByField(queryField, ctx, authorizationResolver);
463463
}
464464
else
465465
{
@@ -877,19 +877,33 @@ private void AddGraphQLFields(IReadOnlyList<ISelectionNode> selections, RuntimeC
877877
/// }
878878
/// }
879879
/// </summary>
880-
private void ProcessGroupByField(FieldNode groupByField, IMiddlewareContext ctx)
880+
private void ProcessGroupByField(FieldNode groupByField, IMiddlewareContext ctx, IAuthorizationResolver authorizationResolver)
881881
{
882882
// Extract 'fields' argument
883883
ArgumentNode? fieldsArg = groupByField.Arguments.FirstOrDefault(a => a.Name.Value == QueryBuilder.GROUP_BY_FIELDS_FIELD_NAME);
884884
HashSet<string> fieldsInArgument = new();
885885

886+
string roleOfGraphQLRequest = Authorization.AuthorizationResolver.GetRoleOfGraphQLRequest(ctx);
887+
886888
if (fieldsArg is { Value: ListValueNode fieldsList })
887889
{
888890
foreach (EnumValueNode value in fieldsList.Items)
889891
{
890892
string fieldName = value.Value;
891893
string columnName = MetadataProvider.TryGetBackingColumn(EntityName, fieldName, out string? backingColumn) ? backingColumn : fieldName;
892894

895+
// Validate that the current role has access to groupBy argument fields
896+
IEnumerable<string> roles = authorizationResolver.GetRolesForField(EntityName, field: columnName, operation: EntityActionOperation.Read);
897+
if (roles != null && !roles.Contains(roleOfGraphQLRequest, StringComparer.OrdinalIgnoreCase))
898+
{
899+
// raising exception for the first unauthorized groupBy field found
900+
throw new DataApiBuilderException(
901+
message: string.Format(DataApiBuilderException.GRAPHQL_GROUPBY_FIELD_AUTHZ_FAILURE, fieldName),
902+
statusCode: HttpStatusCode.Forbidden,
903+
subStatusCode: DataApiBuilderException.SubStatusCodes.AuthorizationCheckFailed
904+
);
905+
}
906+
893907
GroupByMetadata.Fields[columnName] = new Column(DatabaseObject.SchemaName, DatabaseObject.Name, columnName, SourceAlias);
894908
AddColumn(fieldName, backingColumn ?? fieldName);
895909
fieldsInArgument.Add(fieldName);
@@ -913,7 +927,7 @@ private void ProcessGroupByField(FieldNode groupByField, IMiddlewareContext ctx)
913927

914928
case QueryBuilder.GROUP_BY_AGGREGATE_FIELD_NAME:
915929
GroupByMetadata.RequestedAggregations = true;
916-
ProcessAggregations(field, ctx);
930+
ProcessAggregations(field, ctx, authorizationResolver, roleOfGraphQLRequest);
917931
break;
918932
}
919933
}
@@ -963,7 +977,7 @@ private void ProcessGroupByFieldSelections(FieldNode groupByFieldSelection, Hash
963977
/// </summary>
964978
/// <param name="aggregationsField">The FieldNode representing the aggregations field in the GraphQL query.</param>
965979
/// <param name="ctx"> middleware context.</param>
966-
private void ProcessAggregations(FieldNode aggregationsField, IMiddlewareContext ctx)
980+
private void ProcessAggregations(FieldNode aggregationsField, IMiddlewareContext ctx, IAuthorizationResolver authorizationResolver, string roleOfGraphQLRequest)
967981
{
968982
// If there are no selections in the aggregation field, exit early
969983
if (aggregationsField.SelectionSet == null)
@@ -1010,7 +1024,18 @@ private void ProcessAggregations(FieldNode aggregationsField, IMiddlewareContext
10101024
if (MetadataProvider.TryGetBackingColumn(EntityName, fieldName, out string? backingColumn))
10111025
{
10121026
columnName = backingColumn;
1013-
fieldName = backingColumn;
1027+
}
1028+
1029+
// Validate that the current role has access to field in the aggregation function argument
1030+
IEnumerable<string> roles = authorizationResolver.GetRolesForField(EntityName, field: columnName, operation: EntityActionOperation.Read);
1031+
if (roles != null && !roles.Contains(roleOfGraphQLRequest, StringComparer.OrdinalIgnoreCase))
1032+
{
1033+
// raising exception for the first unauthorized field found
1034+
throw new DataApiBuilderException(
1035+
message: string.Format(DataApiBuilderException.GRAPHQL_AGGREGATION_FIELD_AUTHZ_FAILURE, fieldName, operation),
1036+
statusCode: HttpStatusCode.Forbidden,
1037+
subStatusCode: DataApiBuilderException.SubStatusCodes.AuthorizationCheckFailed
1038+
);
10141039
}
10151040

10161041
// Use the field alias if provided, otherwise default to the operation name

‎src/Service.Tests/Authorization/GraphQL/GraphQLAuthorizationHandlerTests.cs‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,5 +71,73 @@ public async Task FieldAuthorizationProcessing(bool isAuthenticated, string clie
7171
SqlTestHelper.PerformTestEqualJsonStrings(expectedResult, actual.ToString());
7272
}
7373
}
74+
75+
/// <summary>
76+
/// Tests that a GraphQL query with a groupBy operation on fields not allowed for aggregation results in an
77+
/// appropriate error message.
78+
/// </summary>
79+
/// <returns></returns>
80+
[TestMethod]
81+
public async Task Query_GroupBy_FieldNotAllowed()
82+
{
83+
string graphQLQueryName = "booksNF";
84+
string graphQLQuery = @"{
85+
booksNF {
86+
groupBy (fields: [id, publisher_id]) {
87+
fields {
88+
id
89+
publisher_id
90+
}
91+
}
92+
}
93+
}
94+
";
95+
96+
JsonElement actual = await ExecuteGraphQLRequestAsync(
97+
graphQLQuery,
98+
graphQLQueryName,
99+
isAuthenticated: true,
100+
clientRoleHeader: "TestFieldExcludedForAggregation");
101+
102+
SqlTestHelper.TestForErrorInGraphQLResponse(
103+
actual.ToString(),
104+
message: "Access forbidden to field 'publisher_id' referenced in the groupBy argument.",
105+
path: @"[""booksNF""]"
106+
);
107+
}
108+
109+
/// <summary>
110+
/// Tests that a GraphQL query with a group by aggregation on a field not allowed for aggregation results in an
111+
/// appropriate error message.
112+
/// </summary>
113+
/// <returns></returns>
114+
[TestMethod]
115+
public async Task Query_GroupBy_Aggregation_FieldNotAllowed()
116+
{
117+
string graphQLQueryName = "booksNF";
118+
string graphQLQuery = @"{
119+
booksNF {
120+
groupBy {
121+
aggregations {
122+
max (field: id)
123+
min (field: publisher_id)
124+
}
125+
}
126+
}
127+
}
128+
";
129+
130+
JsonElement actual = await ExecuteGraphQLRequestAsync(
131+
graphQLQuery,
132+
graphQLQueryName,
133+
isAuthenticated: true,
134+
clientRoleHeader: "TestFieldExcludedForAggregation");
135+
136+
SqlTestHelper.TestForErrorInGraphQLResponse(
137+
actual.ToString(),
138+
message: "Access forbidden to field 'publisher_id' referenced in the aggregation function 'min'.",
139+
path: @"[""booksNF""]"
140+
);
141+
}
74142
}
75143
}

‎src/Service.Tests/Snapshots/ConfigurationTests.TestReadingRuntimeConfigForMsSql.verified.txt‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3442,6 +3442,19 @@
34423442
Action: Read
34433443
}
34443444
]
3445+
},
3446+
{
3447+
Role: TestFieldExcludedForAggregation,
3448+
Actions: [
3449+
{
3450+
Action: Read,
3451+
Fields: {
3452+
Exclude: [
3453+
publisher_id
3454+
]
3455+
}
3456+
}
3457+
]
34453458
}
34463459
],
34473460
Mappings: {

0 commit comments

Comments
 (0)