Skip to content

Commit fe85e6a

Browse files
tobias-tenglertobias-tengler
authored
Properly produce authorization errors for items returned by the nodes field (#8435)
1 parent e0dc913 commit fe85e6a

File tree

3 files changed

+148
-32
lines changed

3 files changed

+148
-32
lines changed

‎src/HotChocolate/Core/src/Types/Types/Relay/NodeFieldResolvers.cs

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ public static async ValueTask ResolveManyNodeAsync(
7474
}
7575

7676
var tasks = ArrayPool<Task<object?>>.Shared.Rent(list.Items.Count);
77-
var result = new object?[list.Items.Count];
77+
var results = new object?[list.Items.Count];
7878
var ct = context.RequestAborted;
7979

8080
for (var i = 0; i < list.Items.Count; i++)
@@ -116,34 +116,42 @@ public static async ValueTask ResolveManyNodeAsync(
116116
{
117117
if (task.Exception is null)
118118
{
119-
result[i] = task.Result;
119+
if (task.Result is IError error)
120+
{
121+
results[i] = null;
122+
context.ReportError(error.WithPath(context.Path.Append(i)));
123+
}
124+
else
125+
{
126+
results[i] = task.Result;
127+
}
120128
}
121129
else
122130
{
123-
result[i] = null;
131+
results[i] = null;
124132
ReportError(context, i, task.Exception);
125133
}
126134
}
127135
else
128136
{
129137
try
130138
{
131-
result[i] = await task;
139+
results[i] = await task;
132140
}
133141
catch (Exception ex)
134142
{
135-
result[i] = null;
143+
results[i] = null;
136144
ReportError(context, i, ex);
137145
}
138146
}
139147
}
140148

141-
context.Result = result;
149+
context.Result = results;
142150
ArrayPool<Task<object?>>.Shared.Return(tasks, true);
143151
}
144152
else
145153
{
146-
var result = new object?[1];
154+
var results = new object?[1];
147155
var nodeId = context.ArgumentLiteral<StringValueNode>(Ids);
148156
var deserializedId = serializer.Parse(nodeId.Value, Unsafe.As<Schema>(context.Schema));
149157
var typeName = deserializedId.TypeName;
@@ -157,11 +165,21 @@ public static async ValueTask ResolveManyNodeAsync(
157165
SetLocalContext(nodeContext, nodeId, deserializedId, type);
158166
TryReplaceArguments(nodeContext, feature.NodeResolver, Ids, nodeId);
159167

160-
result[0] = await ExecutePipelineAsync(nodeContext, feature.NodeResolver);
168+
var result = await ExecutePipelineAsync(nodeContext, feature.NodeResolver);
169+
170+
if (result is IError error)
171+
{
172+
results[0] = null;
173+
context.ReportError(error.WithPath(context.Path.Append(0)));
174+
}
175+
else
176+
{
177+
results[0] = result;
178+
}
161179
}
162180
else
163181
{
164-
result[0] = null;
182+
results[0] = null;
165183

166184
context.ReportError(
167185
ErrorHelper.Relay_NoNodeResolver(
@@ -170,7 +188,7 @@ public static async ValueTask ResolveManyNodeAsync(
170188
context.Selection.SyntaxNodes));
171189
}
172190

173-
context.Result = result;
191+
context.Result = results;
174192
}
175193
return;
176194

‎src/HotChocolate/Core/test/Authorization.Tests/AnnotationBasedAuthorizationTests.cs

Lines changed: 112 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -795,6 +795,79 @@ public async Task Authorize_Nodes_Field()
795795
Assert.Equal(401, value);
796796
}
797797

