Skip to content

Should AllowedHosts be in the template appsettings.json? #47862

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

Closed
eerhardt opened this issue Apr 24, 2023 · 3 comments
Closed

Should AllowedHosts be in the template appsettings.json? #47862

eerhardt opened this issue Apr 24, 2023 · 3 comments
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions

Comments

@eerhardt
Copy link
Member

By default the appsettings.json file in the templates have "AllowedHosts": "*".

From everything I can tell from the code, this appears to be used for "host filtering":

// Fallback
services.PostConfigure<HostFilteringOptions>(options =>
{
if (options.AllowedHosts == null || options.AllowedHosts.Count == 0)
{
// "AllowedHosts": "localhost;127.0.0.1;[::1]"
var hosts = hostingContext.Configuration["AllowedHosts"]?.Split(new[] { ';' }, StringSplitOptions.RemoveEmptyEntries);
// Fall back to "*" to disable.
options.AllowedHosts = (hosts?.Length > 0 ? hosts : new[] { "*" });
}
});
// Change notification
services.AddSingleton<IOptionsChangeTokenSource<HostFilteringOptions>>(
new ConfigurationChangeTokenSource<HostFilteringOptions>(hostingContext.Configuration));
services.AddTransient<IStartupFilter, HostFilteringStartupFilter>();

public Task Invoke(HttpContext context)
{
var allowedHosts = EnsureConfigured();
if (!CheckHost(context, allowedHosts))
{
return HostValidationFailed(context);
}

However, as you can see above the default value, if not configured, is *. So there might not be value in putting this configuration value in appsettings for all the templates.

We should consider removing this config value from the templates if it doesn't add value.

See the discussion here.

@Tratcher
Copy link
Member

Tratcher commented Apr 25, 2023

cc @blowdart
It was put there for discoverability since it's a security related setting we encourage users to set.

@blowdart
Copy link
Contributor

I said that in a PR :)

It should stay

@amcasey
Copy link
Member

amcasey commented Apr 26, 2023

Any final thoughts, @eerhardt?

@eerhardt eerhardt closed this as not planned Won't fix, can't repro, duplicate, stale Apr 26, 2023
@ghost ghost locked as resolved and limited conversation to collaborators May 26, 2023
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Aug 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions
Projects
None yet
Development

No branches or pull requests

4 participants