Set SameSiteMode for cookies in authentication tests (#25281)

[captainsafia]

Description

Backports the same site fix in #25281 to unblock test failures.

Chrome (as of v80) enforces a strict check on cookie settings. Cookies set with SameSiteMode=None must be marked as secure.

The Identity.Server sets the SameSiteMode to None by default. This PR sets the SameSiteMode to Lax by default since the WASM auth tests do not run on an HTTPS server.

I verified this by confirming that a Register -> Log in -> visit preferences flow works on the Authentication app.

Customer Impact

This PR resolves an issue where tests are failing due to an upgrade in the Chrome version that was used for testing in our application. This has no customer impact but will unblock the builds for other servicing PRs.

Regression?

No.

Risk

Low risk, no customer-facing changes.

[dougbu]

I like this on its own but would appreciate seeing a 🟢 (green if that looks as red to you as it does to me) amalgam of this and #25473. Without that we (a) will waste time cherry-picking from multiple branches when trying to see if we've fixed 3.1 issues and (b) cannot be sure we're ready for the branches to open.

Could one of you please take that action item❔ When the branches open, we can use the amalgam validation build for both this and #25473 as long as they don't need further changes.

As I mentioned in #25283

You'll probably need to … skip some of the Kestrel Interop tests to get that [amalgam] PR working.

I.e. we probably need something like that here or in #25473

[dougbu]

we probably need something like that here or in #25473

Or in a third PR included in the amalgam for an extra 💯 no-op points 😺

[BrennanConroy]

Cherry-picked the other change here

[SteveSandersonMS]

Approving for the Components changes. The SignalR ones look good to me too but would defer to somone more in that area for a confirmation :)

[dougbu]

Added a commit because we really need to get this in tomorrow morning i.e. before opening up PRs for 3.1.9 branding.

[SteveSandersonMS]

Added a commit because we really need to get this in tomorrow morning i.e. before opening up PRs for 3.1.9 branding.

If that commit gets merged into release/3.1, what's tracking fixing it in this branch? Does that change put us at risk of losing the E2E test coverage in the LTS branch forever? That would be a serious loss and a hazard for future patch releases.

What alternatives do we have? I see that the release/3.1 branch has been green for the great majority of the last 30 or so commits, so if this implies the level of unreliability in the infrastructure is relatively low, perhaps it's better just to re-run the tests a few times to get a passing build.

[SteveSandersonMS]

Note: I re-ran CI for the previous commit in this PR (e7cb2dd) and it passed without having to disable the E2E tests.

[BrennanConroy]

I re-ran CI for the previous commit in this PR

I don't think that did what you think it should. If you look at the logs you'll see that it pulls the branch and not the commit. And the test runs didn't include Components.E2ETests.

I see that the release/3.1 branch has been green for the great majority of the last 30 or so commits, so if this implies the level of unreliability in the infrastructure is relatively low

The failure rate is 100% currently, it's a recent issue so you can't look at past runs.

what's tracking fixing it in this branch?

You can mention that on the issue tracking these test issues #25322

[SteveSandersonMS]

I don't think that did what you think it should. If you look at the logs you'll see that it pulls the branch and not the commit. And the test runs didn't include Components.E2ETests.

I see - that is very misleading! Thanks for pointing this out.

The failure rate is 100% currently, it's a recent issue so you can't look at past runs.

Are you saying the defect has been introduced by an external environmental change (the new Chrome version?), and so we should expect a 100% failure rate now? If so this makes sense.

You can mention that on the issue tracking these test issues #25322

I will do so, but in general would hope that anyone making a change to disable any test anywhere would also take responsibility for ensuring we track re-enabling it there.

[dougbu]

in general would hope that anyone making a change to disable any test anywhere would also take responsibility for ensuring we track re-enabling it there.

@JunTaoLuo is working on #25322 in #25330. We'll backport that once it's working.

But, in general, no, the person disabling or quarantining a test is rarely the person responsible for fixing it. As a group, we open, assign, and fix test problems.

[dougbu]

@Pilchie @wtgodbe @mmitche are 3.1 branches open to start working on our next release❔ We need to get this in to unblock PRs and get working on branding.

[SteveSandersonMS]

But, in general, no, the person disabling or quarantining a test is rarely the person responsible for fixing it. As a group, we open, assign, and fix test problems.

To be clear, I said track fixing it (e.g., filing an issue, or updating an existing one). Not actually implementing the fix!

[dougbu]

To be clear, I said track fixing it

Good point, sorry. In this case do we need anything beyond #25322❔

[SteveSandersonMS]

I would expect the existing comment in #25322 will suffice now. If the "backport to release/3.1" part has to be split out into a separate issue later that can be decided at the time. Just wanted to be sure we didn't lose track of this 😄