"To log in to this site, your browser must accept cookies from the domain" error message displayed when user goes back and reload the page

This is possibly a duplicate of 🐛 Log out show error message "... your browser must accept cookies ..." Needs work .

The steps to reproduce sound a lot like what happens with cookies off when you try to login

So yes this might be by design

Was this issue ever solved?

In our situation, we encountered this error after updating the site to version 9.5. We had also installed the Automated Logout module. Based on discussions here, it seemed this module might be the cause:

However, we applied a patch to the Drupal core, and currently, everything appears to be functioning well. I've attached the patch for community review. https://www.drupal.org/forum/support/installing-drupal/2023-02-13/to-log... → .

However, we patched the Drupal core and as of now seems to be working fine. Attaching the patch for community review.

Thanks

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.)

Hello,
I'm encountering the same issue on la site.localhot with Drupal 10.2.6, on Google Chrome, as my third part cookies are configured in the browser as not alloed in the private session only.
The current path is well applied on my local and I can log, but the /admin path is not allowed while my profile is an administrator role.
So please take care: this patch can't solve all the cases.

FYI: I can log on Firefox.

So it's a strange issue, maybe encontered only on google chrome, isn't it ?

Regards

I am facing the same issue with the Drupal 10.3 version so attaching the updated patch.

@sourabhjain

I think this patch file is unnecessary as the exact same code is in the MR already. I do not see any update in the patch file.

Hi @prashant
Thanks for the confirmation.
Actually I was facing an issue in my Drupal 10 code and the MR is related to the 11.x so I have created it for 10.3.x.
I am not sure how we can use the MR code in the composer patches. Sorry for that.

Error in samesite
ini_set('session.cookie_samesite', 'None');

Hi @losewn @sourabhjain
I have added the line below to settings.php. Still, the error is not resolved. Whenever I try to log in to the Drupal site it's showing the below error.

Drupal version is 10.3.0
Error: To log in to this site, your browser must accept cookies from the domain

Code added in settings.php:
ini_set('session.cookie_samesite', 'None');

This bug has also been causing us some grief. I have added a test for the bug using the steps to reproduce outlined in the issue summary.

The bug can even be replicated by simply visiting user/login?check_logged_in=1 as an anonymous user.

Where we have experienced the issue is users being redirected after login to a page that contains the check_logged_in, then for whatever reason they are logged out but they reload the page that contains the query parameter and the confusing and incorrect error message appears.

I have reverted the previous fix in the merge request as it was causing the existing testCookiesNotAccepted() test to fail. If cookies are disabled then the session will never contain the 'uid' so the error message is not displaying when it should.

As far as how to solve the issue, I can't think if a way to do so with the current query parameter approach without doing something hacky like setting the check_logged_in to a timestamp and then checking the time and only displaying the error if it is less than X seconds old.

As an alternative to a query parameter, could we instead in use a kernal.request event listener and check if the request is for the user.login route using the POST method, indicating the user login form is being submitted, and display the error message if the session doesn't exist.

I will work on this more tomorrow and see if that is a viable approach unless I get other feedback in the meantime.

Removed sleep(10) function from the Functional test. The test works the same without it.
With a failing test in place, a code fix is now required.

@tame4tex WRT #19, If there is an existing event listener that some code logic can be added to to fix this issue would be good. Creating a new event listener might be overkill and not sure if it could impact performance.

This seems to be the code that controls the error message in the Cookie.php file:

  public function applies(Request $request) {
    $applies = $this->sessionConfiguration->hasSession($request);
    if (!$applies && $request->query->has('check_logged_in')) {
      $domain = ltrim(ini_get('session.cookie_domain'), '.') ?: $request->getHttpHost();
      $this->messenger->addMessage($this->t('To log in to this site, your browser must accept cookies from the domain %domain.', ['%domain' => $domain]), 'error');
    }

It takes the Request object as a parameter. i.e. the Response event has not been dispatched yet.

At line 118 in Cookie.php you have this:

  /**
   * Adds a query parameter to check successful log in redirect URL.
   *
   * @param \Symfony\Component\HttpKernel\Event\ResponseEvent $event
   *   The Event to process.
   */
  public function addCheckToUrl(ResponseEvent $event) {
    $response = $event->getResponse();
    if ($response instanceof RedirectResponse) {
      if ($event->getRequest()->getSession()->has('check_logged_in')) {
        $event->getRequest()->getSession()->remove('check_logged_in');
........
........

This is an existing kernel event listener that listens to the Response event. The Response event precedes the sending of the response to the user-agent (browser). So manipulation of the response can be done prior to sending the response to the user-agent.

1. A fix should be attempted using the existing event listener addCheckToUrl()
2. If necessary the fix might add conditional logic to public function applies(Request $request)
3. Failing 1. and 2. a new event listender can be added to Cookie.php. It could listen to the Response event, for example.

Ah thanks for removing that @oily, it was cruft that I forgot to remove while I was testing possible solutions. Thanks also for the insight.

Going to dig in now for an hour and see what I can come up with.

This could be useful:
Symfony kernel.response documentation

So this turned out to be a challenging bug to fix!

After reviewing the original issue ( 🐛 Login fails and no warning is issued if cookies are not enabled Fixed ) which added the code in the first place, I noted a few mentions of a double redirect approach to solve the original issue. I decided to attempt to implement that approach but only if the request contains the check_logged_in query parameter and there is no session for the request. So the extra redirects would only ever happen for users who have cookies disabled or happen to directly access a url with the check_logged_in query parameter when they are logged out. So it would rarely be experienced by regular users.

This resulted in adding a new /user/verify-cookies page which does the checking for a cookie added to the response and displays an error message if cookies are disabled or redirects the user to their original request if cookies are enabled.

Open to any feedback on the title and text displayed on this new page. I am also wondering if it should NOT be within the user module and instead in a more generic location given it could be used by other functionality or contrib modules that need to verify cookies are enabled.

I also added this new page to robots.txt as it shouldn't be crawled.

With the new approach I was unable to successfully test when cookies were disabled using the existing Functional UserLoginTest as cookies seem to be enabled after the POST request to the user login form. Therefore I switched it to a Functional Javascript test and modified the chrome options to ensure cookies were disabled.

I have updated the Issue Summary with the proposed resolution.

Ready for review and feedback

I note that all tests in the pipeline are green since the last commits by @tame4tex. Manually running the 'test-only' test gives expected result.

Test coverage is now in place. There is now a test that fails without the fix and passes with the fix applied.

It would be interesting to find out if the fix in this issue fixes the related issue related issue 3255711 🐛 Log out show error message "... your browser must accept cookies ..." Needs work . Could then look at closing that issue along with this one.

tame4tex → changed the visibility of the branch 3397718-to-log-in-10.3.x to hidden.

tame4tex → changed the visibility of the branch 10.3.x to hidden.

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

Smsll comment on MR.

Tagging for framework as comment #3 mentions by design potentially

This predates my tenure as subsystem maintainer so will ping Moshe and see if he has anything to add. Will hold of reviewing any code until we get some clarity.

Okay so Moshe doesn't recall any relevant discussions and yielded to me.

At this point I'm inclined to not add extra code to core for an edge case that is manually triggered in a way where other web apps would also show confusing messages the user might not understand.

Also, it only applies to the single time you are redirected right after login, as that's where check_logged_in is added to the query arguments. Any other tab you had open and is refreshed after logout won't show the error message because those tabs will most definitely not have check_logged_in in their query args.

So going to decline this MR as working as intended. There is no reasonable use case where this is an actual problem, so I don't see why we would add so much code for it.

If throughout the work on your MR you found that some of the existing code is sub-par, we can have that discussion in a separate issue. But after some brief digging it seems to be doing its job rather well.

Catch found the original issue: 🐛 Login fails and no warning is issued if cookies are not enabled Fixed

@kristiaanvandeneynde This is not IMO working as designed. This is a bug. There is a related issue which is a related bug that @catch has already acknowledged is a bug. You state in #32

Also, it only applies to the single time you are redirected right after login, as that's where check_logged_in is added to the query arguments

but so long as the url with the query string remains in the browser history the user is liable to return to it numerous times.

Your review(s) are obviously highly-informed and valuable and have progressed the issue. But there are in fact 2 related issues and I am not sure that you are aware that the fix in this issue has not been tested on those other issues and might fix them too.

This bug and the related bugs can be characterised as 'cosmetic' and so easily dismissed as 'frivolous'. Your review highlights that there is a lot of code in the fix and not proportionate to the result. I for one agree with that and ideally the community would explore whether the fix can be achieved in a less invasive manner and with fewer lines. But Rome wasnt built in a day and the current fix is useful even to show the way we do not want to fix it.

I set this to working as designed because for this particular case it seems like it is. I saw mention on Slack of other issues related to this problem space and those might be actual bugs. I'd appreciate if someone could link them here so we can investigate if this patch helps any of those other issues.

To summarize what the code does now:

1. A login redirect is hi-jacked to add the check_logged_in to the query arguments of the destination URL
2. Any page with check_logged_in in its query args will throw the error from the issue title if no session was found

So as far as I am concerned, no URL should have the check_logged_in query argument other than the one you are redirected to after login. As soon as you start browsing, the query arg goes away. So for this purpose, it's doing exactly what it's supposed to be doing. The downside of this lightweight mechanism is that we cannot know whether someone was login redirected to a page or whether someone manually added the query arg or refreshed a stale browser tab.

So you could design a new system that is more complex and therefore likely more expensive or invasive, but for the issue reported here I would argue that it's absolute overkill. Again, if other issues are linked here where the error is shown when it shouldn't be shown, then we can discuss how we want to move forward.

Tempted to close this issue again but will leave open until the related issues are linked.

Thank you for the review @smustgrave and I will make the fix once there is confirmation this issue is not going to be closed.

Thank you for your comments @kristiaanvandeneynde. While I appreciate the points raised, I agree with @oily and do believe this is a bug, at least for the types of sites we manage.

I can understand how this might not be a significant issue for Drupal sites where most users browse multiple pages after logging in. However, it does create challenges in scenarios where:

1. Most users on the site are authorized (i.e. most of the content is not accessible to anonymous users),
2. A number of those users are NOT browsing to other pages after logging in, and
3. There is a backend process that logs users out (e.g. for security after a period of inactivity)

We have sites where users are logging in to check a report or dashboard and often don't browse away from the page they are redirected to after logged in. You can recreate this scenario by doing the following:

1. On a Drupal test site, visit the login form with a destination redirect ie /user/login?destination=node.
2. Log in and open a separate window to log out, simulating the backend process logging out the user.
3. Return to the original window and reload the page, simulating the user’s attempt to refresh their report.
4. Notice the incorrect error message that is displayed.

This behavior has led to confusion among users and, in our experience, unnecessary client support time spent addressing their concerns. I would appreciate it if the "working as designed" designation could be reconsidered.

As one of the authors of the MR, I reviewed the original issue in detail (as outlined in comment #24) and explored several simpler options, though unfortunately without success. That said, I remain open to any informed suggestions for alternatives that address this use case.

Thank you for taking the time to consider this feedback. I hope we can find a resolution that improves the user experience for affected sites.

+1 #37. Can we move this to 'Needs review'?