Fix #4492: Updates Culture and Visitor cookies to use "Lax" SameSite and Secure Cookie Options

[thabaum]

Fixes #4492

Sets options on cookies for samesite, secure and httponly for Culture and Visitor cookies to lax (default) in order to remove browser warnings.

Visitor cookie adds these options:

                        SameSite = Microsoft.AspNetCore.Http.SameSiteMode.Lax, // Set SameSite attribute
                        Secure = true, // Ensure the cookie is only sent over HTTPS
                        HttpOnly = true // Optional: Helps mitigate XSS attacks

Culture cookie adds these options:

                        Expires = DateTimeOffset.UtcNow.AddYears(1),
                        SameSite = Microsoft.AspNetCore.Http.SameSiteMode.Lax, // Set SameSite attribute
                        Secure = true, // Ensure the cookie is only sent over HTTPS
                        HttpOnly = true // Optional: Helps mitigate XSS attacks

Some added cookie options such as Expires/Secure/HttpOnly may need reviewed/adjusted or maybe this is not the fix but thought I would open up the discussion with this pull request.

Secure over HTTPS only, not sure if this would mess up a HTTP load balanced backend for example. Maybe this would need to be able to be set to false in an environment setting?

I set to lax since this was default and would allow more flexibility however these could be set to strict as well. I am not sure if any issues with third party logins or things of this nature so I left as lax until further discussion. Merge and make adjustments or suggest edits I can apply them to this PR, or close for a better solution/fix.

I set these to SameSite = Microsoft.AspNetCore.Http.SameSiteMode.Lax since that is what they default and what it was using prior to avoid breaking anyone sites so if you have subdomain sites the cookie can be passed between them. Setting this to SameSite = Microsoft.AspNetCore.Http.SameSiteMode.Strict is more secure however I am not sure it would allow having the cookie follow a user between sites with the same top level domain from my understanding. options.Cookie.SameSite = SameSiteMode.Strict; the AntiForgeryToken Cookie uses.

When set to strict on a default installation the site still work for the standalone site. Testing scenarios with sub-domain sites that need to maintain settings between them is what would need to be reviewed more/tested.

We can set this to strict if desired as well I just didn't want to break anything with this PR so left it default=lax for that option and am waiting for feedback relating.

I also added HttpOnly = true // Optional: Helps mitigate XSS attacks to all three cookies in this PR.

And Secure = true, // Ensure the cookie is only sent over HTTPS which may or may not be desired with a load balancer so not sure 100% on this, but for general web app purposes makes sense.

As shown in screenshot this will resolve these issues with warnings in console developer tools:

[sbwalker]

@thabaum changing the HttpOnly option in Startup actually breaks the authentication system, so we cannot make this change.

[thabaum]

@sbwalker I have removed that change in the startup.

[sbwalker]

@thabaum setting the Expires property on the Localization cookie changes it from a session cookie to a permanent cookie (with a lifetime of 1 year) - was this intentional?

[sbwalker]

@thabaum I was mistaken about the AntiForgeryToken cookie - it can be set to HttpOnly - so please go ahead and add this back to the PR.

https://learn.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.antiforgery.antiforgeryoptions.cookie?view=aspnetcore-8.0#remarks

[thabaum]

@sbwalker I had just thought it would be something that does not need to expire and should expire eventually.

I really put all these with comments to show what we can do and after review make changes as desired. I can re-add the HttpOnly back to the startup cookie, let me know if you like me to remove the expire setting. I also figure if you revisit the site the language will stay the same?!

[thabaum]

@sbwalker the update to startup.cs has been applied to use options.Cookie.HttpOnly = true; in the AntiForgery cookie builder. Thank you for the further detailed review. I will await feedback on the expire option. I assume "Lax" is OK for these cookies as well instead of "Strict" which is what it defaults to currently.

I can apply/remove whatever settings you feel fits the Oqtane Framework best once you have time to review. Thanks again!

[sbwalker]

@thabaum I am not sure why we would want to change the SameSite security for these cookies? Lax basically means you are willing to share the value of these cookies on cross-site requests - whereas Strict indicates you do not. If the default is Strict currently, it would make sense to preserve this level of security.

EDIT: my apologies - the current default is None and you are suggesting it should be set to Lax as that is that standard browser default when not specified - correct?

[thabaum]

@sbwalker correct, it is defaulting to "Lax" or will in the future, it is currently none.

I am trying to understand these better so this is a chance to review. The question was should we use "Strict" or "Lax"? I set to "Lax" in the PR due to it being default treated as lax. I am ok with setting this to strict however sites that authenticate as subdomains that may have normally kept a user logged in may not anymore if set to strict from my understand. Lax will be domain and subdomain friendly but not allow third party. I may not be 100% correct here I could be off.

So default treated is "Lax" for cookies that have nothing set to be clear and currently nothing is set.