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

Avoid antiforgery issues #13

Merged
merged 3 commits into from
Mar 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 13 additions & 0 deletions src/SmartComponents.AspNetCore/DefaultSmartComponentsBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,17 @@ public ISmartComponentsBuilder WithInferenceBackend<T>(string? name) where T : c

return this;
}

public ISmartComponentsBuilder WithAntiforgeryValidation()
{
services.AddSingleton<SmartComponentsAntiforgeryValidation>();
return this;
}

internal static bool HasEnabledAntiForgeryValidation(IServiceProvider services)
{
return services.GetService<SmartComponentsAntiforgeryValidation>() is not null;
}

internal sealed class SmartComponentsAntiforgeryValidation { }
}
4 changes: 3 additions & 1 deletion src/SmartComponents.AspNetCore/ISmartComponentsBuilder.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Licensed to the .NET Foundation under one or more agreements.
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using SmartComponents.StaticAssets.Inference;
Expand All @@ -8,4 +8,6 @@ namespace Microsoft.AspNetCore.Builder;
public interface ISmartComponentsBuilder
{
public ISmartComponentsBuilder WithInferenceBackend<T>(string? name = null) where T : class, IInferenceBackend;

public ISmartComponentsBuilder WithAntiforgeryValidation();
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,16 @@ public static IEndpointRouteBuilder MapSmartComboBox(this IEndpointRouteBuilder

private static IEndpointRouteBuilder MapSmartComboBoxCore(this IEndpointRouteBuilder builder, string url, Func<SmartComboBoxRequest, Task<IEnumerable<string>>> suggestions)
{
var validateAntiforgery = DefaultSmartComponentsBuilder.HasEnabledAntiForgeryValidation(builder.ServiceProvider);

var endpoint = builder.MapPost(url, async (HttpContext httpContext,
[FromServices] IAntiforgery antiforgery) =>
{
#if NET8_0_OR_GREATER
// We use DisableAntiforgery and validate manually so that it works whether
// or not you have UseAntiforgery middleware in the pipeline. Without doing that,
// people will get errors like https://stackoverflow.com/questions/61829324
//
// On .NET 6, we can't enforce antiforgery at all because there's no way for Blazor
// Server or WebAssembly to get/set a token arbitrarily during interactive rendering
// (e.g., may have to set a cookie). This is not really a problem since these endpoints
// don't mutate any state anyway so the protection is not really required - we only do
// it on .NET 8+ because there's no reason not to.
await antiforgery.ValidateRequestAsync(httpContext);
#endif
// See comment in SmartComponentsServiceCollectionExtensions to explain the antiforgery handling
if (validateAntiforgery)
{
await antiforgery.ValidateRequestAsync(httpContext);
}

// Can't use [FromForm] on net6.0
var form = httpContext.Request.Form;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,16 +36,21 @@ public Action<IApplicationBuilder> Configure(Action<IApplicationBuilder> next) =
{
next(builder);

var validateAntiforgery = DefaultSmartComponentsBuilder.HasEnabledAntiForgeryValidation(builder.ApplicationServices);

builder.UseEndpoints(app =>
{
var smartPasteEndpoint = app.MapPost("/_smartcomponents/smartpaste", async ([FromServices] IInferenceBackend inference, HttpContext httpContext, [FromServices] IAntiforgery antiforgery, [FromServices] SmartPasteInference smartPasteInference) =>
{
#if NET8_0_OR_GREATER
// We use DisableAntiforgery and validate manually so that it works whether
// or not you have UseAntiforgery middleware in the pipeline. Without doing that,
// people will get errors like https://stackoverflow.com/questions/61829324
await antiforgery.ValidateRequestAsync(httpContext);
#endif
// The rules about whether antiforgery are enabled by default vary across different
// ASP.NET Core versions. To make it consistent, we disable the default enablement on
// .NET 8, and manually validate it if you've opted in.
// Also note that antiforgery handling has issues (https://github.com/dotnet/aspnetcore/issues/54533)
// so until that's resolved we need this to be off by default.
if (validateAntiforgery)
{
await antiforgery.ValidateRequestAsync(httpContext);
}

// Can't use [FromForm] on net6.0
if (!httpContext.Request.Form.TryGetValue("dataJson", out var dataJson))
Expand All @@ -59,10 +64,11 @@ public Action<IApplicationBuilder> Configure(Action<IApplicationBuilder> next) =

var smartTextAreaEndpoint = app.MapPost("/_smartcomponents/smarttextarea", async ([FromServices] IInferenceBackend inference, HttpContext httpContext, [FromServices] IAntiforgery antiforgery, [FromServices] SmartTextAreaInference smartTextAreaInference) =>
{
#if NET8_0_OR_GREATER
// See above for why we validate antiforgery manually
await antiforgery.ValidateRequestAsync(httpContext);
#endif
if (validateAntiforgery)
{
// See above for why we validate antiforgery manually
await antiforgery.ValidateRequestAsync(httpContext);
}

// Can't use [FromForm] on net6.0
var form = httpContext.Request.Form;
Expand All @@ -79,6 +85,7 @@ public Action<IApplicationBuilder> Configure(Action<IApplicationBuilder> next) =
});

#if NET8_0_OR_GREATER
// These APIs only exist on .NET 8+. It wasn't enabled by default in prior versions.
smartPasteEndpoint.DisableAntiforgery();
smartTextAreaEndpoint.DisableAntiforgery();
#endif
Expand Down
18 changes: 12 additions & 6 deletions src/SmartComponents.StaticAssets/typescript/SmartComboBox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,18 +70,24 @@ class SmartComboBox extends HTMLElement {
async _requestSuggestions() {
this.currentAbortController = new AbortController();

const body = {
inputValue: this.inputElem.value,
maxResults: this.getAttribute('data-max-suggestions'),
similarityThreshold: this.getAttribute('data-similarity-threshold'),
};

const antiforgeryName = this.getAttribute('data-antiforgery-name');
if (antiforgeryName) {
body[antiforgeryName] = this.getAttribute('data-antiforgery-value');
}

let response: Response;
const requestInit: RequestInit = {
method: 'post',
headers: {
'content-type': 'application/x-www-form-urlencoded',
},
body: new URLSearchParams({
[this.getAttribute('data-antiforgery-name')]: this.getAttribute('data-antiforgery-value'),
inputValue: this.inputElem.value,
maxResults: this.getAttribute('data-max-suggestions'),
similarityThreshold: this.getAttribute('data-similarity-threshold'),
}),
body: new URLSearchParams(body),
signal: this.currentAbortController.signal,
};

Expand Down
20 changes: 13 additions & 7 deletions src/SmartComponents.StaticAssets/typescript/SmartPaste.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,18 @@ function inferFieldDescription(form: HTMLFormElement, element: HTMLElement): str
async function getSmartPasteResponse(button: HTMLButtonElement, formConfig, clipboardContents): Promise<Response> {
const formFields = formConfig.map(entry => restrictProperties(entry, ['identifier', 'description', 'allowedValues', 'type']));

const body = {
dataJson: JSON.stringify({
formFields,
clipboardContents,
})
};

const antiforgeryName = button.getAttribute('data-antiforgery-name');
if (antiforgeryName) {
body[antiforgeryName] = button.getAttribute('data-antiforgery-value');
}

// We rely on the URL being pathbase-relative for Blazor, or a ~/... URL that would already
// be resolved on the server for MVC
const url = button.getAttribute('data-url');
Expand All @@ -223,13 +235,7 @@ async function getSmartPasteResponse(button: HTMLButtonElement, formConfig, clip
headers: {
'content-type': 'application/x-www-form-urlencoded',
},
body: new URLSearchParams({
[button.getAttribute('data-antiforgery-name')]: button.getAttribute('data-antiforgery-value'),
dataJson: JSON.stringify({
formFields,
clipboardContents,
})
})
body: new URLSearchParams(body)
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,19 +117,24 @@ export class SmartTextArea extends HTMLElement {
cursorPosition: this.textArea.selectionStart,
};

const body = {
// TODO: Limit the amount of text we send, e.g., to 100 characters before and after the cursor
textBefore: snapshot.textAreaValue.substring(0, snapshot.cursorPosition),
textAfter: snapshot.textAreaValue.substring(snapshot.cursorPosition),
config: this.getAttribute('data-config'),
};

const antiforgeryName = this.getAttribute('data-antiforgery-name');
if (antiforgeryName) {
body[antiforgeryName] = this.getAttribute('data-antiforgery-value');
}

const requestInit: RequestInit = {
method: 'post',
headers: {
'content-type': 'application/x-www-form-urlencoded',
},
body: new URLSearchParams({
[this.getAttribute('data-antiforgery-name')]: this.getAttribute('data-antiforgery-value'),

// TODO: Limit the amount of text we send, e.g., to 100 characters before and after the cursor
textBefore: snapshot.textAreaValue.substring(0, snapshot.cursorPosition),
textAfter: snapshot.textAreaValue.substring(snapshot.cursorPosition),
config: this.getAttribute('data-config'),
}),
body: new URLSearchParams(body),
signal: snapshot.abortSignal,
};

Expand Down
3 changes: 2 additions & 1 deletion test/testassets/TestBlazorApp/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ private static void Main(string[] args)
.AddInteractiveWebAssemblyComponents();
builder.Services.AddScoped<SmartPasteInference, SmartPasteInferenceForTests>();
builder.Services.AddSmartComponents()
.WithInferenceBackend<OpenAIInferenceBackend>();
.WithInferenceBackend<OpenAIInferenceBackend>()
.WithAntiforgeryValidation(); // This doesn't benefit most apps, but we'll validate it works in E2E tests

var app = builder.Build();

Expand Down
3 changes: 2 additions & 1 deletion test/testassets/TestMvcApp/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ private static void Main(string[] args)
builder.Services.AddControllersWithViews();
builder.Services.AddScoped<SmartPasteInference, SmartPasteInferenceForTests>();
builder.Services.AddSmartComponents()
.WithInferenceBackend<OpenAIInferenceBackend>();
.WithInferenceBackend<OpenAIInferenceBackend>()
.WithAntiforgeryValidation(); // This doesn't benefit most apps, but we'll validate it works in E2E tests

var app = builder.Build();

Expand Down
Loading