Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use the IP address of the user if userId is empty #179

Closed
githubDante opened this issue Jan 21, 2020 · 8 comments
Closed

Use the IP address of the user if userId is empty #179

githubDante opened this issue Jan 21, 2020 · 8 comments
Labels
enhancement New feature or request feature: 📊 responses & statistics wontfix This will not be worked on

Comments

@githubDante
Copy link

Which Forms version are you running? (see apps page)
git [master]
Nextcloud or ownCloud?:
Nextcloud
Nextcloud/ownCloud version: (see Nextcloud admin page)
17.0.2

Description

If the patch proposed in #178 is applied, the form is not configured as Anonymous Form and Access is set to Public access the Name column in the results view will be left empty. The patch included below is using the IP address from where the request is received as userId and as a result the IP address will be shown in Name column.

diff --git a/lib/Controller/PageController.php b/lib/Controller/PageController.php
index dcb60f0..5ba188b 100644
--- a/lib/Controller/PageController.php
+++ b/lib/Controller/PageController.php
@@ -278,7 +278,11 @@ class PageController extends Controller {
                $questions = $this->questionMapper->findByForm($form->getId());

                try {
-                       $notification = $this->notificationMapper->findByUserAndForm($form->getId(), $this->userId);
+                       if($this->userId === null) {
+                               $notification = $this->notificationMapper->findByUserAndForm($form->getId(), '');
+                       } else {
+                               $notification = $this->notificationMapper->findByUserAndForm($form->getId(), $this->userId);
+                       }
                } catch (DoesNotExistException $e) {
                        $notification = null;
                }
@@ -400,6 +404,9 @@ class PageController extends Controller {
                                if($form->getIsAnonymous()){
                                                $vote->setUserId($anonID);
                                }else{
+                                               if ($userId === '') {
+                                                       $userId = $this->request->getRemoteAddress();
+                                               }
                                                $vote->setUserId($userId);
                                }
                                $vote->setVoteOptionText(htmlspecialchars($questions[$i]['text']));
@oscaropenness
Copy link

I like your suggestion of using some identifier for unidentfied users.
I tried to apply your suggestion in my Nextcloud instance. My userId column is still empty.
There is no error in the log or anything. Am I missing something?

Another thought:
From a privacy point of view it would be more appropriate to store some hash of the RemoteIp since it contains already some information about the user (location, workplace, country...)

githubDante added a commit to githubDante/forms that referenced this issue Feb 12, 2020
Additionally will use the IP address from the request if the form
is not Anonymous and the user is not lgged in. See nextcloud#179
@githubDante
Copy link
Author

I just tested again with the master branch and the IP address is displayed correctly in the Name column. I'm sending a pull request.

githubDante added a commit to githubDante/forms that referenced this issue Feb 12, 2020
Additionally will use the IP address from the request if the form
is not Anonymous and the user is not lgged in. See nextcloud#179

Signed-off-by: githubDante <github@dante.tk>
githubDante added a commit to githubDante/forms that referenced this issue Feb 24, 2020
Additionally will use the IP address from the request if the form
is not Anonymous and the user is not lgged in. See nextcloud#179

Signed-off-by: githubDante <github@dante.tk>
@makakken
Copy link

seems this error is still not fixed?

Exception: Argument 2 passed to OCA\Forms\Db\NotificationMapper::findByUserAndForm() must be of the type string, null given, called in /var/www/cloud.offene-werkstaetten.org/apps/forms/lib/Controller/PageController.php on line 277

when i have a pbulic form, and share the link with a non logged-in user, i get a 500 Error.

@jotoeri
Copy link
Member

jotoeri commented Mar 20, 2020

Things are mixed up here a bit.
The internal server Error itself is removed on master, just a release is pending.

The Proposal above is still open, but does not produce any Error if not implemented. Just an empty Name-Field in Results appears for unauthenticated users.

@skjnldsv skjnldsv added 1. to develop Accepted and waiting to be taken care of enhancement New feature or request labels Apr 29, 2020
@skjnldsv skjnldsv changed the title [Proposal] - Use the IP address of the user if userId is empty Use the IP address of the user if userId is empty Apr 29, 2020
@jotoeri
Copy link
Member

jotoeri commented May 3, 2020

@skjnldsv @jancborchardt John & me talked about this for a moment and store these users just like anonymous users now, as John said he wouldn't see any advantage of using the ip.
So the initial problem of an empty user here is solved. Is this issue to develop or just to close? ;)

@jancborchardt
Copy link
Member

@jotoeri @skjnldsv I’d say if we have a different system which we consider better, we can close this? But your call :)

@skjnldsv
Copy link
Member

skjnldsv commented May 3, 2020

The only real advantage I could see here is using a browser's footprint to store and make the "only one submission" option available for public views too. But this is far more complicated and not 100% foolproof. So I would close this indeed as out of scope :)

@skjnldsv skjnldsv closed this as completed May 3, 2020
@skjnldsv skjnldsv added wontfix This will not be worked on and removed 1. to develop Accepted and waiting to be taken care of labels May 3, 2020
@warrenwong87
Copy link

how to do this?

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request feature: 📊 responses & statistics wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

7 participants