Don't automatically set a cookie domain

Removing change record tag, as we have one of those.

Hiding patches because now there's an MR.

Adding cookie compliance table link to the issue summary.

We still need manual testing across browsers here, added table to track that to the issue summary.

Left a review, but leaving it as needs review so we can get some comments from the security team and some manual testing

Added manual testing steps to the issue summary and pinged in #security-discussions to try and solicit some help

My tests:

URL for all: https://8080-shaal-drupalpod-qhjy155me7p.ws-eu87.gitpod.io

Set up:

CHROME
* SESS... Domain value: .8080-shaal-drupalpod-qhjy155me7p.ws-eu87.gitpod.io

FIREFOX
* SESS... Domain value: .8080-shaal-drupalpod-qhjy155me7p.ws-eu87.gitpod.io

SAFARI
* SESS... Domain value: .8080-shaal-drupalpod-qhjy155me7p.ws-eu87.gitpod.io

Re-roll for 9.5.x

Removing credit from #51 as this issue is 10.1.x only

Crediting @fjgarlin for manual testing,

Updated remaining tasks - which is testing in Opera and Edge

The remaining tests:

URL for all: https://8080-shaal-drupalpod-qhjy155me7p.ws-eu87.gitpod.io

EDGE
* SESS... Domain value: .8080-shaal-drupalpod-qhjy155me7p.ws-eu87.gitpod.io

OPERA
* SESS... Domain value: .8080-shaal-drupalpod-qhjy155me7p.ws-eu87.gitpod.io

I think this makes all the listed browsers.

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

The above error seems to be reported against the patch for 9.5, not the MR. Setting back to "Needs review".
Note that I tested the remaining browsers as well, but not sure if further tests or review is needed.

I did some stuff to the MR:

• rebased on to 10.1.x
• Updated mentions to 9.2.0 to 10.1.0
• changed str_pos() to str_contains()

Oh wow MR 173 not even a 4 digit one.

There are some open threads in the MR from @larowlan that should be looked at.

Updating status of manual tests to add the ones done at #53.

MR had a bunch of merge conflicts since 📌 Set SameSite on session cookies Fixed and 🐛 SessionConfiguration::getCookieDomain() return value doc is incorrect. Fixed landed. Manually rebased. Attaching the conflicts.

I'm not sure I resolved the conflicts correctly in core/lib/Drupal/Core/Session/SessionConfiguration.php. Definitely needs another pair of eyes on what I did.

Think the merge conflicts look good!

Only moving to NW as there is a comment in the MR

Should we have a todo for when we plan to phase this out?
Should we emit a deprecation warning here so sites know they need to update their services.yml?

That (I can't tell at least) has been answered

Updated IS to show manual testing in the 5 targeted browsers is done (between #50/#53).

Maybe this isn't the ticket to mention this, but when

SessionConfiguration::getOptions is called we do:
$options['cookie_domain'] = $this->getCookieDomain($request) ?: '';

A few lines after we do:
$options['name'] = $this->getName($request);

=> getName calls getUnprefixedName which in turn depends on $this->options['cookie_domain']:
elseif (isset($this->options['cookie_domain'])) {

but at this point $this->options doesn't have cookie_domain key, since we are using a locally scoped variable $options in the getOptions call.

prudloff → changed the visibility of the branch 9.2.x to hidden.

prudloff → changed the visibility of the branch 11.x to hidden.

prudloff → changed the visibility of the branch 2940879-dont-automatically-set to hidden.