Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem
Voting API config form suggests that there is a rollover window that you can choose in order for anonymous (and registered) users, to cast multiple votes (re-cast a new vote without deleting the old one) from the same source (the IP address originally, e.g. internet cafes, schools, compagnies), after the rollover window ends.
But in fact, Voting API never uses them either for anonymous or registered users. I would consider this as a major issue if it was described on the module's page.
It's even more confusing, when you can see the description of these settings saying
Setting this to 'never' will eliminate most double-voting
when there is actually no 'never' option.
Related issues
In fact, there are even some issues already openned about the missing 'never' option, and that having a '0 minute' option would be great :
#2791129: Vote rollover settings are missing the "Never" option
#3055835: Vote rollover should have an option for "- no rollover -"
There also is an issue about the fact that different anonymous users on the same network cannot vote more than 1 time and suggests to use a different source than IP address (#123916: Allow multiple votes from anonymous users on a shared IP address), while using a different source than the IP address is a possible solution, using the Drupal's session is easily bypassable by just restarting the web browser (as stated in the #7 comment of the mentioned issue).
I created this issue to regroup all those kind of issues in the same place so we can finally fix them and may be have a stable module.
Suggestions
First of all, I think that having an "Immediately" and a "Never" options for the rollover window, are a must have :
- "Immediately" : for compagnies' internal projects with a lot of votes at the same time, where you except well-behaving users and you don't want to prevent several users from being considered as the same one within even a short rollover window.
- "Never" : for projects where registered users can only vote one time.
Then, of course, the rollover window must be used in the code, but unlike in this issue : #2791129: Vote rollover settings are missing the "Never" option
I think it would be better to handle it in the VoteStorage::getUserVotes, because eventually, after the rollover window ends, a user from the same source is indeed considered as a different user, and while deleting the vote in the Vote::preSave would avoid double-voting, you would still see old votes being yours after the rollover window has ended, which we do not want
And finally, having different source's options would be a great addition, and while using the Drupal's session as a vote source is not a great idea, creating a randomly generated token stored in a cookie is really good, in fact, having a source that combine the IP address + a long duration cookie would be a great solution.
Real use cases
1. You have an internal project to your compagny where anonymous users can vote on a poll only one time, but because they all have the same IP address, you can't rely on it, instead you switch to the "cookie" source, and set the rollover to a high value (i.e. 1 week), so, if by any chance, a user ends up genarating the same cookie as another user (which should not happens soon), the user would still be able to vote.
2. You have a public project that is subject to a lot of users and is potentially open to spam/attack where anonymous users can vote, but you don't excpect user on the same network to vote, because relying on the cookie would be too dangerous, instead you switch to the "ip" source and choose a moderated rollover value (i.e. 1 day).
3. You have a public project where anonymous users can vote and you except that users from the same network to vote, but you wan't to avoid the maximum amount of double-voting, so you choose "ip + cookie" coupled to a low rollover value (i.e. 5 minutes).
Conclusion
Of course, as soon as you open your database to anonymous write without moderation, there is no perfect solution.
But, in my opinon the, "ip + cookie" source and a low rollover value combo is the best solution at the moment (if you want anonymous user to be able to vote), as not only it prevent double-voting from the same IP, but also prevent double-voting from the same device (by switching from Wi-Fi to 4G, going from your home to your work network, or using a VPN), but it still alows different users from the same network to be able to cast vote after the rollover window ends.
A possible amelioration would be to define several rollover windows for each source, so you could have low value on the IP and high value on the cookie, so users from the same network don't have to wait long to cast a new vote, and user from the same device that have forgotten if they have already vote, would not be able to double-vote before a longer time. But such a solution would need a lot more change in the module.
I'll attach my patch in a futur comment, and mention this issue in the related issues, to focus on a better solution here.
Comments
Comment #2
jmarcou commentedI added a description on the config's form to warn the user from potential spamming issue when allowing anonymous user to vote (but if the patch ends up the module, there should definitely have a warning on the module's page).
Because I didn't wanted to rewrite a lot of code and still wanted to have the "ip + cookie" option, I ended up concatenating different sources using a separator and a prefix to differentiate them, which even allows to switch from one source to another and keep backward compatibility while avoiding collision between differente sources that may end up with the same hash.
I must also point out, that using the "cookie" source may be a concern for cached pages as sometime people use them to create different cached versions of the same page. It also is the reason why I choosed "ip" instead of "ip + cookie" as the default source option even tough I think "ip + cookie" is the best one, the "ip" being the more neutral and the original intent of this module.
And because people may not be aware of it, I think it would be great to have another warning on the module's page to remember the developers that votingapi doesn't handle the display cache and it's up to them to handle it so they avoid displaying vote results of other users.
Comment #3
jmarcou commentedComment #4
pifagorDear @jmarcou
I do not agree to use a cookie instead of an IP address. Because cookies are easy to delete and able to vote many times. In my opinion, it's not the right approach.
But what about the backward compatibility of already existing values?
Comment #5
pifagorComment #6
jmarcou commentedI added a hook_update, so old votes are considered as "ip" source, it would have worked anyway but every body who already voted would have been allowed to vote again.
I agree with you that the cookie is easy to bypass, that's why I added a description
I don't consider it as an ordinary option, but it seems some people have that need (https://www.drupal.org/project/votingapi/issues/123916), where they except their users to be well-behaving (i.e. internal project where all the users are your employees).
For the "ip + cookie" option, you need both you IP and cookie to change to cast a new vote from the same device, which is just a little bit safer than just the "ip" option, as mobile devices are becoming more and more used, and mobile devices can often run on different networks (like going from Wi-Fi to 4G...), in fact, I mostly consider this option as a reminder for people using mobile devices, that they have already voted, more than a security option, which adds to the user experience.
(I also removed the .txt extension from the patch)
Comment #7
jmarcou commentedI updated the patch with a new cookie name, I replaced dots with underscores as PHP doesn't like dots in cookies' name and replaces them with underscores.
If you wonder why I don't use the user_cookie_save() function, it's because it's really bad.
It doesn't provide any "get" method and you have to manually add the prefix "Drupal.visitor." (with dots replaced by undescores) yourself, so I prefer to do it myself which also allows us to define the duration we want for the cookie.
Comment #8
tr commentedPatch no longer applies and needs to be re-rolled.
I also think that it's extremely important that this functionality include tests, both in order to document how the functionality is supposed to work and to ensure that future changes to the Voting API don't break this functionality. It's up to the maintainer to decide if lack of tests should block this issue, but I would say yes - all new features should have tests before they are committed (that's core Drupal policy too).
Comment #9
tr commentedComment #10
mrinalini9 commentedRerolled patch #7. Still need to add tests as mentioned in #8.
Comment #11
KuroiRoy commentedRerolled patch #10 against 8.x-3.0-beta2. Still need to add tests as mentioned in #8.
Comment #12
KuroiRoy commentedFixed usage of
self::getRequestTime()that doesn't exist and is simply an alias for\Drupal::time()->getRequestTime()Comment #13
tr commented@KuroiRoy: All patches need to be made against the current HEAD. Branches like beta2 are fixed and unchangeable - only the latest -dev version can be modified. Your patch fails because you included some fixes that are already included in -dev.
Please post a new patch based on #10 with your changes included. And please provide an interdiff so we can easily see how your new patch differs from #10.
Comment #14
danlew commentedI have Rerolled patch #12 for 8.x-3.x. I also replaced another usage of
Vote::getRequestTime()in theVoteStorageclass. I have also added an interdiff from #10.This still needs the tests as mentioned in #8 and #10.
Let me know if I've missed something, this is my first time actually contributing.
Comment #15
danlew commentedI've noticed I somehow managed to remove the additional config form field in my previous patch. I have made sure it's back there this time round. Still need to look at the failing tests, and adding any new ones as suggested in #8/#10.
Comment #16
tr commentedI haven't reviewed the functionality of this patch, I have just looked for obvious things that need work. Here are some of the things I found:
PHP superglobals like $_COOKIE should not be used in Drupal 8. See the change notice from more than 7 years ago for a brief description of how to use cookies: https://www.drupal.org/node/2150267. There is a lot of documentation on how to do this in D8, and of course core Drupal is full of examples on the proper usage.
Likewise, static \Drupal:: calls should not be used in object-oriented code. Rather, the service should be injected into the class where it is needed. Again, lots of documentation on this. We've spent some effort removing that sort of usage in this module, and I don't want to go backwards by introducing it in new places like this patch does. (Note, this can't always be done, because of limitations in Drupal core, but if that's the case here then there should be an explicit code comment stating why \Drupal:: was used.)
There are several new coding standards violations added by this patch. Again, no reason to be adding new violations - let's do it right when we add new code so we don't have to "fix" it later on.
The test failures are real failures in functionality, and they are caused by this patch. These failures have existed since the patch #7. They certainly need to be understood and fixed before moving forward, because they indicate this patch is breaking currently-working code.
Anonymous vote rollover was never ported from D7. Last year I took some steps to triage the issue queue and consolidate duplicate issues and tackle the initial part of the problem - please read #2791129-22: Vote rollover settings are missing the "Never" option. I think this current issue needs to have its issue summary rewritten to reflect the current state of the votingapi code and to describe exactly what the patch is trying to accomplish, because this issue is too broad and fuzzy. IMO we first need to make the functionality work in D8 as it did in D7, then we can discuss how to make it better. Trying to re-invent the way rollover functionality works without first making it work and without having tests to prove it works does not seem like a successful strategy to me. I would prefer to make it work first, write test cases to we can prove it works as defined and so we can prevent breaking the functionality, and THEN try some drastic changes to the way the functionality is implemented. Any new approaches can be effectively evaluated by the tests.
The maintainer, @pifagor, mentioned in #4 that he didn't think cookies were the right approach for this issue, and that backwards compatibility with existing votes needs to be addressed. It would probably be worthwhile to review all the existing issues around this feature again, and consolidate them or at least gather them into one meta issue with non-overlapping sub-issues so that we can effectively get this part of the Voting API sorted out and working again. https://www.drupal.org/project/issues/votingapi?text=rollover&status=Ope...
Comment #17
tr commentedAlso ... what is this?
$uuid = new Php();If this patch had even minimal tests, this sort of flaw would have been noticed two years ago. As it is, a lot of time and effort has been wasted.
Comment #18
tr commentedAlso, this needs to now be put into an MR so that it can be tested by the automated testing system. Patches are no longer supported on drupal.org.