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

HSTS and UseHttpsRedirection #5902

Merged
merged 9 commits into from
Apr 13, 2018
Merged

HSTS and UseHttpsRedirection #5902

merged 9 commits into from
Apr 13, 2018

Conversation

Rick-Anderson
Copy link
Contributor

@Rick-Anderson Rick-Anderson commented Apr 7, 2018

Fixes #4568
Fixes Option to opt-out of HTTPS #5960

Review URL

@Rick-Anderson Rick-Anderson changed the title WIP: HSTS and UseHttpsRedirection HSTS and UseHttpsRedirection Apr 10, 2018
@Rick-Anderson Rick-Anderson requested a review from jkotalik April 10, 2018 00:08

[!code-csharp[sample](enforcing-ssl/sample/Startup.cs?name=snippet1&highlight=10)]

`UseHsts` not recommend in development because calling `UseHsts` excludes the local loopback address.
Copy link
Contributor

@jkotalik jkotalik Apr 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"UseHsts not recommend in development because the HSTS header is highly cachable by browsers. By default, UseHsts excludes the local loopback address."


* `localhost` : The IPv4 loopback address.
* `127.0.0.1` : The IPv4 loopback address.
* `[::1]` : The IPv6 loopback address.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add that you also may specify other excluded hosts on the HstsOptions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No setter
See source

Copy link
Contributor

@jkotalik jkotalik Apr 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can still call options.ExcludedHosts.Add(...) to add onto the list.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some reason I can't get the version that has ExcludedHosts
I'm using <PackageReference Include="Microsoft.AspNetCore.App" Version="2.1.0-preview2-final" />

image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is part of the HstsOptions, not the HttpsRedirectionOptions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you send me the code to do that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never mind, wrong place.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

 services.AddHsts(options =>
 {
     options.Preload = true;
     options.IncludeSubDomains = true;
     options.MaxAge = TimeSpan.FromDays(60);
     options.ExcludedHosts.Add("example.com");
     options.ExcludedHosts.Add("www.example.com");
 });

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah looks good!

* `127.0.0.1` : The IPv4 loopback address.
* `[::1]` : The IPv6 loopback address.


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Link to sample here?

@jkotalik
Copy link
Contributor

Looks good to me doc wise. I'll spin up the sample once I get the chance to and make sure that works.

## Require HTTPS

::: moniker range=">= aspnetcore-2.1"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like you're missing the ::: moniker-end for this block.

@Rick-Anderson Rick-Anderson merged commit 15eae12 into master Apr 13, 2018
@Rick-Anderson Rick-Anderson deleted the HSTS/ra branch April 13, 2018 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants