RequireHttps - should it be redirect 302 by default?

[mikes-gh]

RequireHttps uses a 301 permanent redirect.

Mvc/src/Microsoft.AspNetCore.Mvc.Core/RequireHttpsAttribute.cs

Line 87 in ee2cfa1

filterContext.Result = new RedirectResult(newUrl, permanent: true);

It used to be 302 in MVC 5 AFAIK so this adds to the confusion.
All the major browsers cache 301 indefinitely so if you use RequireHttps in your code you can never remove it.
Otherwise you will have code that may not match the behaviour of your site depending on the state of the users browser cache. This can cause some confusion without an in depth knowledge of browser behaviour and 301 vs. 302.

I realise I could override the attribute to say something like RequireHttpsNotPermanent but given the one way street that is 301 I propose making RequireHttps 302 (like it used to be) by default. Make 301 an opt in so you are aware of the consequences.

Perhaps with an overridden RequireHttpsPermanent or RequireHttps("Permanent") attribute for 301.

Discussed at length here
aspnet/Security#798

[Eilon]

@blowdart any thoughts on this? It's not directly security related but I figured you might have some thoughts.

[blowdart]

Make it configurable.

[Eilon]

301 by default?

[blowdart]

302 by default.

[mikes-gh]

I think 302 can make it for RC2.
Anyone coming from MVC5 will be unaware they are disabling their http version of their page/domain for ever. Thats not something you can fix later.

Its a minimal one line change and fixes this bug and is a candidate for ask mode given the consequences on peoples paid for domains.

Mvc/src/Microsoft.AspNetCore.Mvc.Core/RequireHttpsAttribute.cs

Line 87 in ee2cfa1

filterContext.Result = new RedirectResult(newUrl, permanent: true);

changed to
filterContext.Result = new RedirectResult(newUrl);

Leave the up for grabs for the enhancement.

[Eilon]

@mikes-gh good point, I logged #4578 to track changing it to a 302. I'll move this bug to Backlog as it's not critical for the release.

[Eilon]

@sebastienros we have a PR for this at #4602 that just needs to be reviewed and merged.