"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'?

That said, I remain open to any informed suggestions for alternatives that address this use case.

Swap out the 'user.authentication.cookie' service in your code base with an extension of the Cookie class where applies() does not set the message.

  /**
   * {@inheritdoc}
   */
  public function applies(Request $request) {
    return $this->sessionConfiguration->hasSession($request);;
  }

That's 5 minutes of work and an adequate solution to the problem you're facing. I am still hesitant to overhauling the current system based on the problem space detailed in this issue. It's like swatting a fly with a bazooka.

Hardly anyone will ever run into this problem, let alone as frequently as you do here. You'd have to specifically design your site as a single-page dashboard where you don't strip the query arguments after login for this to even remotely be a re-occurring problem.

Going to move to WAD

In that case there are two related issues. Based on your reasoning they should be closed too. But if we do that then any bug if minor should be closed too.

In that case there are two related issues. Based on your reasoning they should be closed too. But if we do that then any bug if minor should be closed too.

https://www.drupal.org/project/drupal/issues/3255711 🐛 Log out show error message "... your browser must accept cookies ..." Needs work

#39 There's a saying 'the customer's always right'. #37 details user complaints.

This behavior has led to confusion among users and, in our experience, unnecessary client support time spent addressing their concerns

There are many issues lacking any specific customer interraction feedback like #37. I think that should elevate the importance of this issue and the 2x related ones. I dont think the site setup in #37 is unique. Probability is there are other sites like it with customers feeling the same way.

Re #40:
We can judge both of those on their own merit. I have yet to get to those and it may very well be one of them will have a more convincing case. This issue, however, I've explained in #36 thoroughly as WAD.

Reviewing it even further, the entire goal of the current code is to check for the session in a way that we do not need a session to carry information across requests. That is why a query argument was chosen to begin with. Your code still adheres to that but adds two redirects to every login request, even when most sites won't ever run into the problem explained here. That is a performance hit we cannot incur for an edge case.

I've given you a solution that won't take much work and that doesn't even come with the performance hit the MR introduces.

Re #41:
I'd appreciate if you didn't put words in my mouth.

Re #42:
But you're not Drupal core's customer and this is not how things work in open source. If we'd have to give in to every little problem people are having we'd have a gigantically bloated core. There's a reason we have modules instead of putting everything into core.

You have a problem and asked for help. Then you were given a solution that you yourself can easily carry out and will 100% fix your problem. We just cannot add this to core directly because the cost far outweighs the benefit.

That is how open source works, by making tough decisions and telling people no if you have to but making sure your system is flexible enough for them to still be able to alleviate their problem.

@kristiaanvandeneynde RE: #43 First of all, I did not supply the code to fix this. @tame4tex did. I have tried to fix the related issue. I used a code approach (to begin with) supplied by @catch, a one-liner. It did not fix it. Others tried varations on the one-liner. None work.

I do not fully understand the fix you are proposing. The way it works in Open source in general is if we think we can fix it we create the code and run it through the pipeline. Until your fix is through the pipeline and it runs green etc how does anyone know it works? I wish it was all that easy!

You seem to imply that you are dealing with a bunch of numpties who cannot tie their shoelaces?

Ok I'm out. I asked to stop putting words in my mouth and you instantly turn around and do it again.

Want people to stop trying to help you in an open source setting? Your comments are a great example on how to achieve that.

@kristiaanvandeneynde No, please stay! : )

I've reported this to the CWG to deal with. It's clear you won't take no for an answer.

Not sure what setting back to NW is going to do as there's two people responsible for this subsystem and the other one yielded to me to handle it. I've patiently explained my position, why I don't think the MR is a good fit for Drupal and gave an example of a workaround that I'm positive will work.

As far as I am concerned this issue is WAD, but if you want to change the status regardless, be my guest.

#45

Ok I'm out. I asked to stop putting words in my mouth and you instantly turn around and do it again.

I have never put any words in your mouth. You are making baseless allegations. That in itself is annoying.

Your comments are a great example on how to achieve that.

That is an utterly baseless allegation. This together with the abo ve is a targetted attempt to humiliate me on the platform and to make me appear to be a wrongdoer.

I suggest you consult your solicitor. If you are considering continuing this bullying.

Hi folks @larowlan chiming in here as a subsystem maintainer.

Wondering if there's a simpler solution that is some javascript in the redirected page that does this:

const query = new URLSearchParams(window.location.search);
query.delete('check_logged_in');
const queryString = query.toString();
let url = window.location.pathname;
if (queryString.length) {
  url = `${url}?${queryString}`;
}
history.replaceState({}, "", url) 

Then when the user presses back they don't land on the URL with check_logged_in

This is much less maintenance overhead and feels much more reasonable than adding additional redirects every each login

You can test this yourself on https://drupal.org/?check_logged_in=1
Paste the code above into the console.
Then click on any link
Then press back.
It's as though you never visited https://drupal.org/?check_logged_in=1

This feature of the web platform is designed entirely for this problem, so we should leverage it in my book.

I'm changing the category here to a feature request. The check_logged_in behaviour is working the way it was intended.

I think we can improve it with the above approach and keep everyone happy.

@larowlan That definitely looks promising. However have worked quite a lot on the related issue, I believe @tame4tex's is more likely to fix it too. If this is closed I have no problem with opening a feature request so long as the code fix she created can be accessed. Danger with closed works as intended is people abandon it not knowing it contains a working solution. I do believe I tested her fix and it worked. Postponed might be better.

The issue is not only when a user goes back to a previous page. On our site it happens like this:

A user on foo.com/bar logs into the site and is redirected to www.foo.com/bar?check_logged_in=1
Now they copy their URL to share with someone else.
Error is displayed to anonymous user.

This even happens to me while working on the site- I log in and copy the URL, then open an incognito window to compare the authenticated page to anonymous page- error displays on anonymous page.

We could just remove the error message considering that it is seems more likely to get the message in error than to get it in a useful way. If your browser doesn't accept cookies, you will probably figure out sooner or later that you really can't log into anything- not sure we need to take responsibility for explaining that one.

Apologies for the delay in jumping back in. There’s a lot to unpack, so please bear with me.

Clarifying a Misconception

@kristiaanvandeneynde thank you for taking the time to further review the MR. In #43 🐛 "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 Needs work , you state:

Your code still adheres to that but adds two redirects to every login request, even when most sites won’t ever run into the problem explained here. That is a performance hit we cannot incur for an edge case.

This is not the case. The MR only introduces a redirect in situations where an error message would currently be shown, specifically:

1. If a user attempts to log in with cookies disabled.
2. If an anonymous user visits a URL containing the check_logged_in query parameter (see the issue summary, #19 🐛 "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 Needs work , #37 🐛 "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 Needs work , #56 🐛 "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 Needs work ).

The redirect logic in \Drupal\user\Authentication\Provider\Cookie::onKernelRequestVerifyCookies() only runs if verifyCookies is TRUE, and that is only set to TRUE by \Drupal\user\Authentication\Provider\Cookie::applies() in the same condition that currently triggers the error message. Thus, the performance impact is minimal and I think justified to prevent incorrect error messages.

Addressing the Alternate Solution (Re: #39 🐛 "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 Needs work )

@kristiaanvandeneynde, thanks for your suggestion. It does resolve the issue, but it also removes the error message in cases where it should still appear, specifically, when users have cookies disabled and cannot log in.

When I mentioned in #37 🐛 "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 Needs work that I was open to suggestions, I was hoping for ideas on simplifying the MR while still ensuring the error message appears correctly when needed and doesn’t appear when it shouldn’t.

Re:

You'd have to specifically design your site as a single-page dashboard where you don't strip the query arguments after login for this to even remotely be a re-occurring problem.

I understand the concern that this might only affect single-page dashboards, but in our case, that’s not true. Our client’s site has over 1 million pages and 6000+ active users. It does, however, have "dashboard" type pages that "manager" type users visit to gain an overview of what other users are doing. So it's not that we only have one-page site, rather we have users that tend to only visit one page.

I don't feel like our client's site is that unusual. Sure, virtually all pages are access controlled, and we have an EventSubscriber that redirects anonymous users to the login form, but apart from that we do our best to work with core as much as possible as it is easier to maintain.

Re: #54 🐛 "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 Needs work & JavaScript-Based Solutions

@larowlan, thank you for the input. Unfortunately, for us this issue occurs when users save/share URLs with check_logged_in or reload such URLs when logged out. That’s why your suggestion wouldn’t work for us. Due to discussions in 🐛 Login fails and no warning is issued if cookies are not enabled Fixed (particularly #272 🐛 Login fails and no warning is issued if cookies are not enabled Fixed ), I also aimed to avoid a JavaScript-based approach.

Re: #56 🐛 "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 Needs work

@jody lynn, thanks for sharing your experience, and you raise a good point. Although, I am really hoping we can come to a consensus on a solution where we don't have to choose between showing an error message incorrectly vs not showing error message when we should.

Seeking Clarity on “Feature Request” vs. “Bug”

Since the current behaviour results in incorrect error messages, I’d love to better understand what qualifies this as a Feature Request rather than a Bug?

Moving Forward: Specific Feedback on Complexity

For those who feel the MR is too complex, could you point out specific areas where you see unnecessary complexity? Are there parts that could be simplified while still achieving the intended behaviour? This would give us something constructive to work with.

Okay reading the MR again, I noticed the weights of the subscribers and indeed as it is written now it will only run when there is no session but we have the query argument to check for one.

Some observations:

1. Sharing of URLs

If we have a session, the URL is not altered and you can still share a URL with check_logged_in on it. Ideally we want to prevent this completely, something @larowlan's suggestion from #54 takes care of.

2. Old behavior did more than it advertised

While the old behavior resides in the Cookie class, it really was only checking for the presence of a session. That means that there could be other reasons a session no longer exists and we would still blame it on the absence of cookies. This seems to be the root problem for some people in this issue.

So one could argue that, if we merely rephrased the error message to actually cover what it checks, this would fix the issue. E.g.: "A session could not be found. This could happen because you got logged out due to inactivity, you just tried to log in but your browser does not accept cookies or someone shared a URL with you that expects you to be logged in."

I would also argue we still go for @larowlan's solution to strip the query argument using JavaScript so that it becomes almost impossible to share a URL with check_logged_in on it. Then we can drop the last bit of the suggested message.

3. New behavior works differently

If we don't have a session, a custom cookie is set and we are redirected to a verification page. On that page we show the old error if we couldn't find the custom cookie we tried to set. But what happens if we do find a cookie? In other words when we did not find a session but we were able to set a cookie. Now we're dealing with "user was logged out in the background" territory and we basically show no error message whatsoever.

This is a behavioral change and you could argue that not showing anything also fixes the problem when you share a URL with check_logged_in on it. But if you just tried to log in and somehow weren't even though cookies are enabled, you are now facing a page wondering why you weren't logged in because there was no error message.

I was hoping for ideas on simplifying the MR while still ensuring the error message appears correctly when needed and doesn’t appear when it shouldn’t.

Well my suggestion would be to keep the old behavior, but to expand the message to cover what it's actually doing. We can then debate whether it still belongs in a class called Cookie in a follow-up.

Since the current behaviour results in incorrect error messages, I’d love to better understand what qualifies this as a Feature Request rather than a Bug?

It doesn't qualify as a bug because the code behaves exactly the way it was intended to. It doesn't seem to cover all use cases, though, as demonstrated by this issue. So basically you're asking for it to cover more use cases and that is a feature request.

could you point out specific areas where you see unnecessary complexity

Setting (and removing) cookies along with a double redirect for pages where the check is present and a session is missing.

Unfortunately, for us this issue occurs when users save/share URLs with check_logged_in

Yes, I'm not a JS expert and I don't see a way to fix that. Maybe someone else does. it does, however, fix the history and back button argument mentioned in the comments, so at least it fixes 2 out of 3 ways for the problem to occur. Which is why I think it's still a win overall.