798+
[Fact]
799+
public async Task Authorize_Nodes_Field_Different_Ids_BeforeResolver()
800+
{
801+
// arrange
802+
var handler = new AuthHandler(
803+
resolver: (_, _) => AuthorizeResult.NotAllowed,
804+
validation: (_, _) => AuthorizeResult.Allowed);
805+
var services = CreateServices(handler);
806+
var executor = await services.GetRequestExecutorAsync();
807+
808+
// act
809+
var result = await executor.ExecuteAsync(builder =>
810+
builder.SetDocument(
811+
"""
812+
query($ids: [ID!]!) {
813+
nodes(ids: $ids) {
814+
__typename
815+
}
816+
}
817+
""")
818+
.SetVariableValues(new Dictionary<string, object?>
819+
{
820+
{
821+
"ids",
822+
new List<string>
823+
{
824+
Convert.ToBase64String("BlogPage:1"u8),
825+
Convert.ToBase64String("Order:1"u8),
826+
Convert.ToBase64String("BlogPage:2"u8)
827+
}
828+
}
829+
}));
830+
831+
// assert
832+
Snapshot
833+
.Create()
834+
.Add(result)
835+
.MatchInline(
836+
"""
837+
{
838+
"errors": [
839+
{
840+
"message": "The current user is not authorized to access this resource.",
841+
"locations": [
842+
{
843+
"line": 2,
844+
"column": 3
845+
}
846+
],
847+
"path": [
848+
"nodes",
849+
1
850+
],
851+
"extensions": {
852+
"code": "AUTH_NOT_AUTHORIZED"
853+
}
854+
}
855+
],
856+
"data": {
857+
"nodes": [
858+
{
859+
"__typename": "BlogPage"
860+
},
861+
null,
862+
{
863+
"__typename": "BlogPage"
864+
}
865+
]
866+
}
867+
}
868+
""");
869+
}
870+
798871
[Fact]
799872
public async Task Skip_Authorize_On_Node_Field()
800873
{
@@ -871,18 +944,17 @@ public async Task Assert_UserState_Exists()
871944
var executor = await services.GetRequestExecutorAsync();
872945

873946
// act
874-
var result = await executor.ExecuteAsync(
875-
builder =>
876-
builder
877-
.SetDocument(
878-
"""
879-
{
880-
nodes(ids: "abc") {
881-
__typename
882-
}
883-
}
884-
""")
885-
.SetUser(new ClaimsPrincipal()));
947+
var result = await executor.ExecuteAsync(builder =>
948+
builder
949+
.SetDocument(
950+
"""
951+
{
952+
nodes(ids: "abc") {
953+
__typename
954+
}
955+
}
956+
""")
957+
.SetUser(new ClaimsPrincipal()));
886958

887959
// assert
888960
Snapshot
@@ -930,16 +1002,15 @@ public async Task Skip_After_Validation_For_Null()
9301002
var executor = await services.GetRequestExecutorAsync();
9311003

9321004
// act
933-
var result = await executor.ExecuteAsync(
934-
builder =>
935-
builder
936-
.SetDocument(
937-
"""
938-
{
939-
null
940-
}
941-
""")
942-
.SetUser(new ClaimsPrincipal()));
1005+
var result = await executor.ExecuteAsync(builder =>
1006+
builder
1007+
.SetDocument(
1008+
"""
1009+
{
1010+
null
1011+
}
1012+
""")
1013+
.SetUser(new ClaimsPrincipal()));
9431014

9441015
// assert
9451016
Snapshot
@@ -965,6 +1036,8 @@ private static IServiceProvider CreateServices(
9651036
.AddType<Street>()
9661037
.AddTypeExtension(typeof(StreetExtensions))
9671038
.AddType<City>()
1039+
.AddType<Order>()
1040+
.AddType<BlogPage>()
9681041
.AddGlobalObjectIdentification()
9691042
.AddAuthorizationHandler(_ => handler)
9701043
.ModifyAuthorizationOptions(configure ?? (_ => { }))
@@ -1014,6 +1087,23 @@ public sealed record City(string? Value) : ICityOrStreet;
10141087
[UnionType]
10151088
public interface ICityOrStreet;
10161089

1090+
[Node]
1091+
[Authorize(ApplyPolicy.BeforeResolver)]
1092+
public sealed record Order(string Id)
1093+
{
1094+
[NodeResolver]
1095+
public static Order GetOrderById(string id)
1096+
=> new(id);
1097+
}
1098+
1099+
[Node]
1100+
public sealed record BlogPage(string Id)
1101+
{
1102+
[NodeResolver]
1103+
public static BlogPage GetBlogPageById(string id)
1104+
=> new(id);
1105+
}
1106+
10171107
[Node]
10181108
[ExtendObjectType<Street>]
10191109
public static class StreetExtensions

‎src/HotChocolate/Core/test/Authorization.Tests/__snapshots__/AnnotationBasedAuthorizationTests.Authorize_Node_Field_Schema.graphql

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,18 @@ interface Node {
77
id: ID!
88
}
99

10+
type BlogPage implements Node {
11+
id: ID!
12+
}
13+
1014
type City @authorize(policy: "READ_CITY", apply: AFTER_RESOLVER) {
1115
value: String
1216
}
1317

18+
type Order implements Node @authorize {
19+
id: ID!
20+
}
21+
1422
type Person implements Node @authorize(policy: "READ_PERSON", apply: AFTER_RESOLVER) {
1523
id: ID!
1624
name: String

0 commit comments

Comments
 (0)