Steps to replicate:
- enable ctools_wizard_test
- Open one browser, let's say Firefox, log in as admin, and navigate to /ctools/wizard
- Enter "foo" on form one and click next
- Open another browser, e.g. Chrome, not logged into that site, and navigate to /ctools/wizard
- Observe "foo" is prepopulated
Steps to fix:
I believe this can be simply fixed by swapping uses of SharedTempStoreFactory for PrivateTempStoreFactory.
The best illustration of this is the attached screenshot from my database. I have been testing this with two wizards: WizardTest is the ctools_wizard_test module, otherwise unaltered, offerdoc is something a bit more elaborate I have extended from FormWizardBase.
You can immediately see that with tempstore.shared only a single row is created per wizard, with the name *wizard_machine_name*, whereas with tempstore.private, one row is created per-wizard-per-user, name *user_id:wizard_machine_name*, which, unless I very thoroughly misunderstand the intention of the WizardBase classes, is clearly the expected and desired behaviour. (1:WizardTest is me-as-admin, uid 1; the long hashes represent me-as-anon sessions in my second browser)
Although the tempstore.shared collections do include an 'owner' key there is only one store for everyone. When user t1lD8O... comes along and the Wizard code requests the contents of shared tempstore with the id WizardTest, they are provided with the contents of tempstore as provided by user 1. Although that tempstore data contains the key owner:1, per the comments at the top of SharedTempStore.php in core,
"It is the responsibility of the implementation to decide when and whether one owner can use or update another owner's data."
The ctools / wizard code does not seem to take on this responsibility. It could, for example, use $tempstore->getIfOwner() and corresponding setter to ensure that user t1lD8O... does not see/set the contents of the tempstore previously set by user 1, but we see e.g. ->set rather than ->setIfOwner on line 287 of FormWizardBase. In other words, all users completing the same wizard will share the same cache of values. As they progress through the wizard, they will merrily read and overwrite each others cached values.
So far simply changing Shared to Private within the Wizard code seems to have created the desired behaviour of giving each user their own set of cached values, however as yet I haven't changed TempstoreConverter.php and I suspect sooner or later I will run into a mismatch between classes here. However I'm less confident changing them there because I have no idea what other aspects of ctools besides wizards (which is all I've looked at so far) are relying on it, and perhaps they definitely need Shared tempstores. So perhaps this needs to be refactored to handle either/both...
This is where I would ideally just attach a .patch file, however I cannot conveniently do that because it needs to fix "only this issue and nothing but this issue". However meanwhile all the uses of SharedTempStoreFactory are from the deprecated \User\ namespace and not the \Core\ namespace. As per
https://www.drupal.org/project/ctools/issues/2936263#comment-12726840 →
I have patched this, however that would mean any new patch I make for this issue would included changes not strictly relating to this issue (change of namespace for all TempStores including those not relating to Wizards), OR, if I rolled back those changes and made a patch that only changed Shared to Private without addressing the deprecated namespace, I would be making a patch that introduced deprecated code, OR, if I both Shared->Private and de-deprecated in this patch but for Wizards only I would be knowingly ignoring all the other non-Wizard deprecations, and that is also silly, so basically... the deprecations need replacing first, be that via my other patch, or someone else's better job of it, and then I can meaningfully roll a patch for this one.
Needs review
3.0
Wizard
Not all content is available!
It's likely this issue predates Contrib.social: some issue and comment data are missing.