Contrib.social

Views exposed filter reset creates session for anonymous

Open on Drupal.org →
Created on 30 August 2024, 8 months ago

Problem/Motivation

If you have a View with an exposed filter and reset button Drupal creates a session and returns the cookie for anonymous visitors as part of the the http/303 response from using the reset button. The session is only created once per uncached response.

This issue was identified in an unauthenticated (anonymous) Webinspect scan which included a finding about a cookie not meeting security criteria. This was unusual because I thought Drupal didn't create sessions / provide cookies for anonymous. When I dug into it I realized Views reset was creating the session.

Expected behavior

Drupal should not create a session for the anonymous visitor.

What happened instead

Drupal created a session for the anonymous visitor.

Steps to reproduce

You can reproduce this bug by:
1. Installing Drupal core Standard profile.
2. As admin, editing the Frontpage view to add an exposed keyword search field (using search, title field, body, etc. -- it doesn't even have to work), set the "Advanced" > "Exposed form style" settings to "Include reset button" and save the View.
3. As admin, add content of type article, noting words used in the title or body for searching later.
4. As anonymous in an incognito window, other browser (or with curl) and with the browser inspector network tab open visit the site's /node page/path. Click the Application tab in the browser inspector and notice you should not have any cookie from the site.
5. Fill in the search keywords field with a keyword used in the content added earlier and click Apply. Notice you still don't have any cookies.
6. With the browser inspector still open, click/tap the Reset button on the views exposed filter. Notice in the network tab you should have a 303 redirect response which sets the session cookie and sends your browser to the /node page/path. Check the browser inspector Application tab and see there is now a cookie from the site with name "SSESS..."

If you try to repeat this again with a different browser / new browser / curl session with the exact same path & parameters, Drupal will not create a new session or send the cookie and the x-drupal-cache will be "HIT" instead of "MISS". To recreate subsequent times, you need to change something in the parameter (like the keywords).

Proposed resolution

Fix this code as suggested by larowlan:

The code in question is in \Drupal\views\Plugin\views\exposed_form\ExposedFormPluginBase::resetForm

    $session = $this->view->getRequest()->getSession();
    $views_session = $session->get('views', []);
    if (isset($views_session[$this->view->storage->id()][$display_id])) {
      unset($views_session[$this->view->storage->id()][$display_id]);
    }
    $session->set('views', $views_session);

The issue is the unqualified call to set the session

$session->set('views', $views_session);

This should be

if (!empty($views_session)) {
    $session->set('views', $views_session);
}
else {
    $session->remove('views');
}

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

🐛 Bug report
Status

Active

Version

11.0 🔥

Component
Views  →

Last updated about 4 hours ago

Created by

🇺🇸United States timwood Rockville, Maryland

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @timwood
  • First commit to issue fork.
  • Merge request !9791Don't start session on filter reset → (Closed) created by Lendude
  • Comment 6 months ago →
    🇳🇱Netherlands Lendude Amsterdam

    Added the proposed code and a test assertion. I can see this happen when testing manually, but the test isn't catching it.
    The assertion detects the session when I log a user in a test, so that part works, but in the test, the reset is not triggering the bug. Haven't figured out why yet

  • Pipeline finished with Success
    6 months ago
    Total: 1188s
    #304770
  • Status changed to Needs work about 1 month ago
  • Comment about 1 month ago →
    🇺🇸United States tregonia

    Currently rubbing up against this issue in D11. I can confirm that the patch does apply, but does not not prevent the anonymous sessions from being created.

  • Comment about 1 month ago →
    🇺🇸United States tregonia

    In my instance, I am finding that the code never reaches the resetForm function.

    The call to resetForm is on line 292, but does not make it into the if statement. This is due to op not being in the form_state within the exposedFormSubmit function.
    https://git.drupalcode.org/project/drupal/-/blob/9.4.x/core/modules/views/src/Plugin/views/exposed_form/ExposedFormPluginBase.php#L292

  • Comment about 1 month ago →
    🇺🇸United States timwood Rockville, Maryland

    In testing the MR on a 10.3.13 site, I can confirm that the patch applies there and prevents anonymous sessions from being created.

    I also tested the 11.x branch on simplytest.me with and without the MR patch. When using the patch, I do not see cookies being set when clicking a reset button. When not using the patch the cookie is still set.

    @tregonia how did you validate the anonymous session? Did you use an incognito window or other browser to ensure you didn't already have a cookie?

  • Comment about 1 month ago →
    🇺🇸United States tregonia

    @timwood
    My process was as follows.

    1. Find a form that includes a reset button and open it in a different browser.
    2. Ping the database using drush sql:cli to get a count on anonymous sessions.
      SELECT count(*) FROM sessions WHERE uid = 0;
      Also used this to look at them.
      SELECT * FROM sessions WHERE uid = 0 ORDER BY timestamp DESC;
    3. Clicked the reset button.
    4. Run the 2 sql:cli commands again.

    Note the sessions count increases, and a new session is created in the database.

    I am currently running D11.1.

  • Comment about 1 month ago →
    🇺🇸United States timwood Rockville, Maryland

    Good tip about the DB query before and after. Can you confirm in your browser whether a session cookie was sent with the 303 redirect response to the user after clicking the reset button?

    Update issue description to add better ordered list formatting.

  • First commit to issue fork.
  • Pipeline finished with Success
    about 1 month ago
    Total: 1112s
    #447865
  • Comment about 1 month ago →
    godotislate

    I was able to the test-only job to fail by changing the test line from drupalGet to submitForm, so that a reset button press is actually performed.

    @tregonia With the MR diff applied, I was not able to reproduce the your reported issue with anonymous session entries in the database table, using the Standard profile install steps detailed in the IS. Are you using any custom or contrib modules that touch forms or views? One other thing to check is to inspect the Reset button in the browser and check whether the name="op" attribute is on the button element. If it's not, then something is removing the attribute that core form API adds.

  • Comment about 1 month ago →
    godotislate
  • Comment about 1 month ago →
    🇺🇸United States smustgrave

    Left a comment on the MR

    If you are another contributor eager to jump in, please allow the previous poster at least 48 hours to respond to feedback first, so they have the opportunity to finish what they started!

  • Pipeline finished with Success
    29 days ago
    Total: 944s
    #449766
  • Comment 29 days ago →
    godotislate

    Addressed MR feedback

  • Comment 29 days ago →
    🇺🇸United States smustgrave

    Feed back appears to be addressed

  • Comment 29 days ago →
    System Message
    • catch → committed 91faaba1 on 11.x 
      Issue #3471220 by godotislate, lendude, timwood, smustgrave: Views...
  • Comment 29 days ago →
    System Message
    • catch → committed 2536e3ac on 10.5.x 
      Issue #3471220 by godotislate, lendude, timwood, smustgrave: Views...
  • Comment 29 days ago →
    🇬🇧United Kingdom catch

    Committed/pushed to 11.x and cherry-picked to 10.5.x, thanks!

  • Comment 29 days ago →
    System Message
  • Comment 15 days ago →
    System Message

    Automatically closed - issue fixed for 2 weeks with no activity.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024