Fix cookie conflicts on shared domains

Created on 5 December 2011, over 12 years ago
Updated 31 August 2023, 10 months ago

When hosting multiple Drupal sites on a shared domain, users are unable to log into more than one site because all the sites use the same session cookie name. The cookie can only contain the session of the last site they log into.

The session cookie prefix should be configurable in settings.php for upstream processing by a reverse proxy, and a database hash should be included in the cookie name so even sites that do not set a custom cookie prefix will not overwrite another site's session cookie. The current prefix is SESS for HTTP connections and SSESS for secure-only cookies. Our fix allows a configurable prefix to be added before the SESS or SSESS prefix.

✨ Feature request
Status

Needs work

Version

11.0 πŸ”₯

Component
BaseΒ  β†’

Last updated 38 minutes ago

Created by

πŸ‡ΊπŸ‡ΈUnited States erikwebb

Live updates comments and jobs are added and updated live.
  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

  • Needs framework manager review

    It is used to alert the framework manager core committer(s) that an issue significantly impacts (or has the potential to impact) multiple subsystems or represents a significant change or addition in architecture or public APIs, and their signoff is needed (see the governance policy draft for more information). If an issue significantly impacts only one subsystem, use Needs subsystem maintainer review instead, and make sure the issue component is set to the correct subsystem.

Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Tagging for framework manager reviews for their thoughts.

    Either way think this will require a test case for the new feature.

    Also MR has failures.

  • last update about 1 year ago
    29,332 pass, 20 fail
  • πŸ‡ΈπŸ‡°Slovakia poker10

    Thanks for working on this @Darren Oh!

    I would say we need to split the patch, because it seems to me that it is combining two separate changes:

    Adding a cookie prefix:

    @@ -66,13 +67,14 @@ public function getOptions(Request $request) {
       protected function getName(Request $request) {
    +    $prefix = Settings::get('session_cookie_prefix', '');
         ...
    -    $prefix = $request->isSecure() ? 'SSESS' : 'SESS';
    +    $prefix .= $request->isSecure() ? 'SSESS' : 'SESS';
    

    Fixing a problem with overwriting site cookies:

    @@ -102,6 +104,10 @@ protected function getUnprefixedName(Request $request) {
    +    // Add hash salt to prevent sites on shared domains with separate databases
    +    // from overwriting each other's cookies.
    +    $session_name .= Settings::get('hash_salt', '');
    

    I think the second change (seems like a bug-fix?) should be in a separate issue. Or the issue title + summary needs to be updated and explained why there are these two changes together. Thanks!

  • last update about 1 year ago
    29,395 pass, 20 fail
  • πŸ‡ΊπŸ‡ΈUnited States Darren Oh Lakeland, Florida
  • πŸ‡ΊπŸ‡ΈUnited States Darren Oh Lakeland, Florida

    poker10, I have updated the issue description. Both changes are addressing the same bug. The configurable prefix was the original solution, but it requires site owners to take action to prevent the bug. I added the database hash as a more reliable solution that does not require action from site owners. I believe both solutions are required because the issue also affects reverse proxies that filter cookies, and the database hash does not help with that.

  • last update 10 months ago
    30,025 pass, 20 fail
  • πŸ‡ΊπŸ‡ΈUnited States Darren Oh Lakeland, Florida
  • Status changed to Needs review 10 months ago
  • πŸ‡ΊπŸ‡ΈUnited States Darren Oh Lakeland, Florida
  • Status changed to Needs work 10 months ago
  • πŸ‡ΊπŸ‡ΈUnited States DamienMcKenna NH, USA

    This could be tested by leaving the prefix blank, logging into the site, changing the prefix, reloading the site and confirming that the session is no longer logged in, then removing the prefix and confirming the original session is logged in again; might be a little fiddly to do, but it'd be simpler than trying to create concurrent installs of core.

    And I suspect the test coverage failures are related to the addition of the hash_salt setting to the session name, maybe this should be optional and it should just use the prefix?

Production build 0.69.0 2024