Log out show error message "... your browser must accept cookies ..."

Created on 22 December 2021, over 2 years ago
Updated 9 April 2024, 3 days ago

Problem/Motivation

Drupal 9.3.0. After logged out I get message and url have attached /?check_logged_in=1

Here patch with helped me

Steps to reproduce

- Install a new site via the Drupal UI (do not use drush to automate the installation).
- Select the 'standard' installation profile in the dropdown.
- Type in the superadmin username, password, th admin email address, database credentials etc into the form(s).
- After you complete the installation you will be logged in as user 1.
- Logout.
- The notice in the screenshot appears.

Logging in and out with supeuser, uid 1, doesn't make the notice appear again, nor does creating a new user and logging in and out with that.

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

๐Ÿ› Bug report
Status

Needs work

Version

11.0 ๐Ÿ”ฅ

Component
User moduleย  โ†’

Last updated about 5 hours ago

Created by

๐Ÿ‡บ๐Ÿ‡ฆUkraine cosolom

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.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

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

  • ๐Ÿ‡จ๐Ÿ‡ดColombia jedihe

    Patch #4 works on Drupal 9.5.8. We were having a problem where hitting /user/logout triggers the "To log in to this site..." message, after the user is properly logged out. A closer look into Drupal core showed that Drupal/Core/Session/SessionManager::destroy() calls session_destroy() instead of Session::invalidate() due to unwanted side-effects (details there); due to this, I also checked and found that both the session record and the browser-session-cookie are correctly cleared, which means the patch doesn't cause redundant session rows or bad impacts on caching.

    I also ran some testing in a simplytest.me instance (vanilla 9.5.10-dev) and found:

    • Chrome set to block cookies gets to show the "To log in to this site..." error message after submitting /user/login with valid login credentials.
    • Chrome with cookies allowed: on logout, everything is properly cleared: browser-cookie as well as session record (checked this using views_sessions module).

    Given all of the above, I think #4 is in great shape. Hopefully we can get some tests written, so it can be reviewed for merging.

  • ๐Ÿ‡ง๐Ÿ‡ชBelgium BramDriesen Belgium ๐Ÿ‡ง๐Ÿ‡ช

    Since Drupal 9 will be EOL soon, I think this needs to target D11 and get a backport to D10

  • ๐Ÿ‡ท๐Ÿ‡บRussia batkor Irkutsk

    Thanks, PAtch #4 worked for me.
    Drupal 10.1.1)

  • First commit to issue fork.
  • Merge request !5634Resolve #3255711 "Log out show" โ†’ (Open) created by djsagar
  • Status changed to Needs review 4 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia djsagar
  • Merge request !563511.x โ†’ (Open) created by djsagar
  • Pipeline finished with Failed
    4 months ago
    Total: 182s
    #57729
  • Status changed to Needs work 4 months ago
  • The Needs Review Queue Bot โ†’ tested this issue.

    While you are making the above changes, we recommend that you convert this patch to a merge request โ†’ . Merge requests are preferred over patches. Be sure to hide the old patch files as well. (Converting an issue to a merge request without other contributions to the issue will not receive credit.)

  • Merge request !5663Djsagar 293afe7d patch 2e47 โ†’ (Open) created by djsagar
  • Status changed to Needs review 4 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia djsagar
  • Pipeline finished with Failed
    4 months ago
    Total: 170s
    #58600
  • Status changed to Needs work 4 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    Previously tagged for tests. When changing to review please take a look at the issue summary and tags to make sure itโ€™s ready.

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom andrew.farquharson

    andrew.farquharson โ†’ made their first commit to this issueโ€™s fork.

  • Pipeline finished with Failed
    10 days ago
    Total: 160s
    #135737
  • Pipeline finished with Failed
    10 days ago
    Total: 160s
    #135788
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom andrew.farquharson

    Merge request !5634 has patch #4 applied.

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom andrew.farquharson
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom andrew.farquharson

    The proposed fix at #18 seems the best solution. It fixes the original issue as strictly described. I have tested site instalaltion using drush site:install (Drupal 11.x) command. The bug does not occur. I believe that fix #18 since it targets the file that controls site installation done via the Drupal UI. Fix at #4 is far more general in its action and could have side-effects.

    @smustgrave I am not sure how to test the installation via the Drupal UI? Is it possible to create a test that tests the equivalent of a Drupal UI installation? If a test is not feasible, it is simple to re-create the issue using the summary and testing manually.

    There is a more general issue, possibly, as described at #22. But that should be dealt with as a new issue.

  • Pipeline finished with Canceled
    3 days ago
    Total: 813s
    #142030
  • Pipeline finished with Canceled
    3 days ago
    Total: 142s
    #142048
  • Pipeline finished with Failed
    3 days ago
    Total: 992s
    #142049
  • Pipeline finished with Success
    3 days ago
    #142075
  • Status changed to Needs review 3 days ago
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom andrew.farquharson

    I have applied the one liner fix at #18. I have not added any test because I do not know how a fresh installation using the UI is possible in, say, a kernel test. Unless someone knows how to create such a test, please review and test the fix manually.

    To see how the issue is isolated to installations via the UI, try a site install using drush and note that the issue does not arise.

  • Status changed to Needs work 3 days ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    Still putting in NW as tests will be needed.

    But does anyone have a suggestion for #41

    Know can do a fresh install using functional tests.

  • Pipeline finished with Failed
    1 day ago
    Total: 1079s
    #144205
Production build https://api.contrib.social 0.62.1