Skip to content

Commit df411fe

Browse files
erlendellefsenerlendellefsen
authored
feat: validate filter paths against AllowedIncludes (#65)
- Add ValidateFilterPaths to IncludeValidator for filter path validation - Update AllowedIncludesAttribute to validate filter relationship paths - Filters like filter[admin.password] now blocked if admin not in AllowedIncludes - Reduces log verbosity for security (logs request ID instead of schema)
1 parent eb6e886 commit df411fe

5 files changed

Lines changed: 778 additions & 35 deletions

File tree

‎JsonApiToolkit.Tests/Attributes/AllowedIncludesFilterPathTests.cs‎

Lines changed: 327 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,327 @@
1+
using JsonApiToolkit.Attributes;
2+
using JsonApiToolkit.Models.Errors;
3+
using Microsoft.AspNetCore.Http;
4+
using Microsoft.AspNetCore.Mvc;
5+
using Microsoft.AspNetCore.Mvc.Abstractions;
6+
using Microsoft.AspNetCore.Mvc.Filters;
7+
using Microsoft.AspNetCore.Routing;
8+
using Microsoft.Extensions.DependencyInjection;
9+
using Microsoft.Extensions.Logging;
10+
using Microsoft.Extensions.Primitives;
11+
using Moq;
12+
13+
namespace JsonApiToolkit.Tests.Attributes;
14+
15+
public class AllowedIncludesFilterPathTests
16+
{
17+
private ActionExecutingContext CreateContext(
18+
Dictionary<string, StringValues>? queryParams = null,
19+
string? actionName = null
20+
)
21+
{
22+
var httpContext = new DefaultHttpContext();
23+
var services = new ServiceCollection();
24+
services.AddSingleton<ILogger<AllowedIncludesAttribute>>(
25+
Mock.Of<ILogger<AllowedIncludesAttribute>>()
26+
);
27+
httpContext.RequestServices = services.BuildServiceProvider();
28+
29+
if (queryParams != null && queryParams.Count > 0)
30+
{
31+
httpContext.Request.Query = new QueryCollection(queryParams);
32+
}
33+
34+
var actionContext = new ActionContext(
35+
httpContext,
36+
new RouteData(),
37+
new ActionDescriptor { DisplayName = actionName ?? "TestAction" }
38+
);
39+
40+
return new ActionExecutingContext(
41+
actionContext,
42+
new List<IFilterMetadata>(),
43+
new Dictionary<string, object?>(),
44+
controller: new object()
45+
);
46+
}
47+
48+
// ─────────────────────────────────────────────────────────────────────────
49+
// Filter path validation - basic cases
50+
// ─────────────────────────────────────────────────────────────────────────
51+
52+
[Fact]
53+
public void OnActionExecuting_FilterOnAllowedRelationship_AllowsRequest()
54+
{
55+
var attribute = new AllowedIncludesAttribute("author", "posts");
56+
var context = CreateContext(
57+
new Dictionary<string, StringValues> { ["filter[author.name][eq]"] = "John" }
58+
);
59+
60+
attribute.OnActionExecuting(context);
61+
62+
Assert.Null(context.Result);
63+
}
64+
65+
[Fact]
66+
public void OnActionExecuting_FilterOnForbiddenRelationship_ReturnsForbidden()
67+
{
68+
var attribute = new AllowedIncludesAttribute("author");
69+
var context = CreateContext(
70+
new Dictionary<string, StringValues> { ["filter[admin.password][like]"] = "%" }
71+
);
72+
73+
attribute.OnActionExecuting(context);
74+
75+
Assert.NotNull(context.Result);
76+
var objectResult = Assert.IsType<ObjectResult>(context.Result);
77+
Assert.Equal(403, objectResult.StatusCode);
78+
79+
var errorResponse = Assert.IsType<JsonApiErrorResponse>(objectResult.Value);
80+
Assert.Single(errorResponse.Errors);
81+
Assert.Equal("403", errorResponse.Errors[0].Status);
82+
Assert.Equal(JsonApiErrorCodes.FilterNotAllowed, errorResponse.Errors[0].Code);
83+
Assert.Contains("admin", errorResponse.Errors[0].Detail);
84+
}
85+
86+
[Fact]
87+
public void OnActionExecuting_SimpleFilterNoRelationship_AllowsRequest()
88+
{
89+
// Filters without dots (e.g., filter[name]) should always be allowed
90+
var attribute = new AllowedIncludesAttribute("author");
91+
var context = CreateContext(
92+
new Dictionary<string, StringValues> { ["filter[name][eq]"] = "test" }
93+
);
94+
95+
attribute.OnActionExecuting(context);
96+
97+
Assert.Null(context.Result);
98+
}
99+
100+
// ─────────────────────────────────────────────────────────────────────────
101+
// Security vulnerability test cases
102+
// ─────────────────────────────────────────────────────────────────────────
103+
104+
[Fact]
105+
public void OnActionExecuting_FilterOnSensitiveRelationship_Blocked()
106+
{
107+
// This is the security hole we're fixing:
108+
// filter[admin.password][like]=% should be blocked when admin is not allowed
109+
var attribute = new AllowedIncludesAttribute("profile", "posts");
110+
var context = CreateContext(
111+
new Dictionary<string, StringValues>
112+
{
113+
["filter[admin.secretKey][like]"] = "%",
114+
["include"] = "profile", // Valid include
115+
}
116+
);
117+
118+
attribute.OnActionExecuting(context);
119+
120+
Assert.NotNull(context.Result);
121+
var objectResult = Assert.IsType<ObjectResult>(context.Result);
122+
Assert.Equal(403, objectResult.StatusCode);
123+
124+
var errorResponse = Assert.IsType<JsonApiErrorResponse>(objectResult.Value);
125+
Assert.Contains("admin", errorResponse.Errors[0].Detail);
126+
}
127+
128+
[Fact]
129+
public void OnActionExecuting_FilterOnNestedForbiddenPath_Blocked()
130+
{
131+
// filter[author.admin.password] should check that author.admin is allowed
132+
var attribute = new AllowedIncludesAttribute("author");
133+
var context = CreateContext(
134+
new Dictionary<string, StringValues>
135+
{
136+
["filter[author.admin.password][eq]"] = "secret",
137+
}
138+
);
139+
140+
attribute.OnActionExecuting(context);
141+
142+
Assert.NotNull(context.Result);
143+
var objectResult = Assert.IsType<ObjectResult>(context.Result);
144+
Assert.Equal(403, objectResult.StatusCode);
145+
}
146+
147+
// ─────────────────────────────────────────────────────────────────────────
148+
// Wildcard pattern tests
149+
// ─────────────────────────────────────────────────────────────────────────
150+
151+
[Fact]
152+
public void OnActionExecuting_FilterMatchingWildcard_AllowsRequest()
153+
{
154+
var attribute = new AllowedIncludesAttribute("author.*");
155+
var context = CreateContext(
156+
new Dictionary<string, StringValues> { ["filter[author.posts.title][eq]"] = "test" }
157+
);
158+
159+
attribute.OnActionExecuting(context);
160+
161+
Assert.Null(context.Result);
162+
}
163+
164+
[Fact]
165+
public void OnActionExecuting_FilterNotMatchingWildcard_Blocked()
166+
{
167+
var attribute = new AllowedIncludesAttribute("author.*");
168+
var context = CreateContext(
169+
new Dictionary<string, StringValues>
170+
{
171+
["filter[comments.author.name][eq]"] = "John", // comments not allowed
172+
}
173+
);
174+
175+
attribute.OnActionExecuting(context);
176+
177+
Assert.NotNull(context.Result);
178+
var objectResult = Assert.IsType<ObjectResult>(context.Result);
179+
Assert.Equal(403, objectResult.StatusCode);
180+
}
181+
182+
// ─────────────────────────────────────────────────────────────────────────
183+
// Combined include and filter validation
184+
// ─────────────────────────────────────────────────────────────────────────
185+
186+
[Fact]
187+
public void OnActionExecuting_ValidIncludeInvalidFilter_ReturnsForbidden()
188+
{
189+
var attribute = new AllowedIncludesAttribute("author", "posts");
190+
var context = CreateContext(
191+
new Dictionary<string, StringValues>
192+
{
193+
["include"] = "author",
194+
["filter[admin.password][eq]"] = "secret",
195+
}
196+
);
197+
198+
attribute.OnActionExecuting(context);
199+
200+
Assert.NotNull(context.Result);
201+
var objectResult = Assert.IsType<ObjectResult>(context.Result);
202+
Assert.Equal(403, objectResult.StatusCode);
203+
204+
var errorResponse = Assert.IsType<JsonApiErrorResponse>(objectResult.Value);
205+
// Should be filter error, not include error
206+
Assert.Equal(JsonApiErrorCodes.FilterNotAllowed, errorResponse.Errors[0].Code);
207+
}
208+
209+
[Fact]
210+
public void OnActionExecuting_InvalidIncludeValidFilter_ReturnsForbiddenInclude()
211+
{
212+
var attribute = new AllowedIncludesAttribute("author", "posts");
213+
var context = CreateContext(
214+
new Dictionary<string, StringValues>
215+
{
216+
["include"] = "admin", // Invalid
217+
["filter[author.name][eq]"] = "John", // Valid
218+
}
219+
);
220+
221+
attribute.OnActionExecuting(context);
222+
223+
Assert.NotNull(context.Result);
224+
var objectResult = Assert.IsType<ObjectResult>(context.Result);
225+
Assert.Equal(403, objectResult.StatusCode);
226+
227+
var errorResponse = Assert.IsType<JsonApiErrorResponse>(objectResult.Value);
228+
// Include validation runs first, so should be include error
229+
Assert.Equal(JsonApiErrorCodes.IncludeNotAllowed, errorResponse.Errors[0].Code);
230+
}
231+
232+
[Fact]
233+
public void OnActionExecuting_BothValidIncludeAndFilter_AllowsRequest()
234+
{
235+
var attribute = new AllowedIncludesAttribute("author", "posts");
236+
var context = CreateContext(
237+
new Dictionary<string, StringValues>
238+
{
239+
["include"] = "author,posts",
240+
["filter[author.name][eq]"] = "John",
241+
["filter[posts.title][like]"] = "%test%",
242+
}
243+
);
244+
245+
attribute.OnActionExecuting(context);
246+
247+
Assert.Null(context.Result);
248+
}
249+
250+
// ─────────────────────────────────────────────────────────────────────────
251+
// Multiple forbidden filters
252+
// ─────────────────────────────────────────────────────────────────────────
253+
254+
[Fact]
255+
public void OnActionExecuting_MultipleInvalidFilters_ReturnsAllInError()
256+
{
257+
var attribute = new AllowedIncludesAttribute("author");
258+
var context = CreateContext(
259+
new Dictionary<string, StringValues>
260+
{
261+
["filter[admin.password][eq]"] = "secret",
262+
["filter[secrets.key][eq]"] = "value",
263+
}
264+
);
265+
266+
attribute.OnActionExecuting(context);
267+
268+
Assert.NotNull(context.Result);
269+
var objectResult = Assert.IsType<ObjectResult>(context.Result);
270+
var errorResponse = Assert.IsType<JsonApiErrorResponse>(objectResult.Value);
271+
272+
var meta = errorResponse.Errors[0].Meta;
273+
Assert.NotNull(meta);
274+
275+
var forbiddenPaths = meta["forbiddenFilterPaths"] as List<string>;
276+
Assert.NotNull(forbiddenPaths);
277+
Assert.Equal(2, forbiddenPaths.Count);
278+
Assert.Contains("admin", forbiddenPaths);
279+
Assert.Contains("secrets", forbiddenPaths);
280+
}
281+
282+
// ─────────────────────────────────────────────────────────────────────────
283+
// Edge cases
284+
// ─────────────────────────────────────────────────────────────────────────
285+
286+
[Fact]
287+
public void OnActionExecuting_EmptyAllowedIncludes_AllowsSimpleFilters()
288+
{
289+
// Empty allowed includes means no relationships allowed
290+
// But simple filters (no dot) should still work
291+
var attribute = new AllowedIncludesAttribute();
292+
var context = CreateContext(
293+
new Dictionary<string, StringValues> { ["filter[name][eq]"] = "test" }
294+
);
295+
296+
attribute.OnActionExecuting(context);
297+
298+
// No includes requested, simple filter allowed
299+
Assert.Null(context.Result);
300+
}
301+
302+
[Fact]
303+
public void OnActionExecuting_NoFilters_AllowsRequest()
304+
{
305+
var attribute = new AllowedIncludesAttribute("author");
306+
var context = CreateContext(
307+
new Dictionary<string, StringValues> { ["sort"] = "-createdAt" }
308+
);
309+
310+
attribute.OnActionExecuting(context);
311+
312+
Assert.Null(context.Result);
313+
}
314+
315+
[Fact]
316+
public void OnActionExecuting_CaseInsensitiveFilterPath_AllowsRequest()
317+
{
318+
var attribute = new AllowedIncludesAttribute("Author");
319+
var context = CreateContext(
320+
new Dictionary<string, StringValues> { ["filter[author.Name][eq]"] = "John" }
321+
);
322+
323+
attribute.OnActionExecuting(context);
324+
325+
Assert.Null(context.Result);
326+
}
327+
}

0 commit comments

Comments
 (0)